feat(linter): make disable directives work for type aware rules

Cargo.toml

@@ -178,6 +178,7 @@ constcat = "0.6.1"
 convert_case = "0.8.0"
 cow-utils = "0.1.3"
 criterion2 = { version = "3.0.2", default-features = false }
+dashmap = "6.1.0"
 dragonbox_ecma = "0.0.5"
 encoding_rs = "0.8.35"
 encoding_rs_io = "0.1.7"

apps/oxlint/Cargo.toml

@@ -28,6 +28,7 @@ oxc_span = { workspace = true }
 
 bpaf = { workspace = true, features = ["autocomplete", "bright-color", "derive"] }
 cow-utils = { workspace = true }
+dashmap = { workspace = true }
 ignore = { workspace = true, features = ["simd-accel"] }
 miette = { workspace = true }
 napi = { workspace = true, features = ["async"], optional = true }

apps/oxlint/fixtures/tsgolint_disable_directives/test.ts

@@ -0,0 +1,39 @@
+// Test file for disable directives with type-aware rules
+const myPromise = new Promise(() => {})
+
+// Test oxlint-disable-next-line
+// oxlint-disable-next-line typescript/no-floating-promises
+myPromise
+
+// Test eslint-disable-next-line with @typescript-eslint prefix
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+myPromise
+
+// Test oxlint-disable/enable block
+/* oxlint-disable typescript/no-floating-promises */
+myPromise
+myPromise
+/* oxlint-enable typescript/no-floating-promises */
+
+// This should still report an error (no disable directive)
+myPromise
+
+// Test with different rule name formats
+// oxlint-disable-next-line no-floating-promises
+myPromise
+
+// eslint-disable-next-line typescript-eslint/no-floating-promises
+myPromise
+
+// Test disable-line variant
+myPromise // oxlint-disable-line typescript/no-floating-promises
+
+// Multiple promises in one block
+/* eslint-disable @typescript-eslint/no-floating-promises */
+Promise.resolve(1)
+Promise.resolve(2)
+Promise.resolve(3)
+/* eslint-enable @typescript-eslint/no-floating-promises */
+
+// Should report error
+Promise.resolve(4)

apps/oxlint/fixtures/tsgolint_disable_directives/tsconfig.json

@@ -0,0 +1,10 @@
+{
+  "compilerOptions": {
+    "target": "es2020",
+    "module": "esnext",
+    "strict": true,
+    "esModuleInterop": true,
+    "skipLibCheck": true,
+    "forceConsistentCasingInFileNames": true
+  }
+}

apps/oxlint/fixtures/tsgolint_disable_directives/unused.ts

@@ -0,0 +1,24 @@
+// Test unused disable directives with type-aware rules
+const myPromise = new Promise(() => {})
+
+// This disable directive is USED (suppresses the error below)
+// oxlint-disable-next-line typescript/no-floating-promises
+myPromise
+
+// This disable directive is UNUSED (no error to suppress)
+// oxlint-disable-next-line typescript/no-floating-promises
+console.log("hello")
+
+// This disable directive is UNUSED (wrong rule name)
+// oxlint-disable-next-line typescript/no-unsafe-assignment
+myPromise
+
+// This disable directive is USED
+/* eslint-disable @typescript-eslint/no-floating-promises */
+myPromise
+/* eslint-enable @typescript-eslint/no-floating-promises */
+
+// This disable directive is UNUSED (no floating promise here)
+/* eslint-disable @typescript-eslint/no-floating-promises */
+const x = 1 + 2
+/* eslint-enable @typescript-eslint/no-floating-promises */

apps/oxlint/src/lint.rs

