fix: resolve the three open CodeQL alerts - #77
Merged
Conversation
All three are in code this cycle added, not pre-existing. #208 cs/local-not-disposed (warning) -- AtomicFile.WriteTempAsync assigned the FileStream to a local and then opened `await using (stream...)`. It is disposed in practice, but the analyser cannot prove it for the window between construction and the using taking effect. Rewritten as an `await using` declaration, which is both provably correct and shorter. #207 cs/linq/missed-select (note) -- ResolveStoredProviderName ran a filter-then-project foreach. Expressed as Select + FirstOrDefault, which says what it does. #209 cs/inefficient-containskey (note) -- a test called ContainsKey and then the indexer, two lookups where TryGetValue is one, and the out value makes the assertion read better besides. No behaviour change; 416 tests unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The entry was missing from the previous commit: the scripted edit asserted on a unique '### Fixed' heading, which is no longer unique now that [Unreleased] has both Breaking and Fixed sections. The assertion fired, the CHANGELOG was left untouched, and the commit went ahead with only the source changes. Co-Authored-By: Claude Opus 5 <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.
Closes the three open code scanning alerts. All three are in code added during this cycle — none are pre-existing, so this is cleaning up after the recent work rather than paying down old debt.
cs/local-not-disposedAtomicFile.cs:71cs/linq/missed-selectLibsecretCredentialManager.cs:514cs/inefficient-containskeyFileCredentialManagerTests.cs:317#208 — the only one that is more than style
WriteTempAsyncassigned theFileStreamto a local and then openedawait using (stream.ConfigureAwait(false)). It is disposed in practice, but there is a window between construction and theusingtaking effect that the analyser cannot prove, and it was flagged accordingly. Rewritten as anawait usingdeclaration, which is provably correct and two lines shorter.Worth noting this is code from the #54 permissions fix — the alert is a fair catch on my own change.
#207 and #209 — notes
ResolveStoredProviderNameran a filter-then-projectforeach; it now says so withSelect+FirstOrDefault. A test pairedContainsKeywith the indexer whereTryGetValueis one lookup, and theoutvalue makes the assertion read better.Verification
Build clean, 416 tests (358 passed, 58 skipped) — unchanged. No behaviour change; the CodeQL run on this PR is the actual check.
🤖 Generated with Claude Code