Fix CI Release-build analyzer errors#33
Merged
Merged
Conversation
CI's pr.yaml fetches .editorconfig from main before building (the 'protected configuration files' guard). Locally, our .editorconfig sets several rules to 'suggestion' so they don't fail the build, but on CI the file appears to be ignored (the fetch step writes UTF-8 with BOM, which can prevent the analyzer from picking it up). Either way, the findings are real, so fix them at the source rather than leaning on .editorconfig overrides. src: - InMemoryLogger / InMemoryLogger<T>: suppress MA0049 (type name matches containing namespace) on each class with a SuppressMessage attribute and justification - the package, namespace, and primary type intentionally share a name. - InMemoryLogger: use System.Threading.Lock on net9+ (MA0158) via conditional compilation; keep object-based lock on older TFMs. - InMemoryLogger.ScopeDisposable.Dispose: merge nested ifs (S1066). - InMemoryLoggerProvider.LogEntries / Loggers: suppress S2365 (properties copying collections) - this is a deliberate test-helper API; cost is documented in <remarks>. - InMemoryLoggerProvider.Loggers: pass StringComparer.Ordinal to the Dictionary copy constructor (MA0002), matching the inner ConcurrentDictionary. tests: - InMemoryLoggerOfTTests.Ctor_when_default_parameters_creates_instance: Assert.NotNull(sut) instead of relying on a ReSharper comment to silence S1481/S2699. - InMemoryLoggerProviderTests.Dispose_does_not_throw: capture and assert on Record.Exception so the test has a real assertion (S2699). Verified locally: build clean both with and without .editorconfig present (simulating CI), 52/52 tests pass on net462, net47, net471, net472, net48, net481, net6.0, net7.0, net8.0, net9.0, net10.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
PR #32's CI (failed run) surfaced 5 distinct analyzer errors in Release that didn't fire locally. Cause:
pr.yaml's "Fetch trusted configuration files from main branch" step writes.editorconfigwithOut-File -Encoding UTF8, which prefixes a BOM that the .NET analyzer engine appears to ignore — so the project's severity overrides (e.g.MA0002 = suggestion) don't apply on CI. (Separate issue — flagging for a follow-up to switch to[System.IO.File]::WriteAllText(..., (Get-Content ... -Raw), $utf8NoBom).)Rather than relying on
.editorconfigoverrides at all, this PR fixes the underlying findings.src
InMemoryLoggerandInMemoryLogger<T>: type name matches namespace. The package, namespace, and primary type intentionally share a name; suppressed via[SuppressMessage]with justification.InMemoryLogger._lock: useSystem.Threading.Lockonnet9.0+via#if NET9_0_OR_GREATER; keepobjectlock on older TFMs.ScopeDisposable.Dispose: merge nestedifs with&&; preserve the comment.InMemoryLoggerProvider.LogEntries/Loggers: properties materialize a snapshot, which is the deliberate test-helper API. Suppressed via[SuppressMessage]with justification; cost documented in<remarks>.InMemoryLoggerProvider.Loggers: passStringComparer.Ordinalto theDictionarycopy constructor, matching the innerConcurrentDictionary's comparer.tests
InMemoryLoggerOfTTests.Ctor_when_default_parameters_creates_instance:Assert.NotNull(sut)instead of relying on a ReSharper comment to silence the unused-variable rule and the missing-assertion rule.InMemoryLoggerProviderTests.Dispose_does_not_throw: capture withRecord.ExceptionandAssert.Null(ex)so the test has a real assertion.Test plan
dotnet build "src/Wolfgang.Extensions.Logging.InMemoryLogger/Wolfgang.Extensions.Logging.InMemoryLogger.csproj" -c Release— 0 errors with.editorconfigpresent.editorconfigtemporarily renamed (simulating the CI BOM bug) — 0 errorsdotnet test -c Release— 52/52 passing onnet462,net47,net471,net472,net48,net481,net6.0,net7.0,net8.0,net9.0,net10.0After this lands on
develop, PR #32 (develop -> main) will re-run CI.🤖 Generated with Claude Code