fix(linter): Fix syntax error reporting in some output formatters. #19590
fix(linter): Fix syntax error reporting in some output formatters. #19590graphite-app[bot] merged 1 commit intomainfrom
Conversation
|
Confirmed that this impacted the checkstyle, github, junit, and unix formatters at least. Not stylish or default, though. |
There was a problem hiding this comment.
Pull request overview
Fixes truncated diagnostics (notably parser errors containing :) by removing overly aggressive message parsing in the diagnostics reporter, and adds regression coverage to prevent formatter regressions.
Changes:
- Remove colon-based message truncation from
Info::new(usediagnostic.to_string()as-is). - Add a Unix reporter unit regression test for messages containing colons.
- Add snapshot regression coverage using a parser-error fixture across multiple CLI output formatters.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/oxc_diagnostics/src/reporter.rs | Stops truncating diagnostic messages on : when building Info, preventing loss of parser error text. |
| apps/oxlint/src/output_formatter/unix.rs | Adds a unit regression test ensuring Unix output preserves messages containing :. |
| apps/oxlint/src/output_formatter/mod.rs | Adds a snapshot regression test running multiple output formatters against a parser-error fixture. |
| apps/oxlint/fixtures/output_formatter_diagnostic/parser-error.js | Adds a fixture that triggers a parser error containing : in the message. |
Merging this PR will not alter performance
Comparing Footnotes
|
|
I am not sure what is causing the failure in the tests 🤔 |
Merge activity
|
…19590) Add snapshot tests for all the formatters with this case to make sure we don't regress on this for any of them in the future. Basically, we were being overly aggressive in the stripping of the rule ID from the message, which caused messages with colons in them (e.g. parser errors like `Expected `;` but found `:``) to be truncated to just the character after the first colon. We don't actually need to do this anymore, as the `diagnostic.to_string()` method doesn't include the rule ID (if it ever did), so we can just use the full string as the message without any additional parsing. Generated with Claude Code. Fixes #19588.
09507d0 to
7958b56
Compare
### 🚀 Features - e814049 oxc_data_structure/rope: Add `get_offset_from_line_and_column` (#18133) (Sysix) ### 🐛 Bug Fixes - 7958b56 linter: Fix syntax error reporting in some output formatters. (#19590) (connorshea) - e316694 codegen: Avoid sourcemap panic on `U+2028`/`U+2029` (#19548) (camc314) - 933ff72 semantic: Emit correct error code for reserved type name (#19545) (camc314) ### ⚡ Performance - b5fa195 codegen: Remove bounds check from `SourcemapBuilder` (#19578) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
# Oxlint ### 🚀 Features - 46177dd linter: Implement unicorn/prefer-module (#19603) (camc314) - 42f78bb linter: Implement unicorn/prefer-ternary (#19605) (camc314) ### 🐛 Bug Fixes - 43df857 react/exhaustive-deps: Normalize .current callback deps (#19610) (camc314) - 574f48f linter/no-throw-literal: Close warning block (#19612) (camc314) - 79fe3b4 linter/prefer-mock-return-shorthand: Avoid unsafe autofixes for call-like returns (#19581) (camc314) - 85045e8 linter: Check protected members in explicit-module-boundary-types (#19594) (camc314) - e38115e linter: Catch missing return type on exported arrow/function expressions (#19587) (Peter Wagenet) - 419d3fd linter: Fix false negatives in typescript/no-require-imports (#19589) (Peter Wagenet) - 7958b56 linter: Fix syntax error reporting in some output formatters. (#19590) (connorshea) - 024f51c linter: Add help text to more eslint diagnostics (#19591) (Anthony Amaro) - a8489a1 linter: Warning `eslint/no-throw-literal` rule to be deprecated, better use `typescript/only-throw-error` (#19593) (Said Atrahouch) - 50fc70d linter/type-aware: Use correct span for disable directives (#19576) (camc314) - 421a99c linter: Add help guidance to eslint diagnostic messages (#19562) (Anthony Amaro) - e81364a linter: Add help text to eslint rule diagnostics (#19560) (Anthony Amaro) - 89b58d0 linter: Add help text to more eslint rule diagnostics (#19561) (Anthony Amaro) - 74f7833 linter/jest/prefer-mock-return-shorthand: Preserve typed arrow returns (#19556) (camc314) - bdd6f34 linter: Restrict prefer-import-in-mock to mock calls (#19555) (camc314) ### 📚 Documentation - a331993 linter: Improve docs for `eslint/radix` rule. (#19611) (connorshea) ### 🛡️ Security - c67f9dc linter: Update ajv version. (#19613) (connorshea) # Oxfmt ### 🚀 Features - 984dc07 oxfmt: Strip `"experimental"SortXxx` prefix (#19567) (leaysgur) ### 🐛 Bug Fixes - d7b63a4 oxfmt: Update API types for `sortPackageJsonOptions` (#19569) (leaysgur) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Add snapshot tests for all the formatters with this case to make sure we don't regress on this for any of them in the future.
Basically, we were being overly aggressive in the stripping of the rule ID from the message, which caused messages with colons in them (e.g. parser errors like
Expected;but found:``) to be truncated to just the character after the first colon.We don't actually need to do this anymore, as the
diagnostic.to_string()method doesn't include the rule ID (if it ever did), so we can just use the full string as the message without any additional parsing.Generated with Claude Code.
Fixes #19588.