FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: remove always-false conditions flagged by SonarCloud by glennawatson · Pull Request #1604 · reactiveui/splat · GitHub

/ splat Public

fix: remove always-false conditions flagged by SonarCloud - #1604

Merged
glennawatson merged 1 commit into
mainfrom
fix/sonar-always-false-conditions
Jun 12, 2026
ConversationCommits (1)ChecksFiles changed
Merged

fix: remove always-false conditions flagged by SonarCloud#1604
glennawatson merged 1 commit into
mainfrom
fix/sonar-always-false-conditions

Conversation

glennawatson commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix / code-smell cleanup (no behavioural change).

What is the new behavior?

Removes redundant conditions that SonarCloud reports as always evaluating to False:

  • Splat.Logging/MemoizingMRUCache.cs (InvalidateAll) — oldEntries is guaranteed to be assigned on the only path that leaves the lock without returning, so oldEntries is null was always false. The local is now declared non-nullable (which also satisfies nullable flow analysis) and the dead disjunct is gone, leaving the meaningful _releaseFunction is null guard.
  • Splat.Ninject/NinjectDependencyResolver.cs (UnregisterCurrent/UnregisterAll, 4 sites) — kernel.GetBindings(serviceType).ToArray() never returns null, so bindings is null was always false. The guards now check only bindings.Length < 1.

What is the current behavior?

The code carried redundant x is null checks that the analyzer flagged as always-false (MemoizingMRUCache.cs L268; NinjectDependencyResolver.cs L361, L383, L413, L438) — five Major "Code Smell" maintainability issues.

What might this PR break?

None. Every removed sub-condition was provably unreachable, so behaviour is identical. Public API is unchanged.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

No tests added — the changes are behaviour-preserving removals of dead conditions. Existing suites pass (Ninject 83, MemoizingMRUCache 27/27) and both projects build clean under -warnaserror.

fix: remove always-false conditions flagged by SonarCloud
48ba450 
Drop redundant null checks that the analyzer reports as constantly false:

- MemoizingMRUCache.InvalidateAll: oldEntries is guaranteed assigned on the
  only path that exits the lock without returning, so `oldEntries is null` is
  always false. Declare it non-nullable (satisfying nullable flow) and remove
  the dead disjunct.
- NinjectDependencyResolver UnregisterCurrent/UnregisterAll (x4): the result of
  kernel.GetBindings(...).ToArray() is never null, so `bindings is null` is
  always false. Check only the length.

Behaviour is unchanged; both projects build clean under -warnaserror.
glennawatson enabled auto-merge (squash) June 12, 2026 11:12

sonarqubecloud Bot commented Jun 12, 2026

Copy link
Copy Markdown

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
73.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.23%. Comparing base (4a1505b) to head (48ba450).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/Splat.Ninject/NinjectDependencyResolver.cs 0.00% 0 Missing and 4 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1604      +/-   ##
==========================================
- Coverage   82.30%   82.23%   -0.08%     
==========================================
  Files         124      124              
  Lines        7036     7036              
  Branches     1080     1080              
==========================================
- Hits         5791     5786       -5     
- Misses        937      940       +3     
- Partials      308      310       +2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

glennawatson disabled auto-merge June 12, 2026 11:20
glennawatson merged commit 7156cd0 into main Jun 12, 2026
12 of 14 checks passed
glennawatson deleted the fix/sonar-always-false-conditions branch June 12, 2026 11:20

github-actions Bot commented Jun 27, 2026

Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions Bot locked as resolved and limited conversation to collaborators Jun 27, 2026
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
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

ChrisPulman ChrisPulman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL