Skip to content

Commit 4ad88f1

Browse files
SysixSysix
committed
feat(language_server): respect disable directives for type-aware rules
1 parent 039883c commit 4ad88f1

File tree

6 files changed

+198
-34
lines changed

6 files changed

+198
-34
lines changed

‎crates/oxc_language_server/fixtures/linter/tsgolint/unused_disabled_directives/.oxlintrc.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"rules": {
3+
"no-unused-expressions": "off",
4+
"@typescript-eslint/no-floating-promises": "error"
5+
}
6+
}

‎crates/oxc_language_server/fixtures/linter/tsgolint/unused_disabled_directives/test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Test file for disable directives with type-aware rules
2+
const myPromise = new Promise(() => {})
3+
4+
// Test oxlint-disable-next-line
5+
// oxlint-disable-next-line typescript/no-floating-promises
6+
myPromise
7+
8+
// Test eslint-disable-next-line with @typescript-eslint prefix
9+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
10+
myPromise
11+
12+
// Test oxlint-disable/enable block
13+
/* oxlint-disable typescript/no-floating-promises */
14+
myPromise
15+
myPromise
16+
/* oxlint-enable typescript/no-floating-promises */
17+
18+
// This should still report an error (no disable directive)
19+
myPromise
20+
21+
// Test with different rule name formats
22+
// oxlint-disable-next-line no-floating-promises
23+
myPromise
24+
25+
// eslint-disable-next-line typescript-eslint/no-floating-promises
26+
myPromise
27+
28+
// Test disable-line variant
29+
myPromise // oxlint-disable-line typescript/no-floating-promises
30+
31+
// Multiple promises in one block
32+
/* eslint-disable @typescript-eslint/no-floating-promises */
33+
Promise.resolve(1)
34+
Promise.resolve(2)
35+
Promise.resolve(3)
36+
/* eslint-enable @typescript-eslint/no-floating-promises */
37+
38+
// Should report error
39+
Promise.resolve(4)

‎crates/oxc_language_server/src/linter/server_linter.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,19 @@ mod test {
460460
.test_and_snapshot_single_file("test.js");
461461
}
462462

