fix(linter/plugins): report() accept loc with no end#16859
fix(linter/plugins): report() accept loc with no end#16859graphite-app[bot] merged 1 commit intomainfrom
report() accept loc with no end#16859Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The main risk is that report() currently treats any object with a start property as a valid location and may fail later with less actionable errors; adding lightweight shape validation would improve debuggability and robustness. Separately, endLine?: number | undefined / endColumn?: number | undefined is redundant and adds API noise; the meaningful behavior is driven by hasOwn checks rather than the type itself.
Additional notes (1)
- Readability |
apps/oxlint/src-js/package/rule_tester.ts:250-255
ErrorBase.endLine?: number | undefinedandendColumn?: number | undefinedare semantically redundant: an optional property already implies it may beundefined. The real behavioral change here comes from thehasOwn(error, "endLine")checks, not the type.
If the intent is to encourage authors to explicitly include endLine: undefined in test cases, the type change doesn’t add much but does add noise to the public test-case interface. Consider keeping the simpler type and documenting the hasOwn behavior (or provide a dedicated sentinel/flag) so the API stays clean.
Summary of changes
Summary of changes
This PR updates JS-side lint reporting and tests to better match ESLint’s accepted loc shapes for Context#report().
-
apps/oxlint/src-js/plugins/report.ts- Expands
DiagnosticBase.locto accept either:- a
{ start, end? }shape whereendmay be omitted/null/undefined(LocationWithOptionalEnd), or - a shorthand
{ line, column }(LineColumn).
- a
- Updates
report()to normalize these inputs intostart/endoffsets, defaultingendtostartwhen missing. - Adds runtime narrowing via
typeAssertIsfor the supported shapes.
- Expands
-
apps/oxlint/src-js/package/rule_tester.ts- Allows
endLine/endColumnto be explicitlyundefined. - Adjusts location assertions in ESLint compat mode so tests can expect
end*to beundefinedwhenlocwas provided as{ line, column }and oxlint internally setsend === start.
- Allows
-
Conformance snapshots updated accordingly (
apps/oxlint/conformance/snapshot.md).
There was a problem hiding this comment.
Pull request overview
This PR enhances the Context#report() function to accept multiple formats for the loc property, matching ESLint's flexible API. Previously, loc required both start and end properties with LineColumn objects. Now it accepts five different formats: full location with start and end, start with null/undefined end, start only, or a plain line/column pair.
Key Changes
- Updated type definitions to allow
LocationWithOptionalEnd | LineColumnfor thelocproperty - Added logic to handle
locformats with optional or missingendcoordinates - Enhanced rule tester to properly validate test cases in ESLint compatibility mode when
endLine/endColumnare undefined
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/oxlint/src-js/plugins/report.ts | Added LocationWithOptionalEnd interface and updated report() to handle five different loc formats with appropriate branching logic |
| apps/oxlint/src-js/package/rule_tester.ts | Updated ErrorBase interface and assertInvalidTestCaseLocationIsCorrect() to handle endLine/endColumn being explicitly undefined in ESLint compat mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
Make `Context#report` accept multiple formats for `loc` property. These are all legal in ESLint:
```
{ start: { line, column }, end: { line, column } }
{ start: { line, column }, end: null }
{ start: { line, column }, end: undefined }
{ start: { line, column } }
{ line, column }
```
graphite-app
bot
force-pushed
the
12-14-fix_linter_plugins_report_accept_loc_with_no_end_
branch
from
82cbdb6 to
eccffcf
Compare
graphite-app
bot
deleted the
12-14-fix_linter_plugins_report_accept_loc_with_no_end_
branch
Make
Context#reportaccept multiple formats forlocproperty. These are all legal in ESLint: