Repository navigation
Refuse a diagnostic catalogue the built-in descriptors weren't allocated from [patch] - #70
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…ted from [patch] GeneratorBase required a Diagnostics catalogue but never read it, so a null catalogue, or one with a different prefix from the MetadataFileMissing and MetadataParseFailed descriptors, was silently accepted. Initialize now checks that both descriptors are in the catalogue and throws an InvalidOperationException naming the descriptor and the catalogue otherwise. Fixes #58 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3R5YfaNjqL9oXpRg6
|
This file contains hidden or 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
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.
Fixes #58
GeneratorBase.Diagnosticswas abstract but nothing read it. A generator could returnnull, or a catalogue with a different prefix from itsMetadataFileMissing/MetadataParseFaileddescriptors, and nothing caught the mismatch. That is exactly the prefix driftDiagnosticCatalogexists to prevent.Change
This is option (a), as the triage recommends.
Initializenow checks first thatDiagnosticsis non-null and that both built-in descriptors are among itsDescriptors(matched by ID). If either check fails, it throwsInvalidOperationExceptionnaming the generator, the member, the descriptor ID, the catalogue's category, and the IDs the catalogue actually holds. The Roslyn driver turns that into its CS8784 warning, and the generator emits nothing.The XML docs on
Diagnosticsand the README property table now state the requirement.Behaviour note for consumers: a generator whose two built-in descriptors come from a catalogue other than the one it names will now stop generating, where before it generated silently. That is the intent of the issue. Every test generator in this repository already complies.
Tests
ANullDiagnosticCatalogueIsRefusedADescriptorFromAnotherCatalogueIsRefused: the generator names anOTHcatalogue while its descriptors areTST001/TST002. The test checks that the message namesMetadataFileMissing (TST001)and the'Other'catalogue.EnsureCataloged()call removed and pass with it. The full suite passes (82 tests).Generated by Claude Code