Skip to content

Add a local SonarCloud check; simplify AdjustForContrast; cover PatternValidationRule - #162

Merged
matt-edmondson merged 1 commit into
mainfrom
chore/sonar-complexity
Aug 14, 2026
Merged

Add a local SonarCloud check; simplify AdjustForContrast; cover PatternValidationRule#162
matt-edmondson merged 1 commit into
mainfrom
chore/sonar-complexity

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Three things: a workflow fix, one S3776 refactor, and closing the coverage gap that failed #161's gate.

A local SonarCloud check

CI injects the Sonar analyzers via the scanner, so a plain dotnet build never runs them — findings only surface after a push, a ~10 minute round trip per attempt. This session spent a lot of time on exactly that loop. Ported the opt-in from KtsuBuild:

dotnet build -p:CustomBeforeMicrosoftCommonProps=$PWD/.sonarlint/sonar-local.props

.sonarlint/sonar-local.globalconfig raises the rules CI reports that the analyzer package ships disabled, and silences the ones CI's profile does not report, so a clean local run means a clean CI run. Nothing imports it automatically — normal builds, CI and packaging are unaffected. Documented in CLAUDE.md.

Known limitation, stated up front: it currently only reaches Semantics.SourceGenerators. That project declares its SDK with the <Project Sdk="..."> attribute form, whereas the ktsu.Sdk projects use <Project> with <Sdk Name="..." /> elements, and CustomBeforeMicrosoftCommonProps does not reach those. Findings in Strings, Paths, Music, Color and Quantities still have to be read from SonarCloud. Worth solving properly in the SDK — probably a Directory.Build.props hook gated on a property — but that belongs in ktsu.Sdk rather than here.

Even limited to one project it is useful: Semantics.SourceGenerators holds 7 of the 9 remaining S3776 findings and most of the remaining S1192.

S3776 — AdjustForContrast (complexity 19)

Two changes, both of which also remove duplication:

  • The WCAG thresholds were written out twice — a switch here and a pair of ternaries in AccessibilityLevelAgainst. Both now call a shared RequiredContrast(level, largeText).
  • The binary search had a four-way branch. The interval always shrinks toward the end that satisfies the requirement, and when darkening the roles simply swap, so it reduces to one comparison:
if (meets == goLighter) { hi = mid; } else { lo = mid; }

Coverage — PatternValidationRule

This is what failed #161's new_coverage condition: the class had no tests at all, which matters more now that it carries a regex match timeout. Added six, including one that proves the hardening works rather than assuming it:

PatternValidationRule rule = new("^(a+)+$");          // classic catastrophic backtracking
Assert.ThrowsExactly<RegexMatchTimeoutException>(() => rule.Validate(value));

Without the timeout added in #161 that call does not finish in any practical time. So the S6444 fix is now verified, not asserted.

Verification

  • dotnet build — 0 errors, 0 warnings, all target frameworks
  • dotnet test1095/1095 pass

Remaining — ~48

S3776 ×7 (all in Semantics.SourceGenerators, now locally checkable — QuantitiesGenerator has two at complexity 42 and 43, DimensionsMetadata one at 41), S1133 ×10, S1192 ×15, S4136 ×8 in SemanticString.cs, S1172 ×4, S3267 ×4, S2342 ×2, S4144 ×1.

…t PatternValidationRule

Local Sonar reproduction (ported from KtsuBuild):
  dotnet build -p:CustomBeforeMicrosoftCommonProps=$PWD/.sonarlint/sonar-local.props
CI injects the Sonar analyzers via the scanner, so a plain build never runs
them and findings only surface after a push - a ~10 minute round trip per
attempt, which this session spent a lot of time on. Documented in CLAUDE.md
including the limitation that it currently only reaches
Semantics.SourceGenerators: that project declares its SDK with the
<Project Sdk="..."> attribute form, and CustomBeforeMicrosoftCommonProps does
not reach the ktsu.Sdk projects that use <Sdk Name="..." /> elements.

S3776 - AdjustForContrast was at complexity 19:
- The WCAG thresholds were spelled out twice, as a switch here and as a pair
  of ternaries in AccessibilityLevelAgainst. Both now call RequiredContrast.
- The binary search had a four-way branch. The interval always shrinks toward
  the end satisfying the requirement, and when darkening the roles simply
  swap, so it collapses to "did the midpoint land on the goLighter side".

PatternValidationRule had no tests at all, which is what failed the coverage
gate on #161 and matters more now that it carries a match timeout. Added six,
including one proving the catastrophic-backtracking case
("^(a+)+$" against a non-matching run of 40 a s) raises
RegexMatchTimeoutException instead of hanging - so the S6444 fix from #161 is
now actually verified rather than assumed.

Build clean on all TFMs, 1095/1095 tests pass.
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 413a2de into main Aug 14, 2026
6 checks passed
@matt-edmondson
matt-edmondson deleted the chore/sonar-complexity branch August 14, 2026 09:37
Damon3000s pushed a commit to Damon3000s/Semantics that referenced this pull request Aug 16, 2026
All 16 S1192 findings in Semantics.SourceGenerators, verified cleared with
the local Sonar check added in ktsu-dev#162 - the whole rule is now clean in that
project.

The generators built C# by repeating bare string literals: "public" 45
times, "static" 28, "/// <summary>" and "/// </summary>" 26 each, plus the
"value" and "right" parameter names, the diagnostic category, and a 150
character CA2225 SuppressMessage attribute spelled out four times. Collected
into an internal Emit class, so a typo in a keyword or a documentation
delimiter is now a compile error rather than malformed generated source.

141 literal replacements across 8 generator files.

Generated output is byte-identical - the constants carry the same values -
verified by zero drift under Semantics.Quantities/Generated.

Build clean on all TFMs, 1095/1095 tests pass.
Damon3000s pushed a commit to Damon3000s/Semantics that referenced this pull request Aug 16, 2026
Added S1172 to .sonarlint/sonar-local.globalconfig first - the analyzer
package ships it disabled, so the local check added in ktsu-dev#162 was not seeing
it even though CI reports it. It now surfaces locally like the rest.

Removed, with their call sites updated:
- EmitOverloadType(typeFormMap)
- EmitVectorOverloadType(dim)
- EmitVectorCrossDimOperators(ownerFullType, components)

EmitVectorOverloadType computes its own components array from dims, which is
why the caller-supplied one was redundant.

Verified with the local check: S1172 in Semantics.SourceGenerators is now
clean. Generated output byte-identical, no drift. Build clean on all TFMs,
1095/1095 tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant