feat(lint): `deno lint --fix` and lsp quick fixes (#22615)

Adds a `--fix` option to deno lint. This currently doesn't work for
basically any rules, but we can add them over time to deno lint.

7 files changed, 302 insertions, 39 deletions

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index a0d46ebc2..939dfee74 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -69,7 +69,7 @@ deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"]
 deno_doc = { version = "=0.113.1", features = ["html"] }
 deno_emit = "=0.38.2"
 deno_graph = { version = "=0.69.9", features = ["tokio_executor"] }
-deno_lint = { version = "=0.57.1", features = ["docs"] }
+deno_lint = { version = "=0.58.0", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_npm = "=0.17.0"
 deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index 07b60331e..22c7d8e6d 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -199,6 +199,7 @@ pub struct UninstallFlags {
 pub struct LintFlags {
   pub files: FileFlags,
   pub rules: bool,
+  pub fix: bool,
   pub maybe_rules_tags: Option<Vec<String>>,
   pub maybe_rules_include: Option<Vec<String>>,
   pub maybe_rules_exclude: Option<Vec<String>>,
@@ -2006,6 +2007,12 @@ Ignore linting a file by adding an ignore comment at the top of the file:
     .defer(|cmd| {
       cmd
         .arg(
+          Arg::new("fix")
+            .long("fix")
+            .help("Fix any linting errors for rules that support it")
+            .action(ArgAction::SetTrue),
+        )
+        .arg(
           Arg::new("rules")
             .long("rules")
             .help("List available rules")
@@ -3622,6 +3629,7 @@ fn lint_parse(flags: &mut Flags, matches: &mut ArgMatches) {
     Some(f) => f.collect(),
     None => vec![],
   };
+  let fix = matches.get_flag("fix");
   let rules = matches.get_flag("rules");
   let maybe_rules_tags = matches
     .remove_many::<String>("rules-tags")
@@ -3642,6 +3650,7 @@ fn lint_parse(flags: &mut Flags, matches: &mut ArgMatches) {
       include: files,
       ignore,
     },
+    fix,
     rules,
     maybe_rules_tags,
     maybe_rules_include,
@@ -5015,6 +5024,7 @@ mod tests {
             include: vec!["script_1.ts".to_string(), "script_2.ts".to_string(),],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5042,6 +5052,7 @@ mod tests {
             include: vec!["script_1.ts".to_string(), "script_2.ts".to_string()],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5070,6 +5081,7 @@ mod tests {
             include: vec!["script_1.ts".to_string(), "script_2.ts".to_string()],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5085,8 +5097,12 @@ mod tests {
       }
     );
 
-    let r =
-      flags_from_vec(svec!["deno", "lint", "--ignore=script_1.ts,script_2.ts"]);
+    let r = flags_from_vec(svec![
+      "deno",
+      "lint",
+      "--fix",
+      "--ignore=script_1.ts,script_2.ts"
+    ]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -5095,6 +5111,7 @@ mod tests {
             include: vec![],
             ignore: vec!["script_1.ts".to_string(), "script_2.ts".to_string()],
           },
+          fix: true,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5116,6 +5133,7 @@ mod tests {
             include: vec![],
             ignore: vec![],
           },
+          fix: false,
           rules: true,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5142,6 +5160,7 @@ mod tests {
             include: vec![],
             ignore: vec![],
           },
+          fix: false,
           rules: true,
           maybe_rules_tags: Some(svec!["recommended"]),
           maybe_rules_include: None,
@@ -5169,6 +5188,7 @@ mod tests {
             include: vec![],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: Some(svec![""]),
           maybe_rules_include: Some(svec!["ban-untagged-todo", "no-undef"]),
@@ -5190,6 +5210,7 @@ mod tests {
             include: vec!["script_1.ts".to_string()],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5218,6 +5239,7 @@ mod tests {
             include: vec!["script_1.ts".to_string()],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
@@ -5247,6 +5269,7 @@ mod tests {
             include: vec!["script_1.ts".to_string()],
             ignore: vec![],
           },
+          fix: false,
           rules: false,
           maybe_rules_tags: None,
           maybe_rules_include: None,
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index d957b8f80..cb4473ca2 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -391,6 +391,7 @@ pub struct LintOptions {
   pub rules: LintRulesConfig,
   pub files: FilePatterns,
   pub reporter_kind: LintReporterKind,
+  pub fix: bool,
 }
 
 impl LintOptions {
@@ -399,6 +400,7 @@ impl LintOptions {
       rules: Default::default(),
       files: FilePatterns::new_with_base(base),
       reporter_kind: Default::default(),
+      fix: false,
     }
   }
 
@@ -407,6 +409,7 @@ impl LintOptions {
     maybe_lint_flags: Option<LintFlags>,
     initial_cwd: &Path,
   ) -> Result<Self, AnyError> {
+    let fix = maybe_lint_flags.as_ref().map(|f| f.fix).unwrap_or(false);
     let mut maybe_reporter_kind =
       maybe_lint_flags.as_ref().and_then(|lint_flags| {
         if lint_flags.json {
@@ -464,6 +467,7 @@ impl LintOptions {
         maybe_rules_include,
         maybe_rules_exclude,
       ),
+      fix,
     })
   }
 }
diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs
index 71f07275e..54073edaf 100644
--- a/cli/lsp/analysis.rs
+++ b/cli/lsp/analysis.rs
@@ -19,6 +19,7 @@ use deno_core::anyhow::anyhow;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::serde::Deserialize;
+use deno_core::serde::Serialize;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::ModuleSpecifier;
@@ -78,6 +79,19 @@ static IMPORT_SPECIFIER_RE: Lazy<Regex> =
 
 const SUPPORTED_EXTENSIONS: &[&str] = &[".ts", ".tsx", ".js", ".jsx", ".mjs"];
 
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct DataQuickFixChange {
+  pub range: Range,
+  pub new_text: String,
+}
+
+/// A quick fix that's stored in the diagnostic's data field.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct DataQuickFix {
+  pub description: String,
+  pub changes: Vec<DataQuickFixChange>,
+}
+
 /// Category of self-generated diagnostic messages (those not coming from)
 /// TypeScript.
 #[derive(Debug, PartialEq, Eq)]
@@ -87,6 +101,7 @@ pub enum Category {
     message: String,
     code: String,
     hint: Option<String>,
+    quick_fixes: Vec<DataQuickFix>,
   },
 }
 
@@ -104,6 +119,7 @@ impl Reference {
         message,
         code,
         hint,
+        quick_fixes,
       } => lsp::Diagnostic {
         range: self.range,
         severity: Some(lsp::DiagnosticSeverity::WARNING),
@@ -120,19 +136,26 @@ impl Reference {
         },
         related_information: None,
         tags: None, // we should tag unused code
-        data: None,
+        data: if quick_fixes.is_empty() {
+          None
+        } else {
+          serde_json::to_value(quick_fixes).ok()
+        },
       },
     }
   }
 }
 
-fn as_lsp_range(diagnostic: &LintDiagnostic) -> Range {
-  let start_lc = diagnostic
-    .text_info
-    .line_and_column_index(diagnostic.range.start);
-  let end_lc = diagnostic
-    .text_info
-    .line_and_column_index(diagnostic.range.end);
+fn as_lsp_range_from_diagnostic(diagnostic: &LintDiagnostic) -> Range {
+  as_lsp_range(diagnostic.range, &diagnostic.text_info)
+}
+
+fn as_lsp_range(
+  source_range: SourceRange,
+  text_info: &SourceTextInfo,
+) -> Range {
+  let start_lc = text_info.line_and_column_index(source_range.start);
+  let end_lc = text_info.line_and_column_index(source_range.end);
   Range {
     start: Position {
       line: start_lc.line_index as u32,
@@ -156,11 +179,26 @@ pub fn get_lint_references(
     lint_diagnostics
       .into_iter()
       .map(|d| Reference {
-        range: as_lsp_range(&d),
+        range: as_lsp_range_from_diagnostic(&d),
         category: Category::Lint {
           message: d.message,
           code: d.code,
           hint: d.hint,
+          quick_fixes: d
+            .fixes
+            .into_iter()
+            .map(|f| DataQuickFix {
+              description: f.description.to_string(),
+              changes: f
+                .changes
+                .into_iter()
+                .map(|change| DataQuickFixChange {
+                  range: as_lsp_range(change.range, &d.text_info),
+                  new_text: change.new_text.to_string(),
+                })
+                .collect(),
+            })
+            .collect(),
         },
       })
       .collect(),
@@ -668,7 +706,57 @@ impl CodeActionCollection {
     Ok(())
   }
 
-  pub fn add_deno_lint_ignore_action(
+  pub fn add_deno_lint_actions(
+    &mut self,
+    specifier: &ModuleSpecifier,
+    diagnostic: &lsp::Diagnostic,
+    maybe_text_info: Option<SourceTextInfo>,
+    maybe_parsed_source: Option<deno_ast::ParsedSource>,
+  ) -> Result<(), AnyError> {
+    if let Some(data_quick_fixes) = diagnostic
+      .data
+      .as_ref()
+      .and_then(|d| serde_json::from_value::<Vec<DataQuickFix>>(d.clone()).ok())
+    {
+      for quick_fix in data_quick_fixes {
+        let mut changes = HashMap::new();
+        changes.insert(
+          specifier.clone(),
+          quick_fix
+            .changes
+            .into_iter()
+            .map(|change| lsp::TextEdit {
+              new_text: change.new_text.clone(),
+              range: change.range,
+            })
+            .collect(),
+        );
+        let code_action = lsp::CodeAction {
+          title: quick_fix.description.to_string(),
+          kind: Some(lsp::CodeActionKind::QUICKFIX),
+          diagnostics: Some(vec![diagnostic.clone()]),
+          command: None,
+          is_preferred: None,
+          disabled: None,
+          data: None,
+          edit: Some(lsp::WorkspaceEdit {
+            changes: Some(changes),
+            change_annotations: None,
+            document_changes: None,
+          }),
+        };
+        self.actions.push(CodeActionKind::DenoLint(code_action));
+      }
+    }
+    self.add_deno_lint_ignore_action(
+      specifier,
+      diagnostic,
+      maybe_text_info,
+      maybe_parsed_source,
+    )
+  }
+
+  fn add_deno_lint_ignore_action(
     &mut self,
     specifier: &ModuleSpecifier,
     diagnostic: &lsp::Diagnostic,
@@ -1087,6 +1175,7 @@ mod tests {
             message: "message1".to_string(),
             code: "code1".to_string(),
             hint: None,
+            quick_fixes: Vec::new(),
           },
           range,
         },
@@ -1105,6 +1194,7 @@ mod tests {
             message: "message2".to_string(),
             code: "code2".to_string(),
             hint: Some("hint2".to_string()),
+            quick_fixes: Vec::new(),
           },
           range,
         },
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index fe6904830..7b969b8ab 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -1691,6 +1691,7 @@ let c: number = "a";
       rules: Default::default(),
       files: FilePatterns::new_with_base(temp_dir.path().to_path_buf()),
       reporter_kind: Default::default(),
+      fix: false,
     };
 
     // test enabled
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 7d9c4318b..de5f7e357 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -2195,7 +2195,7 @@ impl Inner {
               })?
           }
           Some("deno-lint") => code_actions
-            .add_deno_lint_ignore_action(
+            .add_deno_lint_actions(
               &specifier,
               diagnostic,
               asset_or_doc.document().map(|d| d.text_info()),
diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs
index 251efd941..bf96eca06 100644
--- a/cli/tools/lint/mod.rs
+++ b/cli/tools/lint/mod.rs
@@ -10,6 +10,7 @@ use deno_ast::SourceRange;
 use deno_ast::SourceTextInfo;
 use deno_config::glob::FilePatterns;
 use deno_core::anyhow::bail;
+use deno_core::anyhow::Context;
 use deno_core::error::generic_error;
 use deno_core::error::AnyError;
 use deno_core::parking_lot::Mutex;
@@ -216,9 +217,10 @@ async fn lint_files(
 
   futures.push({
     let has_error = has_error.clone();
-    let lint_rules = lint_rules.rules.clone();
+    let linter = create_linter(lint_rules.rules);
     let reporter_lock = reporter_lock.clone();
     let incremental_cache = incremental_cache.clone();
+    let fix = lint_options.fix;
     deno_core::unsync::spawn(async move {
       run_parallelized(paths, {
         move |file_path| {
@@ -229,12 +231,15 @@ async fn lint_files(
             return Ok(());
           }
 
-          let r = lint_file(&file_path, file_text, lint_rules);
-          if let Ok((file_diagnostics, file_source)) = &r {
+          let r = lint_file(&linter, &file_path, file_text, fix);
+          if let Ok((file_source, file_diagnostics)) = &r {
             if file_diagnostics.is_empty() {
               // update the incremental cache if there were no diagnostics
-              incremental_cache
-                .update_file(&file_path, file_source.text_info().text_str())
+              incremental_cache.update_file(
+                &file_path,
+                // ensure the returned text is used here as it may have been modified via --fix
+                file_source.text_info().text_str(),
+              )
             }
           }
 
@@ -322,22 +327,145 @@ pub fn create_linter(rules: Vec<&'static dyn LintRule>) -> Linter {
 }
 
 fn lint_file(
+  linter: &Linter,
   file_path: &Path,
   source_code: String,
-  lint_rules: Vec<&'static dyn LintRule>,
-) -> Result<(Vec<LintDiagnostic>, ParsedSource), AnyError> {
+  fix: bool,
+) -> Result<(ParsedSource, Vec<LintDiagnostic>), AnyError> {
   let specifier = specifier_from_file_path(file_path)?;
   let media_type = MediaType::from_specifier(&specifier);
 
-  let linter = create_linter(lint_rules);
+  if fix {
+    lint_file_and_fix(linter, &specifier, media_type, source_code, file_path)
+  } else {
+    linter
+      .lint_file(LintFileOptions {
+        specifier,
+        media_type,
+        source_code,
+      })
+      .map_err(AnyError::from)
+  }
+}
 
-  let (source, file_diagnostics) = linter.lint_file(LintFileOptions {
-    specifier,
+fn lint_file_and_fix(
+  linter: &Linter,
+  specifier: &ModuleSpecifier,
+  media_type: MediaType,
+  source_code: String,
+  file_path: &Path,
+) -> Result<(ParsedSource, Vec<LintDiagnostic>), deno_core::anyhow::Error> {
+  // initial lint
+  let (source, diagnostics) = linter.lint_file(LintFileOptions {
+    specifier: specifier.clone(),
     media_type,
-    source_code: source_code.clone(),
+    source_code,
   })?;
 
-  Ok((file_diagnostics, source))
+  // Try applying fixes repeatedly until the file has none left or
+  // a maximum number of iterations is reached. This is necessary
+  // because lint fixes may overlap and so we can't always apply
+  // them in one pass.
+  let mut source = source;
+  let mut diagnostics = diagnostics;
+  let mut fix_iterations = 0;
+  loop {
+    let change = apply_lint_fixes_and_relint(
+      specifier,
+      media_type,
+      linter,
+      source.text_info(),
+      &diagnostics,
+    )?;
+    match change {
+      Some(change) => {
+        source = change.0;
+        diagnostics = change.1;
+      }
+      None => {
+        break;
+      }
+    }
+    fix_iterations += 1;
+    if fix_iterations > 5 {
+      log::warn!(
+        concat!(
+          "Reached maximum number of fix iterations for '{}'. There's ",
+          "probably a bug in Deno. Please fix this file manually.",
+        ),
+        specifier,
+      );
+      break;
+    }
+  }
+
+  if fix_iterations > 0 {
+    // everything looks good and the file still parses, so write it out
+    fs::write(file_path, source.text_info().text_str())
+      .context("Failed writing fix to file.")?;
+  }
+
+  Ok((source, diagnostics))
+}
+
+fn apply_lint_fixes_and_relint(
+  specifier: &ModuleSpecifier,
+  media_type: MediaType,
+  linter: &Linter,
+  text_info: &SourceTextInfo,
+  diagnostics: &[LintDiagnostic],
+) -> Result<Option<(ParsedSource, Vec<LintDiagnostic>)>, AnyError> {
+  let Some(new_text) = apply_lint_fixes(text_info, diagnostics) else {
+    return Ok(None);
+  };
+  linter
+    .lint_file(LintFileOptions {
+      specifier: specifier.clone(),
+      source_code: new_text,
+      media_type,
+    })
+    .map(Some)
+    .context(
+      "An applied lint fix caused a syntax error. Please report this bug.",
+    )
+}
+
+fn apply_lint_fixes(
+  text_info: &SourceTextInfo,
+  diagnostics: &[LintDiagnostic],
+) -> Option<String> {
+  if diagnostics.is_empty() {
+    return None;
+  }
+
+  let file_start = text_info.range().start;
+  let mut quick_fixes = diagnostics
+    .iter()
+    // use the first quick fix
+    .filter_map(|d| d.fixes.first())
+    .flat_map(|fix| fix.changes.iter())
+    .map(|change| deno_ast::TextChange {
+      range: change.range.as_byte_range(file_start),
+      new_text: change.new_text.to_string(),
+    })
+    .collect::<Vec<_>>();
+  if quick_fixes.is_empty() {
+    return None;
+  }
+  // remove any overlapping text changes, we'll circle
+  // back for another pass to fix the remaining
+  quick_fixes.sort_by_key(|change| change.range.start);
+  for i in (1..quick_fixes.len()).rev() {
+    let cur = &quick_fixes[i];
+    let previous = &quick_fixes[i - 1];
+    let is_overlapping = cur.range.start < previous.range.end;
+    if is_overlapping {
+      quick_fixes.remove(i);
+    }
+  }
+  let new_text =
+    deno_ast::apply_text_changes(text_info.text_str(), quick_fixes);
+  Some(new_text)
 }
 
 /// Lint stdin and write result to stdout.
@@ -346,7 +474,7 @@ fn lint_file(
 fn lint_stdin(
   file_path: &Path,
   lint_rules: Vec<&'static dyn LintRule>,
-) -> Result<(Vec<LintDiagnostic>, ParsedSource), AnyError> {
+) -> Result<(ParsedSource, Vec<LintDiagnostic>), AnyError> {
   let mut source_code = String::new();
   if stdin().read_to_string(&mut source_code).is_err() {
     return Err(generic_error("Failed to read from stdin"));
@@ -354,24 +482,24 @@ fn lint_stdin(
 
   let linter = create_linter(lint_rules);
 
-  let (source, file_diagnostics) = linter.lint_file(LintFileOptions {
-    specifier: specifier_from_file_path(file_path)?,
-    source_code: source_code.clone(),
-    media_type: MediaType::TypeScript,
-  })?;
-
-  Ok((file_diagnostics, source))
+  linter
+    .lint_file(LintFileOptions {
+      specifier: specifier_from_file_path(file_path)?,
+      source_code: source_code.clone(),
+      media_type: MediaType::TypeScript,
+    })
+    .map_err(AnyError::from)
 }
 
 fn handle_lint_result(
   file_path: &str,
-  result: Result<(Vec<LintDiagnostic>, ParsedSource), AnyError>,
+  result: Result<(ParsedSource, Vec<LintDiagnostic>), AnyError>,
   reporter_lock: Arc<Mutex<Box<dyn LintReporter + Send>>>,
 ) -> bool {
   let mut reporter = reporter_lock.lock();
 
   match result {
-    Ok((mut file_diagnostics, _source)) => {
+    Ok((_source, mut file_diagnostics)) => {
       file_diagnostics.sort_by(|a, b| match a.specifier.cmp(&b.specifier) {
         std::cmp::Ordering::Equal => a.range.start.cmp(&b.range.start),
         file_order => file_order,
@@ -493,17 +621,26 @@ struct LintError {
 
 struct PrettyLintReporter {
   lint_count: u32,
+  fixable_diagnostics: u32,
 }
 
 impl PrettyLintReporter {
   fn new() -> PrettyLintReporter {
-    PrettyLintReporter { lint_count: 0 }
+    PrettyLintReporter {
+      lint_count: 0,
+      fixable_diagnostics: 0,
+    }
   }
 }
 
 impl LintReporter for PrettyLintReporter {
   fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) {
     self.lint_count += 1;
+    if let LintOrCliDiagnostic::Lint(d) = d {
+      if !d.fixes.is_empty() {
+        self.fixable_diagnostics += 1;
+      }
+    }
 
     eprintln!("{}", d.display());
   }
@@ -514,9 +651,17 @@ impl LintReporter for PrettyLintReporter {
   }
 
   fn close(&mut self, check_count: usize) {
+    let fixable_suffix = if self.fixable_diagnostics > 0 {
+      colors::gray(format!(" ({} fixable via --fix)", self.fixable_diagnostics))
+        .to_string()
+    } else {
+      "".to_string()
+    };
     match self.lint_count {
-      1 => info!("Found 1 problem"),
-      n if n > 1 => info!("Found {} problems", self.lint_count),
+      1 => info!("Found 1 problem{}", fixable_suffix),
+      n if n > 1 => {
+        info!("Found {} problems{}", self.lint_count, fixable_suffix)
+      }
       _ => (),
     }