Convert the undocumented_function linter to use trait

Summary:
Even if the `undocumented_function` is supposed to be disabled by default, it still runs. This is due to a flawed logic in `diagnostics.rs`, originally introduced in D59272525.

This is problematic, since open-source projects such as OTP see a huge number of errors when running the `elp lint` command without specifying a custom configuration.

By converting the linter to use the new trait, we incidentally fix the bug.

This was the last linter using the `default_disabled: true` logic. At this point the enabled/disabled logic can be simplified.

Reviewed By: alanz

Differential Revision: D87060203

fbshipit-source-id: 1121d286c766fb461420e3268aca817169020d7d

Author: robertoaloi

crates/elp/src/resources/test/buck_tests_2/resolves_generated_includes.stdout

@@ -5,8 +5,7 @@ Diagnostics reported in 3 modules:
   top_includer: 2
       0:1-0:1::[Error] [L0000] Issue in included file
       12:4-12:10::[Error] [E1508] undefined macro 'THIRD/2'
-  wa_buck2_module_search: 5
-      19:0-19:18::[WeakWarning] [W0040] The function is non-trivial, exported, but not documented.
+  wa_buck2_module_search: 4
       54:18-54:31::[WeakWarning] [W0051] Binary string can be written using sigil syntax.
       56:18-56:28::[WeakWarning] [W0051] Binary string can be written using sigil syntax.
       56:38-56:51::[WeakWarning] [W0051] Binary string can be written using sigil syntax.

crates/ide/src/diagnostics.rs

@@ -1500,7 +1500,6 @@ pub fn diagnostics_descriptors<'a>() -> Vec<&'a DiagnosticDescriptor<'a>> {
         &unspecific_include::DESCRIPTOR,
         &edoc::DESCRIPTOR,
         &macro_precedence_suprise::DESCRIPTOR,
-        &undocumented_function::DESCRIPTOR,
         &duplicate_module::DESCRIPTOR,
         &no_dialyzer_attribute::DESCRIPTOR,
         &no_catch::DESCRIPTOR,
@@ -1584,6 +1583,7 @@ const GENERIC_LINTERS: &[&dyn GenericDiagnostics] = &[
     &unused_macro::LINTER,
     &missing_compile_warn_missing_spec::LINTER,
     &undocumented_module::LINTER,
+    &undocumented_function::LINTER,
 ];
 
 /// Unified registry for all types of linters

crates/ide/src/diagnostics/undocumented_function.rs

@@ -8,11 +8,12 @@
  * above-listed licenses.
  */
 
-use elp_ide_assists::helpers::unwrap_parens;
 // Diagnostic: undocumented-function
+use elp_ide_assists::helpers::unwrap_parens;
 use elp_ide_db::elp_base_db::FileId;
 use elp_syntax::ast;
 use elp_syntax::ast::Atom;
+use elp_text_edit::TextRange;
 use fxhash::FxHashSet;
 use hir::AsName;
 use hir::FunctionDef;
@@ -21,37 +22,66 @@ use hir::Semantic;
 use hir::form_list::ModuleDocAttribute;
 use hir::known;
 
