Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_diagnostics): allow diagnostic locations to be created without a resource #3834

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Nov 23, 2022

Summary

Fixes #3829

This change makes the resource field of a diagnostics Location optional instead of the location itself. This allows a diagnostic to be initially created with a span but no resource, and have the resource only injected later.

Test Plan

This change is mostly at the type level and should be checked by the Rust compiler. Since it only allows diagnostics to be combined in new ways and no code is making use of the new feature yet, it shouldn't have any visible change on existing diagnostics.

@leops leops requested a review from a team November 23, 2022 11:22
@netlify
Copy link

netlify bot commented Nov 23, 2022

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit 9fa6984
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/637e1b495ed7e90008216dc4

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44936 44936 0
Failed 943 943 0
Panics 0 0 0
Coverage 97.94% 97.94% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1757 1757 0
Failed 4189 4189 0
Panics 0 0 0
Coverage 29.55% 29.55% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for flagging it! This will make things way easier and allow us to remove a bunch of code.

@leops leops added the A-Diagnostic Area: errors and diagnostics label Nov 23, 2022
@ematipico ematipico merged commit 6807934 into main Nov 23, 2022
@ematipico ematipico deleted the fix/diagnostic-location-resource branch November 23, 2022 13:32
jeysal added a commit to jeysal/rometools that referenced this pull request Nov 24, 2022
* upstream/main: (73 commits)
  fix(semantic_analyzers): style/noShoutyConstants does not recognize multiple uses of a constant. (rome#3789)
  feat(rome_js_analyze): useDefaultSwitchClauseLast (rome#3791)
  chore: run rustfmt and typo fix (rome#3840)
  feat(rome_js_analyze): use exhaustive deps support properties (rome#3581)
  website(docs): Fix text formatting (rome#3828)
  feat(rome_js_analyze): `noVoidTypeReturn` (rome#3806)
  feat(rome_cli): expose the `--verbose` flag to the CLI (rome#3812)
  fix(rome_diagnostics): allow diagnostic locations to be created without a resource (rome#3834)
  feat(rome_js_analyze): add noExtraNonNullAssertion rule (rome#3797)
  fix(rome_lsp): lsp friendly catch unwind (rome#3740)
  feat(rome_js_semantic): model improvements (rome#3825)
  feat(rome_json_parser): JSON Lexer (rome#3809)
  feat(rome_js_analyze): implement `noDistractingElements` (rome#3820)
  fix(rome_js_formatter): shothanded named import line break with default import (rome#3826)
  feat(rome_js_analyze): `noConstructorReturn` (rome#3805)
  feat(website): change enabledNurseryRules to All/Recommended select (rome#3810)
  feat(rome_js_analyze): noSetterReturn
  feat(rome_js_analyze): noConstructorReturn
  feat(rome_analyze): suppress rule via code actions (rome#3572)
  feat(rome_js_analyze): `noVar` (rome#3765)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Diagnostic Area: errors and diagnostics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Diagnostics don't inherit file path when creating a Location type
2 participants