diff --git a/cli/tools/lint/linter.rs b/cli/tools/lint/linter.rs index 3b6e8c68f4..5c71472322 100644 --- a/cli/tools/lint/linter.rs +++ b/cli/tools/lint/linter.rs @@ -197,7 +197,7 @@ impl CliLinter { media_type, &self.linter, self.deno_lint_config.clone(), - source.text_info_lazy(), + &source, &diagnostics, &external_linter_container, )?; @@ -215,7 +215,7 @@ impl CliLinter { log::warn!( concat!( "Reached maximum number of fix iterations for '{}'. There's ", - "probably a bug in Deno. Please fix this file manually.", + "probably a bug in the lint rule. Please fix this file manually.", ), specifier, ); @@ -243,25 +243,75 @@ fn apply_lint_fixes_and_relint( media_type: MediaType, linter: &DenoLintLinter, config: DenoLintConfig, - text_info: &SourceTextInfo, + original_source: &ParsedSource, diagnostics: &[LintDiagnostic], external_linter_container: &ExternalLinterContainer, ) -> Result)>, AnyError> { + let text_info = original_source.text_info_lazy(); let Some(new_text) = apply_lint_fixes(text_info, diagnostics) else { return Ok(None); }; - let (source, diagnostics) = linter - .lint_file(LintFileOptions { + let lint_with_text = |new_text: String| { + let (source, diagnostics) = linter.lint_file(LintFileOptions { specifier: specifier.clone(), source_code: new_text, media_type, - config, + config: config.clone(), external_linter: external_linter_container.get_callback(), - }) - .context( - "An applied lint fix caused a syntax error. Please report this bug.", - )?; + })?; + let mut new_diagnostics = source.diagnostics().clone(); + new_diagnostics.retain(|d| !original_source.diagnostics().contains(d)); + if let Some(diagnostic) = new_diagnostics.pop() { + return Err(AnyError::from(diagnostic)); + } + Ok((source, diagnostics)) + }; + + let (source, diagnostics) = match lint_with_text(new_text) { + Ok(result) => result, + Err(err) => { + let utf16_map = Utf16Map::new(text_info.text_str()); + // figure out which diagnostic caused a syntax error + let mut diagnostics = diagnostics.to_vec(); + while let Some(last_diagnostic) = diagnostics.pop() { + let Some(lint_fix) = last_diagnostic.details.fixes.first() else { + continue; + }; + let success = match apply_lint_fixes(text_info, &diagnostics) { + Some(new_text) => lint_with_text(new_text).is_ok(), + None => true, + }; + if success { + let mut changes_text = String::new(); + for change in &lint_fix.changes { + let utf8_start = + (change.range.start - text_info.range().start) as u32; + let utf8_end = (change.range.end - text_info.range().start) as u32; + let utf16_start = utf16_map + .utf8_to_utf16_offset(utf8_start.into()) + .unwrap_or(utf8_start.into()); + let utf16_end = utf16_map + .utf8_to_utf16_offset(utf8_end.into()) + .unwrap_or(utf8_end.into()); + changes_text.push_str(&format!( + "Range: [{}, {}]\n", + u32::from(utf16_start), + u32::from(utf16_end) + )); + changes_text.push_str(&format!("Text: {:?}\n\n", &change.new_text)); + } + return Err(err).context(format!( + "The '{}' rule caused a syntax error applying '{}'.\n\n{}", + last_diagnostic.details.code, lint_fix.description, changes_text + )); + } + } + return Err(err).context( + "A lint fix caused a syntax error. This is a bug in a lint rule.", + ); + } + }; if let Some(err) = external_linter_container.take_error() { return Err(err); diff --git a/cli/tools/lint/reporters.rs b/cli/tools/lint/reporters.rs index 447086a0d2..2aa50b6de8 100644 --- a/cli/tools/lint/reporters.rs +++ b/cli/tools/lint/reporters.rs @@ -55,11 +55,19 @@ impl LintReporter for PrettyLintReporter { fn visit_error(&mut self, file_path: &str, err: &AnyError) { log::error!("Error linting: {file_path}"); - if let Some(CoreError::Js(js_error)) = err.downcast_ref::() { - log::error!(" {}", format_js_error(js_error)); - return; + let text = + if let Some(CoreError::Js(js_error)) = err.downcast_ref::() { + format_js_error(js_error) + } else { + format!("{err:#}") + }; + for line in text.split('\n') { + if line.is_empty() { + log::error!(""); + } else { + log::error!(" {}", line); + } } - log::error!(" {err}"); } fn close(&mut self, check_count: usize) { diff --git a/tests/specs/lint/lint_plugin_fix_error/__test__.jsonc b/tests/specs/lint/lint_plugin_fix_error/__test__.jsonc new file mode 100644 index 0000000000..57da106ff6 --- /dev/null +++ b/tests/specs/lint/lint_plugin_fix_error/__test__.jsonc @@ -0,0 +1,6 @@ +{ + "tempDir": true, + "args": "lint --fix", + "output": "fix.out", + "exitCode": 1 +} diff --git a/tests/specs/lint/lint_plugin_fix_error/deno.json b/tests/specs/lint/lint_plugin_fix_error/deno.json new file mode 100644 index 0000000000..57b9dcb364 --- /dev/null +++ b/tests/specs/lint/lint_plugin_fix_error/deno.json @@ -0,0 +1,5 @@ +{ + "lint": { + "plugins": ["./plugin.ts"] + } +} diff --git a/tests/specs/lint/lint_plugin_fix_error/fix.out b/tests/specs/lint/lint_plugin_fix_error/fix.out new file mode 100644 index 0000000000..aed9d4df8a --- /dev/null +++ b/tests/specs/lint/lint_plugin_fix_error/fix.out @@ -0,0 +1,11 @@ +Error linting: [WILDLINE]main.ts + The 'test-plugin/my-rule' rule caused a syntax error applying 'Fix this test-plugin/my-rule problem'. + + Range: [14, 18] + Text: "garbage test test" + + : Expected a semicolon at file:///[WILDLINE]/main.ts:1:23 + + const value = garbage test test; + ~~~~ +Checked 2 files diff --git a/tests/specs/lint/lint_plugin_fix_error/main.ts b/tests/specs/lint/lint_plugin_fix_error/main.ts new file mode 100644 index 0000000000..5a277eecbb --- /dev/null +++ b/tests/specs/lint/lint_plugin_fix_error/main.ts @@ -0,0 +1,2 @@ +const value = "𝄞"; +console.log(value); diff --git a/tests/specs/lint/lint_plugin_fix_error/plugin.ts b/tests/specs/lint/lint_plugin_fix_error/plugin.ts new file mode 100644 index 0000000000..3df9f8655d --- /dev/null +++ b/tests/specs/lint/lint_plugin_fix_error/plugin.ts @@ -0,0 +1,20 @@ +export default { + name: "test-plugin", + rules: { + "my-rule": { + create(context) { + return { + VariableDeclarator(node) { + context.report({ + node: node.init, + message: 'should be equal to string "1"', + fix(fixer) { + return fixer.replaceText(node.init, "garbage test test"); + }, + }); + }, + }; + }, + }, + }, +}; diff --git a/tests/specs/lint/lint_plugin_infinite_edits/__test__.jsonc b/tests/specs/lint/lint_plugin_infinite_edits/__test__.jsonc new file mode 100644 index 0000000000..57da106ff6 --- /dev/null +++ b/tests/specs/lint/lint_plugin_infinite_edits/__test__.jsonc @@ -0,0 +1,6 @@ +{ + "tempDir": true, + "args": "lint --fix", + "output": "fix.out", + "exitCode": 1 +} diff --git a/tests/specs/lint/lint_plugin_infinite_edits/deno.json b/tests/specs/lint/lint_plugin_infinite_edits/deno.json new file mode 100644 index 0000000000..57b9dcb364 --- /dev/null +++ b/tests/specs/lint/lint_plugin_infinite_edits/deno.json @@ -0,0 +1,5 @@ +{ + "lint": { + "plugins": ["./plugin.ts"] + } +} diff --git a/tests/specs/lint/lint_plugin_infinite_edits/fix.out b/tests/specs/lint/lint_plugin_infinite_edits/fix.out new file mode 100644 index 0000000000..4dde757781 --- /dev/null +++ b/tests/specs/lint/lint_plugin_infinite_edits/fix.out @@ -0,0 +1,12 @@ +Reached maximum number of fix iterations for 'file:///[WILDLINE]/main.ts'. There's probably a bug in the lint rule. Please fix this file manually. +error[test-plugin/my-rule]: should be equal to string "1" + --> [WILDLINE]main.ts:1:15 + | +1 | const value = [WILDLINE]; + | [WILDLINE] + + docs: https://docs.deno.com/lint/rules/test-plugin/my-rule + + +Found 1 problem (1 fixable via --fix) +Checked 2 files diff --git a/tests/specs/lint/lint_plugin_infinite_edits/main.ts b/tests/specs/lint/lint_plugin_infinite_edits/main.ts new file mode 100644 index 0000000000..5a277eecbb --- /dev/null +++ b/tests/specs/lint/lint_plugin_infinite_edits/main.ts @@ -0,0 +1,2 @@ +const value = "𝄞"; +console.log(value); diff --git a/tests/specs/lint/lint_plugin_infinite_edits/plugin.ts b/tests/specs/lint/lint_plugin_infinite_edits/plugin.ts new file mode 100644 index 0000000000..5926fb868c --- /dev/null +++ b/tests/specs/lint/lint_plugin_infinite_edits/plugin.ts @@ -0,0 +1,20 @@ +export default { + name: "test-plugin", + rules: { + "my-rule": { + create(context) { + return { + VariableDeclarator(node) { + context.report({ + node: node.init, + message: 'should be equal to string "1"', + fix(fixer) { + return fixer.replaceText(node.init, Date.now().toString()); + }, + }); + }, + }; + }, + }, + }, +}; diff --git a/tests/specs/lint/lint_plugin_utf16/__test__.jsonc b/tests/specs/lint/lint_plugin_utf16/__test__.jsonc new file mode 100644 index 0000000000..e04db0eaf0 --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/__test__.jsonc @@ -0,0 +1,22 @@ +{ + "tests": { + "lint": { + "args": "lint", + "output": "lint.out", + "exitCode": 1 + }, + "fix": { + "tempDir": true, + "steps": [{ + "args": "lint --fix", + "output": "fix.out" + }, { + "args": [ + "eval", + "console.log(Deno.readTextFileSync('main.ts').trim())" + ], + "output": "fixed.out" + }] + } + } +} diff --git a/tests/specs/lint/lint_plugin_utf16/deno.json b/tests/specs/lint/lint_plugin_utf16/deno.json new file mode 100644 index 0000000000..57b9dcb364 --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/deno.json @@ -0,0 +1,5 @@ +{ + "lint": { + "plugins": ["./plugin.ts"] + } +} diff --git a/tests/specs/lint/lint_plugin_utf16/fix.out b/tests/specs/lint/lint_plugin_utf16/fix.out new file mode 100644 index 0000000000..158c556c29 --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/fix.out @@ -0,0 +1 @@ +Checked 2 files diff --git a/tests/specs/lint/lint_plugin_utf16/fixed.out b/tests/specs/lint/lint_plugin_utf16/fixed.out new file mode 100644 index 0000000000..46538595af --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/fixed.out @@ -0,0 +1,2 @@ +const value = "1"; +console.log(value); diff --git a/tests/specs/lint/lint_plugin_utf16/lint.out b/tests/specs/lint/lint_plugin_utf16/lint.out new file mode 100644 index 0000000000..cf78dcf9f3 --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/lint.out @@ -0,0 +1,11 @@ +error[test-plugin/my-rule]: should be equal to string "1" + --> [WILDLINE]main.ts:1:15 + | +1 | const value = "𝄞"; + | ^^^ + + docs: https://docs.deno.com/lint/rules/test-plugin/my-rule + + +Found 1 problem (1 fixable via --fix) +Checked 2 files diff --git a/tests/specs/lint/lint_plugin_utf16/main.ts b/tests/specs/lint/lint_plugin_utf16/main.ts new file mode 100644 index 0000000000..5a277eecbb --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/main.ts @@ -0,0 +1,2 @@ +const value = "𝄞"; +console.log(value); diff --git a/tests/specs/lint/lint_plugin_utf16/plugin.ts b/tests/specs/lint/lint_plugin_utf16/plugin.ts new file mode 100644 index 0000000000..40894f5e0e --- /dev/null +++ b/tests/specs/lint/lint_plugin_utf16/plugin.ts @@ -0,0 +1,22 @@ +export default { + name: "test-plugin", + rules: { + "my-rule": { + create(context) { + return { + VariableDeclarator(node) { + if (node.init.type !== "Literal" || node.init.value !== "1") { + context.report({ + node: node.init, + message: 'should be equal to string "1"', + fix(fixer) { + return fixer.replaceText(node.init, '"1"'); + }, + }); + } + }, + }; + }, + }, + }, +};