Skip to content

Clean up assorted analyzer warnings (S1066, S1192, S6444, CA1309, CA1825)#88

Merged
JohannBlais merged 2 commits into
masterfrom
chore/assorted-analyzer-cleanup
May 30, 2026
Merged

Clean up assorted analyzer warnings (S1066, S1192, S6444, CA1309, CA1825)#88
JohannBlais merged 2 commits into
masterfrom
chore/assorted-analyzer-cleanup

Conversation

@JohannBlais

Copy link
Copy Markdown
Owner

Summary

A focused batch of safe, valuable analyzer fixes — and an explicit list of what I deliberately left alone and why. No behavior change; both .NET Framework 4.8 projects build and all 96 tests pass.

Fixed (7 warnings)

Rule Where Change
S6444 InformationBoxForm.cs The auto-close label Regex now passes a 1-second timeout (new Regex(@"…", RegexOptions.None, TimeSpan.FromSeconds(1))), hardening against pathological matching. Matches the timeout convention already used in TextHelper.
CA1309 InformationBoxForm.cs Name.Equals("Help")Equals("Help", StringComparison.Ordinal). The control Name is a programmatic identifier — ordinal is both correct and faster.
S1192 InformationBoxForm.cs The auto-close countdown format "{0} ({1})" (used 4×) extracted to a ButtonCountdownFormat const.
S1066 ×2 InformationBoxDesigner.cs Two nested if blocks in GetAutoClose() merged into single && conditions.
CA1825 ×2 InformationBoxArgsParserTests.cs new T[0]Array.Empty<T>().

Deliberately NOT changed (with reasons)

Rule Why left alone
S2342 (rename InformationBoxCheckBox) Would break the public API.
S1450 (definedParameters → local) False positive — the field is read cross-instance via InformationBoxScope.Current.definedParameters, so it can't be a local.
S3878 (PaintingEngine.AddLines) False positive — GraphicsPath.AddLines has no params overload, so the explicit array is required.
S3011 (reflection on OnHelpRequested) Intentional and necessary to raise help on an arbitrary parent form; this is a review-only warning.
S1075 (hardcoded lorem-api URL), S3267 (loop → LINQ) Low value / would reduce clarity.

This leaves the remaining warnings as the genuinely "design-level" ones (S3776 cognitive complexity ×10, S107 too-many-params ×4) that touch the public API shape and warrant separate discussion rather than a mechanical fix.

Test plan

  • msbuild InfoBox.csproj + InfoBox.Designer.csproj (.NET 4.8) — build
  • dotnet test — InfoBoxCore.Tests 88/88, InfoBoxCore.Designer.Tests 8/8
  • Azure DevOps CI build

🤖 Generated with Claude Code

JohannBlais and others added 2 commits May 30, 2026 18:10
…825)

A focused batch of safe, valuable analyzer fixes. No behavior change;
both .NET Framework 4.8 projects build and all 96 tests pass.

- S6444 (InformationBoxForm.cs): the auto-close label Regex
  `new Regex(@".*?\(\d+\)")` now passes a 1-second timeout, hardening
  against pathological matching (matches the timeout convention already
  used in TextHelper).
- CA1309 (InformationBoxForm.cs): the "Help" button-name check uses
  StringComparison.Ordinal. The Name is a programmatic identifier, so
  ordinal is both correct and faster than the implicit culture-aware
  comparison.
- S1192 (InformationBoxForm.cs): the auto-close countdown format string
  "{0} ({1})", repeated 4x, is extracted to a ButtonCountdownFormat const.
- S1066 (InformationBoxDesigner.cs): two nested `if` statements in
  GetAutoClose() are merged into single conditions with `&&`.
- CA1825 (InformationBoxArgsParserTests.cs): two `new T[0]` allocations
  replaced with Array.Empty<T>().

Deliberately left untouched (with reasons):
- S2342 (rename InformationBoxCheckBox): would break the public API.
- S1450 (definedParameters -> local): false positive; the field is read
  cross-instance via InformationBoxScope.Current.definedParameters.
- S3878 (PaintingEngine AddLines): false positive; GraphicsPath.AddLines
  has no params overload, so the array is required.
- S3011 (reflection on OnHelpRequested): intentional and necessary to
  trigger help on an arbitrary parent form; review-only warning.
- S1075 (hardcoded lorem-api URL), S3267 (loop->LINQ): low value /
  would reduce clarity.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extracting the auto-close format string to a const (previous commit)
made the analyzer surface CA1863, which suggests caching a
System.Text.CompositeFormat. CompositeFormat only exists on .NET 8+,
but the shared source also compiles on .NET Framework 4.8, so the
suggested fix is unavailable - the same dual-build constraint that
already applies to CA2263. Suppressed in .editorconfig with the same
rationale, keeping the readability win of the named const without
trading it for four unfixable warnings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohannBlais
JohannBlais merged commit 2cd52ed into master May 30, 2026
1 check was pending
@JohannBlais

Copy link
Copy Markdown
Owner Author

Follow-up commit on this branch: extracting the format string to a const surfaced CA1863 ×4 (suggests CompositeFormat, which only exists on .NET 8+ and so is unavailable to the net48 shared source — same constraint as CA2263). Suppressed it in .editorconfig with the matching rationale, keeping the readable const. Azure DevOps build #287 green; total unique warnings now 25 (down from 32 at the start of this batch).

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