@@ -311,9 +311,12 @@ impl LintRunner {
             .filter(|path| !ignore_matcher.should_ignore(Path::new(path)))
             .collect::<Vec<Arc<OsStr>>>();
 
+        let linter_report_unused =
+            if self.options.type_aware { None } else { report_unused_directives };
+
         let linter = Linter::new(LintOptions::default(), config_store.clone(), external_linter)
             .with_fix(fix_options.fix_kind())
-            .with_report_unused_directives(report_unused_directives);
+            .with_report_unused_directives(linter_report_unused);
 
         let number_of_files = files_to_lint.len();
 
@@ -340,11 +343,13 @@ impl LintRunner {
 
         let cwd = options.cwd().to_path_buf();
 
-        // Spawn linting in another thread so diagnostics can be printed immediately from diagnostic_service.run.
-        {
+        let disable_directives_map = std::sync::Arc::new(dashmap::DashMap::new());
+
+        let regular_lint_handle = {
             let tx_error = tx_error.clone();
             let files_to_lint = files_to_lint.clone();
-            rayon::spawn(move || {
+            let disable_directives_map_clone = disable_directives_map.clone();
+            std::thread::spawn(move || {
                 let has_external_linter = linter.has_external_linter();
 
                 let mut lint_service = LintService::new(linter, options);
@@ -371,20 +376,88 @@ impl LintRunner {
                     );
                 }
 
+                lint_service.set_disable_directives_map(disable_directives_map_clone);
                 lint_service.run(&tx_error);
-            });
-        }
+            })
+        };
 
         // Run type-aware linting through tsgolint
         // TODO: Add a warning message if `tsgolint` cannot be found, but type-aware rules are enabled
         if self.options.type_aware {
-            if let Err(err) = TsGoLintState::try_new(&cwd, config_store).and_then(|state| {
-                state.with_silent(misc_options.silent).lint(&files_to_lint, tx_error)
-            }) {
+            regular_lint_handle.join().unwrap();
+            if let Err(err) =
+                TsGoLintState::try_new(&cwd, config_store, disable_directives_map.clone()).and_then(
+                    |state| state.with_silent(misc_options.silent).lint(&files_to_lint, tx_error),
+                )
+            {
                 print_and_flush_stdout(stdout, &err);
                 return CliRunResult::TsGoLintError;
             }
+
+            if let Some(severity) = report_unused_directives {
+                for entry in disable_directives_map.iter() {
+                    let (path, directives) = (entry.key(), entry.value());
+                    let unused = directives.collect_unused_disable_comments();
+                    if !unused.is_empty() {
+                        use oxc_linter::RuleCommentType;
+
+                        let mut diagnostics = Vec::new();
+                        let message_for_disable =
+                            "Unused eslint-disable directive (no problems were reported).";
+
+                        for unused_comment in unused {
+                            let span = unused_comment.span;
+                            match unused_comment.r#type {
+                                RuleCommentType::All => {
+                                    diagnostics.push(
+                                        OxcDiagnostic::warn(message_for_disable)
+                                            .with_label(span)
+                                            .with_severity(if severity == AllowWarnDeny::Deny {
+                                                oxc_diagnostics::Severity::Error
+                                            } else {
+                                                oxc_diagnostics::Severity::Warning
+                                            }),
+                                    );
+                                }
+                                RuleCommentType::Single(rules) => {
+                                    for rule in rules {
+                                        let rule_message = format!(
+                                            "Unused eslint-disable directive (no problems were reported from {}).",
+                                            rule.rule_name
+                                        );
+                                        diagnostics.push(
+                                            OxcDiagnostic::warn(rule_message)
+                                                .with_label(rule.name_span)
+                                                .with_severity(
+                                                    if severity == AllowWarnDeny::Deny {
+                                                        oxc_diagnostics::Severity::Error
+                                                    } else {
+                                                        oxc_diagnostics::Severity::Warning
+                                                    },
+                                                ),
+                                        );
+                                    }
+                                }
+                            }
+                        }
+
+                        if !diagnostics.is_empty() {
+                            let source_text =
+                                std::fs::read_to_string(path.as_path()).unwrap_or_default();
+                            let wrapped = oxc_diagnostics::DiagnosticService::wrap_diagnostics(
+                                &cwd,
+                                path.clone(),
+                                &source_text,
+                                diagnostics,
+                            );
+                            tx_error.send((path.clone(), wrapped)).unwrap();
+                        }
+                    }
+                }
+            }
+            drop(tx_error);
         } else {
+            regular_lint_handle.join().unwrap();
             drop(tx_error);
         }
 
@@ -1308,4 +1381,24 @@ mod test {
         let args = &["--type-aware", "test.svelte"];
         Tester::new().with_cwd("fixtures/tsgolint".into()).test_and_snapshot(args);
     }
+
+    #[cfg(not(target_endian = "big"))]
+    #[test]
+    fn test_tsgolint_unused_disable_directives() {
+        // Test that unused disable directives are reported with type-aware rules
+        let args = &["--type-aware", "--report-unused-disable-directives", "unused.ts"];
+        Tester::new()
+            .with_cwd("fixtures/tsgolint_disable_directives".into())
+            .test_and_snapshot(args);
+    }
+
+    #[cfg(not(target_endian = "big"))]
+    #[test]
+    fn test_tsgolint_disable_directives() {
+        // Test that disable directives work with type-aware rules
+        let args = &["--type-aware", "test.ts"];
+        Tester::new()
+            .with_cwd("fixtures/tsgolint_disable_directives".into())
+            .test_and_snapshot(args);
+    }
 }

