feat(linter): let fixer functions return a None fix (#4210)

Part of #4187.

Adds `CompositeFix::None`, which enables fixer functions to decide not to fix some code.

While I was in the area, I took the liberty of adding some doc comments.

Author: DonIsaac

crates/oxc_linter/src/context.rs

@@ -1,3 +1,4 @@
+#![allow(rustdoc::private_intra_doc_links)] // useful for intellisense
 use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};
 
 use oxc_cfg::ControlFlowGraph;
@@ -18,11 +19,16 @@ use crate::{
 pub struct LintContext<'a> {
     semantic: Rc<Semantic<'a>>,
 
+    /// Diagnostics reported by the linter.
+    ///
+    /// Contains diagnostics for all rules across all files.
     diagnostics: RefCell<Vec<Message<'a>>>,
 
     disable_directives: Rc<DisableDirectives<'a>>,
 
-    /// Whether or not to apply code fixes during linting.
+    /// Whether or not to apply code fixes during linting. Defaults to `false`.
+    ///
+    /// Set via the `--fix` CLI flag.
     fix: bool,
 
     file_path: Rc<Path>,
@@ -32,13 +38,24 @@ pub struct LintContext<'a> {
     // states
     current_rule_name: &'static str,
 
+    /// Current rule severity. Allows for user severity overrides, e.g.
+    /// ```json
+    /// // .oxlintrc.json
+    /// {
+    ///   "rules": {
+    ///     "no-debugger": "error"
+    ///   }
+    /// }
+    /// ```
     severity: Severity,
 }
 
 impl<'a> LintContext<'a> {
     /// # Panics
     /// If `semantic.cfg()` is `None`.
     pub fn new(file_path: Box<Path>, semantic: Rc<Semantic<'a>>) -> Self {
+        const DIAGNOSTICS_INITIAL_CAPACITY: usize = 128;
+
         // We should always check for `semantic.cfg()` being `Some` since we depend on it and it is
         // unwrapped without any runtime checks after construction.
         assert!(
@@ -50,7 +67,7 @@ impl<'a> LintContext<'a> {
                 .build();
         Self {
             semantic,
-            diagnostics: RefCell::new(vec![]),
+            diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
             disable_directives: Rc::new(disable_directives),
             fix: false,
             file_path: file_path.into(),
@@ -60,6 +77,7 @@ impl<'a> LintContext<'a> {
         }
     }
 
+    /// Enable/disable automatic code fixes.
     #[must_use]
     pub fn with_fix(mut self, fix: bool) -> Self {
         self.fix = fix;
@@ -112,14 +130,17 @@ impl<'a> LintContext<'a> {
         span.source_text(self.semantic().source_text())
     }
 
+    /// [`SourceType`] of the file currently being linted.
     pub fn source_type(&self) -> &SourceType {
         self.semantic().source_type()
     }
 
+    /// Path to the file currently being linted.
     pub fn file_path(&self) -> &Path {
         &self.file_path
     }
 
+    /// Plugin settings
     pub fn settings(&self) -> &OxlintSettings {
         &self.eslint_config.settings
     }
@@ -128,6 +149,9 @@ impl<'a> LintContext<'a> {
         &self.eslint_config.globals
     }
 
+    /// Runtime environments turned on/off by the user.
+    ///
+    /// Examples of environments are `builtin`, `browser`, `node`, etc.
     pub fn env(&self) -> &OxlintEnv {
         &self.eslint_config.env
     }
@@ -174,6 +198,11 @@ impl<'a> LintContext<'a> {
     }
 
     /// 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`].
+    ///
+    /// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
     pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
     where
         C: Into<CompositeFix<'a>>,
@@ -189,23 +218,37 @@ impl<'a> LintContext<'a> {
         }
     }
 
+    /// AST nodes
+    ///
+    /// Shorthand for `self.semantic().nodes()`.
     pub fn nodes(&self) -> &AstNodes<'a> {
         self.semantic().nodes()
     }
 
+    /// Scope tree
+    ///
+    /// Shorthand for `ctx.semantic().scopes()`.
     pub fn scopes(&self) -> &ScopeTree {
         self.semantic().scopes()
     }
 
+    /// Symbol table
+    ///
+    /// Shorthand for `ctx.semantic().symbols()`.
     pub fn symbols(&self) -> &SymbolTable {
         self.semantic().symbols()
     }
 
+    /// Imported modules and exported symbols
+    ///
+    /// Shorthand for `ctx.semantic().module_record()`.
     pub fn module_record(&self) -> &ModuleRecord {
         self.semantic().module_record()
     }
 
-    /* JSDoc */
+    /// JSDoc comments
+    ///
+    /// Shorthand for `ctx.semantic().jsdoc()`.
     pub fn jsdoc(&self) -> &JSDocFinder<'a> {
         self.semantic().jsdoc()
     }

crates/oxc_linter/src/fixer.rs

@@ -2,16 +2,22 @@ use std::borrow::Cow;
 
 use oxc_codegen::Codegen;
 use oxc_diagnostics::OxcDiagnostic;
-use oxc_span::{GetSpan, Span};
+use oxc_span::{GetSpan, Span, SPAN};
 
 use crate::LintContext;
 
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Clone)]
 pub struct Fix<'a> {
     pub content: Cow<'a, str>,
     pub span: Span,
 }
 
