Fix all analyzer warnings and enable TreatWarningsAsErrors - #223
Merged
Conversation
ReferenceEquals on a long?/DateTime? boxes the value, so the null checks only worked by the accident that a null nullable boxes to a null reference. Use plain null comparisons instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Check TryParse success explicitly at each call site instead of relying on the out value being zeroed on failure. This also fixes a real bug in ArtApiHandler: a failed parse of the size parameter overwrote the Int32.MaxValue sentinel with 0, so an invalid size attempted a 0-pixel resize instead of skipping the resize. The stream/length locals in TranscodeApiHandler were leftovers from before the TranscodeStreamer refactor and are now deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
WebRequest.Create is obsolete (SYSLIB0014); the two last.fm token/session GETs now go through a shared static HttpClient. Error semantics are unchanged: both APIs throw on a failed request. ServerInfo.TempFolder (never reassigned) becomes a get-only property and UserPurge.Queue becomes a property, resolving CA2211. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Convert dictionary reads guarded by ContainsKey to single TryGetValue lookups across the API handlers and the session/user repository caches. No behavior change; short-circuit ordering of compound conditions is preserved at every site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Declare locals, fields, and private helpers with their concrete types (List, Dictionary, HashSet, FileStream) instead of interfaces, and mark members that touch no instance state as static, updating call sites. The 3-arg IApiHandler.Process implementations stay instance methods; only the non-interface overloads and helpers changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
A full rebuild surfaced analyzer warnings that incremental builds had
hidden. The behavior-relevant ones:
- CA2013: more nullable value types passed to ReferenceEquals in
Playlist, User, and UserRepository; use plain null comparisons.
- CA2208: FavoritesForAlbumArtistId threw ArgumentNullException with
the wrong parameter name ("artistId", a copy-paste from the artist
overload), and InsertTypeExtensions.QueryText passed the type name
instead of the parameter name; both now use nameof.
- CA1873: Log.Write now checks ILogger.IsEnabled before logging so
disabled levels skip message formatting.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
CA1861 constant array arguments become params Split/Trim calls or static readonly fields; CA2263 Enum.GetNames uses the generic overload; CA1846/CA1866/CA1834 switch to span, char, and Append(char) overloads; CA1840 uses Environment.CurrentManagedThreadId. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Constructor guards use ArgumentNullException.ThrowIfNull; the nullable int? parameter guards in FavoriteRepository keep explicit throws (to avoid CA1871 boxing) but use nameof for the parameter names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
P/Invoke externs become internal (no callers outside the assembly) with BestFitMapping disabled on the string-marshaling imports; GetMapping calls with compile-time types use the generic overload; Bind* results are explicitly discarded; PreparedSqlLiteInsertCommand now declares IDisposable so its existing dispose pattern is recognized; plus small span/char-overload and Length-over-LINQ cleanups. The ArgumentException in DoSavePointExecute had its message passed as paramName; the arguments are now in the right order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Test-class Dispose() methods call GC.SuppressFinalize (CA1816), E2E HTTP calls pass TestContext.Current.CancellationToken so test cancellation stays responsive (xUnit1051), and the starred-songs count assertion uses Assert.Single (xUnit2013). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF
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
Eliminates all ~183 unique analyzer warnings (a full
--no-incrementalrebuild reported more than the original ~143 estimate — incremental builds hide warnings from up-to-date projects) and turns onTreatWarningsAsErrorsinDirectory.Build.propsso new warnings fail the build. Fixes are grouped into 11 commits by warning category, ordered bug-risk first, intended for rebase-and-merge.Real bugs fixed along the way
Int32.TryParseof thesizeparameter overwrote theInt32.MaxValuesentinel with 0, so an invalid size attempted a 0-pixel resize instead of skipping (CA1806)ArgumentNullExceptionnaming"artistId"— copy-paste from the artist overload (CA2208)ArgumentExceptionmessage and paramName were swapped (CA2208)stream/lengthlocals left over from the TranscodeStreamer refactor (CS0219)ReferenceEquals, which only worked because a null nullable boxes to a null reference (CA2013)WebRequestto a sharedHttpClient(SYSLIB0014)Mechanical groups
ContainsKey+ indexer →TryGetValue(47 sites, API handlers + repository caches)Enum.GetNames, span/char overloadsArgumentNullException.ThrowIfNull(explicit throws kept forint?params to avoid CA1871 boxing)BestFitMapping = falseon string-marshaling imports (deliberately noThrowOnUnmappableChar— would change behavior), genericGetMapping, discardedBind*results,IDisposabledeclared onPreparedSqlLiteInsertCommandGC.SuppressFinalizeinDispose()(CA1816),TestContext.Current.CancellationTokenon E2E HTTP calls (xUnit1051),Assert.Single(xUnit2013)Test plan
dotnet build WaveBox.slnx --no-incremental— 0 warnings, 0 errors (with warnings-as-errors on)dotnet test— 463 passed, 0 failed (Core 193, Server 251, E2E 19), verified after each commit🤖 Generated with Claude Code
https://claude.ai/code/session_019u4sFGNHRSAKp1Hjdr1oRF