Fix the open SonarCloud issues and unfreeze the Uno app's stale ones - #10
Merged
Conversation
Clears all 95 OPEN/CONFIRMED findings. They split in two, because the
analysis scope changed underneath them: sonar.yml replaced automatic
analysis but only builds the shared core + CLI + tests, so the Uno app's
78 findings have been frozen since mid-July with nothing to re-evaluate
them -- neither a fix nor a false positive could ever close them.
Real fixes (18 of these are in files the scan still analyses):
- LwpCommands: the connect-retry loop had an empty stop condition
(S1994). Bounded for-loop plus an explicit final attempt; same three
tries, same delay, same wrapped error.
- LwpAddressingMapping: Split(':', '-') also binds to the
Split(char, int count) overload, since '-' converts to int. Pass an
explicit separator array (S3220).
- pi-bt-info.sh / setup-bluez.sh: '[' -> '[[', explicit status-preserving
returns in the six helpers, positional param into a local, and a
constant for the six-times-repeated sed expression (S7688, S7682,
S7679, S1192).
- AppManifest.js: drop the bare var (S3504). Assigns window.UnoAppManifest
rather than using const -- a top-level const in a classic script is not
a window property, which would silently break the bootstrapper's global
lookup.
- Train: switch expression for SpeedDirectionLabel, and the two duplicated
nested ternaries folded into one FormulaDisplay helper (S3358).
- TrackGeometry: Seg1..Seg8 constants across all four tables, so a typo
can no longer desync them (S1192). Also removes the magic "SEG-2" in
SecondViewModel.
- MainPage: flattened the nested ternary to match the identical line above
it (S3358).
- Desktop/WASM/iOS entry points: static class (S1118).
- ShellViewModel: dropped the unread _navigator field and its now-pointless
constructor (S4487).
- MainViewModel.Hub: Cancel() -> await CancelAsync() (S6966).
Scope fix: exclude Source/Trackify/**/*.cs from Sonar indexing.
60 of the app's 78 findings were false positives from analysing without a
compilation -- S2325 "make it static" on [ObservableProperty]-generated
property reads and on IValueConverter members (which cannot be static),
and S8970 "nullable warnings are disabled here" when Directory.Build.props
sets <Nullable>enable</Nullable>. That the correctly-unflagged
ArrowRotationDeg reads plain properties confirms the mechanism.
Excluding only the .cs keeps the WasmScripts JS and any XML analysed, and
costs nothing that was being analysed anyway. The workflow header records
why, and that the exclusion should go if the heads are ever built inside
begin/end.
Verified: CLI + core build clean, 33/33 tests pass, android/desktop/wasm/
windows heads build, both scripts pass bash -n, CLI --help works, desktop
app launches with no startup exceptions. The iOS entry-point change needs
a Mac to build.
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
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.



Clears all 95 OPEN/CONFIRMED findings. They split in two, because the analysis scope changed underneath them: sonar.yml replaced automatic analysis but only builds the shared core + CLI + tests, so the Uno app's 78 findings have been frozen since mid-July with nothing to re-evaluate them -- neither a fix nor a false positive could ever close them.
Real fixes (18 of these are in files the scan still analyses):
Scope fix: exclude Source/Trackify/**/*.cs from Sonar indexing.
60 of the app's 78 findings were false positives from analysing without a compilation -- S2325 "make it static" on [ObservableProperty]-generated property reads and on IValueConverter members (which cannot be static), and S8970 "nullable warnings are disabled here" when Directory.Build.props sets enable. That the correctly-unflagged ArrowRotationDeg reads plain properties confirms the mechanism.
Excluding only the .cs keeps the WasmScripts JS and any XML analysed, and costs nothing that was being analysed anyway. The workflow header records why, and that the exclusion should go if the heads are ever built inside begin/end.
Verified: CLI + core build clean, 33/33 tests pass, android/desktop/wasm/ windows heads build, both scripts pass bash -n, CLI --help works, desktop app launches with no startup exceptions. The iOS entry-point change needs a Mac to build.