Close the gaps in the documentation generator's configuration key extractor - #1421
Merged
Merged
Conversation
…ractor
`GetRuleConfigurationKeys` only recognized fields whose type was exactly
`ConfigurationDefinition<T>`, and `TryGetRuleIdPrefix` gave up unless the key
started with `MA`. As a result `dotnet_diagnostic.MA0048.excluded_symbol_names`
was never attributed to MA0048, so the documentation validation never checked
that MA0048.md mentions it, and the definitions held in the MA0220 registry were
not walked at all. Those keys are documented today, by hand: the only automated
guard that a configurable rule documents its options simply was not running on
them.
The extractor now walks arrays and collections of definitions, strips a leading
`dotnet_diagnostic.` before matching the rule id, and fails the run on a key
that cannot be attributed to any rule, with an allow list for the options that
are not owned by a rule (`max_line_length`). A container with a shape the walker
cannot traverse throws instead of silently dropping its keys.
The option keys also used three casings. `ConfigurationDefinition<T>` now holds
the current name of the option followed by its legacy names, the first key set
in the configuration winning, so the odd ones are renamed to snake_case without
breaking the existing configurations:
- MA0007.IgnoreCatchAllArm -> MA0007.ignore_catch_all_arm
- MA0032.allowOverloadsWithOptionalParameters
-> MA0032.allow_overloads_with_optional_parameters
- MA0115.ReportPascalCaseUnmatchedParameter
-> MA0115.report_pascal_case_unmatched_parameter
This replaces the `IsHidden` property, whose only use was to hide the legacy
name of an option from the documentation: MA0053 and MA0104 no longer need a
second definition and a second lookup to keep their legacy names working, and
the generator documents the current name while still checking that every name
resolves to a rule.
`main` reworked the same code in the meantime: - `ConfigurationDefinition<T>` gained `IsRegex`/`RegexOptions`, and MA0220 now discovers the regex-valued options by reflection instead of a hardcoded array. The definitions carry their key list, so MA0220 iterates `Keys` to keep validating the legacy name of an option, and the regex helpers of `AnalyzerOptionsExtensions` resolve the definition instead of its current name only. - MA0104 read the legacy option by picking between two definitions. The two are merged into one carrying both names, so `GetNamespacesRegex` no longer has to choose. - The documentation generator extracts the definitions once into `GetConfigurationDefinitions` and validates that the keys mentioned in the documentation are declared. It now collects every key of a definition, so the legacy names mentioned in the documentation are declared, while only the current name is documented and validated per rule. The container detection of the generator is restricted to the enumerable types: `Func<ConfigurationDefinition<string>, string>`, the cached lambda of the MA0220 discovery, is a generic type mentioning a definition but is not a container. `AllOptionsEndingWithRegexAreValidatedByTheRule` asserted that `MA0104.namepaces_regex` is one of the keys returned by the MA0220 discovery. MA0220 still validates it, but as a key of the `namespaces_regex` definition, so the test considers every key of a definition.
This was referenced Sep 6, 2026
Closed
This was referenced Sep 24, 2026
Open
Open
Open
Open
Open
Open
Open
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.
What
GetRuleConfigurationKeysin the documentation generator only recognized fields whose type was exactlyConfigurationDefinition<T>, andTryGetRuleIdPrefixgave up unless the key's first two characters wereMA. Three consequences:dotnet_diagnostic.MA0048.excluded_symbol_names— the one option written with adotnet_diagnostic.prefix — was never attributed to MA0048, soValidateRuleDocumentationContainsConfigurationKeysnever checked thatMA0048.mdmentions it, and the generated "Configurable" tooltip listed 8 keys instead of 9.InvalidRegexConfigurationAnalyzer.RegexConfigurations, an array) were not walked at all.Every one of those keys is in fact documented today, by hand. The documentation check is the only automated guard that a configurable rule documents its options, and it had holes exactly where a rule deviates from the norm — which is when documentation is most likely to be missed.
Changes
Extractor (
src/DocumentationGenerator/Program.cs)ConfigurationDefinition<T>are walked in addition to bare fields. The type filter still runs beforeGetValue, so no unrelated static constructors are triggered. A container whose shape the walker cannot traverse now throws rather than dropping its keys quietly.dotnet_diagnostic.is stripped before matching the rule id.max_line_length).Option key casing
The keys used three casings (
.exclude_tostring_methods,MA0007.IgnoreCatchAllArm,MA0032.allowOverloadsWithOptionalParameters), which worked because Roslyn's key comparer is case-insensitive but made the option surface unguessable.ConfigurationDefinition<T>now holds the current name of an option followed by its legacy names, the first key set in the configuration winning:MA0007.ignore_catch_all_armMA0007.IgnoreCatchAllArmMA0032.allow_overloads_with_optional_parametersMA0032.allowOverloadsWithOptionalParametersMA0115.report_pascal_case_unmatched_parameterMA0115.ReportPascalCaseUnmatchedParameterThis replaces the
IsHiddenproperty, whose only use was hiding the legacy name of an option from the documentation. MA0053 (class_with_virtual_member_shoud_be_sealed) and MA0104 (namepaces_regex) no longer need a second definition and a "read the legacy key, pass it as the default" lookup, andGetConfigurationValue(ISymbol, definition, defaultValue)is removed with its last caller. The generator documents the current name only, while still checking that every name — legacy included — resolves to a rule.Notes for the reviewer
this([key], …), so the ~60 one-key definitions are untouched.Keysis anImmutableArray<string>built from astring[]constructor parameter rather than a collection-expression target: the netstandard2.0 / roslyn4.8 build'sSystem.Collections.ImmutablepredatesCollectionBuilderAttribute(CS9210). This keeps call sites written as collection expressions and the lookup'sforeachallocation-free.configuration.Keys, so an invalid regex is still reported against the key the user actually wrote.Keysrefactor itself: the documented key set is identical before and after.Testing
dotnet buildclean across all five Roslyn versions.dotnet run --project src/DocumentationGeneratorexits 0 and is idempotent. I verified the new guard fires by temporarily emptying the allow list, and that the array walking finds the MA0220 registry's 3 definitions where it previously found none.roslyn5.9suite: 3913 passed, 0 failed. The 349–351 configuration-related tests pass on 4.8 / 4.14 / 5.0 / 5.6 / 5.9; they include the pre-existing tests that setMA0053.class_with_virtual_member_shoud_be_sealedandMA0104.namepaces_regex, so the older aliases are covered through the new mechanism. The three renamed options got their existing tests parameterized over both the new and the legacy key.