refactor(language_server): move sub option for flags to the root + deprecate flags

Author: Sysix

crates/oxc_language_server/README.md

@@ -19,16 +19,27 @@ 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                                                                |
-| `tsConfigPath`            | `<string>` \| `null`           | `null`     | Path to a TypeScript configuration file. If your `tsconfig.json` is not at the root, alias paths will not be resolve correctly for the `import` plugin |
-| `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            |
-| `typeAware`               | `true` \| `false`              | `false`    | Enables type-aware linting                                                                                                                             |
-| `flags`                   | `Map<string, string>`          | `<empty>`  | Special oxc language server flags, currently only one flag key is supported: `disable_nested_config`                                                   |
-| `fmt.experimental`        | `true` \| `false`              | `false`    | Enables experimental formatting with `oxc_formatter`                                                                                                   |
-| `fmt.configPath`          | `<string>` \| `null`           | `null`     | Path to a oxfmt configuration file, when `null` is passed, the server will use `.oxfmtrc.json` and the workspace root                                  |
+| 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                                                                |
+| `tsConfigPath`            | `<string>` \| `null`              | `null`     | Path to a TypeScript configuration file. If your `tsconfig.json` is not at the root, alias paths will not be resolve correctly for the `import` plugin |
+| `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            |
+| `typeAware`               | `true` \| `false`                 | `false`    | Enables type-aware linting                                                                                                                             |
+| `disableNestedConfig`     | `false` \| `true`                 | `false`    | Disabled nested configuration and searches only for `configPath`.                                                                                      |
+| `fixKind`                 | [fixKind values](#fixkind-values) | `safe_fix` | The level of a possible fix for d diagnostic, will be applied for the complete workspace (diagnostic, code action, commands and more).                 |
+| `fmt.experimental`        | `true` \| `false`                 | `false`    | Enables experimental formatting with `oxc_formatter`                                                                                                   |
+| `fmt.configPath`          | `<string>` \| `null`              | `null`     | Path to a oxfmt configuration file, when `null` is passed, the server will use `.oxfmtrc.json` and the workspace root                                  |
+| `flags`                   | `Map<string, string>`             | `<empty>`  | (deprecated) Custom flags passed to the language server.                                                                                               |
+
+### `fixKind` values:
+
+- `"safe_fix"` (default)
+- `"safe_fix_or_suggestion`
+- `"dangerous_fix"`
+- `"dangerous_fix_or_suggestion"`
+- `"none"`
+- `"all"`
 
 ## Supported LSP Specifications from Server
 
@@ -47,18 +58,19 @@ The client can pass the workspace options like following:
       "tsConfigPath": null,
       "unusedDisableDirectives": "allow",
       "typeAware": false,
-      "flags": {},
+      "disableNestedConfig": false,
+      "fixKind": "safe_fix",
       "fmt.experimental": false,
       "fmt.configPath": null
     }
   }]
 }
 ```
 
-#### Flags
+#### Flags (deprecated)
 
 - `key: disable_nested_config`: Disabled nested configuration and searches only for `configPath`
-- `key: fix_kind`: default: `"safe_fix"`, possible values `"safe_fix" | "safe_fix_or_suggestion" | "dangerous_fix" | "dangerous_fix_or_suggestion" | "none" | "all"`
+- `key: fix_kind`: see [FixKind values](#fixkind-values) for possible values
 
 ### [initialized](https://microsoft.github.io/language-server-protocol/specification#initialized)
 
@@ -85,7 +97,8 @@ The client can pass the workspace options like following:
       "tsConfigPath": null,
       "unusedDisableDirectives": "allow",
       "typeAware": false,
-      "flags": {},
+      "disableNestedConfig": false,
+      "fixKind": "safe_fix",
       "fmt.experimental": false,
       "fmt.configPath": null
     }
@@ -180,7 +193,8 @@ The client can return a response like:
   "tsConfigPath": null,
   "unusedDisableDirectives": "allow",
   "typeAware": false,
-  "flags": {},
+  "disableNestedConfig": false,
+  "fixKind": "safe_fix",
   "fmt.experimental": false,
   "fmt.configPath": null
 }]

crates/oxc_language_server/src/linter/options.rs

@@ -1,4 +1,3 @@
-use log::info;
 use rustc_hash::{FxBuildHasher, FxHashMap};
 use serde::{Deserialize, Deserializer, Serialize, de::Error};
 use serde_json::Value;
@@ -30,27 +29,38 @@ pub struct LintOptions {
     pub ts_config_path: Option<String>,
     pub unused_disable_directives: UnusedDisableDirectives,
     pub type_aware: bool,
-    pub flags: FxHashMap<String, String>,
+    pub disable_nested_config: bool,
+    pub fix_kind: LintFixKindFlag,
 }
 
-impl LintOptions {
-    pub fn use_nested_configs(&self) -> bool {
-        !self.flags.contains_key("disable_nested_config") && self.config_path.is_none()
+#[derive(Debug, Default, Serialize, PartialEq, Eq, Deserialize, Clone)]
+#[serde(rename_all = "snake_case")]
+pub enum LintFixKindFlag {
+    #[default]
+    SafeFix,
+    SafeFixOrSuggestion,
+    DangerousFix,
+    DangerousFixOrSuggestion,
+    None,
+    All,
+}
+
+impl From<LintFixKindFlag> for FixKind {
+    fn from(flag: LintFixKindFlag) -> Self {
+        match flag {
+            LintFixKindFlag::SafeFix => FixKind::SafeFix,
+            LintFixKindFlag::SafeFixOrSuggestion => FixKind::SafeFixOrSuggestion,
+            LintFixKindFlag::DangerousFix => FixKind::DangerousFix,
+            LintFixKindFlag::DangerousFixOrSuggestion => FixKind::DangerousFixOrSuggestion,
+            LintFixKindFlag::None => FixKind::None,
+            LintFixKindFlag::All => FixKind::All,
+        }
     }
+}
 
-    pub fn fix_kind(&self) -> FixKind {
-        self.flags.get("fix_kind").map_or(FixKind::SafeFix, |kind| match kind.as_str() {
-            "safe_fix" => FixKind::SafeFix,
-            "safe_fix_or_suggestion" => FixKind::SafeFixOrSuggestion,
-            "dangerous_fix" => FixKind::DangerousFix,
-            "dangerous_fix_or_suggestion" => FixKind::DangerousFixOrSuggestion,
-            "none" => FixKind::None,
-            "all" => FixKind::All,
-            _ => {
-                info!("invalid fix_kind flag `{kind}`, fallback to `safe_fix`");
-                FixKind::SafeFix
-            }
-        })
+impl LintOptions {
+    pub fn use_nested_configs(&self) -> bool {
+        !self.disable_nested_config && self.config_path.is_none()
     }
 }
 
@@ -72,17 +82,17 @@ impl TryFrom<Value> for LintOptions {
             return Err("no object passed".to_string());
         };
 
+        // deprecated flags field
         let mut flags = FxHashMap::with_capacity_and_hasher(2, FxBuildHasher);
         if let Some(json_flags) = object.get("flags").and_then(|value| value.as_object()) {
             if let Some(disable_nested_config) =
                 json_flags.get("disable_nested_config").and_then(|value| value.as_str())
             {
-                flags
-                    .insert("disable_nested_config".to_string(), disable_nested_config.to_string());
+                flags.insert("disable_nested_config".to_string(), disable_nested_config);
             }
 
             if let Some(fix_kind) = json_flags.get("fix_kind").and_then(|value| value.as_str()) {
-                flags.insert("fix_kind".to_string(), fix_kind.to_string());
+                flags.insert("fix_kind".to_string(), fix_kind);
             }
         }
 
@@ -107,14 +117,30 @@ impl TryFrom<Value> for LintOptions {
             type_aware: object
                 .get("typeAware")
                 .is_some_and(|key| serde_json::from_value::<bool>(key.clone()).unwrap_or_default()),
-            flags,
+            disable_nested_config: object
+                .get("disableNestedConfig")
+                .and_then(|key| serde_json::from_value::<bool>(key.clone()).ok())
+                .unwrap_or(flags.contains_key("disable_nested_config")),
+            fix_kind: object
+                .get("fixKind")
+                .and_then(|key| serde_json::from_value::<LintFixKindFlag>(key.clone()).ok())
+                .unwrap_or_else(|| match flags.get("fix_kind") {
+                    Some(&"safe_fix") => LintFixKindFlag::SafeFix,
+                    Some(&"safe_fix_or_suggestion") => LintFixKindFlag::SafeFixOrSuggestion,
+                    Some(&"dangerous_fix") => LintFixKindFlag::DangerousFix,
+                    Some(&"dangerous_fix_or_suggestion") => {
+                        LintFixKindFlag::DangerousFixOrSuggestion
+                    }
+                    Some(&"none") => LintFixKindFlag::None,
+                    Some(&"all") => LintFixKindFlag::All,
+                    _ => LintFixKindFlag::default(),
+                }),
         })
     }
 }
 
 #[cfg(test)]
 mod test {
-    use rustc_hash::FxHashMap;
     use serde_json::json;
 
     use super::{LintOptions, Run, UnusedDisableDirectives};
@@ -126,19 +152,17 @@ mod test {
             "configPath": "./custom.json",
             "unusedDisableDirectives": "warn",
             "typeAware": true,
-            "flags": {
-                "disable_nested_config": "true",
-                "fix_kind": "dangerous_fix"
-            }
+            "disableNestedConfig": true,
+            "fixKind": "dangerous_fix"
         });
 
         let options = LintOptions::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!(options.type_aware);
-        assert_eq!(options.flags.get("disable_nested_config"), Some(&"true".to_string()));
-        assert_eq!(options.flags.get("fix_kind"), Some(&"dangerous_fix".to_string()));
+        assert!(options.disable_nested_config);
+        assert_eq!(options.fix_kind, super::LintFixKindFlag::DangerousFix);
     }
 
     #[test]
@@ -150,7 +174,8 @@ mod test {
         assert_eq!(options.config_path, None);
         assert_eq!(options.unused_disable_directives, UnusedDisableDirectives::Allow);
         assert!(!options.type_aware);
-        assert!(options.flags.is_empty());
+        assert!(!options.disable_nested_config);
+        assert_eq!(options.fix_kind, super::LintFixKindFlag::SafeFix);
     }
 
     #[test]
@@ -163,24 +188,6 @@ mod test {
         let options = LintOptions::try_from(json).unwrap();
         assert_eq!(options.run, Run::OnType); // fallback
         assert_eq!(options.config_path, Some("./custom.json".into()));
-        assert!(options.flags.is_empty());
-    }
-
-    #[test]
-    fn test_invalid_flags_options_json() {
-        let json = json!({
-            "configPath": "./custom.json",
-            "flags": {
-                "disable_nested_config": true, // should be string
-                "fix_kind": "dangerous_fix"
-            }
-        });
-
-        let options = LintOptions::try_from(json).unwrap();
-        assert_eq!(options.run, Run::OnType); // fallback
-        assert_eq!(options.config_path, Some("./custom.json".into()));
-        assert_eq!(options.flags.get("disable_nested_config"), None);
-        assert_eq!(options.flags.get("fix_kind"), Some(&"dangerous_fix".to_string()));
     }
 
     #[test]
@@ -192,10 +199,70 @@ mod test {
             LintOptions { config_path: Some("config.json".to_string()), ..Default::default() };
         assert!(!options.use_nested_configs());
 
-        let mut flags = FxHashMap::default();
-        flags.insert("disable_nested_config".to_string(), "true".to_string());
-
-        let options = LintOptions { flags, ..Default::default() };
+        let options = LintOptions { disable_nested_config: true, ..Default::default() };
         assert!(!options.use_nested_configs());
     }
+
+    mod deprecated_flags {
+        use serde_json::json;
+
+        use crate::linter::options::LintFixKindFlag;
+
+        use super::{LintOptions, Run, UnusedDisableDirectives};
+
+        #[test]
+        fn test_valid_options_json_deprecated_flags() {
+            let json = json!({
+                "run": "onSave",
+                "configPath": "./custom.json",
+                "unusedDisableDirectives": "warn",
+                "typeAware": true,
+                "flags": {
+                    "disable_nested_config": "true",
+                    "fix_kind": "dangerous_fix"
+                }
+            });
+
+            let options = LintOptions::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!(options.type_aware);
+            assert!(options.disable_nested_config);
+            assert_eq!(options.fix_kind, LintFixKindFlag::DangerousFix);
+        }
+
+        #[test]
+        fn test_invalid_flags_options_json() {
+            let json = json!({
+                "configPath": "./custom.json",
+                "flags": {
+                    "disable_nested_config": true, // should be string
+                    "fix_kind": "dangerous_fix"
+                }
+            });
+
+            let options = LintOptions::try_from(json).unwrap();
+            assert_eq!(options.run, Run::OnType); // fallback
+            assert_eq!(options.config_path, Some("./custom.json".into()));
+            assert!(!options.disable_nested_config); // fallback
+            assert_eq!(options.fix_kind, LintFixKindFlag::DangerousFix);
+        }
+
+        #[test]
+        fn test_root_options_overrides_flags() {
+            let json = json!({
+                "disableNestedConfig": false,
+                "fixKind": "safe_fix_or_suggestion",
+                "flags": {
+                    "disable_nested_config": "true",
+                    "fix_kind": "dangerous_fix"
+                }
+            });
+
+            let options = LintOptions::try_from(json).unwrap();
+            assert!(!options.disable_nested_config); // root option takes precedence
+            assert_eq!(options.fix_kind, LintFixKindFlag::SafeFixOrSuggestion);
+        }
+    }
 }