Skip to content

Address the SonarCloud findings from #219 - #220

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/bold-planck-mxarux
Sep 12, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/bold-planck-mxarux

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Follow-up to #219, which merged before these landed. The quality gate passed there, so nothing blocked the merge — but six new issues were reported without being named, and all six are in code that PR added.

Its dashboard is reachable from the agent sandbox, so these came from the API rather than from guessing.

Four MSTest findings, and why the count mattered

MSTEST0046 wanted Assert.Contains over StringAssert.Contains, and MSTEST0037 wanted Assert.DoesNotContain over Assert.IsFalse(… .Any(…)).

This repository wants exactly that. Semantics.Test has 105 Assert.Contains call sites and had two StringAssert.Contains — both from #219 — plus 10 existing Assert.DoesNotContain. So the new test was the outlier, and these changes move it onto the surrounding convention.

Worth recording why that was checked rather than assumed: the identical rule went the other way in ktsu.Coder earlier today, where StringAssert.Contains has 188 call sites and the bare form has none, so the right answer there was to decline it. Same rule, opposite conclusion, decided by counting.

DoesNotContain takes an expected value and a collection rather than a collection and a predicate, so both assertions now project the refusals to their kinds first — which reads more directly than the IsFalse-around-Any it replaces:

VocabularyIssueKind[] kinds = [.. Vocabulary().Refused.Select(issue => issue.Kind)];

Assert.DoesNotContain(VocabularyIssueKind.UnknownDimension, kinds, "…");

Two nested ternaries, at major severity

S3358 on QuantityVocabulary.cs. Both predate the lift and moved with the file, which is why Sonar counts them as new code — and both are worth extracting on their own merits, since each was a physics rule compressed into nesting.

They are now named methods with the reasoning written out: BoundOf (only the magnitude form is bounded below at all, so a stricter floor on an overload applies there and nowhere else) and Wanted (a cross product defaults to three components because that is where a cross product exists; everything else defaults to every form).

Verification

  • dotnet build and dotnet build -c Release — clean, 0 warnings
  • Semantics.Test: 1136 tests, 0 failed
  • Semantics.Cpp.Test: 27 passed, including the g++/clang++ compile tests
  • Generated output still byte-identical — no drift under Semantics.Quantities/Generated/
  • No suppressions added

🤖 Generated with Claude Code

https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf


Generated by Claude Code

Six, all in code #219 added, and none of them arguable once the repository's
own conventions are checked rather than assumed. The gate passed there and it
merged before these landed.

Four are MSTest analyzer findings on the new test, and this repository wants
exactly what they ask for: Assert.Contains has 105 call sites in Semantics.Test
and StringAssert.Contains had two, both of them from that PR; Assert.DoesNotContain
has ten. Worth noting because the same rule went the other way in ktsu.Coder,
where StringAssert.Contains has 188 call sites and the bare form has none -- the
rule is the same and the right answer is the opposite, which is why the count is
worth taking before the suggestion is.

DoesNotContain takes an expected value and a collection rather than a
collection and a predicate, so the two assertions project the refusals to their
kinds first, which reads more directly than the IsFalse-around-Any they
replace.

The other two are S3358, nested ternaries in the vocabulary, at major severity.
Both predate the lift and moved with the file, which is why they are counted as
new code; both are now named methods -- how far down an overload is bounded, and
which forms a relationship asks for -- with the reasoning that was implicit in
the nesting written out.

Behaviour is unchanged: 1136 and 27 tests pass, and the generated output is
still byte-identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGCMUrT3jBgmANHNHcPmBf
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit f84c080 into main Sep 12, 2026
13 checks passed
@matt-edmondson
matt-edmondson deleted the claude/bold-planck-mxarux branch September 12, 2026 10:52
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.

2 participants