feat(linter): support suggestions and dangerous fixes (#4223)

apps/oxlint/src/command/lint.rs

@@ -1,7 +1,7 @@
 use std::{path::PathBuf, str::FromStr};
 
 use bpaf::Bpaf;
-use oxc_linter::AllowWarnDeny;
+use oxc_linter::{AllowWarnDeny, FixKind};
 
 use super::{
     expand_glob,
@@ -122,6 +122,40 @@ pub struct FixOptions {
     /// Fix as many issues as possible. Only unfixed issues are reported in the output
     #[bpaf(switch)]
     pub fix: bool,
+    /// Apply auto-fixable suggestions. May change program behavior.
+    #[bpaf(switch)]
+    pub fix_suggestions: bool,
+
+    /// Apply dangerous fixes and suggestions.
+    #[bpaf(switch)]
+    pub fix_dangerously: bool,
+}
+
+impl FixOptions {
+    pub fn fix_kind(&self) -> FixKind {
+        let mut kind = FixKind::None;
+
+        if self.fix {
+            kind.set(FixKind::SafeFix, true);
+        }
+
+        if self.fix_suggestions {
+            kind.set(FixKind::Suggestion, true);
+        }
+
+        if self.fix_dangerously {
+            if kind.is_none() {
+                kind.set(FixKind::Fix, true);
+            }
+            kind.set(FixKind::Dangerous, true);
+        }
+
+        kind
+    }
+
+    pub fn is_enabled(&self) -> bool {
+        self.fix || self.fix_suggestions || self.fix_dangerously
+    }
 }
 
 /// Handle Warnings

apps/oxlint/src/lint/mod.rs

@@ -93,7 +93,7 @@ impl Runner for LintRunner {
         let lint_options = LintOptions::default()
             .with_filter(filter)
             .with_config_path(basic_options.config)
-            .with_fix(fix_options.fix)
+            .with_fix(fix_options.fix_kind())
             .with_react_plugin(enable_plugins.react_plugin)
             .with_unicorn_plugin(enable_plugins.unicorn_plugin)
             .with_typescript_plugin(enable_plugins.typescript_plugin)

crates/oxc_language_server/src/linter.rs

@@ -13,7 +13,7 @@ use oxc_linter::{
         AstroPartialLoader, JavaScriptSource, SveltePartialLoader, VuePartialLoader,
         LINT_PARTIAL_LOADER_EXT,
     },
-    LintContext, Linter,
+    FixKind, LintContext, Linter,
 };
 use oxc_parser::Parser;
 use oxc_semantic::SemanticBuilder;
@@ -388,7 +388,7 @@ pub struct ServerLinter {
 
 impl ServerLinter {
     pub fn new() -> Self {
-        let linter = Linter::default().with_fix(true);
+        let linter = Linter::default().with_fix(FixKind::SafeFix);
         Self { linter: Arc::new(linter) }
     }
 

crates/oxc_language_server/src/main.rs

@@ -7,7 +7,7 @@ use futures::future::join_all;
 use globset::Glob;
 use ignore::gitignore::Gitignore;
 use log::{debug, error, info};
-use oxc_linter::{LintOptions, Linter};
+use oxc_linter::{FixKind, LintOptions, Linter};
 use serde::{Deserialize, Serialize};
 use tokio::sync::{Mutex, OnceCell, RwLock, SetError};
 use tower_lsp::{
@@ -345,7 +345,9 @@ impl Backend {
             let mut linter = self.server_linter.write().await;
             *linter = ServerLinter::new_with_linter(
                 Linter::from_options(
-                    LintOptions::default().with_fix(true).with_config_path(Some(config_path)),
+                    LintOptions::default()
+                        .with_fix(FixKind::SafeFix)
+                        .with_config_path(Some(config_path)),
                 )
                 .expect("should have initialized linter with new options"),
             );

crates/oxc_linter/Cargo.toml

@@ -33,6 +33,7 @@ oxc_codegen     = { workspace = true }
 oxc_resolver    = { workspace = true }
 
 rayon               = { workspace = true }
+bitflags            = { workspace = true }
 lazy_static         = { workspace = true }
 serde_json          = { workspace = true }
 serde               = { workspace = true, features = ["derive"] }

crates/oxc_linter/src/context.rs

@@ -10,7 +10,7 @@ use oxc_syntax::module_record::ModuleRecord;
 use crate::{
     config::OxlintRules,
     disable_directives::{DisableDirectives, DisableDirectivesBuilder},
-    fixer::{CompositeFix, Message, RuleFixer},
+    fixer::{FixKind, Message, RuleFix, RuleFixer},
     javascript_globals::GLOBALS,
     AllowWarnDeny, OxlintConfig, OxlintEnv, OxlintGlobals, OxlintSettings,
 };
@@ -26,10 +26,12 @@ pub struct LintContext<'a> {
 
     disable_directives: Rc<DisableDirectives<'a>>,
 
-    /// Whether or not to apply code fixes during linting. Defaults to `false`.
+    /// Whether or not to apply code fixes during linting. Defaults to
+    /// [`FixKind::None`] (no fixing).
     ///
-    /// Set via the `--fix` CLI flag.
-    fix: bool,
+    /// Set via the `--fix`, `--fix-suggestions`, and `--fix-dangerously` CLI
+    /// flags.
+    fix: FixKind,
 
     file_path: Rc<Path>,
 
@@ -69,7 +71,7 @@ impl<'a> LintContext<'a> {
             semantic,
             diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
             disable_directives: Rc::new(disable_directives),
-            fix: false,
+            fix: FixKind::None,
             file_path: file_path.into(),
             eslint_config: Arc::new(OxlintConfig::default()),
             current_rule_name: "",
@@ -79,7 +81,7 @@ impl<'a> LintContext<'a> {
 
     /// Enable/disable automatic code fixes.
     #[must_use]
-    pub fn with_fix(mut self, fix: bool) -> Self {
+    pub fn with_fix(mut self, fix: FixKind) -> Self {
         self.fix = fix;
         self
     }
@@ -190,25 +192,114 @@ impl<'a> LintContext<'a> {
     /// Report a lint rule violation.
     ///
     /// Use [`LintContext::diagnostic_with_fix`] to provide an automatic fix.
+    #[inline]
     pub fn diagnostic(&self, diagnostic: OxcDiagnostic) {
         self.add_diagnostic(Message::new(diagnostic, None));
     }
 
     /// Report a lint rule violation and provide an automatic fix.
     ///
     /// The second argument is a [closure] that takes a [`RuleFixer`] and
-    /// returns something that can turn into a [`CompositeFix`].
+    /// returns something that can turn into a `CompositeFix`.
+    ///
+    /// Fixes created this way should not create parse errors or change the
+    /// semantics of the linted code. If your fix may change the code's
+    /// semantics, use [`LintContext::diagnostic_with_suggestion`] instead. If
+    /// your fix has the potential to create parse errors, use
+    /// [`LintContext::diagnostic_with_dangerous_fix`].
     ///
     /// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
+    #[inline]
     pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
     where
-        C: Into<CompositeFix<'a>>,
+        C: Into<RuleFix<'a>>,
+        F: FnOnce(RuleFixer<'_, 'a>) -> C,
+    {
+        self.diagnostic_with_fix_of_kind(diagnostic, FixKind::SafeFix, fix);
+    }
+
+    /// Report a lint rule violation and provide a suggestion for fixing it.
+    ///
+    /// The second argument is a [closure] that takes a [`RuleFixer`] and
+    /// returns something that can turn into a `CompositeFix`.
+    ///
+    /// Fixes created this way should not create parse errors, but have the
+    /// potential to change the code's semantics. If your fix is completely safe
+    /// and definitely does not change semantics, use [`LintContext::diagnostic_with_fix`].
+    /// If your fix has the potential to create parse errors, use
+    /// [`LintContext::diagnostic_with_dangerous_fix`].
+    ///
+    /// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
+    #[inline]
+    pub fn diagnostic_with_suggestion<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
+    where
+        C: Into<RuleFix<'a>>,
+        F: FnOnce(RuleFixer<'_, 'a>) -> C,
+    {
+        self.diagnostic_with_fix_of_kind(diagnostic, FixKind::Suggestion, fix);
+    }
+
+    /// Report a lint rule violation and provide a potentially dangerous
+    /// automatic fix for it.
+    ///
+    /// The second argument is a [closure] that takes a [`RuleFixer`] and
+    /// returns something that can turn into a `CompositeFix`.
+    ///
+    /// Dangerous fixes should be avoided and are not applied by default with
+    /// `--fix`. Use this method if:
+    /// - Your fix is experimental and you want to test it out in the wild
+    ///   before marking it as safe.
+    /// - Your fix is extremely aggressive and risky, but you want to provide
+    ///   it as an option to users.
+    ///
+    /// When possible, prefer [`LintContext::diagnostic_with_fix`]. If the only
+    /// risk your fix poses is minor(ish) changes to code semantics, use
+    /// [`LintContext::diagnostic_with_suggestion`] instead.
+    ///
+    /// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
+    ///
+    #[inline]
+    pub fn diagnostic_with_dangerous_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
+    where
+        C: Into<RuleFix<'a>>,
+        F: FnOnce(RuleFixer<'_, 'a>) -> C,
+    {
+        self.diagnostic_with_fix_of_kind(diagnostic, FixKind::DangerousFix, fix);
+    }
+
+    pub fn diagnostic_with_fix_of_kind<C, F>(
+        &self,
+        diagnostic: OxcDiagnostic,
+        fix_kind: FixKind,
+        fix: F,
+    ) where
+        C: Into<RuleFix<'a>>,
         F: FnOnce(RuleFixer<'_, 'a>) -> C,
     {
-        if self.fix {
-            let fixer = RuleFixer::new(self);
-            let composite_fix: CompositeFix = fix(fixer).into();
-            let fix = composite_fix.normalize_fixes(self.source_text());
+        // if let Some(accepted_fix_kind) = self.fix {
+        //     let fixer = RuleFixer::new(fix_kind, self);
+        //     let rule_fix: RuleFix<'a> = fix(fixer).into();
+        //     let diagnostic = match (rule_fix.message(), &diagnostic.help) {
+        //         (Some(message), None) => diagnostic.with_help(message.to_owned()),
+        //         _ => diagnostic,
+        //     };
+        //     if rule_fix.kind() <= accepted_fix_kind {
+        //         let fix = rule_fix.into_fix(self.source_text());
+        //         self.add_diagnostic(Message::new(diagnostic, Some(fix)));
+        //     } else {
+        //         self.diagnostic(diagnostic);
+        //     }
+        // } else {
+        //     self.diagnostic(diagnostic);
+        // }
+        let fixer = RuleFixer::new(fix_kind, self);
+        let rule_fix: RuleFix<'a> = fix(fixer).into();
+        let diagnostic = match (rule_fix.message(), &diagnostic.help) {
+            (Some(message), None) => diagnostic.with_help(message.to_owned()),
+            _ => diagnostic,
+        };
+        if self.fix.can_apply(rule_fix.kind()) {
+            let fix = rule_fix.into_fix(self.source_text());
             self.add_diagnostic(Message::new(diagnostic, Some(fix)));
         } else {
             self.diagnostic(diagnostic);