-use super::Diagnostic;
-use super::DiagnosticCode;
-use super::DiagnosticConditions;
-use super::DiagnosticDescriptor;
-use super::Severity;
-
-const DIAGNOSTIC_CODE: DiagnosticCode = DiagnosticCode::UndocumentedFunction;
-const DIAGNOSTIC_MESSAGE: &str = "The function is non-trivial, exported, but not documented.";
-const DIAGNOSTIC_SEVERITY: Severity = Severity::WeakWarning;
-
-pub(crate) static DESCRIPTOR: DiagnosticDescriptor = DiagnosticDescriptor {
-    conditions: DiagnosticConditions {
-        experimental: false,
-        include_generated: false,
-        include_tests: false,
-        default_disabled: true,
-    },
-    checker: &|diags, sema, file_id, _ext| {
-        check(diags, sema, file_id);
-    },
-};
-
-fn check(diagnostics: &mut Vec<Diagnostic>, sema: &Semantic, file_id: FileId) {
-    let callbacks = sema.resolve_callbacks(file_id);
-    if !contains_moduledoc_hidden_attribute(sema, file_id) {
-        sema.def_map_local(file_id)
-            .get_functions()
-            .for_each(|(_arity, def)| check_function(diagnostics, sema, def, &callbacks));
+use crate::diagnostics::DiagnosticCode;
+use crate::diagnostics::GenericLinter;
+use crate::diagnostics::GenericLinterMatchContext;
+use crate::diagnostics::Linter;
+use crate::diagnostics::Severity;
+
+pub(crate) struct UndocumentedFunctionLinter;
+
+impl Linter for UndocumentedFunctionLinter {
+    fn id(&self) -> DiagnosticCode {
+        DiagnosticCode::UndocumentedFunction
+    }
+
+    fn description(&self) -> &'static str {
+        "The function is non-trivial, exported, but not documented."
+    }
+
+    fn severity(&self) -> Severity {
+        Severity::WeakWarning
+    }
+
+    fn is_enabled(&self) -> bool {
+        false
+    }
+
+    fn should_process_test_files(&self) -> bool {
+        false
     }
 }
 
+#[derive(Debug, Default, Clone, PartialEq, Eq)]
+pub struct Context {
+    range: TextRange,
+}
+
+impl GenericLinter for UndocumentedFunctionLinter {
+    type Context = Context;
+
+    fn matches(
+        &self,
+        sema: &Semantic,
+        file_id: FileId,
+    ) -> Option<Vec<GenericLinterMatchContext<Context>>> {
+        let mut res = Vec::new();
+        let callbacks = sema.resolve_callbacks(file_id);
+        if !contains_moduledoc_hidden_attribute(sema, file_id) {
+            sema.def_map_local(file_id)
+                .get_functions()
+                .for_each(|(_arity, def)| {
+                    if let Some(match_context) = check_function(sema, def, &callbacks) {
+                        res.push(match_context);
+                    }
+                });
+        }
+        Some(res)
+    }
+}
+
+pub static LINTER: UndocumentedFunctionLinter = UndocumentedFunctionLinter;
+
 fn contains_moduledoc_hidden_attribute(sema: &Semantic, file_id: FileId) -> bool {
     sema.form_list(file_id)
         .moduledoc_attributes()
@@ -87,20 +117,22 @@ fn function_should_be_checked(
 }
 
 fn check_function(
-    diagnostics: &mut Vec<Diagnostic>,
     sema: &Semantic,
     def: &FunctionDef,
     callbacks: &FxHashSet<NameArity>,
-) {
+) -> Option<GenericLinterMatchContext<Context>> {
     if function_should_be_checked(sema, def, callbacks)
         && !def.has_doc_attribute()
         && !def.has_doc_attribute_metadata()
         && def.edoc_comments(sema.db).is_none()
         && let Some(name_range) = def.name_range(sema.db)
     {
-        let diagnostic = Diagnostic::new(DIAGNOSTIC_CODE, DIAGNOSTIC_MESSAGE, name_range)
-            .with_severity(DIAGNOSTIC_SEVERITY);
-        diagnostics.push(diagnostic);
+        Some(GenericLinterMatchContext {
+            range: name_range,
+            context: Context { range: name_range },
+        })
+    } else {
+        None
     }
 }
 
@@ -130,7 +162,7 @@ mod tests {
     -module(main).
     -export([main/0]).
     main() ->
- %% ^^^^ weak: W0040: The function is non-trivial, exported, but not documented.
+ %% ^^^^ 💡 weak: W0040: The function is non-trivial, exported, but not documented.
       [ok,
        ok,
        ok,
@@ -147,7 +179,7 @@ mod tests {
     -module(main).
     -export([main/0]).
     main() ->
- %% ^^^^ weak: W0040: The function is non-trivial, exported, but not documented.
+ %% ^^^^ 💡 weak: W0040: The function is non-trivial, exported, but not documented.
       [ok,
        ok,
        ok,
@@ -317,7 +349,7 @@ mod tests {
     -export([handle_call/1]).
 
     main() ->
-    %%<^ weak: W0040: The function is non-trivial, exported, but not documented.
+    %%<^ 💡 weak: W0040: The function is non-trivial, exported, but not documented.
       [ok,
        ok,
        ok,
@@ -349,7 +381,7 @@ mod tests {
       ok.
 
     complex() ->
-    %%<^^^^ weak: W0040: The function is non-trivial, exported, but not documented.
+    %%<^^^^ 💡 weak: W0040: The function is non-trivial, exported, but not documented.
       [ok,
        ok,
        ok,
@@ -370,7 +402,7 @@ mod tests {
       ok.
 
     complex(a) ->
-    %%<^^^^ weak: W0040: The function is non-trivial, exported, but not documented.
+    %%<^^^^ 💡 weak: W0040: The function is non-trivial, exported, but not documented.
       [ok];
     complex(b) ->
       [ok,