feat(language_server): add unusedDisableDirectives option (#11645)

closes #11618

VSCode users are now able to set the severity of unused disabled directives via `oxc.unusedDisableDirectives`.
Other Clients can use this options too, by passing `unusedDisableDirectives` as a new config key.

![screenshot of vscode editor showing the new reporting error](https://github.com/user-attachments/assets/7581db72-7756-4f55-9df1-e873b3ca0f59)

Author: Sysix

crates/oxc_language_server/README.md

@@ -19,11 +19,12 @@ This crate provides an [LSP](https://microsoft.github.io/language-server-protoco
 
 These options can be passed with [initialize](#initialize), [workspace/didChangeConfiguration](#workspace/didChangeConfiguration) and [workspace/configuration](#workspace/configuration).
 
-| Option Key   | Value(s)               | Default    | Description                                                                                          |
-| ------------ | ---------------------- | ---------- | ---------------------------------------------------------------------------------------------------- |
-| `run`        | `"onSave" \| "onType"` | `"onType"` | Should the server lint the files when the user is typing or saving                                   |
-| `configPath` | `<string>` \| `null`   | `null`     | Path to a oxlint configuration file, passing a string will disable nested configuration              |
-| `flags`      | `Map<string, string>`  | `<empty>`  | Special oxc language server flags, currently only one flag key is supported: `disable_nested_config` |
+| Option Key                | Value(s)                       | Default    | Description                                                                                                                                 |
+| ------------------------- | ------------------------------ | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------- |
+| `run`                     | `"onSave" \| "onType"`         | `"onType"` | Should the server lint the files when the user is typing or saving                                                                          |
+| `configPath`              | `<string>` \| `null`           | `null`     | Path to a oxlint configuration file, passing a string will disable nested configuration                                                     |
+| `unusedDisableDirectives` | `"allow" \| "warn"` \| "deny"` | `"allow"`  | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway |
+| `flags`                   | `Map<string, string>`          | `<empty>`  | Special oxc language server flags, currently only one flag key is supported: `disable_nested_config`                                        |
 
 ## Supported LSP Specifications from Server
 
@@ -39,6 +40,7 @@ The client can pass the workspace options like following:
     "options": {
       "run": "onType",
       "configPath": null,
+      "unusedDisableDirectives": "allow",
       "flags": {}
     }
   }]
@@ -72,6 +74,7 @@ The client can pass the workspace options like following:
     "options": {
       "run": "onType",
       "configPath": null,
+      "unusedDisableDirectives": "allow",
       "flags": {}
     }
   }]
@@ -155,11 +158,10 @@ Only will be requested when the `ClientCapabilities` has `workspace.configuratio
 The client can return a response like:
 
 ```json
-{
-    [{
-        "run": "onType",
-        "configPath": null,
-        "flags": {}
-    }]
-}
+[{
+  "run": "onType",
+  "configPath": null,
+  "unusedDisableDirectives": "allow",
+  "flags": {}
+}]
 ```

crates/oxc_language_server/fixtures/linter/unused_disabled_directives/test.js

@@ -0,0 +1,4 @@
+// oxlint-disable-next-line no-debugger -- on wrong line
+(() => {
+  debugger;
+})();

crates/oxc_language_server/src/linter/server_linter.rs

@@ -7,13 +7,16 @@ use log::{debug, warn};
 use rustc_hash::{FxBuildHasher, FxHashMap};
 use tower_lsp_server::lsp_types::Uri;
 
-use oxc_linter::{Config, ConfigStore, ConfigStoreBuilder, LintOptions, Linter, Oxlintrc};
+use oxc_linter::{
+    AllowWarnDeny, Config, ConfigStore, ConfigStoreBuilder, LintOptions, Linter, Oxlintrc,
+};
 use tower_lsp_server::UriExt;
 
 use crate::linter::{
     error_with_position::DiagnosticReport,
     isolated_lint_handler::{IsolatedLintHandler, IsolatedLintHandlerOptions},
 };
+use crate::options::UnusedDisableDirectives;
 use crate::{ConcurrentHashMap, Options};
 
 use super::config_walker::ConfigWalker;
@@ -65,7 +68,15 @@ impl ServerLinter {
         extended_paths.extend(config_builder.extended_paths.clone());
         let base_config = config_builder.build();
 
-        let lint_options = LintOptions { fix: options.fix_kind(), ..Default::default() };
+        let lint_options = LintOptions {
+            fix: options.fix_kind(),
+            report_unused_directive: match options.unused_disable_directives {
+                UnusedDisableDirectives::Allow => None, // or AllowWarnDeny::Allow, should be the same?
+                UnusedDisableDirectives::Warn => Some(AllowWarnDeny::Warn),
+                UnusedDisableDirectives::Deny => Some(AllowWarnDeny::Deny),
+            },
+            ..Default::default()
+        };
 
         let config_store = ConfigStore::new(
             base_config,
@@ -358,4 +369,18 @@ mod test {
         )
         .test_and_snapshot_single_file("forward_ref.ts");
     }
+
+    #[test]
+    fn test_report_unused_directives() {
+        use crate::options::UnusedDisableDirectives;
+        Tester::new(
+            "fixtures/linter/unused_disabled_directives",
+            Some(Options {
+                unused_disable_directives: UnusedDisableDirectives::Deny,
+                ..Default::default()
+            }),
+        )
+        // ToDo: this should be fixable
+        .test_and_snapshot_single_file("test.js");
+    }
 }

crates/oxc_language_server/src/options.rs

@@ -13,11 +13,21 @@ pub enum Run {
     OnType,
 }
 
+#[derive(Debug, Serialize, Deserialize, Default, PartialEq, Eq, Clone, Copy)]
+#[serde(rename_all = "camelCase")]
+pub enum UnusedDisableDirectives {
+    #[default]
+    Allow,
+    Warn,
+    Deny,
+}
+
 #[derive(Debug, Default, Serialize, Clone)]
 #[serde(rename_all = "camelCase")]
 pub struct Options {
     pub run: Run,
     pub config_path: Option<String>,
+    pub unused_disable_directives: UnusedDisableDirectives,
     pub flags: FxHashMap<String, String>,
 }
 
@@ -79,6 +89,13 @@ impl TryFrom<Value> for Options {
                 .get("run")
                 .map(|run| serde_json::from_value::<Run>(run.clone()).unwrap_or_default())
                 .unwrap_or_default(),
+            unused_disable_directives: object
+                .get("unusedDisableDirectives")
+                .map(|key| {
+                    serde_json::from_value::<UnusedDisableDirectives>(key.clone())
+                        .unwrap_or_default()
+                })
+                .unwrap_or_default(),
             config_path: object
                 .get("configPath")
                 .and_then(|config_path| serde_json::from_value::<String>(config_path.clone()).ok()),
@@ -99,13 +116,14 @@ mod test {
     use rustc_hash::FxHashMap;
     use serde_json::json;
 
-    use super::{Options, Run, WorkspaceOption};
+    use super::{Options, Run, UnusedDisableDirectives, WorkspaceOption};
 
     #[test]
     fn test_valid_options_json() {
         let json = json!({
             "run": "onSave",
             "configPath": "./custom.json",
+            "unusedDisableDirectives": "warn",
             "flags": {
                 "disable_nested_config": "true",
                 "fix_kind": "dangerous_fix"
@@ -115,6 +133,7 @@ mod test {
         let options = Options::try_from(json).unwrap();
         assert_eq!(options.run, Run::OnSave);
         assert_eq!(options.config_path, Some("./custom.json".into()));
+        assert_eq!(options.unused_disable_directives, UnusedDisableDirectives::Warn);
         assert_eq!(options.flags.get("disable_nested_config"), Some(&"true".to_string()));
         assert_eq!(options.flags.get("fix_kind"), Some(&"dangerous_fix".to_string()));
     }
@@ -126,6 +145,7 @@ mod test {
         let options = Options::try_from(json).unwrap();
         assert_eq!(options.run, Run::OnType);
         assert_eq!(options.config_path, None);
+        assert_eq!(options.unused_disable_directives, UnusedDisableDirectives::Allow);
         assert!(options.flags.is_empty());
     }
 

crates/oxc_language_server/src/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap

@@ -0,0 +1,28 @@
+---
+source: crates/oxc_language_server/src/tester.rs
+input_file: crates/oxc_language_server/fixtures/linter/unused_disabled_directives/test.js
+---
+code: "eslint(no-debugger)"
+code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html"
+message: "`debugger` statement is not allowed\nhelp: Remove the debugger statement"
+range: Range { start: Position { line: 2, character: 2 }, end: Position { line: 2, character: 11 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/unused_disabled_directives/test.js"
+related_information[0].location.range: Range { start: Position { line: 2, character: 2 }, end: Position { line: 2, character: 11 } }
+severity: Some(Warning)
+source: Some("oxc")
+tags: None
+fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 2, character: 2 }, end: Position { line: 2, character: 11 } } })
+
+
+code: ""
+code_description.href: "None"
+message: "Unused eslint-disable directive (no problems were reported from no-debugger)."
+range: Range { start: Position { line: 0, character: 2 }, end: Position { line: 0, character: 56 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/unused_disabled_directives/test.js"
+related_information[0].location.range: Range { start: Position { line: 0, character: 2 }, end: Position { line: 0, character: 56 } }
+severity: Some(Error)
+source: Some("oxc")
+tags: None
+fixed: None

crates/oxc_language_server/src/worker.rs

@@ -127,6 +127,7 @@ impl WorkspaceWorker {
         old_options.config_path != new_options.config_path
             || old_options.use_nested_configs() != new_options.use_nested_configs()
             || old_options.fix_kind() != new_options.fix_kind()
+            || old_options.unused_disable_directives != new_options.unused_disable_directives
     }
 
     pub async fn should_lint_on_run_type(&self, current_run: Run) -> bool {

editors/vscode/README.md

@@ -37,11 +37,12 @@ Following configuration are supported via `settings.json` and effect the window
 
 Following configuration are supported via `settings.json` and can be changed for each workspace:
 
-| Key              | Default Value | Possible Values          | Description                                                                 |
-| ---------------- | ------------- | ------------------------ | --------------------------------------------------------------------------- |
-| `oxc.lint.run`   | `onType`      | `onSave` \| `onType`     | Run the linter on save (onSave) or on type (onType)                         |
-| `oxc.configPath` | `null`        | `null`\| `<string>`      | Path to ESlint configuration. Keep it empty to enable nested configuration. |
-| `oxc.flags`      | -             | `Record<string, string>` | Custom flags passed to the language server.                                 |
+| Key                           | Default Value | Possible Values             | Description                                                                                                                                  |
+| ----------------------------- | ------------- | --------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- |
+| `oxc.lint.run`                | `onType`      | `onSave` \| `onType`        | Run the linter on save (onSave) or on type (onType)                                                                                          |
+| `oxc.configPath`              | `null`        | `null` \| `<string>`        | Path to ESlint configuration. Keep it empty to enable nested configuration.                                                                  |
+| `oxc.unusedDisableDirectives` | `allow`       | `allow` \| `warn` \| `deny` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway. |
+| `oxc.flags`                   | -             | `Record<string, string>`    | Custom flags passed to the language server.                                                                                                  |
 
 #### Flags
 

editors/vscode/client/WorkspaceConfig.ts

@@ -5,6 +5,8 @@ export const oxlintConfigFileName = '.oxlintrc.json';
 
 export type Trigger = 'onSave' | 'onType';
 
+type UnusedDisableDirectives = 'allow' | 'warn' | 'deny';
+
 /**
  * See `"contributes.configuration"` in `package.json`
  */
@@ -24,6 +26,16 @@ export interface WorkspaceConfigInterface {
    * @default 'onType'
    */
   run: Trigger;
+
+  /**
+   * Define how directive comments like `// oxlint-disable-line` should be reported,
+   * when no errors would have been reported on that line anyway
+   *
+   * `oxc.unusedDisableDirectives`
+   *
+   * @default 'allow'
+   */
+  unusedDisableDirectives: UnusedDisableDirectives;
   /**
    * Additional flags to pass to the LSP binary
    * `oxc.flags`
@@ -36,6 +48,7 @@ export interface WorkspaceConfigInterface {
 export class WorkspaceConfig {
   private _configPath: string | null = null;
   private _runTrigger: Trigger = 'onType';
+  private _unusedDisableDirectives: UnusedDisableDirectives = 'allow';
   private _flags: Record<string, string> = {};
 
   constructor(private readonly workspace: WorkspaceFolder) {
@@ -53,6 +66,8 @@ export class WorkspaceConfig {
     this._runTrigger = this.configuration.get<Trigger>('lint.run') || 'onType';
     this._configPath = this.configuration.get<string | null>('configPath') ||
       (useNestedConfigs ? null : oxlintConfigFileName);
+    this._unusedDisableDirectives = this.configuration.get<UnusedDisableDirectives>('unusedDisableDirectives') ??
+      'allow';
     this._flags = flags;
   }
 
@@ -63,6 +78,9 @@ export class WorkspaceConfig {
     if (event.affectsConfiguration(`${ConfigService.namespace}.lint.run`, this.workspace)) {
       return true;
     }
+    if (event.affectsConfiguration(`${ConfigService.namespace}.unusedDisableDirectives`, this.workspace)) {
+      return true;
+    }
     if (event.affectsConfiguration(`${ConfigService.namespace}.flags`, this.workspace)) {
       return true;
     }
@@ -91,6 +109,15 @@ export class WorkspaceConfig {
     return this.configuration.update('configPath', value, ConfigurationTarget.WorkspaceFolder);
   }
 
+  get unusedDisableDirectives(): UnusedDisableDirectives {
+    return this._unusedDisableDirectives;
+  }
+
+  updateUnusedDisableDirectives(value: UnusedDisableDirectives): PromiseLike<void> {
+    this._unusedDisableDirectives = value;
+    return this.configuration.update('unusedDisableDirectives', value, ConfigurationTarget.WorkspaceFolder);
+  }
+
   get flags(): Record<string, string> {
     return this._flags;
   }
@@ -104,6 +131,7 @@ export class WorkspaceConfig {
     return {
       run: this.runTrigger,
       configPath: this.configPath ?? null,
+      unusedDisableDirectives: this.unusedDisableDirectives,
       flags: this.flags,
     };
   }

editors/vscode/package.json

@@ -110,6 +110,22 @@
           "default": null,
           "description": "Path to ESlint configuration. Keep it empty to enable nested configuration."
         },
+        "oxc.unusedDisableDirectives": {
+          "type": "string",
+          "scope": "resource",
+          "enum": [
+            "allow",
+            "warn",
+            "deny"
+          ],
+          "enumDescriptions": [
+            "Allow",
+            "Warn",
+            "Deny"
+          ],
+          "default": "allow",
+          "description": "Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway."
+        },
         "oxc.flags": {
           "type": "object",
           "scope": "resource",

editors/vscode/tests/WorkspaceConfig.spec.ts

@@ -7,7 +7,7 @@ suite('WorkspaceConfig', () => {
   setup(async () => {
     const workspaceConfig = workspace.getConfiguration('oxc', WORKSPACE_FOLDER);
     const globalConfig = workspace.getConfiguration('oxc');
-    const keys = ['lint.run', 'configPath', 'flags'];
+    const keys = ['lint.run', 'configPath', 'flags', 'unusedDisableDirectives'];
 
     await Promise.all(keys.map(key => workspaceConfig.update(key, undefined, ConfigurationTarget.WorkspaceFolder)));
     // VSCode will not save different workspace configuration inside a `.code-workspace` file.
@@ -18,7 +18,7 @@ suite('WorkspaceConfig', () => {
   teardown(async () => {
     const workspaceConfig = workspace.getConfiguration('oxc', WORKSPACE_FOLDER);
     const globalConfig = workspace.getConfiguration('oxc');
-    const keys = ['lint.run', 'configPath', 'flags'];
+    const keys = ['lint.run', 'configPath', 'flags', 'unusedDisableDirectives'];
 
     await Promise.all(keys.map(key => workspaceConfig.update(key, undefined, ConfigurationTarget.WorkspaceFolder)));
     // VSCode will not save different workspace configuration inside a `.code-workspace` file.
@@ -30,6 +30,7 @@ suite('WorkspaceConfig', () => {
     const config = new WorkspaceConfig(WORKSPACE_FOLDER);
     strictEqual(config.runTrigger, 'onType');
     strictEqual(config.configPath, null);
+    strictEqual(config.unusedDisableDirectives, 'allow');
     deepStrictEqual(config.flags, {});
   });
 
@@ -61,13 +62,15 @@ suite('WorkspaceConfig', () => {
     await Promise.all([
       config.updateRunTrigger('onSave'),
       config.updateConfigPath('./somewhere'),
+      config.updateUnusedDisableDirectives('deny'),
       config.updateFlags({ test: 'value' }),
     ]);
 
     const wsConfig = workspace.getConfiguration('oxc', WORKSPACE_FOLDER);
 
     strictEqual(wsConfig.get('lint.run'), 'onSave');
     strictEqual(wsConfig.get('configPath'), './somewhere');
+    strictEqual(wsConfig.get('unusedDisableDirectives'), 'deny');
     deepStrictEqual(wsConfig.get('flags'), { test: 'value' });
   });
 });