S3776: reduce cognitive complexity in seven methods - #167
Merged
Conversation
Splits the seven methods SonarCloud flags for cognitive complexity into named helpers. Pure extraction — no logic changes. The generated sources under Generated/ are byte-identical, which is the load-bearing check for four of the five files. Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs CollectAllOperators (43) and CollectAllProducts (42) each split per relationship kind, with the innermost per-form body extracted again: CollectIntegralOperators/CollectDerivativeOperators + Add*OpsForForm, CollectDotProducts/CollectCrossProducts + AddDotProductForForm and AddCrossProduct. GenerateInner (22) hands its Phase B loop body to EmitDimensionTypes and the overload loop to EmitOverloadTypes. Semantics.SourceGenerators/Models/DimensionsMetadata.cs Validate (37) -> ValidateDimension -> ValidateAvailableUnits and ValidateVectorForms -> ValidateVectorForm. The two seen-name sets stay threaded through so duplicates are still detected document-wide, and the issue ordering is unchanged. Semantics.SourceGenerators/Generators/UnitsGenerator.cs GenerateInner (23) -> BuildUnitToDimensionsMap, BuildUnitClass, BuildUnitsCatalogue. Semantics.SourceGenerators/Templates/PropertyTemplate.cs WriteTo (17) uses guard-clause returns; the long auto-accessor predicate becomes CanUseShorthand and the duplicated getter/setter branches collapse into one WriteAccessor helper. Semantics.Music/Chord.cs ConsumeModifiers (17) -> ConsumeOmissions, ConsumeTensions, ConsumeFifthAlteration, plus TakeEither for the ASCII/Unicode token pairs. Consumption order is load-bearing (the parse is destructive) and is preserved exactly; TakeEither keeps the original short-circuit, so only one spelling of a token is ever consumed. Verified: 8-TFM build clean (0 warnings), 1095/1095 tests pass, no drift under Generated/, and the local Sonar analyzers report no remaining S3776 in Semantics.SourceGenerators.
|
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
1 participant
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.
Clears all seven open
S3776cognitive-complexity findings. Pure extraction into named helpers — no logic changes anywhere.Sites
QuantitiesGenerator.csCollectAllOperatorsQuantitiesGenerator.csCollectAllProductsQuantitiesGenerator.csGenerateInnerDimensionsMetadata.csValidateUnitsGenerator.csGenerateInnerPropertyTemplate.csWriteToChord.csConsumeModifiersAll are now under the 15 threshold.
Notes for review
The generated sources under
Generated/are byte-identical. Four of the five files (QuantitiesGenerator,UnitsGenerator,DimensionsMetadata,PropertyTemplate) exist only to produce that emitted code, so zero drift is direct proof those extractions preserved behavior — stronger than test coverage alone.Chord.csis the one resting on tests instead.Chord.ConsumeModifiersis a destructive parse and its consumption order is load-bearing — omissions, then the flat sixth (before any bare6), then the altered tensions (multi-character tokens before bare numbers), then the fifth alteration, thenadd9. The split preserves that order exactly.The new
TakeEither(ref body, ascii, unicode)helper keeps the originalTake(a) || Take(b)short-circuit, so a body containing both spellings of a token still loses only the ASCII one. That is existing behavior, preserved deliberately rather than "fixed" — if it's a latent bug it deserves its own change with its own test.Chord.cswas a seventh site not on the original worklist (which had six, all inSemantics.SourceGenerators). It's inSemantics.Music, which the local Sonar setup can't reach, so it's the one site verified only by CI.Verification
Generated/.sonarlint/sonar-local.props, withS3776enabled atwarning) report zero remaining S3776 inSemantics.SourceGenerators🤖 Generated with Claude Code
https://claude.ai/code/session_01AgFo81FUAF3qx6o5ifnAwq