Skip to content

Let DiagnosticManager take Locatables instead of Locations - #1878

Merged
TwitchBronBron merged 8 commits into
flatten-locationsfrom
diagnostic-locatables
Oct 15, 2026
Merged

TwitchBronBron merged 8 commits into
flatten-locationsfrom
diagnostic-locatables

Conversation

@markwpearce

@markwpearce markwpearce commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

DiagnosticManager.register() now takes a Locatable (node/token) anywhere it took a Location, including relatedInformation. Only pos/end/source get kept, so diagnostics don't hold onto the AST, and nothing gets resolved into a Location until getDiagnostics(). Suppressed diagnostics never get resolved.

  • clearing/filtering by file still works off source.uri
  • getDiagnostics() output is unchanged, still plain Locations for plugins/LSP
  • no call sites changed yet, that's a follow-up (location: util.getLocation(x) -> location: x)

Also fixes register() never merging relatedInformation when the same diagnostic was registered again (the check was backwards).

Benchmark: v1 (2ae472d) vs. v1 + all location PRs (#1845, #1865, #1866, #1867, #1868, #1876, #1878, #1879, #1880, #1881). Best of 5 runs. Diagnostics identical on every project.

Project Load Validate Heap Peak RSS Edit re-validation
roku-googlephotos 110 → 100 ms (−9%) 183 → 181 ms (−1%) 38.8 → 28.4 MB (−27%) 231 → 206 MB −2 to −7%
jellyfin-roku 388 → 342 ms (−12%) 1016 → 980 ms (−4%) 259 → 195 MB (−25%) 632 → 513 MB 0 to −3%
brighterscript-game-engine 744 → 518 ms (−30%) 1589 → 1573 ms (−1%) 313 → 234 MB (−25%) 762 → 570 MB 0 to −7%
jellyrock 504 → 424 ms (−16%) 1653 → 1586 ms (−4%) 335 → 246 MB (−27%) 873 → 723 MB 0 to −5%
Unamed OTT App 2088 → 1896 ms (−9%) 5104 → 5055 ms (−1%) 774 → 643 MB (−17%) 1294 → 1213 MB −1 to −7%
jellyfin-fork 382 → 330 ms (−14%) 1438 → 1425 ms (−1%) 286 → 241 MB (−16%) 963 → 863 MB −2 to −3%

🤖 Generated with Claude Code

register() now accepts a Locatable (node/token) anywhere it took a Location, including relatedInformation. Only pos/end/source get kept, so the AST isn't retained, and nothing gets turned into a Location until getDiagnostics(). Suppressed diagnostics never get resolved at all.

Also fixes register() never merging relatedInformation when the same diagnostic was registered again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@markwpearce markwpearce added this to the v1.0.0 milestone Oct 5, 2026
markwpearce and others added 7 commits October 6, 2026 08:22
Locatable version of `util.getLocation(a) ?? util.getLocation(b)`, for diagnostics that fall back to another node when the first one is synthetic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
They have no pos, so they were being stored (and returned) as if they were a Location. Check for a Location's uri/range instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tokens and nodes can have deprecated range/location getters (#1876), which made them look like a Location.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ic-locatables

# Conflicts:
#	src/DiagnosticManager.spec.ts
#	src/DiagnosticManager.ts
#	src/util.spec.ts
#	src/util.ts
@TwitchBronBron
TwitchBronBron merged commit c6b5053 into flatten-locations Oct 15, 2026
11 checks passed
@TwitchBronBron
TwitchBronBron deleted the diagnostic-locatables branch October 15, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants