Skip to content

Commit 20b8d99

Browse files
robertoaloimeta-codesync[bot]
robertoaloi
authored and
meta-codesync[bot]
committed
Convert undocumented_module linter to use trait
Summary: Even if the `undocumented_module` 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. One more linters uses a `default_disabled: true` property in its descriptor: `undocumented_function`. It will be converted next, and the logic to handle the `default_disabled` will eventually be removed. Reviewed By: alanz Differential Revision: D87059857 fbshipit-source-id: 7fb6eff8773c786c6894548c914b8e3c78c38745
1 parent aca24c8 commit 20b8d99

File tree

6 files changed

+96
-104
lines changed

6 files changed

+96
-104
lines changed

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
Reporting all diagnostics codes
2-
Diagnostics reported in 5 modules:
3-
app_a: 2
2+
Diagnostics reported in 3 modules:
3+
app_a: 1
44
5:9-5:31::[WeakWarning] [W0037] Unspecific include.
5-
0:8-0:13::[WeakWarning] [W0046] The module is not documented.
6-
auto_gen_a: 1
7-
0:8-0:18::[WeakWarning] [W0046] The module is not documented.
8-
eqwalizer: 1
9-
1:8-1:17::[WeakWarning] [W0046] The module is not documented.
10-
top_includer: 3
11-
0:8-0:20::[WeakWarning] [W0046] The module is not documented.
5+
top_includer: 2
126
0:1-0:1::[Error] [L0000] Issue in included file
137
12:4-12:10::[Error] [E1508] undefined macro 'THIRD/2'
14-
wa_buck2_module_search: 6
8+
wa_buck2_module_search: 5
159
19:0-19:18::[WeakWarning] [W0040] The function is non-trivial, exported, but not documented.
16-
2:8-2:30::[WeakWarning] [W0046] The module is not documented.
1710
54:18-54:31::[WeakWarning] [W0051] Binary string can be written using sigil syntax.
1811
56:18-56:28::[WeakWarning] [W0051] Binary string can be written using sigil syntax.
1912
56:38-56:51::[WeakWarning] [W0051] Binary string can be written using sigil syntax.

