We had at the beginning some rules' diagnostics like `<rule-name>: diagnostic message`, from that message we did create a code action `Fix this <rule-name> problem`, but this is buggy.
I searched in `crates/oxc_linter` for `OxcDiagnostic::\w+\(".*:` and found only `no-invalid-regexp` to be a useful case.
For other rules this could be wrong like:
- Unexpected `javascript:` url
- `<Document />` from `next/document` should not be imported outside of `pages/_document.js`. See: https:/..`
- Always prefer `const x: T = { ... }`.
Closes#25278
## Overview
This PR resolves an oxlint language server crash that could occur when
requesting code actions for diagnostics reported by JS plugins,
particularly when the diagnostic appears after the beginning of a file.
JS plugin diagnostics can now safely provide regular fixes as well as
line-level and file-level disable actions.
## Root cause
1. In `run_external_rules()`, the `semantic` is being taken to run the
external JS plugin rules, leaving the former `semantic` inside the
context host empty:
https://github.com/oxc-project/oxc/blob/e23dccf338d568ac0c2be3b1223f213c797923f7/crates/oxc_linter/src/lib.rs#L539
2. The language server enables `with_ignore_fixes` to generate ignore
fixes, but inside the `add_ignore_fix()`, it doesn't check the
`section_source_text`.
3. The context host with the empty `semantic` passes the empty source
text to the `add_ignore_fix()` as `section_source_text`, and the empty
string just causes the crash.
https://github.com/oxc-project/oxc/blob/e23dccf338d568ac0c2be3b1223f213c797923f7/crates/oxc_linter/src/context/host.rs#L329-L334
## What changed
A new property `source_text` is added to the context sub host, which
stores a reference to the real source text string. Even if the
`semantic` is taken and becomes empty, the reference can still get the
real string and make things work.
```diff
pub struct ContextSubHost<'a> {
/// Semantic information about the file being linted, which includes scopes, symbols and AST nodes.
/// See [`Semantic`].
pub(super) semantic: Semantic<'a>,
// ...
+ /// Stable source text for this script section
+ /// which remains available even after `semantic` is taken while running JS plugins.
+ pub(super) source_text: &'a str,
/// The source text offset of the sub host
pub(super) source_text_offset: u32,
}
```
And I added a new utility method to get the `source_text` from the
context sub host.
```rs
#[inline]
pub fn source_text(&self) -> &'a str {
self.current_sub_host().source_text()
}
```
When passing source text to `add_ignore_fix()`, just use the new
`source_text()` method.
```diff
if self.with_ignore_fixes {
- let source_text = self.semantic().source_text();
+ let source_text = self.source_text();
for diagnostic in &mut diagnostics {
diagnostic.add_ignore_fix(self.current_sub_host().source_text_offset, source_text);
}
}
// (the same as other occurs
```
Also, I added the necessary check for the `section_source_text` in
`add_ignore_fix()`.
## Notes
This is my first contribution to oxc project. Thanks for reviewing!
`console.log` and `debugger;` are utils while developing.
`console.log` is already a suggestion (aligned with ESLint), but `no-debugger` would be autofixed too.
ESLint does not have a fixer for his rule, but we do.
This is a part of #22195, where do not autofix suggestions on the editor side, aligning with ESLint.
`debugger;` should not be autofixed on save.
ESLint shows by default suggestions in the editor. We did not have the right architecture to difference between safe or dangerous fix / suggestions. With https://github.com/oxc-project/oxc/pull/19113 we know now what kind of fix this is.
This allows us now to align more with the ESLint editor experience. The last found tech dep was https://github.com/oxc-project/oxc/pull/19795.
This will not apply for editors which manage its own configurations like VS Code editor and IntelliJ Plugin (cc @nrayburn-tech). These editors need their own default value changed.
When using auto save `source.fixAll.oxc` or `source.fixAll` code action, or the `oxc.fixAll` command, the suggestions will be applied too. Because this is always in a context of one file only, I guess this is safe. Or we can change the line for this: https://github.com/oxc-project/oxc/blob/a45b9f4f808e4761ee615c31ea32bccfad322602/apps/oxlint/src/lsp/code_actions.rs#L96-L100
Sysix