+impl Default for Fix<'_> {
+    fn default() -> Self {
+        Self::empty()
+    }
+}
+
 impl<'a> Fix<'a> {
     pub const fn delete(span: Span) -> Self {
         Self { content: Cow::Borrowed(""), span }
@@ -20,10 +26,21 @@ impl<'a> Fix<'a> {
     pub fn new<T: Into<Cow<'a, str>>>(content: T, span: Span) -> Self {
         Self { content: content.into(), span }
     }
+
+    /// Creates a [`Fix`] that doesn't change the source code.
+    #[inline]
+    pub const fn empty() -> Self {
+        Self { content: Cow::Borrowed(""), span: SPAN }
+    }
 }
 
+#[derive(Debug, Default)]
 pub enum CompositeFix<'a> {
+    /// No fixes
+    #[default]
+    None,
     Single(Fix<'a>),
+    /// Several fixes that will be merged into one, in order.
     Multiple(Vec<Fix<'a>>),
 }
 
@@ -33,9 +50,22 @@ impl<'a> From<Fix<'a>> for CompositeFix<'a> {
     }
 }
 
+impl<'a> From<Option<Fix<'a>>> for CompositeFix<'a> {
+    fn from(fix: Option<Fix<'a>>) -> Self {
+        match fix {
+            Some(fix) => CompositeFix::Single(fix),
+            None => CompositeFix::None,
+        }
+    }
+}
+
 impl<'a> From<Vec<Fix<'a>>> for CompositeFix<'a> {
     fn from(fixes: Vec<Fix<'a>>) -> Self {
-        CompositeFix::Multiple(fixes)
+        if fixes.is_empty() {
+            CompositeFix::None
+        } else {
+            CompositeFix::Multiple(fixes)
+        }
     }
 }
 
@@ -46,19 +76,21 @@ impl<'a> CompositeFix<'a> {
         match self {
             CompositeFix::Single(fix) => fix,
             CompositeFix::Multiple(fixes) => Self::merge_fixes(fixes, source_text),
+            CompositeFix::None => Fix::empty(),
         }
     }
-    // Merges multiple fixes to one, returns an `Fix::default`(which will not fix anything) if:
-    // 1. `fixes` is empty
-    // 2. contains overlapped ranges
-    // 3. contains negative ranges (span.start > span.end)
-    // <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L147-L179>
+    /// Merges multiple fixes to one, returns an `Fix::default`(which will not fix anything) if:
+    ///
+    /// 1. `fixes` is empty
+    /// 2. contains overlapped ranges
+    /// 3. contains negative ranges (span.start > span.end)
+    ///
+    /// <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L147-L179>
     fn merge_fixes(fixes: Vec<Fix<'a>>, source_text: &str) -> Fix<'a> {
         let mut fixes = fixes;
-        let empty_fix = Fix::default();
         if fixes.is_empty() {
             // Do nothing
-            return empty_fix;
+            return Fix::empty();
         }
         if fixes.len() == 1 {
             return fixes.pop().unwrap();
@@ -77,33 +109,35 @@ impl<'a> CompositeFix<'a> {
             // negative range or overlapping ranges is invalid
             if span.start > span.end {
                 debug_assert!(false, "Negative range is invalid: {span:?}");
-                return empty_fix;
+                return Fix::empty();
             }
             if last_pos > span.start {
                 debug_assert!(
                     false,
                     "Fix must not be overlapped, last_pos: {}, span.start: {}",
                     last_pos, span.start
                 );
-                return empty_fix;
+                return Fix::empty();
             }
 
             let Some(before) = source_text.get((last_pos) as usize..span.start as usize) else {
                 debug_assert!(false, "Invalid range: {}, {}", last_pos, span.start);
-                return empty_fix;
+                return Fix::empty();
             };
 
+            output.reserve(before.len() + content.len());
             output.push_str(before);
             output.push_str(content);
             last_pos = span.end;
         }
 
         let Some(after) = source_text.get(last_pos as usize..end as usize) else {
             debug_assert!(false, "Invalid range: {:?}", last_pos as usize..end as usize);
-            return empty_fix;
+            return Fix::empty();
         };
 
         output.push_str(after);
+        output.shrink_to_fit();
         Fix::new(output, Span::new(start, end))
     }
 }
@@ -121,6 +155,8 @@ impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
         Self { ctx }
     }
 
+    /// Get a snippet of source text covered by the given [`Span`]. For details,
+    /// see [`Span::source_text`].
     pub fn source_range(self, span: Span) -> &'a str {
         self.ctx.source_range(span)
     }
@@ -186,6 +222,11 @@ impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
     pub fn codegen(self) -> Codegen<'a, false> {
         Codegen::<false>::new()
     }
+
+    #[allow(clippy::unused_self)]
+    pub fn noop(self) -> Fix<'a> {
+        Fix::empty()
+    }
 }
 
 pub struct FixResult<'a> {

crates/oxc_linter/src/lib.rs

@@ -49,6 +49,7 @@ fn size_asserts() {
     assert_eq_size!(RuleEnum, [u8; 16]);
 }
 
+#[derive(Debug)]
 pub struct Linter {
     rules: Vec<RuleWithSeverity>,
     options: LintOptions,
@@ -83,6 +84,7 @@ impl Linter {
         self
     }
 
+    /// Enable/disable automatic code fixes.
     #[must_use]
     pub fn with_fix(mut self, yes: bool) -> Self {
         self.options.fix = yes;

crates/oxc_linter/src/rule.rs

@@ -100,7 +100,7 @@ impl fmt::Display for RuleCategory {
     }
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct RuleWithSeverity {
     pub rule: RuleEnum,
     pub severity: AllowWarnDeny,