Repository navigation
chore: clear the six CodeQL alerts raised since 30 October - #3154
Merged
Merged
Conversation
CodeQL raised six alerts between 30 October and 3 November, and none
failed a pull request's check, which passes on warnings and notes.
- AboutViewModel compared release.Tag with _latest?.Tag after an
`is not { } release` check, twice (cs/constant-condition). Both call
IsOnTheCard now, which keeps the null check where it can matter.
- DnsService's capture check and GamingProfile.HasAnyThatCanRun had
more than three nested && and || each (cs/complex-condition). Their
parts are named.
- The regex generator's matcher for AppAlertService.VersionNumber
checked a loop count that is never zero where it was checked. The
version's digits are an atomic group now: giving any back would leave
a digit or a dot next, so it never made a match.
- UninstallerViewModel created a CancellationTokenSource for a batch
and disposed it later in the same method, outside a finally
(cs/dispose-not-called-on-throw). The four places that replace the
field's source call RenewCancellation, so the field owns it.
No behavior changes. A new test holds the static-source capture check.
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
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.
Why
The code scanning baseline has been zero since September. Six alerts appeared between 30 October and 3 November. A pull request's CodeQL check passes on warnings and notes, and the default alert list shows main only, so none of them stopped a merge: five were on main by 2 November, and the sixth arrived with #3152 today.
What changes
AboutViewModel(1744, 1745,cs/constant-condition). Both re-checks after an await comparedrelease.Tag != _latest?.Tag, after_latest is not { } releaseat the top of the method, so CodeQL read the?.as dead. It is:_latestis only ever assigned a release, never null. Both callIsOnTheCard(release)now. It compares the tags with the null check kept, in case a later change ever clears_latest, and carries the comment the first check had.DnsService.CaptureSnapshotAsync(1746) andGamingProfile.HasAnyThatCanRun(1747,cs/complex-condition). Each condition had more than three nested&&and||. Their parts are named:staticWithoutServersandabsentWithServers, andforTheGameandasAdministrator.AppAlertService.VersionNumber(1748,cs/constant-conditionin the generatedRegexGenerator.g.cs). The generator's matcher backtracked into\d+(?:\.\d+)+, and at that point checked a loop count that cannot be zero there. Giving digits back can never make a match: the next character would be a digit or a dot, and neither the optional letter nor the end of the version can be either. The digits and dots are an atomic group now, so the matcher keeps no backtracking for them, and the dead check is gone from the generated code.UninstallerViewModel(1749,cs/dispose-not-called-on-throw). The batch created itsCancellationTokenSourceand, since fix: uninstall apps from any drive, and let Cancel stop the leftover search #3152, disposed it later in the same method to start the leftover search's, outside afinally. The field owns the source: whatever replaces it next, orDispose, disposes it, so a throw in between left nothing behind. The four places that replace it callRenewCancellation()now, marked[MemberNotNull(nameof(_cts))], which states that ownership in one place.Behavior is unchanged
1 9 . v V a x K _ - space (, and on 19 product names, in .NET and in Python'sre. 868 of the generated strings hold a version, so the comparison is not over strings neither pattern matches.VersionNumbermatcher loses its loop backtracking and the check CodeQL flagged. Everything else inRegexGenerator.g.csis the same apart from the pattern in its doc comment.IsOnTheCardalways true: 2 About tests fail, the download and the install that a newer release stops.RenewCancellationkeeping the last source: 1 fails, the cancelled batch whose leftover search still has to run.CaptureSnapshotAsync_AStaticSourceThatNamesNoServer_Throws, for IPv4 and IPv6, a check no test held before. FiveWithoutVersionrows at the end of a version: one letter, two letters, a dot and a word, a letter between two parts, and a digit after the letter.Checks
dotnet format --verify-no-changesis clean.