fix: resolve 5 SonarQube S2589 issues and improve null handling

[sonarqube-agent]

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Fixed redundant null checks and boolean conditions flagged by SonarQube static analysis across multiple files. These changes remove gratuitous pattern matches and null-conditional operators that were guaranteed to evaluate to true or false, improving code clarity and eliminating false-positive static analysis warnings.

View Project in SonarCloud

---

Fixed Issues

csharpsquid:S2589 - Change this condition so that it does not always evaluate to 'True'. • MAJOR • View issue

Location: src/ConnectedMode/Persistence/SolutionBindingRepository.cs:50

Why is this an issue?

Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.

What changed

The original code used 'not null' as a pattern match arm, but at that point in the control flow, the value being matched was guaranteed to be non-null (likely because a previous arm already handled the null case). This meant the 'not null' condition always evaluated to true, triggering the static analysis warning about a gratuitous boolean expression. By changing 'not null' to the discard pattern '_', the code uses a catch-all default arm instead of a redundant non-null check, eliminating the always-true condition while preserving the same runtime behavior.

--- a/src/ConnectedMode/Persistence/SolutionBindingRepository.cs
+++ b/src/ConnectedMode/Persistence/SolutionBindingRepository.cs
@@ -50,1 +50,1 @@ BoundSonarQubeProject ILegacySolutionBindingRepository.Read(string configFilePat
-            not null => bindingJsonModelConverter.ConvertFromModelToLegacy(bindingJsonModel, credentialsLoader.Load(bindingJsonModel.ServerUri))
+            _ => bindingJsonModelConverter.ConvertFromModelToLegacy(bindingJsonModel, credentialsLoader.Load(bindingJsonModel.ServerUri))

csharpsquid:S2589 - Remove this unnecessary check for null. • MAJOR • View issue

Location: src/ConnectedMode/Migration/RoslynProjectProvider.cs:61

Why is this an issue?

Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.

What changed

This hunk removes the null-conditional operators (?.) from wrappedWorkspace?.CurrentSolution?. and replaces them with direct member access (wrappedWorkspace.CurrentSolution.). The static analysis determined that wrappedWorkspace is never null at this point in the code, making the null-conditional checks gratuitous. By removing the unnecessary null checks, the code no longer triggers the 'Remove this unnecessary check for null' warning (rule S2589) and becomes clearer about its actual intent.

--- a/src/ConnectedMode/Migration/RoslynProjectProvider.cs
+++ b/src/ConnectedMode/Migration/RoslynProjectProvider.cs
@@ -61,1 +61,1 @@ public IReadOnlyList<Project> Get()
-            return wrappedWorkspace?.CurrentSolution?.Projects.ToList();
+            return wrappedWorkspace.CurrentSolution.Projects.ToList();

csharpsquid:S1168 - Return an empty collection instead of null. • MAJOR • View issue

Location: src/IssueViz/IssueVisualizationControl/ViewModels/Commands/NavigateToRuleDescriptionCommand.cs:86

Why is this an issue?

Returning null or default instead of an actual collection forces the method callers to explicitly test for null, making the code more complex and less readable.

What changed

This hunk replaces return null; with return Array.Empty<object>(); in the NavigateToRuleDescriptionCommandConverter class. The static analysis rule flags returning null from a method that returns a collection type, since it forces callers to check for null and is less readable. By returning an empty array instead of null, the code follows the best practice of returning an empty collection, which eliminates the need for null checks by callers.

--- a/src/IssueViz/IssueVisualizationControl/ViewModels/Commands/NavigateToRuleDescriptionCommand.cs
+++ b/src/IssueViz/IssueVisualizationControl/ViewModels/Commands/NavigateToRuleDescriptionCommand.cs
@@ -86,1 +86,1 @@ public class NavigateToRuleDescriptionCommandConverter : IMultiValueConverter
-            return null;
+            return Array.Empty<object>();

csharpsquid:S2589 - Change this condition so that it does not always evaluate to 'False'. • MAJOR • View issue

Location: src/Integration/Helpers/OutputWindowService.cs:50

Why is this an issue?

Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.

What changed

Removes the Debug.Assert(sonarLintOutputPane != null, ...) line that follows a code path where sonarLintOutputPane is guaranteed to be non-null (since the null case was already handled earlier). The static analyzer flagged the sonarLintOutputPane == null condition within this assert as always evaluating to 'False', making it a gratuitous boolean expression. By removing the entire Debug.Assert statement, the always-false condition is eliminated.

--- a/src/Integration/Helpers/OutputWindowService.cs
+++ b/src/Integration/Helpers/OutputWindowService.cs
@@ -48,1 +47,0 @@ public void Show()
-            Debug.Assert(sonarLintOutputPane != null, "Failed to create SonarLint pane");

csharpsquid:S2589 - Change this condition so that it does not always evaluate to 'False'. • MAJOR • View issue

Location: src/Core/FileMonitor/SingleFileMonitor.cs:139

Why is this an issue?

Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.

What changed

This hunk removes the Debug.Assert line that contained the condition fileChangedHandlers == null || disposedValu which was flagged as always evaluating to 'False'. Since fileChangedHandlers is never null at that point in the code flow, the null check portion of the condition is gratuitous. By removing the entire assertion line, the always-false condition is eliminated, resolving the code smell about a boolean expression that never changes the evaluation result.

--- a/src/Core/FileMonitor/SingleFileMonitor.cs
+++ b/src/Core/FileMonitor/SingleFileMonitor.cs
@@ -138,1 +137,0 @@ private void OnFileChanged(object sender, FileSystemEventArgs args)
-            Debug.Assert(fileChangedHandlers != null, "Not expecting file system events to be monitored if there are no listeners");

Have a suggestion or found an issue? Share your feedback here.

---

SonarQube Remediation Agent uses AI. Check for mistakes.