Fix thread-safety and interface compliance issues

[sonarqube-agent]

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

Why these issues? All issues are CRITICAL or MAJOR severity in production code with clear, actionable fixes. The thread-safety issue in ErrorTagTooltipProvider.cs has the highest impact as a concurrency bug. The three parameter-rename violations in ActiveDocumentTracker.cs share the same rule and file, making them efficiently addressable in a single change. The loop-variable modification issue, while MAJOR severity, represents a valuable automation opportunity to prevent future analysis warnings.

This change resolves 5 SonarQube violations across 3 files: introduces thread-safe static field updates using Interlocked operations, converts a for-loop to a while-loop to permit safe loop-variable modification, and aligns method parameter names with their interface declaration. These fixes improve code correctness, thread-safety, and maintainability while resolving critical static analysis warnings.

View Project in SonarCloud

---

Fixed Issues

csharpsquid:S2696 - Remove this set, which updates a 'static' field from an instance method. • CRITICAL • View issue

Location: src/IssueViz/Editor/ErrorTagging/ErrorTagTooltipProvider.cs:97

Why is this an issue?

Updating a static field from a non-static method introduces significant challenges and potential bugs. Multiple class instances and threads can access and modify the static field concurrently, leading to unintended consequences for other instances or threads (unexpected behavior, race conditions and synchronization problems).

What changed

Adds the System.Threading import which is required for using Interlocked.Increment in the new IncrementInstanceCount helper method. This import is necessary to support the thread-safe replacement of the direct static field update from an instance method.

--- a/src/IssueViz/Editor/ErrorTagging/ErrorTagTooltipProvider.cs
+++ b/src/IssueViz/Editor/ErrorTagging/ErrorTagTooltipProvider.cs
@@ -22,0 +23,1 @@
+using System.Threading;

csharpsquid:S127 - Do not update the stop condition variable 'i' in the body of the for loop. • MAJOR • View issue

Location: src/IssueViz/IssueVisualizationControl/ViewModels/IssueVisualizationViewModel.cs:320

Why is this an issue?

A for loop stop condition should test the loop counter against an invariant value, one that is true at both the beginning and ending of every loop iteration. Ideally, this means that the stop condition is set to a local variable just before the loop begins.

What changed

This hunk converts the for loop into a while loop by extracting the loop counter initialization (var i = 0) to before the loop and changing for (var i = 0; i < flowLocations.Count; i++) to while (i < flowLocations.Count). This addresses the static analysis warning about updating the loop counter variable i within the body of a for loop. By using a while loop instead, the iteration logic is expected to be more complex, and modifying i inside the loop body is acceptable and idiomatic.

--- a/src/IssueViz/IssueVisualizationControl/ViewModels/IssueVisualizationViewModel.cs
+++ b/src/IssueViz/IssueVisualizationControl/ViewModels/IssueVisualizationViewModel.cs
@@ -307,1 +307,2 @@ private IReadOnlyList<ILocationListItem> BuildLocationListItems(IAnalysisIssueFl
-                for (var i = 0; i < flowLocations.Count; i++)
+                var i = 0;
+                while (i < flowLocations.Count)

csharpsquid:S927 - Rename parameter 'elementId' to 'elementid' to match the interface declaration. • CRITICAL • View issue

Location: src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs:96

Why is this an issue?

Parameters are part of the method signature and its identity.

What changed

This hunk renames all three parameters of the OnElementValueChanged method to match the interface declaration of IVsSelectionEvents. It renames elementId to elementid (fixing the parameter name mismatch where the interface declares elementid), oldValue to varValueOld (fixing the parameter name mismatch where the interface declares varValueOld), and newValue to varValueNew (fixing the parameter name mismatch where the interface declares varValueNew). All three parameter names now match the interface's declared parameter names, resolving all three S927 warnings about mismatched parameter names.

--- a/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
+++ b/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
@@ -96,1 +96,1 @@ internal sealed class ActiveDocumentTracker : IActiveDocumentTracker, IVsSelecti
-        int IVsSelectionEvents.OnElementValueChanged(uint elementId, object oldValue, object newValue)
+        int IVsSelectionEvents.OnElementValueChanged(uint elementid, object varValueOld, object varValueNew)

csharpsquid:S927 - Rename parameter 'oldValue' to 'varValueOld' to match the interface declaration. • CRITICAL • View issue

Location: src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs:96

Why is this an issue?

Parameters are part of the method signature and its identity.

What changed

This hunk renames all three parameters of the OnElementValueChanged method to match the interface declaration of IVsSelectionEvents. It renames elementId to elementid (fixing the parameter name mismatch where the interface declares elementid), oldValue to varValueOld (fixing the parameter name mismatch where the interface declares varValueOld), and newValue to varValueNew (fixing the parameter name mismatch where the interface declares varValueNew). All three parameter names now match the interface's declared parameter names, resolving all three S927 warnings about mismatched parameter names.

--- a/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
+++ b/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
@@ -96,1 +96,1 @@ internal sealed class ActiveDocumentTracker : IActiveDocumentTracker, IVsSelecti
-        int IVsSelectionEvents.OnElementValueChanged(uint elementId, object oldValue, object newValue)
+        int IVsSelectionEvents.OnElementValueChanged(uint elementid, object varValueOld, object varValueNew)

csharpsquid:S927 - Rename parameter 'newValue' to 'varValueNew' to match the interface declaration. • CRITICAL • View issue

Location: src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs:96

Why is this an issue?

Parameters are part of the method signature and its identity.

What changed

This hunk renames all three parameters of the OnElementValueChanged method to match the interface declaration of IVsSelectionEvents. It renames elementId to elementid (fixing the parameter name mismatch where the interface declares elementid), oldValue to varValueOld (fixing the parameter name mismatch where the interface declares varValueOld), and newValue to varValueNew (fixing the parameter name mismatch where the interface declares varValueNew). All three parameter names now match the interface's declared parameter names, resolving all three S927 warnings about mismatched parameter names.

--- a/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
+++ b/src/Infrastructure.VS/DocumentEvents/ActiveDocumentTracker.cs
@@ -96,1 +96,1 @@ internal sealed class ActiveDocumentTracker : IActiveDocumentTracker, IVsSelecti
-        int IVsSelectionEvents.OnElementValueChanged(uint elementId, object oldValue, object newValue)
+        int IVsSelectionEvents.OnElementValueChanged(uint elementid, object varValueOld, object varValueNew)

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

---

SonarQube Remediation Agent uses AI. Check for mistakes.