fix issues with diagnostic deduplication #36825
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Non-comparable diagnostics were being adding multiple times, which might also panic in the equality comparison
jbardin
added
no-changelog-needed
dsa0x
previously approved these changes
Apr 2, 2025
|
Found another incorrect comparison for equality, so updated the PR |
mildwonkey
previously approved these changes
Apr 2, 2025
Extra values can be any type, and are only asserted by interface by the party expecting that interface so we can't attempt to compare them for equality. The only diagnostic type with Extra values which implements Equals is and hcl diagnostic.
The source check was skipped if either value was nil, but we can't be sure they are equal if there is no value. This may prove to be problematic is tests, as diagnostic equality for deduplication isn't strict equality, but rather relies on being able to prove the context is also equal.
jbardin
force-pushed
the
jbardin/diag-dedupe
branch
from
784e53c to
291d26c
Compare
mildwonkey
approved these changes
Apr 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Non-comparable diagnostics were being adding multiple times to the final output, and would also panic in the equality comparison when encountering nil values.
The comparison of hcl diagnostics was incorrectly comparing missing Source values, but we can't compare location if it does not exist. This may present a problem with tests relying on
DiagnosticComparerwhich is usingEquals, but since hcl diagnostics should generally not exist without a source it is probably OK in practice.We were also comparing diagnostics with
Extradata as equal, butExtravalues can be any type, and are only asserted by interface by the party expecting that interface, so we can't attempt to compare them for equality.