apps/oxlint/src/snapshots/_--type-aware test.ts@oxlint.snap

@@ -0,0 +1,12 @@
+---
+source: apps/oxlint/src/tester.rs
+---
+########## 
+arguments: --type-aware test.ts
+working directory: 
+----------
+Found 0 warnings and 0 errors.
+Finished in <variable>ms on 0 files with 102 rules using 1 threads.
+----------
+CLI result: LintSucceeded
+----------

apps/oxlint/src/snapshots/fixtures__tsgolint_disable_directives_--type-aware --report-unused-disable-directives unused.ts@oxlint.snap

@@ -0,0 +1,55 @@
+---
+source: apps/oxlint/src/tester.rs
+---
+########## 
+arguments: --type-aware --report-unused-disable-directives unused.ts
+working directory: fixtures/tsgolint_disable_directives
+----------
+
+  ! Unused eslint-disable directive (no problems were reported).
+    ,-[unused.ts:9:3]
+  8 | // This disable directive is UNUSED (no error to suppress)
+  9 | // oxlint-disable-next-line typescript/no-floating-promises
+    :   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ 10 | console.log("hello")
+    `----
+
+  ! Unused eslint-disable directive (no problems were reported).
+    ,-[unused.ts:13:3]
+ 12 | // This disable directive is UNUSED (wrong rule name)
+ 13 | // oxlint-disable-next-line typescript/no-unsafe-assignment
+    :   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ 14 | myPromise
+    `----
+
+  ! typescript-eslint(no-floating-promises): Promises must be awaited.
+    ,-[unused.ts:14:1]
+ 13 | // oxlint-disable-next-line typescript/no-unsafe-assignment
+ 14 | myPromise
+    : ^^^^^^^^^
+ 15 | 
+    `----
+  help: 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.
+
+  ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html\eslint(no-unused-vars)]8;;\: Variable 'x' is declared but never used. Unused variables should start with a '_'.
+    ,-[unused.ts:23:7]
+ 22 | /* eslint-disable @typescript-eslint/no-floating-promises */
+ 23 | const x = 1 + 2
+    :       |
+    :       `-- 'x' is declared here
+ 24 | /* eslint-enable @typescript-eslint/no-floating-promises */
+    `----
+  help: Consider removing this declaration.
+
+  ! Unused eslint-disable directive (no problems were reported).
+    ,-[unused.ts:24:3]
+ 23 | const x = 1 + 2
+ 24 | /* eslint-enable @typescript-eslint/no-floating-promises */
+    :   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    `----
+
+Found 5 warnings and 0 errors.
+Finished in <variable>ms on 1 file with 102 rules using 1 threads.
+----------
+CLI result: LintSucceeded
+----------

apps/oxlint/src/snapshots/fixtures__tsgolint_disable_directives_--type-aware test.ts@oxlint.snap

@@ -0,0 +1,30 @@
+---
+source: apps/oxlint/src/tester.rs
+---
+########## 
+arguments: --type-aware test.ts
+working directory: fixtures/tsgolint_disable_directives
+----------
+
+  ! typescript-eslint(no-floating-promises): Promises must be awaited.
+    ,-[test.ts:19:1]
+ 18 | // This should still report an error (no disable directive)
+ 19 | myPromise
+    : ^^^^^^^^^
+ 20 | 
+    `----
+  help: 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.
+
+  ! typescript-eslint(no-floating-promises): Promises must be awaited.
+    ,-[test.ts:39:1]
+ 38 | // Should report error
+ 39 | Promise.resolve(4)
+    : ^^^^^^^^^^^^^^^^^^
+    `----
+  help: 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.
+
+Found 2 warnings and 0 errors.
+Finished in <variable>ms on 1 file with 102 rules using 1 threads.
+----------
+CLI result: LintSucceeded
+----------

crates/oxc_language_server/Cargo.toml

@@ -30,6 +30,7 @@ oxc_linter = { workspace = true, features = ["language_server"] }
 oxc_parser = { workspace = true }
 
 #
+dashmap = { workspace = true }
 env_logger = { workspace = true, features = ["humantime"] }
 futures = { workspace = true }
 ignore = { workspace = true, features = ["simd-accel"] }