SLVS-1597 Store SLCore issue id in AnalysisIssue (#5823)

SonarSource · Nov 20, 2024 · 6351ee7 · 6351ee7
1 parent d4bb713
commit 6351ee7
Show file tree

Hide file tree

Showing 16 changed files with 84 additions and 51 deletions.
diff --git a/src/Core/Analysis/AnalysisIssue.cs b/src/Core/Analysis/AnalysisIssue.cs
@@ -26,6 +26,7 @@ public class AnalysisIssue : IAnalysisIssue
         private static readonly IReadOnlyList<IQuickFix> EmptyFixes = [];
 
         public AnalysisIssue(
+            Guid? id,
             string ruleKey,
             AnalysisIssueSeverity? severity,
             AnalysisIssueType? type,
@@ -36,6 +37,7 @@ public AnalysisIssue(
             string context = null
             )
         {
+            Id = id;
             RuleKey = ruleKey;
             Severity = severity;
             HighestSoftwareQualitySeverity = highestSoftwareQualitySeverity;
@@ -46,6 +48,8 @@ public AnalysisIssue(
             RuleDescriptionContextKey = context;
         }
 
+        public Guid? Id { get; }
+
         public string RuleKey { get; }
 
         public AnalysisIssueSeverity? Severity { get; }
@@ -65,7 +69,8 @@ public AnalysisIssue(
 
     public class AnalysisHotspotIssue : AnalysisIssue, IAnalysisHotspotIssue
     {
-        public AnalysisHotspotIssue(string ruleKey, 
+        public AnalysisHotspotIssue(Guid id,
+            string ruleKey, 
             AnalysisIssueSeverity? severity, 
             AnalysisIssueType? type, 
             SoftwareQualitySeverity? highestSoftwareQualitySeverity,
@@ -74,7 +79,7 @@ public AnalysisHotspotIssue(string ruleKey,
             IReadOnlyList<IQuickFix> fixes = null, 
             string context = null,
             HotspotPriority? hotspotPriority = null) : 
-            base(ruleKey, severity, type, highestSoftwareQualitySeverity, primaryLocation, flows, fixes, context)
+            base(id, ruleKey, severity, type, highestSoftwareQualitySeverity, primaryLocation, flows, fixes, context)
         {
             HotspotPriority = hotspotPriority;
         }

diff --git a/src/Core/Analysis/IAnalysisIssue.cs b/src/Core/Analysis/IAnalysisIssue.cs
@@ -38,6 +38,11 @@ public interface IAnalysisHotspotIssue : IAnalysisIssue
 
     public interface IAnalysisIssueBase
     {
+        /// <summary>
+        /// The id of the issue that comes from SlCore 
+        /// </summary>
+        Guid? Id { get; }
+
         string RuleKey { get; }
 
         IReadOnlyList<IAnalysisIssueFlow> Flows { get; }

diff --git a/src/Integration.Vsix.UnitTests/CFamily/CFamilyIssueToAnalysisIssueConverterTests.cs b/src/Integration.Vsix.UnitTests/CFamily/CFamilyIssueToAnalysisIssueConverterTests.cs
@@ -18,18 +18,11 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-using System;
-using System.Collections.Generic;
 using System.IO.Abstractions;
-using System.Linq;
-using FluentAssertions;
-using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Utilities;
-using Moq;
 using SonarLint.VisualStudio.CFamily.Rules;
 using SonarLint.VisualStudio.CFamily.SubProcess;
-using SonarLint.VisualStudio.Core;
 using SonarLint.VisualStudio.Core.Analysis;
 using SonarLint.VisualStudio.Core.Configuration;
 using SonarLint.VisualStudio.Core.UserRuleSettings;
@@ -444,6 +437,7 @@ public void Convert_SeverityAndTypeLookup(string ruleKey, AnalysisIssueSeverity
             var testSubject = CreateTestSubject();
             var issue = Convert(testSubject, message);
 
+            issue.Id.Should().BeNull();
             issue.RuleKey.Should().Be($"lang1:{ruleKey}");
             issue.Severity.Should().Be(severity);
             issue.Type.Should().Be(type);

diff --git a/src/Integration.Vsix/CFamily/CFamilyIssueToAnalysisIssueConverter.cs b/src/Integration.Vsix/CFamily/CFamilyIssueToAnalysisIssueConverter.cs
@@ -197,6 +197,7 @@ private IAnalysisIssue ToAnalysisIssue(Message cFamilyIssue,
         {
             return new AnalysisIssue
             (
+                id: null, // until CFamily is migrated to SlCore, its ID will be null 
                 ruleKey: sqLanguage + ":" + cFamilyIssue.RuleKey,
                 severity: Convert(defaultSeverity),
                 type: Convert(defaultType),

diff --git a/src/IssueViz.Security.UnitTests/Hotspots/Models/HotspotTests.cs b/src/IssueViz.Security.UnitTests/Hotspots/Models/HotspotTests.cs
@@ -36,7 +36,7 @@ public class HotspotTests
         [TestMethod]
         public void Ctor_NullLocation_ArgumentNullException()
         {
-            Action act = () => new Hotspot(
+            Action act = () => new Hotspot(id: null,
                 "hotspot key",
                 "server-path",
                 primaryLocation: null,
@@ -49,7 +49,7 @@ public void Ctor_NullLocation_ArgumentNullException()
         [TestMethod]
         public void Ctor_PropertiesSet()
         {
-            var hotspot = new Hotspot(
+            var hotspot = new Hotspot(id: null,
                 "hotspot key",
                 "server-path",
                 primaryLocation: new AnalysisIssueLocation(
@@ -84,7 +84,8 @@ public void Ctor_PropertiesSet()
         public void Ctor_NoFlows_EmptyFlows()
         {
             IReadOnlyList<IAnalysisIssueFlow> flows = null;
-            var hotspot = new Hotspot("hotspot key",
+            var hotspot = new Hotspot(id: null,
+                "hotspot key",
                 "server-path",
                 new AnalysisIssueLocation(
                     "message",
@@ -105,7 +106,8 @@ public void Ctor_NoFlows_EmptyFlows()
         public void Ctor_HasFlows_CorrectFlows()
         {
             var flows = new[] { Mock.Of<IAnalysisIssueFlow>(), Mock.Of<IAnalysisIssueFlow>() };
-            var hotspot = new Hotspot("hotspot key",
+            var hotspot = new Hotspot(id: null,
+                "hotspot key",
                 "server-path",
                 new AnalysisIssueLocation(
                     "message",

diff --git a/src/IssueViz.Security.UnitTests/Taint/Models/TaintIssueTests.cs b/src/IssueViz.Security.UnitTests/Taint/Models/TaintIssueTests.cs
@@ -30,7 +30,7 @@ public class TaintIssueTests
     [TestMethod]
     public void Ctor_NullLocation_ArgumentNullException()
     {
-        Action act = () => new TaintIssue("issue key", "rule key",
+        Action act = () => new TaintIssue( Guid.Empty,"issue key", "rule key",
             null,
             AnalysisIssueSeverity.Major, SoftwareQualitySeverity.High, DateTimeOffset.MinValue, null, null);
 
@@ -41,10 +41,12 @@ public void Ctor_NullLocation_ArgumentNullException()
     public void Ctor_PropertiesSet()
     {
         var created = DateTimeOffset.Parse("2001-01-31T01:02:03+0200");
-        var issue = new TaintIssue("issue key", "rule key",
+        var id = Guid.NewGuid();
+        var issue = new TaintIssue(id, "issue key", "rule key",
             new AnalysisIssueLocation("message", "local-path.cpp", new TextRange(1, 2, 3, 4, "hash")),
             AnalysisIssueSeverity.Major, SoftwareQualitySeverity.High, created, null, "contextKey");
 
+        issue.Id.Should().Be(id);
         issue.IssueKey.Should().Be("issue key");
         issue.RuleKey.Should().Be("rule key");
         issue.Severity.Should().Be(AnalysisIssueSeverity.Major);
@@ -64,7 +66,7 @@ public void Ctor_PropertiesSet()
     public void Ctor_NoFlows_EmptyFlows()
     {
         IReadOnlyList<IAnalysisIssueFlow> flows = null;
-        var issue = new TaintIssue("issue key", "rule key",
+        var issue = new TaintIssue(Guid.NewGuid(), "issue key", "rule key",
             new AnalysisIssueLocation("message", "local-path.cpp", new TextRange(1, 2, 3, 4, "hash")),
             AnalysisIssueSeverity.Major, SoftwareQualitySeverity.High, DateTimeOffset.MinValue, flows, null);
 
@@ -75,7 +77,7 @@ public void Ctor_NoFlows_EmptyFlows()
     public void Ctor_HasFlows_CorrectFlows()
     {
         var flows = new[] { Substitute.For<IAnalysisIssueFlow>(), Substitute.For<IAnalysisIssueFlow>() };
-        var issue = new TaintIssue("issue key", "rule key",
+        var issue = new TaintIssue(Guid.NewGuid(), "issue key", "rule key",
             new AnalysisIssueLocation("message", "local-path.cpp", new TextRange(1, 2, 3, 4, "hash")),
             AnalysisIssueSeverity.Major, SoftwareQualitySeverity.High, DateTimeOffset.MinValue, flows, null);
 
@@ -87,7 +89,8 @@ public void Ctor_HasNoStandardAndNoCCTSeverity_Throws()
     {
         AnalysisIssueSeverity? analysisIssueSeverity = null;
         SoftwareQualitySeverity? highestSoftwareQualitySeverity = null;
-        var act = () => new TaintIssue("issue key",
+        var act = () => new TaintIssue(Guid.NewGuid(),
+            "issue key",
             "rule key",
             new AnalysisIssueLocation("msg", "local-path.cpp", new TextRange(1, 2, 3, 4, "hash")),
             analysisIssueSeverity,

diff --git a/src/IssueViz.Security.UnitTests/Taint/TaintIssueToIssueVisualizationConverterTests.cs b/src/IssueViz.Security.UnitTests/Taint/TaintIssueToIssueVisualizationConverterTests.cs
@@ -55,8 +55,9 @@ public void MefCtor_CheckIsExported() =>
     public void Convert_IssueVizConverterCalledWithCorrectParameters_ReturnsConvertedIssueVizWithReversedLocations()
     {
         var created = DateTimeOffset.Parse("2001-12-30T01:02:03+0000");
+        var id = Guid.Parse("efa697a2-9cfd-4faf-ba21-71b378667a81");
         var taintDto = new TaintVulnerabilityDto(
-            Guid.Parse("efa697a2-9cfd-4faf-ba21-71b378667a81"),
+            id,
             "serverkey",
             true,
             "rulekey:S123",
@@ -97,6 +98,7 @@ public void Convert_IssueVizConverterCalledWithCorrectParameters_ReturnsConverte
         result.Should().BeSameAs(expectedConvertedIssueViz);
         issueVizConverter.Received().Convert(
             Arg.Is((TaintIssue taintIssue) =>
+                taintIssue.Id == id &&
                 taintIssue.IssueKey == "serverkey" &&
                 taintIssue.RuleKey == "rulekey:S123" &&
                 taintIssue.Severity == AnalysisIssueSeverity.Minor &&

diff --git a/src/IssueViz.Security/Hotspots/Models/IHotspot.cs b/src/IssueViz.Security/Hotspots/Models/IHotspot.cs
@@ -40,13 +40,15 @@ internal class Hotspot : IHotspot
     {
         private static readonly IReadOnlyList<IAnalysisIssueFlow> EmptyFlows = Array.Empty<IAnalysisIssueFlow>();
 
-        public Hotspot(string hotspotKey,
+        public Hotspot(Guid? id,
+            string hotspotKey,
             string serverFilePath,
             IAnalysisIssueLocation primaryLocation,
             IHotspotRule rule,
             IReadOnlyList<IAnalysisIssueFlow> flows,
             string context = null)
         {
+            Id = id;
             HotspotKey = hotspotKey;
             ServerFilePath = serverFilePath;
             PrimaryLocation = primaryLocation ?? throw new ArgumentNullException(nameof(primaryLocation));
@@ -56,6 +58,7 @@ public Hotspot(string hotspotKey,
         }
 
         public string HotspotKey { get; }
+        public Guid? Id { get; }
         public string RuleKey => Rule.RuleKey;
         public IHotspotRule Rule { get; }
         public IReadOnlyList<IAnalysisIssueFlow> Flows { get; }

diff --git a/src/IssueViz.Security/OpenInIdeHotspots/HotspotDetailsDtoToHotspotConverter.cs b/src/IssueViz.Security/OpenInIdeHotspots/HotspotDetailsDtoToHotspotConverter.cs
@@ -47,7 +47,8 @@ public HotspotDetailsDtoToHotspotConverter(IChecksumCalculator checksumCalculato
 
     public IAnalysisIssueBase Convert(HotspotDetailsDto hotspotDetailsDto, string rootPath)
     {
-        return new Hotspot(hotspotDetailsDto.key,
+        return new Hotspot(id: null,
+            hotspotDetailsDto.key,
             hotspotDetailsDto.ideFilePath,
             new AnalysisIssueLocation(hotspotDetailsDto.message,
                 Path.Combine(rootPath, hotspotDetailsDto.ideFilePath),

diff --git a/src/IssueViz.Security/Taint/Models/ITaintIssue.cs b/src/IssueViz.Security/Taint/Models/ITaintIssue.cs
@@ -37,7 +37,9 @@ public class TaintIssue : ITaintIssue
     {
         private static readonly IReadOnlyList<IAnalysisIssueFlow> EmptyFlows = Array.Empty<IAnalysisIssueFlow>();
 
-        public TaintIssue(string issueKey,
+        public TaintIssue(
+            Guid? id,
+            string issueKey,
             string ruleKey,
             IAnalysisIssueLocation primaryLocation,
             AnalysisIssueSeverity? severity,
@@ -46,6 +48,7 @@ public TaintIssue(string issueKey,
             IReadOnlyList<IAnalysisIssueFlow> flows,
             string ruleDescriptionContextKey)
         {
+            Id = id;
             IssueKey = issueKey;
             RuleKey = ruleKey;
             PrimaryLocation = primaryLocation ?? throw new ArgumentNullException(nameof(primaryLocation));
@@ -61,6 +64,7 @@ public TaintIssue(string issueKey,
             }
         }
 
+        public Guid? Id { get; }
         public string IssueKey { get; }
         public string RuleKey { get; }
         public AnalysisIssueSeverity? Severity { get; }

diff --git a/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs b/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs
@@ -51,6 +51,7 @@ public IAnalysisIssueVisualization Convert(TaintVulnerabilityDto slcoreTaintIssu
 
     private static IAnalysisIssueBase ConvertToAnalysisIssue(TaintVulnerabilityDto slcoreTaintIssue, string configScopeRoot) =>
         new TaintIssue(
+            slcoreTaintIssue.id,
             slcoreTaintIssue.sonarServerKey,
             slcoreTaintIssue.ruleKey,
             CreateLocation(slcoreTaintIssue.message, slcoreTaintIssue.ideFilePath, configScopeRoot, slcoreTaintIssue.textRange),

diff --git a/src/IssueViz.UnitTests/AnalysisIssueVisualizationConverterTests.cs b/src/IssueViz.UnitTests/AnalysisIssueVisualizationConverterTests.cs
@@ -268,6 +268,7 @@ private void AssertConversion(IAnalysisIssueVisualization expectedIssueVisualiza
         private IAnalysisIssue CreateIssue(params IQuickFix[] quickFixes)
         {
             var issue = new AnalysisIssue(
+                Guid.NewGuid(),
                 Guid.NewGuid().ToString(),
                 AnalysisIssueSeverity.Blocker,
                 AnalysisIssueType.Bug,
@@ -283,6 +284,7 @@ private IAnalysisIssue CreateIssue(params IQuickFix[] quickFixes)
         private IAnalysisIssue CreateIssue(string filePath, params IAnalysisIssueFlow[] flows)
         {
             var issue = new AnalysisIssue(
+                Guid.NewGuid(),
                 Guid.NewGuid().ToString(),
                 AnalysisIssueSeverity.Blocker,
                 AnalysisIssueType.Bug,

diff --git a/src/IssueViz/OpenInIde/IssueDetailDtoToAnalysisIssueConverter.cs b/src/IssueViz/OpenInIde/IssueDetailDtoToAnalysisIssueConverter.cs
@@ -47,7 +47,7 @@ public IssueDetailDtoToAnalysisIssueConverter(IChecksumCalculator checksumCalcul
 
     public IAnalysisIssueBase Convert(IssueDetailDto issueDetailDto, string rootPath)
     {
-        return new ServerIssue(
+        return new ServerIssue(Id: null,
             issueDetailDto.ruleKey,
             new AnalysisIssueLocation(issueDetailDto.message,
                 Path.Combine(rootPath, issueDetailDto.ideFilePath),
@@ -73,9 +73,12 @@ public IAnalysisIssueBase Convert(IssueDetailDto issueDetailDto, string rootPath
     }
 
     private sealed record ServerIssue(
+        Guid? Id,
         string RuleKey,
         IAnalysisIssueLocation PrimaryLocation,
         string RuleDescriptionContextKey,
         IReadOnlyList<IAnalysisIssueFlow> Flows)
-        : IAnalysisIssueBase;
+        : IAnalysisIssueBase
+    {
+    }
 }