Skip to content

[major] Sonar: clear all 24 open issues and the one security hotspot - #170

Merged
matt-edmondson merged 1 commit into
mainfrom
chore/sonar-cleanup
Aug 15, 2026
Merged

[major] Sonar: clear all 24 open issues and the one security hotspot#170
matt-edmondson merged 1 commit into
mainfrom
chore/sonar-cleanup

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Clears every open SonarCloud issue on ktsu-dev_Semantics (24) plus the one open security hotspot.

Two of the 24 (MSTEST0058/0061) were already fixed by #169 and had simply not been re-analyzed, so 22 required work here.

Breaking changes

This is tagged [major] — it cuts 3.0. docs/migration-guide-3.0.md covers both changes and is linked from CLAUDE.md.

S1133 (x10) — the ten [Obsolete] first-class .NET type validation attributes are removed: IsBoolean, IsDateTime, IsDecimal, IsDouble, IsGuid, IsInt32, IsIpAddress, IsTimeSpan, IsUri, IsVersion. They validated that a string parses as a type while still storing it as a string. Callers should wrap the .NET type directly.

Note for anyone reimplementing one: the removed attributes treated an empty string as valid. An equivalent custom attribute needs IsNullOrEmpty(value) || TryParse(...) to match.

S2342 (x2) — [Flags] enums renamed to the plural, matching ChordModifiers and the Chord.Omissions/Chord.Tensions properties that carry them: ChordOmissionChordOmissions, ChordTensionChordTensions. Members are unchanged, so it is a mechanical rename.

Non-breaking

S8969 (x4)Chord, Pitch, PitchClass, Progression.Parse. The null-forgiving operators were only redundant on the modern TFMs; on netstandard2.0/2.1 the BCL guards are annotation-oblivious, so simply deleting the ! would have raised CS8602 there. Replaced the IsNullOrEmpty guards with x is null || x.Length == 0 so the null state is definite on every TFM, then dropped the !. In Progression.TryParse the whitespace guard was already subsumed by the token-count check below it (splitting on whitespace with RemoveEmptyEntries yields no tokens for a blank input), so it reduces to a plain null check.

S3267 (x2)IsCreditCardNumberAttribute's digit scan becomes value.Any(...); IsSentenceCaseAttribute's stateful skip-the-first-letter loop becomes value.Where(char.IsLetter).Skip(1).Any(char.IsUpper).

S1244 (x4) — suppressed rather than changed. Both are false positives where a tolerance would make the code wrong:

  • Hsl.HueDegrees: max is a bit-exact copy of whichever of R/G/B is largest, so == is channel selection, not a numeric comparison. An epsilon would let two near-equal channels both match and pick the wrong hue sector.
  • NormalizedParameter.Denormalize/Normalize: skew == 1.0 is a fast path. Math.Pow(x, 1.0) returns x exactly, so skipping it is behaviourally identical, and a tolerance would change results for skews merely close to one.

S7637 (hotspot) — pinned dependabot/fetch-metadata to a full commit SHA, matching how the one other third-party action in the repo is pinned. Verified that the v3 and v3.1.0 tags both resolve to 25dd0e34.

Docs

The [ValidateAny] example used [IsEmailAddress, IsUri], which no longer compiles. Now [IsEmailAddress, StartsWith("https://")], updated in all four places it appears.

Verification

  • dotnet build clean — 0 warnings, 0 errors across all TFMs including netstandard2.0/2.1, which is what confirms the ! removals are safe.
  • dotnet test — 1083/1083 pass.
  • Generated quantity output has zero content diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CwVawzKnviybKCCzKhd7BS

Two of the 24 (MSTEST0058/0061) were already fixed by #169 and had just
not been re-analyzed. The remaining 22 are addressed here, plus the open
githubactions:S7637 hotspot.

BREAKING CHANGES
----------------
S1133 (x10) - the ten [Obsolete] first-class .NET type validation
  attributes are removed: IsBoolean, IsDateTime, IsDecimal, IsDouble,
  IsGuid, IsInt32, IsIpAddress, IsTimeSpan, IsUri, IsVersion. They
  validated that a string parses as a type while still storing it as a
  string. Callers should wrap the .NET type directly.

  Behavioural note for anyone reimplementing one: the removed attributes
  treated an empty string as VALID, so an equivalent custom attribute
  needs IsNullOrEmpty(value) || TryParse(...).

S2342 (x2) - [Flags] enums renamed to the plural, matching ChordModifiers
  and the Chord.Omissions/Chord.Tensions properties that carry them:
  ChordOmission -> ChordOmissions, ChordTension -> ChordTensions. Members
  are unchanged, so it is a mechanical rename.

docs/migration-guide-3.0.md covers both, linked from CLAUDE.md.

NON-BREAKING
------------
S8969 (x4) - Chord, Pitch, PitchClass, Progression.Parse. The null-
  forgiving operators were only redundant on the modern TFMs; on
  netstandard2.0/2.1 the BCL guards are annotation-oblivious, so simply
  deleting the '!' would have raised CS8602 there. Replaced the
  IsNullOrEmpty guards with 'x is null || x.Length == 0' so the null
  state is definite on every TFM, then dropped the '!'. In
  Progression.TryParse the whitespace guard was already subsumed by the
  token-count check below it (splitting on whitespace with
  RemoveEmptyEntries yields no tokens for a blank input), so it reduces
  to a plain null check.

S3267 (x2) - IsCreditCardNumberAttribute's digit scan becomes
  value.Any(...); IsSentenceCaseAttribute's stateful skip-the-first-
  letter loop becomes value.Where(char.IsLetter).Skip(1).Any(char.IsUpper).

S1244 (x4) - suppressed rather than changed; both are false positives
  where a tolerance would make the code wrong.

  Hsl.HueDegrees: 'max' is a bit-exact copy of whichever of R/G/B is
  largest, so == is channel selection, not a numeric comparison. An
  epsilon would let two near-equal channels both match and pick the
  wrong hue sector.

  NormalizedParameter.Denormalize/Normalize: 'skew == 1.0' is a fast
  path. Math.Pow(x, 1.0) returns x exactly, so skipping it is
  behaviourally identical, and a tolerance would change results for
  skews merely close to one.

S7637 - pinned dependabot/fetch-metadata to a full commit SHA, matching
  how the one other third-party action in the repo is pinned. Verified
  that the v3 and v3.1.0 tags both resolve to 25dd0e34.

Docs: the [ValidateAny] example used [IsEmailAddress, IsUri], which no
longer compiles. Now [IsEmailAddress, StartsWith("https://")], updated in
all four places it appears.

Verified: dotnet build clean (0 warnings, 0 errors) across all TFMs
including netstandard2.0/2.1, which is what confirms the '!' removals.
dotnet test 1083/1083 pass. Generated quantity output has zero content
diff.
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit e39d8ab into main Aug 15, 2026
6 checks passed
@matt-edmondson
matt-edmondson deleted the chore/sonar-cleanup branch August 15, 2026 03:04
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