463+
#[test]
464+
fn test_report_tsgolint_unused_directives() {
465+
Tester::new(
466+
"fixtures/linter/tsgolint/unused_disabled_directives",
467+
Some(LintOptions {
468+
unused_disable_directives: UnusedDisableDirectives::Deny,
469+
type_aware: true,
470+
..Default::default()
471+
}),
472+
)
473+
.test_and_snapshot_single_file("test.ts");
474+
}
475+
463476
#[test]
464477
fn test_root_ignore_patterns() {
465478
let tester = Tester::new("fixtures/linter/ignore_patterns", None);

‎crates/oxc_language_server/src/snapshots/fixtures_linter_tsgolint_unused_disabled_directives@test.ts.snap‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
---
2+
source: crates/oxc_language_server/src/tester.rs
3+
---
4+
##########
5+
file: fixtures/linter/tsgolint/unused_disabled_directives/test.ts
6+
----------
7+
8+
code: "typescript-eslint(no-floating-promises)"
9+
code_description.href: "None"
10+
message: "Promises must be awaited.\nhelp: The promise must end with a call to .catch, or end with a call to .then with a rejection handler, or be explicitly marked as ignored with the `void` operator."
11+
range: Range { start: Position { line: 18, character: 0 }, end: Position { line: 18, character: 9 } }
12+
related_information[0].message: ""
13+
related_information[0].location.uri: "file://<variable>/fixtures/linter/tsgolint/unused_disabled_directives/test.ts"
14+
related_information[0].location.range: Range { start: Position { line: 18, character: 0 }, end: Position { line: 18, character: 9 } }
15+
severity: Some(Error)
16+
source: Some("oxc")
17+
tags: None
18+
fixed: Multiple(
19+
[
20+
FixedContent {
21+
message: Some(
22+
"Disable no-floating-promises for this line",
23+
),
24+
code: "// oxlint-disable-next-line no-floating-promises\n",
25+
range: Range {
26+
start: Position {
27+
line: 18,
28+
character: 0,
29+
},
30+
end: Position {
31+
line: 18,
32+
character: 0,
33+
},
34+
},
35+
},
36+
FixedContent {
37+
message: Some(
38+
"Disable no-floating-promises for this file",
39+
),
40+
code: "// oxlint-disable no-floating-promises\n",
41+
range: Range {
42+
start: Position {
43+
line: 0,
44+
character: 0,
45+
},
46+
end: Position {
47+
line: 0,
48+
character: 0,
49+
},
50+
},
51+
},
52+
],
53+
)
54+
55+
56+
code: "typescript-eslint(no-floating-promises)"
57+
code_description.href: "None"
58+
message: "Promises must be awaited.\nhelp: The promise must end with a call to .catch, or end with a call to .then with a rejection handler, or be explicitly marked as ignored with the `void` operator."
59+
range: Range { start: Position { line: 38, character: 0 }, end: Position { line: 38, character: 18 } }
60+
related_information[0].message: ""
61+
related_information[0].location.uri: "file://<variable>/fixtures/linter/tsgolint/unused_disabled_directives/test.ts"
62+
related_information[0].location.range: Range { start: Position { line: 38, character: 0 }, end: Position { line: 38, character: 18 } }
63+
severity: Some(Error)
64+
source: Some("oxc")
65+
tags: None
66+
fixed: Multiple(
67+
[
68+
FixedContent {
69+
message: Some(
70+
"Disable no-floating-promises for this line",
71+
),
72+
code: "// oxlint-disable-next-line no-floating-promises\n",
73+
range: Range {
74+
start: Position {
75+
line: 38,
76+
character: 0,
77+
},
78+
end: Position {
79+
line: 38,
80+
character: 0,
81+
},
82+
},
83+
},
84+
FixedContent {
85+
message: Some(
86+
"Disable no-floating-promises for this file",
87+
),
88+
code: "// oxlint-disable no-floating-promises\n",
89+
range: Range {
90+
start: Position {
91+
line: 0,
92+
character: 0,
93+
},
94+
end: Position {
95+
line: 0,
96+
character: 0,
97+
},
98+
},
99+
},
100+
],
101+
)

‎crates/oxc_linter/src/lint_runner.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,8 @@ impl LintRunner {
244244
let mut messages = self.lint_service.run_source();
245245

246246
if let Some(type_aware_linter) = &self.type_aware_linter
247-
&& let Ok(tso_messages) = type_aware_linter.lint_source(file, source_text)
247+
&& let Ok(tso_messages) =
248+
type_aware_linter.lint_source(file, source_text, self.directives_store.map())
248249
{
249250
messages.extend(tso_messages);
250251
}

‎crates/oxc_linter/src/tsgolint.rs‎

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -202,39 +202,11 @@ impl TsGoLintState {
202202
continue;
203203
};
204204

205-
let span = Span::new(
206-
tsgolint_diagnostic.range.pos,
207-
tsgolint_diagnostic.range.end,
208-
);
209-
210-
let should_skip = {
211-
if let Some(directives) = disable_directives_map.get(&path)
212-
{
213-
directives.contains(&tsgolint_diagnostic.rule, span)
214-
|| directives.contains(
215-
&format!(
216-
"typescript-eslint/{}",
217-
tsgolint_diagnostic.rule
218-
),
219-
span,
220-
)
221-
|| directives.contains(
222-
&format!(
223-
"@typescript-eslint/{}",
224-
tsgolint_diagnostic.rule
225-
),
226-
span,
227-
)
228-
} else {
229-
debug_assert!(
230-
false,
231-
"disable_directives_map should have an entry for every file we linted"
232-
);
233-
false
234-
}
235-
};
236-
237-
if should_skip {
205+
if should_skip_diagnostic(
206+
&disable_directives_map,
207+
&path,
208+
&tsgolint_diagnostic,
209+
) {
238210
continue;
239211
}
240212

@@ -336,6 +308,7 @@ impl TsGoLintState {
336308
&self,
337309
path: &Arc<OsStr>,
338310
source_text: String,
311+
disable_directives_map: Arc<Mutex<FxHashMap<PathBuf, DisableDirectives>>>,
339312
) -> Result<Vec<Message>, String> {
340313
let mut resolved_configs: FxHashMap<PathBuf, ResolvedLinterState> = FxHashMap::default();
341314
let mut source_overrides = FxHashMap::default();
@@ -394,6 +367,8 @@ impl TsGoLintState {
394367
let stdout = child.stdout.take().expect("Failed to open tsgolint stdout");
395368

396369
let stdout_handler = std::thread::spawn(move || -> Result<Vec<Message>, String> {
370+
let disable_directives_map =
371+
disable_directives_map.lock().expect("disable_directives_map mutex poisoned");
397372
let msg_iter = TsGoLintMessageStream::new(stdout);
398373

399374
let mut result = vec![];
@@ -426,6 +401,14 @@ impl TsGoLintState {
426401
continue;
427402
};
428403

404+
if should_skip_diagnostic(
405+
&disable_directives_map,
406+
&path,
407+
&tsgolint_diagnostic,
408+
) {
409+
continue;
410+
}
411+
429412
let mut message = Message::from_tsgo_lint_diagnostic(
430413
tsgolint_diagnostic,
431414
&source_text,
@@ -880,6 +863,27 @@ impl std::fmt::Display for TsGoLintMessageParseError {
880863
}
881864
}
882865

866+
fn should_skip_diagnostic(
867+
disable_directives_map: &FxHashMap<PathBuf, DisableDirectives>,
868+
path: &Path,
869+
tsgolint_diagnostic: &TsGoLintRuleDiagnostic,
870+
) -> bool {
871+
let span = Span::new(tsgolint_diagnostic.range.pos, tsgolint_diagnostic.range.end);
872+
873+
if let Some(directives) = disable_directives_map.get(path) {
874+
directives.contains(&tsgolint_diagnostic.rule, span)
875+
|| directives.contains(&format!("typescript-eslint/{}", tsgolint_diagnostic.rule), span)
876+
|| directives
877+
.contains(&format!("@typescript-eslint/{}", tsgolint_diagnostic.rule), span)
878+
} else {
879+
debug_assert!(
880+
false,
881+
"disable_directives_map should have an entry for every file we linted"
882+
);
883+
false
884+
}
885+
}
886+
883887
/// Parses a single message from the binary tsgolint output.
884888
// Messages are encoded as follows:
885889
// | Payload Size (uint32 LE) - 4 bytes | Message Type (uint8) - 1 byte | Payload |

0 commit comments

Comments
 (0)