‎crates/elp/src/resources/test/linter/parse_elp_no_lint_specified_json_output.stdout‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,15 @@
11
{"path":"app_a/src/app_a.erl","line":5,"char":5,"code":"ELP","severity":"warning","name":"W0011 (application_get_env)","original":null,"replacement":null,"description":"module `app_a` belongs to app `app_a`, but reads env for `misc`\n\nFor more information see: /erlang-error-index/w/W0011","docPath":"website/docs/erlang-error-index/w/W0011.md"}
22
{"path":"app_a/src/app_a.erl","line":9,"char":1,"code":"ELP","severity":"error","name":"P1700 (head_mismatch)","original":null,"replacement":null,"description":"head mismatch 'fooX' vs 'food'\n\nFor more information see: /erlang-error-index/p/P1700","docPath":null}
33
{"path":"app_a/src/app_a.erl","line":8,"char":7,"code":"ELP","severity":"warning","name":"W0018 (unexpected_semi_or_dot)","original":null,"replacement":null,"description":"Unexpected ';'\n\nFor more information see: /erlang-error-index/w/W0018","docPath":"website/docs/erlang-error-index/w/W0018.md"}
4-
{"path":"app_a/src/app_a.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
54
{"path":"app_a/src/app_a.erl","line":13,"char":5,"code":"ELP","severity":"warning","name":"W0026 (unexported_function)","original":null,"replacement":null,"description":"Function 'app_a:baz/2' is not exported.\n\nFor more information see: /erlang-error-index/w/W0026","docPath":"website/docs/erlang-error-index/w/W0026.md"}
65
{"path":"app_a/src/app_a.erl","line":12,"char":1,"code":"ELP","severity":"warning","name":"L1230 (L1230)","original":null,"replacement":null,"description":"function bar/0 is unused\n\nFor more information see: /erlang-error-index/l/L1230","docPath":null}
76
{"path":"app_a/src/app_a.erl","line":16,"char":1,"code":"ELP","severity":"warning","name":"L1230 (L1230)","original":null,"replacement":null,"description":"function baz/2 is unused\n\nFor more information see: /erlang-error-index/l/L1230","docPath":null}
8-
{"path":"app_a/src/app_a_edoc.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
9-
{"path":"app_a/src/app_a_ssr.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
10-
{"path":"app_a/src/app_a_unused_param.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
117
{"path":"app_a/src/app_a_unused_param.erl","line":5,"char":5,"code":"ELP","severity":"warning","name":"L1268 (L1268)","original":null,"replacement":null,"description":"variable 'X' is unused\n\nFor more information see: /erlang-error-index/l/L1268","docPath":null}
128
{"path":"app_b/src/app_b.erl","line":5,"char":5,"code":"ELP","severity":"warning","name":"W0011 (application_get_env)","original":null,"replacement":null,"description":"module `app_b` belongs to app `app_b`, but reads env for `misc`\n\nFor more information see: /erlang-error-index/w/W0011","docPath":"website/docs/erlang-error-index/w/W0011.md"}
13-
{"path":"app_b/src/app_b.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
14-
{"path":"app_b/src/app_b_unused_param.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
159
{"path":"app_b/src/app_b_unused_param.erl","line":5,"char":5,"code":"ELP","severity":"warning","name":"L1268 (L1268)","original":null,"replacement":null,"description":"variable 'X' is unused\n\nFor more information see: /erlang-error-index/l/L1268","docPath":null}
1610
{"path":"app_a/src/custom_function_matches.erl","line":13,"char":5,"code":"ELP","severity":"warning","name":"W0017 (undefined_function)","original":null,"replacement":null,"description":"Function 'excluded:function/0' is undefined.\n\nFor more information see: /erlang-error-index/w/W0017","docPath":"website/docs/erlang-error-index/w/W0017.md"}
1711
{"path":"app_a/src/custom_function_matches.erl","line":14,"char":5,"code":"ELP","severity":"warning","name":"W0017 (undefined_function)","original":null,"replacement":null,"description":"Function 'not_excluded:function/0' is undefined.\n\nFor more information see: /erlang-error-index/w/W0017","docPath":"website/docs/erlang-error-index/w/W0017.md"}
1812
{"path":"app_a/src/custom_function_matches.erl","line":15,"char":5,"code":"ELP","severity":"warning","name":"W0017 (undefined_function)","original":null,"replacement":null,"description":"Function 'cross:call/0' is undefined.\n\nFor more information see: /erlang-error-index/w/W0017","docPath":"website/docs/erlang-error-index/w/W0017.md"}
19-
{"path":"app_a/src/expression_updates_literal.erl","line":2,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}
2013
{"path":"app_a/src/expression_updates_literal.erl","line":7,"char":1,"code":"ELP","severity":"warning","name":"L1309 (L1309)","original":null,"replacement":null,"description":"missing specification for function a_fun/0\n\nFor more information see: /erlang-error-index/l/L1309","docPath":null}
2114
{"path":"app_a/src/expression_updates_literal.erl","line":8,"char":7,"code":"ELP","severity":"warning","name":"L1318 (L1318)","original":null,"replacement":null,"description":"expression updates a literal\n\nFor more information see: /erlang-error-index/l/L1318","docPath":null}
2215
{"path":"app_a/src/spelling.erl","line":2,"char":2,"code":"ELP","severity":"error","name":"W0013 (misspelled_attribute)","original":null,"replacement":null,"description":"misspelled attribute, saw 'dyalizer' but expected 'dialyzer'\n\nFor more information see: /erlang-error-index/w/W0013","docPath":"website/docs/erlang-error-index/w/W0013.md"}
23-
{"path":"app_a/src/spelling.erl","line":1,"char":9,"code":"ELP","severity":"disabled","name":"W0046 (undocumented_module)","original":null,"replacement":null,"description":"The module is not documented.\n\nFor more information see: /erlang-error-index/w/W0046","docPath":"website/docs/erlang-error-index/w/W0046.md"}

‎crates/elp/src/resources/test/linter/parse_elp_no_lint_specified_output.stdout‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,24 @@
11
Reporting all diagnostics codes
2-
Diagnostics reported in 9 modules:
3-
app_a: 7
2+
Diagnostics reported in 7 modules:
3+
app_a: 6
44
4:4-4:34::[Warning] [W0011] module `app_a` belongs to app `app_a`, but reads env for `misc`
55
8:0-8:4::[Error] [P1700] head mismatch 'fooX' vs 'food'
66
7:6-7:7::[Warning] [W0018] Unexpected ';'
7-
0:8-0:13::[WeakWarning] [W0046] The module is not documented.
87
12:4-12:13::[Warning] [W0026] Function 'app_a:baz/2' is not exported.
98
11:0-11:3::[Warning] [L1230] function bar/0 is unused
109
15:0-15:3::[Warning] [L1230] function baz/2 is unused
11-
app_a_edoc: 1
12-
0:8-0:18::[WeakWarning] [W0046] The module is not documented.
13-
app_a_ssr: 1
14-
0:8-0:17::[WeakWarning] [W0046] The module is not documented.
15-
app_a_unused_param: 2
16-
0:8-0:26::[WeakWarning] [W0046] The module is not documented.
10+
app_a_unused_param: 1
1711
4:4-4:5::[Warning] [L1268] variable 'X' is unused
18-
app_b: 2
12+
app_b: 1
1913
4:4-4:34::[Warning] [W0011] module `app_b` belongs to app `app_b`, but reads env for `misc`
20-
0:8-0:13::[WeakWarning] [W0046] The module is not documented.
21-
app_b_unused_param: 2
22-
0:8-0:26::[WeakWarning] [W0046] The module is not documented.
14+
app_b_unused_param: 1
2315
4:4-4:5::[Warning] [L1268] variable 'X' is unused
2416
custom_function_matches: 3
2517
12:4-12:21::[Warning] [W0017] Function 'excluded:function/0' is undefined.
2618
13:4-13:25::[Warning] [W0017] Function 'not_excluded:function/0' is undefined.
2719
14:4-14:14::[Warning] [W0017] Function 'cross:call/0' is undefined.
28-
expression_updates_literal: 3
29-
1:8-1:34::[WeakWarning] [W0046] The module is not documented.
20+
expression_updates_literal: 2
3021
6:0-6:5::[Warning] [L1309] missing specification for function a_fun/0
3122
7:6-8:15::[Warning] [L1318] expression updates a literal
32-
spelling: 2
23+
spelling: 1
3324
1:1-1:9::[Error] [W0013] misspelled attribute, saw 'dyalizer' but expected 'dialyzer'
34-
0:8-0:16::[WeakWarning] [W0046] The module is not documented.

‎crates/elp/src/resources/test/linter/warnings_as_errors.stdout‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,24 @@
11
Reporting all diagnostics codes
2-
Diagnostics reported in 9 modules:
3-
app_a: 7
2+
Diagnostics reported in 7 modules:
3+
app_a: 6
44
4:4-4:34::[Warning] [W0011] module `app_a` belongs to app `app_a`, but reads env for `misc`
55
8:0-8:4::[Error] [P1700] head mismatch 'fooX' vs 'food'
66
7:6-7:7::[Warning] [W0018] Unexpected ';'
7-
0:8-0:13::[WeakWarning] [W0046] The module is not documented.
87
12:4-12:13::[Warning] [W0026] Function 'app_a:baz/2' is not exported.
98
11:0-11:3::[Error] [L1230] function bar/0 is unused
109
15:0-15:3::[Error] [L1230] function baz/2 is unused
11-
app_a_edoc: 1
12-
0:8-0:18::[WeakWarning] [W0046] The module is not documented.
13-
app_a_ssr: 1
14-
0:8-0:17::[WeakWarning] [W0046] The module is not documented.
15-
app_a_unused_param: 2
16-
0:8-0:26::[WeakWarning] [W0046] The module is not documented.
10+
app_a_unused_param: 1
1711
4:4-4:5::[Error] [L1268] variable 'X' is unused
18-
app_b: 2
12+
app_b: 1
1913
4:4-4:34::[Warning] [W0011] module `app_b` belongs to app `app_b`, but reads env for `misc`
20-
0:8-0:13::[WeakWarning] [W0046] The module is not documented.
21-
app_b_unused_param: 2
22-
0:8-0:26::[WeakWarning] [W0046] The module is not documented.
14+
app_b_unused_param: 1
2315
4:4-4:5::[Error] [L1268] variable 'X' is unused
2416
custom_function_matches: 3
2517
12:4-12:21::[Warning] [W0017] Function 'excluded:function/0' is undefined.
2618
13:4-13:25::[Warning] [W0017] Function 'not_excluded:function/0' is undefined.
2719
14:4-14:14::[Warning] [W0017] Function 'cross:call/0' is undefined.
28-
expression_updates_literal: 3
29-
1:8-1:34::[WeakWarning] [W0046] The module is not documented.
20+
expression_updates_literal: 2
3021
6:0-6:5::[Error] [L1309] missing specification for function a_fun/0
3122
7:6-8:15::[Error] [L1318] expression updates a literal
32-
spelling: 2
23+
spelling: 1
3324
1:1-1:9::[Error] [W0013] misspelled attribute, saw 'dyalizer' but expected 'dialyzer'
34-
0:8-0:16::[WeakWarning] [W0046] The module is not documented.

‎crates/ide/src/diagnostics.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1502,7 +1502,6 @@ pub fn diagnostics_descriptors<'a>() -> Vec<&'a DiagnosticDescriptor<'a>> {
15021502
&macro_precedence_suprise::DESCRIPTOR,
15031503
&undocumented_function::DESCRIPTOR,
15041504
&duplicate_module::DESCRIPTOR,
1505-
&undocumented_module::DESCRIPTOR,
15061505
&no_dialyzer_attribute::DESCRIPTOR,
15071506
&no_catch::DESCRIPTOR,
15081507
&no_nowarn_suppressions::DESCRIPTOR,
@@ -1584,6 +1583,7 @@ const SSR_PATTERN_LINTERS: &[&dyn SsrPatternsDiagnostics] = &[
15841583
const GENERIC_LINTERS: &[&dyn GenericDiagnostics] = &[
15851584
&unused_macro::LINTER,
15861585
&missing_compile_warn_missing_spec::LINTER,
1586+
&undocumented_module::LINTER,
15871587
];
15881588

15891589
/// Unified registry for all types of linters

‎crates/ide/src/diagnostics/undocumented_module.rs‎

Lines changed: 77 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -14,62 +14,89 @@ use elp_ide_db::elp_base_db::FileId;
1414
use elp_ide_db::source_change::SourceChangeBuilder;
1515
use elp_syntax::AstNode;
1616
use elp_text_edit::TextRange;
17-
use elp_text_edit::TextSize;
1817
use hir::Semantic;
1918

20-
use super::Diagnostic;
21-
use super::DiagnosticCode;
22-
use super::DiagnosticConditions;
23-
use super::DiagnosticDescriptor;
24-
use super::Severity;
25-
26-
const DIAGNOSTIC_CODE: DiagnosticCode = DiagnosticCode::UndocumentedModule;
27-
const DIAGNOSTIC_MESSAGE: &str = "The module is not documented.";
28-
const DIAGNOSTIC_SEVERITY: Severity = Severity::WeakWarning;
29-
const FIX_ID: &str = "add_moduledoc_false";
30-
const FIX_LABEL: &str = "Add `-moduledoc false.` attribute";
31-
32-
pub(crate) static DESCRIPTOR: DiagnosticDescriptor = DiagnosticDescriptor {
33-
conditions: DiagnosticConditions {
34-
experimental: false,
35-
include_generated: false,
36-
include_tests: false,
37-
default_disabled: true,
38-
},
39-
checker: &|diags, sema, file_id, _ext| {
40-
check(diags, sema, file_id);
41-
},
42-
};
43-
44-
fn check(diagnostics: &mut Vec<Diagnostic>, sema: &Semantic, file_id: FileId) -> Option<()> {
45-
let def_map = sema.def_map_local(file_id);
46-
let module_attribute = sema.module_attribute(file_id)?;
47-
let module_has_no_docs = def_map.moduledoc.is_empty()
48-
&& def_map.moduledoc_metadata.is_empty()
49-
&& sema
50-
.module_edoc_header(file_id, &module_attribute)
51-
.is_none();
52-
if module_has_no_docs {
53-
let module_name = module_attribute.name()?;
54-
let module_name_range = module_name.syntax().text_range();
55-
let moduledoc_insert_offset = helpers::moduledoc_insert_offset(sema, file_id)?;
56-
let fixes = fixes(file_id, moduledoc_insert_offset, module_name_range);
57-
let diagnostic = Diagnostic::new(DIAGNOSTIC_CODE, DIAGNOSTIC_MESSAGE, module_name_range)
58-
.with_severity(DIAGNOSTIC_SEVERITY)
59-
.with_fixes(Some(fixes));
60-
diagnostics.push(diagnostic);
61-
}
62-
Some(())
19+
use crate::diagnostics::DiagnosticCode;
20+
use crate::diagnostics::GenericLinter;
21+
use crate::diagnostics::GenericLinterMatchContext;
22+
use crate::diagnostics::Linter;
23+
use crate::diagnostics::Severity;
24+
use crate::fix;
25+
26+
pub(crate) struct UndocumentedModuleLinter;
27+
28+
impl Linter for UndocumentedModuleLinter {
29+
fn id(&self) -> DiagnosticCode {
30+
DiagnosticCode::UndocumentedModule
31+
}
32+
33+
fn description(&self) -> &'static str {
34+
"The module is not documented."
35+
}
36+
37+
fn severity(&self) -> Severity {
38+
Severity::WeakWarning
39+
}
40+
41+
fn is_enabled(&self) -> bool {
42+
false
43+
}
44+
45+
fn should_process_test_files(&self) -> bool {
46+
false
47+
}
48+
}
49+
50+
#[derive(Debug, Default, Clone, PartialEq, Eq)]
51+
pub struct Context {
52+
module_name_range: TextRange,
6353
}
6454

65-
fn fixes(file_id: FileId, insert_offset: TextSize, show_range: TextRange) -> Vec<Assist> {
66-
let mut builder = SourceChangeBuilder::new(file_id);
67-
builder.insert(insert_offset, "-moduledoc false.\n");
68-
let source_change = builder.finish();
69-
let fix = crate::fix(FIX_ID, FIX_LABEL, source_change, show_range);
70-
vec![fix]
55+
impl GenericLinter for UndocumentedModuleLinter {
56+
type Context = Context;
57+
58+
fn matches(
59+
&self,
60+
sema: &Semantic,
61+
file_id: FileId,
62+
) -> Option<Vec<GenericLinterMatchContext<Context>>> {
63+
let mut res = Vec::new();
64+
let def_map = sema.def_map_local(file_id);
65+
let module_attribute = sema.module_attribute(file_id)?;
66+
let module_has_no_docs = def_map.moduledoc.is_empty()
67+
&& def_map.moduledoc_metadata.is_empty()
68+
&& sema
69+
.module_edoc_header(file_id, &module_attribute)
70+
.is_none();
71+
if module_has_no_docs {
72+
let module_name = module_attribute.name()?;
73+
let module_name_range = module_name.syntax().text_range();
74+
let context = Context { module_name_range };
75+
res.push(GenericLinterMatchContext {
76+
range: module_name_range,
77+
context,
78+
});
79+
}
80+
Some(res)
81+
}
82+
83+
fn fixes(&self, context: &Context, sema: &Semantic, file_id: FileId) -> Option<Vec<Assist>> {
84+
let insert_offset = helpers::moduledoc_insert_offset(sema, file_id)?;
85+
let mut builder = SourceChangeBuilder::new(file_id);
86+
builder.insert(insert_offset, "-moduledoc false.\n");
87+
let source_change = builder.finish();
88+
let fix = fix(
89+
"add_moduledoc_false",
90+
"Add `-moduledoc false.` attribute",
91+
source_change,
92+
context.module_name_range,
93+
);
94+
Some(vec![fix])
95+
}
7196
}
7297

98+
pub static LINTER: UndocumentedModuleLinter = UndocumentedModuleLinter;
99+
73100
#[cfg(test)]
74101
mod tests {
75102

0 commit comments

Comments
 (0)