MSTEST0058/0061: fix the two MSTest analyzer diagnostics - #169
Merged
Conversation
MSTEST0058 - Semantics.Test/SemanticStringTests.cs
ErrorMessages_ContainTypeInformation asserted inside a catch block.
Replaced the try/catch/Assert.Fail shape with Assert.ThrowsExactly,
which is what the rest of the suite already uses, and asserted on the
returned exception's message.
MSTEST0061 - Semantics.Test/Paths/AbsoluteDirectoryPathParentTests.cs
Parent_OfUncShareRoot_ReturnsTheShareItself branched on
OperatingSystem.IsWindows() and called Assert.Inconclusive. Replaced
with [OSCondition(OperatingSystems.Windows)] so the test is skipped off
Windows rather than reporting inconclusive from inside the body. Also
tagged [TestCategory("OS-Specific")] to match the convention in
CLAUDE.md and PathValidationAttributeTests.
Behaviour note: off Windows this test now reports as skipped instead of
inconclusive. On Windows it runs exactly as before.
Verified: test project builds clean (0 warnings), 1095/1095 pass with 0
skipped on Windows.
|
Damon3000s
pushed a commit
to Damon3000s/Semantics
that referenced
this pull request
Aug 16, 2026
Two of the 24 (MSTEST0058/0061) were already fixed by ktsu-dev#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.
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 the two
external_roslyn:MSTEST*diagnostics — the last actionable items in the SonarCloud queue that aren't triage decisions or documented false positives.MSTEST0058— assert in a catch blockSemanticStringTests.ErrorMessages_ContainTypeInformationused try /Assert.Fail/ catch-and-assert. Replaced withAssert.ThrowsExactly<ArgumentException>, which the rest of the suite already uses, asserting on the returned exception's message.MSTEST0061— runtime OS check instead of[OSCondition]AbsoluteDirectoryPathParentTests.Parent_OfUncShareRoot_ReturnsTheShareItselfbranched onOperatingSystem.IsWindows()and calledAssert.Inconclusive. Replaced with[OSCondition(OperatingSystems.Windows)], plus[TestCategory("OS-Specific")]to match the convention inCLAUDE.mdandPathValidationAttributeTests.Behaviour change worth noting: off Windows this test now reports as skipped rather than inconclusive. On Windows it runs exactly as before. If CI runs on Linux, expect the skipped count to move by one.
Verification
🤖 Generated with Claude Code
https://claude.ai/code/session_01AgFo81FUAF3qx6o5ifnAwq