Repository: denoland/deno_lint (rust)

Issue:
Ability to automatically fix linting errors
It might be useful for lints to include [text changes](https://github.com/denoland/deno_ast/pull/63) in the lint diagnostic that would fix the diagnostic. For example, `no-unused-var` could return text changes that would remove unused imports.

This could then be used by a `deno lint --fix` command.

Pull request description:
feat: quick fixes
Closes #1181
Closes #1013

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index edbf28b..8e6b47d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -220,9 +220,9 @@ checksum = "41b319d1b62ffbd002e057f36bebd1f42b9f97927c9577461d855f3513c4289f"
 
 [[package]]
 name = "deno_ast"
-version = "0.34.0"
+version = "0.34.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "84b28da6d573c30a63e4a6b66b91fa32773ed2cf4c0f26aa19b91f6c542b1a57"
+checksum = "a73fa2b215c93d44b55228a6c69aa92ffa200cfa683ebaf45202ecfbd749bf52"
 dependencies = [
  "deno_media_type",
  "deno_terminal",
@@ -307,9 +307,9 @@ dependencies = [
 
 [[package]]
 name = "dprint-swc-ext"
-version = "0.15.0"
+version = "0.15.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8fcd02c378803dba687471a9f1f1b758c33e53f616333faaddbaeb8840d9b6dd"
+checksum = "5bad772f9e49af3a613fcddf1671d1e2e877e0a6d94f2b7162bfea4ac8140bee"
 dependencies = [
  "allocator-api2",
  "bumpalo",
@@ -994,9 +994,9 @@ dependencies = [
 
 [[package]]
 name = "swc_common"
-version = "0.33.17"
+version = "0.33.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "095c158fe55b36faeebb4274692643a6d7cdc5b7902e1d5968ddbe52b7de1d1c"
+checksum = "c85e8b15d0fb87691e27c8f3cf953748db3ccd2a39e165d6d5275a48fb0d29e3"
 dependencies = [
  "ast_node",
  "better_scoped_tls",
@@ -1019,9 +1019,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_ast"
-version = "0.112.2"
+version = "0.112.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "852a48a24a2533de88298c6b25355bc68fdee31ac21cb4fb8939b7001715353c"
+checksum = "36226eb87bfd2f5620bde04f149a4b869ab34e78496d60cb0d8eb9da765d0732"
 dependencies = [
  "bitflags 2.3.3",
  "is-macro",
@@ -1037,9 +1037,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_loader"
-version = "0.45.19"
+version = "0.45.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7c16051bce5421992a1b49350735bf4d110f761fd68ae7098af17a64ad639b8d"
+checksum = "d0058cf970880f5382effe43eb2b727a73ba09ae41922fa140c2c3fa6ca9b2d1"
 dependencies = [
  "anyhow",
  "pathdiff",
@@ -1051,9 +1051,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_parser"
-version = "0.143.3"
+version = "0.143.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "90ff55811ed5de14b05e9a2979bae2bce3c807582f559b4325948463265307d9"
+checksum = "20823cac99a9adbd4c03fb5e126aaccbf92446afedad99252a0e1fc76e2ffc43"
 dependencies = [
  "either",
  "new_debug_unreachable",
@@ -1073,9 +1073,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_transforms_base"
-version = "0.137.6"
+version = "0.137.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b9489f8f5c6c08e8291bd93eb354aa91903d4eba5eeb72e1b90adf43c8fe7a29"
+checksum = "66539401f619730b26d380a120b91b499f80cbdd9bb15d00aa73bc3a4d4cc394"
 dependencies = [
  "better_scoped_tls",
  "bitflags 2.3.3",
@@ -1096,9 +1096,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_utils"
-version = "0.127.5"
+version = "0.127.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b2de25ac5cc8c8375476985e854054f05d9e841886f3c290716a0eec32eedce3"
+checksum = "14482e455df85486d68a51533a31645d511e56df93a35cadf0eabbe7abe96b98"
 dependencies = [
  "indexmap 2.1.0",
  "num_cpus",
@@ -1114,9 +1114,9 @@ dependencies = [
 
 [[package]]
 name = "swc_ecma_visit"
-version = "0.98.2"
+version = "0.98.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cdb71511a816c7c84ddc96e6939389be261caf20858486a5e76948551f110e1f"
+checksum = "df0127694c36d656ea9eab5c170cdd8ab398246ae2a335de26961c913a4aca47"
 dependencies = [
  "num-bigint",
  "swc_atoms",
@@ -1150,9 +1150,9 @@ dependencies = [
 
 [[package]]
 name = "swc_visit"
-version = "0.5.8"
+version = "0.5.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b27078d8571abe23aa52ef608dd1df89096a37d867cf691cbb4f4c392322b7c9"
+checksum = "358e246dedeb4ae8efacebcce1360dc2f9b6c0b4c1ad8b737cc60f5b6633691a"
 dependencies = [
  "either",
  "swc_visit_macros",
@@ -1160,9 +1160,9 @@ dependencies = [
 
 [[package]]
 name = "swc_visit_macros"
-version = "0.5.9"
+version = "0.5.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fa8bb05975506741555ea4d10c3a3bdb0e2357cd58e1a4a4332b8ebb4b44c34d"
+checksum = "fbbbb9d77d5112f90ed7ea00477135b16c4370c872b93a0b63b766e8710650ad"
 dependencies = [
  "Inflector",
  "pmutil",
diff --git a/Cargo.toml b/Cargo.toml
index 9dc21a3..b413704 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -25,7 +25,7 @@ default = []
 docs = []
 
 [dependencies]
-deno_ast = { version = "0.34.0", features = ["scopes", "transforms", "utils", "visit", "view"] }
+deno_ast = { version = "0.34.1", features = ["scopes", "transforms", "utils", "visit", "view"] }
 log = "0.4.20"
 serde = { version = "1.0.195", features = ["derive"] }
 serde_json = "1.0.111"
diff --git a/src/context.rs b/src/context.rs
index 00c199a..3ca7787 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -1,7 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use crate::control_flow::ControlFlow;
-use crate::diagnostic::LintDiagnostic;
+use crate::diagnostic::{LintDiagnostic, LintFix};
 use crate::ignore_directives::{
   parse_line_ignore_directives, CodeStatus, FileIgnoreDirective,
   LineIgnoreDirective,
@@ -211,6 +211,7 @@ impl<'view> Context<'view> {
           CODE,
           format!("Ignore for code \"{}\" was not used.", unused_code),
           None,
+          Vec::new(),
         );
         diagnostics.push(d);
       }
@@ -229,6 +230,7 @@ impl<'view> Context<'view> {
           CODE,
           format!("Ignore for code \"{}\" was not used.", unused_code),
           None,
+          Vec::new(),
         );
         diagnostics.push(d);
       }
@@ -258,6 +260,7 @@ impl<'view> Context<'view> {
           rules::ban_unknown_rule_code::CODE,
           format!("Unknown rule for code \"{}\"", unknown_rule_code),
           None,
+          Vec::new(),
         );
         diagnostics.push(d);
       }
@@ -272,6 +275,7 @@ impl<'view> Context<'view> {
           rules::ban_unknown_rule_code::CODE,
           format!("Unknown rule for code \"{}\"", unknown_rule_code),
           None,
+          Vec::new(),
         );
         diagnostics.push(d);
       }
@@ -306,6 +310,7 @@ impl<'view> Context<'view> {
       code.to_string(),
       message.to_string(),
       None,
+      Vec::new(),
     );
     self.diagnostics.push(diagnostic);
   }
@@ -317,8 +322,25 @@ impl<'view> Context<'view> {
     message: impl ToString,
     hint: impl ToString,
   ) {
-    let diagnostic =
-      self.create_diagnostic(range, code, message, Some(hint.to_string()));
+    let diagnostic = self.create_diagnostic(
+      range,
+      code,
+      message,
+      Some(hint.to_string()),
+      Vec::new(),
+    );
+    self.diagnostics.push(diagnostic);
+  }
+
+  pub fn add_diagnostic_with_fixes(
+    &mut self,
+    range: SourceRange,
+    code: impl ToString,
+    message: impl ToString,
+    hint: Option<String>,
+    fixes: Vec<LintFix>,
+  ) {
+    let diagnostic = self.create_diagnostic(range, code, message, hint, fixes);
     self.diagnostics.push(diagnostic);
   }
 
@@ -328,6 +350,7 @@ impl<'view> Context<'view> {
     code: impl ToString,
     message: impl ToString,
     maybe_hint: Option<String>,
+    fixes: Vec<LintFix>,
   ) -> LintDiagnostic {
     LintDiagnostic {
       specifier: self.specifier().clone(),
@@ -336,6 +359,7 @@ impl<'view> Context<'view> {
       message: message.to_string(),
       code: code.to_string(),
       hint: maybe_hint,
+      fixes,
     }
   }
 }
diff --git a/src/diagnostic.rs b/src/diagnostic.rs
index 324e37b..06dd0ad 100644
--- a/src/diagnostic.rs
+++ b/src/diagnostic.rs
@@ -14,6 +14,18 @@ use deno_ast::ModuleSpecifier;
 use deno_ast::SourceRange;
 use deno_ast::SourceTextInfo;
 
+#[derive(Debug, Clone)]
+pub struct LintFixChange {
+  pub new_text: Cow<'static, str>,
+  pub range: SourceRange,
+}
+
+#[derive(Debug, Clone)]
+pub struct LintFix {
+  pub description: Cow<'static, str>,
+  pub changes: Vec<LintFixChange>,
+}
+
 #[derive(Clone)]
 pub struct LintDiagnostic {
   pub specifier: ModuleSpecifier,
@@ -22,6 +34,13 @@ pub struct LintDiagnostic {
   pub message: String,
   pub code: String,
   pub hint: Option<String>,
+  /// Fixes that should be shown in the Deno LSP and also
+  /// used for the `deno lint --fix` flag.
+  ///
+  /// Note: If there are multiple fixes for a diagnostic then
+  /// only the first fix will be used for the `--fix` flag, but
+  /// multiple will be shown in the LSP.
+  pub fixes: Vec<LintFix>,
 }
 
 impl Diagnostic for LintDiagnostic {
diff --git a/src/rules/no_window.rs b/src/rules/no_window.rs
index 5fb848a..db01bc7 100644
--- a/src/rules/no_window.rs
+++ b/src/rules/no_window.rs
@@ -1,11 +1,14 @@
 // Copyright 2020-2021 the Deno authors. All rights reserved. MIT license.
 use super::Context;
 use super::LintRule;
+use crate::diagnostic::LintFix;
+use crate::diagnostic::LintFixChange;
 use crate::handler::Handler;
 use crate::handler::Traverse;
 use crate::Program;
 
 use deno_ast::view as ast_view;
+use deno_ast::SourceRange;
 use deno_ast::SourceRanged;
 use if_chain::if_chain;
 
@@ -16,6 +19,7 @@ const CODE: &str = "no-window";
 const MESSAGE: &str =
   "window is deprecated and scheduled for removal in Deno 2.0";
 const HINT: &str = "Instead, use `globalThis`";
+const FIX_DESC: &str = "Rename window to globalThis";
 
 impl LintRule for NoWindow {
   fn tags(&self) -> &'static [&'static str] {
@@ -42,6 +46,24 @@ impl LintRule for NoWindow {
 
 struct NoWindowGlobalHandler;
 
+impl NoWindowGlobalHandler {
+  fn add_diagnostic(&self, ctx: &mut Context, range: SourceRange) {
+    ctx.add_diagnostic_with_fixes(
+      range,
+      CODE,
+      MESSAGE,
+      Some(HINT.to_string()),
+      vec![LintFix {
+        description: FIX_DESC.into(),
+        changes: vec![LintFixChange {
+          new_text: "globalThis".into(),
+          range,
+        }],
+      }],
+    );
+  }
+}
+
 impl Handler for NoWindowGlobalHandler {
   fn member_expr(&mut self, expr: &ast_view::MemberExpr, ctx: &mut Context) {
     if expr.parent().is::<ast_view::MemberExpr>() {
@@ -54,12 +76,7 @@ impl Handler for NoWindowGlobalHandler {
       if ident.sym() == "window";
       if ctx.scope().is_global(&ident.inner.to_id());
       then {
-        ctx.add_diagnostic_with_hint(
-          ident.range(),
-          CODE,
-          MESSAGE,
-          HINT,
-        );
+        self.add_diagnostic(ctx, ident.range());
       }
     }
   }
@@ -71,12 +88,7 @@ impl Handler for NoWindowGlobalHandler {
       if ident.sym() == "window";
       if ctx.scope().is_global(&ident.inner.to_id());
       then {
-        ctx.add_diagnostic_with_hint(
-          ident.range(),
-          CODE,
-          MESSAGE,
-          HINT,
-        );
+        self.add_diagnostic(ctx, ident.range());
       }
     }
   }
@@ -117,16 +129,19 @@ mod tests {
       r#"window.fetch()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, "globalThis.fetch()"),
         }
       ],
       r#"window["fetch"]()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, r#"globalThis["fetch"]()"#),
         }
       ],
       r#"window[`fetch`]()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, "globalThis[`fetch`]()"),
         }
       ],
       r#"
@@ -134,11 +149,16 @@ function foo() {
   const window = 42;
   return window;
 }
-window;
-      "#: [
+window;"#: [
         {
           col: 0,
           line: 6,
+          fix: (FIX_DESC, "
+function foo() {
+  const window = 42;
+  return window;
+}
+globalThis;"),
         }
       ],
     };
diff --git a/src/rules/no_window_prefix.rs b/src/rules/no_window_prefix.rs
index 3e5a75b..7f4da18 100644
--- a/src/rules/no_window_prefix.rs
+++ b/src/rules/no_window_prefix.rs
@@ -2,6 +2,8 @@
 
 use super::Context;
 use super::LintRule;
+use crate::diagnostic::LintFix;
+use crate::diagnostic::LintFixChange;
 use crate::handler::Handler;
 use crate::handler::Traverse;
 use crate::Program;
@@ -19,6 +21,7 @@ const CODE: &str = "no-window-prefix";
 const MESSAGE: &str = "For compatibility between the Window context and the Web Workers, calling Web APIs via `window` is disallowed";
 const HINT: &str =
   "Instead, call this API via `self`, `globalThis`, or no extra prefix";
+const FIX_DESC: &str = "Rename window to globalThis";
 
 impl LintRule for NoWindowPrefix {
   fn tags(&self) -> &'static [&'static str] {
@@ -241,18 +244,25 @@ impl Handler for NoWindowPrefixHandler {
 
     use deno_ast::view::Expr;
     if_chain! {
-      if let Expr::Ident(obj) = &member_expr.obj;
-      let obj_symbol = obj.sym();
+      if let Expr::Ident(obj_ident) = &member_expr.obj;
+      let obj_symbol = obj_ident.sym();
       if obj_symbol == "window";
-      if ctx.scope().is_global(&obj.inner.to_id());
+      if ctx.scope().is_global(&obj_ident.inner.to_id());
       if let Some(prop_symbol) = extract_symbol(member_expr);
       if PROPERTY_DENY_LIST.contains(prop_symbol);
       then {
-        ctx.add_diagnostic_with_hint(
+        ctx.add_diagnostic_with_fixes(
           member_expr.range(),
           CODE,
           MESSAGE,
-          HINT,
+          Some(HINT.into()),
+          vec![LintFix {
+            description: FIX_DESC.into(),
+            changes: vec![LintFixChange {
+              new_text: "globalThis".into(),
+              range: obj_ident.range(),
+            }],
+          }]
         );
       }
     }
@@ -386,28 +396,36 @@ mod tests {
       r#"window.fetch()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, "globalThis.fetch()"),
         }
       ],
       r#"window["fetch"]()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, r#"globalThis["fetch"]()"#),
         }
       ],
       r#"window[`fetch`]()"#: [
         {
           col: 0,
+          fix: (FIX_DESC, "globalThis[`fetch`]()"),
         }
       ],
-      r#"
+      "
 function foo() {
   const window = 42;
   return window;
 }
-window.fetch();
-      "#: [
+window.fetch();": [
         {
           col: 0,
           line: 6,
+          fix: (FIX_DESC, "
+function foo() {
+  const window = 42;
+  return window;
+}
+globalThis.fetch();"),
         }
       ],
     };


Test patch:
diff --git a/src/test_util.rs b/src/test_util.rs
index b58877c..4ce4e1a 100644
--- a/src/test_util.rs
+++ b/src/test_util.rs
@@ -10,6 +10,8 @@ use deno_ast::view as ast_view;
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
 use deno_ast::ParsedSource;
+use deno_ast::SourceTextInfo;
+use deno_ast::TextChange;
 
 #[macro_export]
 macro_rules! assert_lint_ok {
@@ -198,7 +200,7 @@ impl LintErrTester {
   #[track_caller]
   pub fn run(self) {
     let rule_code = self.rule.code();
-    let diagnostics = lint(self.rule, self.src, self.filename);
+    let (parsed_source, diagnostics) = lint(self.rule, self.src, self.filename);
     if self.errors.len() != diagnostics.len() {
       eprintln!(
         "Actual diagnostics:\n{:#?}",
@@ -223,6 +225,7 @@ impl LintErrTester {
         col,
         message,
         hint,
+        fixes,
       } = error;
       assert_diagnostic_2(
         diagnostic,
@@ -232,17 +235,26 @@ impl LintErrTester {
         self.src,
         message,
         hint.as_deref(),
+        fixes,
+        parsed_source.text_info(),
       );
     }
   }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub struct LintErrFix {
+  pub description: String,
+  pub fixed_code: String,
+}
+
 #[derive(Default)]
 pub struct LintErr {
   pub line: usize,
   pub col: usize,
   pub message: String,
   pub hint: Option<String>,
+  pub fixes: Vec<LintErrFix>,
 }
 
 #[derive(Default)]
@@ -251,6 +263,7 @@ pub struct LintErrBuilder {
   col: Option<usize>,
   message: Option<String>,
   hint: Option<String>,
+  fixes: Vec<LintErrFix>,
 }
 
 impl LintErrBuilder {
@@ -280,12 +293,21 @@ impl LintErrBuilder {
     self
   }
 
+  pub fn fix(&mut self, value: (&'static str, &'static str)) -> &mut Self {
+    self.fixes.push(LintErrFix {
+      description: value.0.to_string(),
+      fixed_code: value.1.to_string(),
+    });
+    self
+  }
+
   pub fn build(self) -> LintErr {
     LintErr {
       line: self.line.unwrap_or(1),
       col: self.col.unwrap_or(0),
       message: self.message.unwrap_or_default(),
       hint: self.hint,
+      fixes: self.fixes,
     }
   }
 }
@@ -295,7 +317,7 @@ fn lint(
   rule: &'static dyn LintRule,
   source: &str,
   specifier: &str,
-) -> Vec<LintDiagnostic> {
+) -> (ParsedSource, Vec<LintDiagnostic>) {
   let linter = LinterBuilder::default().rules(vec![rule]).build();
 
   let specifier = ModuleSpecifier::parse(specifier).unwrap();
@@ -306,7 +328,7 @@ fn lint(
     media_type,
   });
   match lint_result {
-    Ok((_, diagnostics)) => diagnostics,
+    Ok((source, diagnostics)) => (source, diagnostics),
     Err(e) => panic!(
       "Failed to lint.\n[cause]\n{}\n\n[source code]\n{}",
       e, source
@@ -343,6 +365,7 @@ pub fn assert_diagnostic(
   );
 }
 
+#[allow(clippy::too_many_arguments)]
 #[track_caller]
 fn assert_diagnostic_2(
   diagnostic: &LintDiagnostic,
@@ -352,6 +375,8 @@ fn assert_diagnostic_2(
   source: &str,
   message: &str,
   hint: Option<&str>,
+  fixes: &[LintErrFix],
+  text_info: &SourceTextInfo,
 ) {
   let line_and_column = diagnostic
     .text_info
@@ -387,6 +412,25 @@ fn assert_diagnostic_2(
     diagnostic.hint.as_deref(),
     source
   );
+  let actual_fixes = diagnostic
+    .fixes
+    .iter()
+    .map(|fix| LintErrFix {
+      description: fix.description.to_string(),
+      fixed_code: deno_ast::apply_text_changes(
+        text_info.text_str(),
+        fix
+          .changes
+          .iter()
+          .map(|change| TextChange {
+            range: change.range.as_byte_range(text_info.range().start),
+            new_text: change.new_text.to_string(),
+          })
+          .collect(),
+      ),
+    })
+    .collect::<Vec<_>>();
+  assert_eq!(actual_fixes, fixes, "Quick fixes did not match.");
 }
 
 #[track_caller]
@@ -395,7 +439,7 @@ pub fn assert_lint_ok(
   source: &str,
   specifier: &'static str,
 ) {
-  let diagnostics = lint(rule, source, specifier);
+  let (_parsed_source, diagnostics) = lint(rule, source, specifier);
   if !diagnostics.is_empty() {
     eprintln!("filename {:?}", specifier);
     panic!(