SLVS-1614 Fix Category in the error list for MQR mode (#5822)

src/Core/Analysis/AnalysisIssue.cs

@@ -30,7 +30,7 @@ public AnalysisIssue(
             string ruleKey,
             AnalysisIssueSeverity? severity,
             AnalysisIssueType? type,
-            SoftwareQualitySeverity? highestSoftwareQualitySeverity, 
+            Impact highestImpact, 
             IAnalysisIssueLocation primaryLocation,
             IReadOnlyList<IAnalysisIssueFlow> flows,
             IReadOnlyList<IQuickFix> fixes = null,
@@ -40,7 +40,7 @@ public AnalysisIssue(
             Id = id;
             RuleKey = ruleKey;
             Severity = severity;
-            HighestSoftwareQualitySeverity = highestSoftwareQualitySeverity;
+            HighestImpact = highestImpact;
             Type = type;
             PrimaryLocation = primaryLocation ?? throw new ArgumentNullException(nameof(primaryLocation));
             Flows = flows ?? EmptyFlows;
@@ -53,8 +53,6 @@ public AnalysisIssue(
         public string RuleKey { get; }
 
         public AnalysisIssueSeverity? Severity { get; }
-
-        public SoftwareQualitySeverity? HighestSoftwareQualitySeverity { get; }
 
         public AnalysisIssueType? Type { get; }
 
@@ -63,23 +61,24 @@ public AnalysisIssue(
         public IAnalysisIssueLocation PrimaryLocation { get; }
 
         public IReadOnlyList<IQuickFix> Fixes { get; }
+        public Impact HighestImpact { get; }
 
         public string RuleDescriptionContextKey { get; }
     }
 
     public class AnalysisHotspotIssue : AnalysisIssue, IAnalysisHotspotIssue
     {
-        public AnalysisHotspotIssue(Guid id,
+        public AnalysisHotspotIssue(Guid? id,
             string ruleKey, 
             AnalysisIssueSeverity? severity, 
             AnalysisIssueType? type, 
-            SoftwareQualitySeverity? highestSoftwareQualitySeverity,
+            Impact highestImpact,
             IAnalysisIssueLocation primaryLocation, 
             IReadOnlyList<IAnalysisIssueFlow> flows, 
             IReadOnlyList<IQuickFix> fixes = null, 
             string context = null,
             HotspotPriority? hotspotPriority = null) : 
-            base(id, ruleKey, severity, type, highestSoftwareQualitySeverity, primaryLocation, flows, fixes, context)
+            base(id, ruleKey, severity, type, highestImpact, primaryLocation, flows, fixes, context)
         {
             HotspotPriority = hotspotPriority;
         }

src/Core/Analysis/CleanCodeTaxonomy.cs

@@ -108,4 +108,6 @@ public enum SoftwareQualitySeverity
         High = 3,
         Blocker = 4
     }
+
+    public record Impact(SoftwareQuality Quality, SoftwareQualitySeverity Severity);
 }

src/Core/Analysis/IAnalysisIssue.cs

@@ -23,12 +23,12 @@ namespace SonarLint.VisualStudio.Core.Analysis
     public interface IAnalysisIssue : IAnalysisIssueBase
     {
         AnalysisIssueSeverity? Severity { get; }
-
-        SoftwareQualitySeverity? HighestSoftwareQualitySeverity { get; }
 
         AnalysisIssueType? Type { get; }
 
         IReadOnlyList<IQuickFix> Fixes { get; }
+
+        Impact HighestImpact { get; }
     }
 
     public interface IAnalysisHotspotIssue : IAnalysisIssue

src/Education/Rule/SLCoreRuleMetaDataProvider.cs

@@ -85,7 +85,7 @@ private static RuleInfo Convert(EffectiveRuleDetailsDto effectiveRuleDetailsAsyn
             Convert(effectiveRuleDetailsAsync.severityDetails.Right?.impacts));
 
     private static Dictionary<SoftwareQuality, SoftwareQualitySeverity> Convert(List<ImpactDto> cleanCodeAttribute) =>
-        cleanCodeAttribute?.ToDictionary(x => Convert(x.softwareQuality), x => x.impactSeverity.ToSoftwareQualitySeverity());
+        cleanCodeAttribute?.ToDictionary(x => x.softwareQuality.ToSoftwareQuality(), x => x.impactSeverity.ToSoftwareQualitySeverity());
 
 
     private static RuleIssueSeverity? Convert(IssueSeverity? issueSeverity) =>
@@ -111,15 +111,6 @@ private static Dictionary<SoftwareQuality, SoftwareQualitySeverity> Convert(List
             _ => throw new ArgumentOutOfRangeException(nameof(ruleType), ruleType, null)
         };
 
-    private static SoftwareQuality Convert(SLCore.Common.Models.SoftwareQuality argSoftwareQuality) =>
-        argSoftwareQuality switch
-        {
-            SLCore.Common.Models.SoftwareQuality.MAINTAINABILITY => SoftwareQuality.Maintainability,
-            SLCore.Common.Models.SoftwareQuality.RELIABILITY => SoftwareQuality.Reliability,
-            SLCore.Common.Models.SoftwareQuality.SECURITY => SoftwareQuality.Security,
-            _ => throw new ArgumentOutOfRangeException(nameof(argSoftwareQuality), argSoftwareQuality, null)
-        };
-
     private static CleanCodeAttribute? Convert(SLCore.Common.Models.CleanCodeAttribute? cleanCodeAttribute) =>
         cleanCodeAttribute switch
         {

src/Integration.Vsix.UnitTests/CFamily/CFamilyIssueToAnalysisIssueConverterTests.cs

@@ -471,11 +471,12 @@ public void Convert_NewCCTEnabled_FillsSoftwareQualitySeverity(bool isCCTEnabled
             var testSubject = CreateTestSubject(connectedModeFeaturesConfiguration: CMConfig.Object);
             var issue = testSubject.Convert(message, "lang", CFamilyconfig.Object);
 
-            issue.HighestSoftwareQualitySeverity.Should().Be(expectedSoftwareQualitySeverity);
+            var highestSoftwareQualitySeverity = issue.HighestImpact?.Severity;
+            highestSoftwareQualitySeverity.Should().Be(expectedSoftwareQualitySeverity);
         }
 
         [TestMethod]
-        [Description("Regression test for https://github.com/SonarSource/sonarlint-visualstudio/issues/2149")]
+        [Microsoft.VisualStudio.TestTools.UnitTesting.Description("Regression test for https://github.com/SonarSource/sonarlint-visualstudio/issues/2149")]
         [DataRow("", "")] // empty should not throw
         [DataRow("a.txt", "a.txt")] // not-rooted should stay the same
         [DataRow("c:\\a.txt", "c:\\a.txt")]
@@ -498,7 +499,7 @@ public void Convert_HasMessageParts_QualifiedFilePath(string originalPath, strin
         }
 
         [TestMethod]
-        [Description("Regression test for https://github.com/SonarSource/sonarlint-visualstudio/issues/2557")]
+        [Microsoft.VisualStudio.TestTools.UnitTesting.Description("Regression test for https://github.com/SonarSource/sonarlint-visualstudio/issues/2557")]
         [DataRow("", "")] // empty should not throw
         [DataRow("a.txt", "a.txt")] // not-rooted should stay the same
         [DataRow("c:\\a.txt", "c:\\a.txt")]
@@ -548,50 +549,75 @@ public void ConvertFromIssueType(IssueType cfamilyIssueType, AnalysisIssueType a
         [DataRow(SoftwareQualitySeverity.Medium, SoftwareQualitySeverity.Medium)]
         [DataRow(SoftwareQualitySeverity.Low, SoftwareQualitySeverity.Low)]
         [DataRow(null, null)]
-        public void GetHighestSoftwareQualitySeverity(SoftwareQualitySeverity? softwareQualitySeverity, SoftwareQualitySeverity? highestSoftwareQualitySeverity)
+        public void GetHighestImpact_ReturnsImpactWithHighestSeverity(SoftwareQualitySeverity? softwareQualitySeverity, SoftwareQualitySeverity? expectedHighestSoftwareQualitySeverity)
         {
             var impacts = new Dictionary<SoftwareQuality, SoftwareQualitySeverity>();
-
             if (softwareQualitySeverity.HasValue)
             {
                 impacts.Add(SoftwareQuality.Maintainability, softwareQualitySeverity.Value);
             }
-
             RuleMetadata ruleMetaData = CreateRuleMetaData(impacts);
 
-            CFamilyIssueToAnalysisIssueConverter.GetHighestSoftwareQualitySeverity(ruleMetaData).Should().Be(highestSoftwareQualitySeverity);
+            var highestSoftwareQualitySeverity = CFamilyIssueToAnalysisIssueConverter.GetHighestImpact(ruleMetaData)?.Severity;
+
+            highestSoftwareQualitySeverity.Should().Be(expectedHighestSoftwareQualitySeverity);
         }
 
         [TestMethod]
         [DataRow(new SoftwareQualitySeverity[] { SoftwareQualitySeverity.Low, SoftwareQualitySeverity.Medium }, SoftwareQualitySeverity.Medium)]
         [DataRow(new SoftwareQualitySeverity[] { SoftwareQualitySeverity.Low, SoftwareQualitySeverity.High }, SoftwareQualitySeverity.High)]
         [DataRow(new SoftwareQualitySeverity[] { SoftwareQualitySeverity.Medium, SoftwareQualitySeverity.High }, SoftwareQualitySeverity.High)]
-        public void GetHighestSoftwareQualitySeverity_HasTwoImpacts_GetsTheHighestOne(SoftwareQualitySeverity[] softwareQualitySeverities, SoftwareQualitySeverity? highestSoftwareQualitySeverity)
+        public void GetHighestImpact_HasTwoImpacts_GetsTheHighestOne(SoftwareQualitySeverity[] softwareQualitySeverities, SoftwareQualitySeverity? expectedHighestSoftwareQualitySeverity)
         {
             var impacts = new Dictionary<SoftwareQuality, SoftwareQualitySeverity>
             {
                 { SoftwareQuality.Maintainability, softwareQualitySeverities[0] },
                 { SoftwareQuality.Reliability, softwareQualitySeverities[1] }
             };
-
             RuleMetadata ruleMetaData = CreateRuleMetaData(impacts);
 
-            CFamilyIssueToAnalysisIssueConverter.GetHighestSoftwareQualitySeverity(ruleMetaData).Should().Be(highestSoftwareQualitySeverity);
+            var highestSoftwareQualitySeverity = CFamilyIssueToAnalysisIssueConverter.GetHighestImpact(ruleMetaData)?.Severity;
+
+            highestSoftwareQualitySeverity.Should().Be(expectedHighestSoftwareQualitySeverity);
         }
 
         [TestMethod]
-        public void GetHighestSoftwareQualitySeverity_HasThreeImpacts_GetsTheHighestOne()
+        public void GetHighestImpact_HasThreeImpacts_GetsTheHighestOne()
         {
             var impacts = new Dictionary<SoftwareQuality, SoftwareQualitySeverity>
             {
                 { SoftwareQuality.Maintainability, SoftwareQualitySeverity.Low },
                 { SoftwareQuality.Reliability, SoftwareQualitySeverity.High },
                 { SoftwareQuality.Security, SoftwareQualitySeverity.Medium }
             };
+            RuleMetadata ruleMetaData = CreateRuleMetaData(impacts);
 
+            var highestImpact = CFamilyIssueToAnalysisIssueConverter.GetHighestImpact(ruleMetaData);
+
+            highestImpact.Severity.Should().Be(SoftwareQualitySeverity.High);
+            highestImpact.Quality.Should().Be(SoftwareQuality.Reliability);
+        }
+
+        [TestMethod]
+        [DataRow(SoftwareQualitySeverity.Blocker)]
+        [DataRow(SoftwareQualitySeverity.High)]
+        [DataRow(SoftwareQualitySeverity.Medium)]
+        [DataRow(SoftwareQualitySeverity.Low)]
+        [DataRow(SoftwareQualitySeverity.Info)]
+        public void GetHighestImpact_HasTwoHighImpactsForDifferentQualities_GetsTheHighestSoftwareQuality(SoftwareQualitySeverity softwareQualitySeverity)
+        {
+            var impacts = new Dictionary<SoftwareQuality, SoftwareQualitySeverity>
+            {
+                { SoftwareQuality.Maintainability, SoftwareQualitySeverity.Info },
+                { SoftwareQuality.Reliability, softwareQualitySeverity },
+                { SoftwareQuality.Security, softwareQualitySeverity }
+            };
             RuleMetadata ruleMetaData = CreateRuleMetaData(impacts);
 
-            CFamilyIssueToAnalysisIssueConverter.GetHighestSoftwareQualitySeverity(ruleMetaData).Should().Be(SoftwareQualitySeverity.High);
+            var highestImpact = CFamilyIssueToAnalysisIssueConverter.GetHighestImpact(ruleMetaData);
+
+            highestImpact.Severity.Should().Be(softwareQualitySeverity);
+            highestImpact.Quality.Should().Be(SoftwareQuality.Security);
         }
 
         [TestMethod]

src/Integration.Vsix.UnitTests/ErrorList/IssuesSnapshotTests _GetValue.cs

@@ -206,7 +206,7 @@ public void GetValue_ErrorCategory_Is_CodeSmell_By_Default()
         }
 
         [TestMethod]
-        public void GetValue_ErrorCategory_Is_Issue_Type()
+        public void GetValue_StandardMode_ErrorCategory_IsSeverityAndType()
         {
             issue.Type = AnalysisIssueType.Bug;
             issue.Severity = AnalysisIssueSeverity.Blocker;
@@ -217,6 +217,19 @@ public void GetValue_ErrorCategory_Is_Issue_Type()
             GetValue(StandardTableKeyNames.ErrorCategory).Should().Be("Blocker Vulnerability");
         }
 
+        [TestMethod]
+        [DataRow(SoftwareQuality.Security, SoftwareQualitySeverity.Blocker)]
+        [DataRow(SoftwareQuality.Security, SoftwareQualitySeverity.High)]
+        [DataRow(SoftwareQuality.Maintainability, SoftwareQualitySeverity.Medium)]
+        [DataRow(SoftwareQuality.Maintainability, SoftwareQualitySeverity.Low)]
+        [DataRow(SoftwareQuality.Reliability, SoftwareQualitySeverity.Info)]
+        public void GetValue_MqrMode_ErrorCategory_IsSoftwareQualityAndSeverity(SoftwareQuality quality, SoftwareQualitySeverity severity)
+        {
+            issue.HighestImpact = new Impact(quality, severity);
+
+            GetValue(StandardTableKeyNames.ErrorCategory).Should().Be(severity + " " + quality);
+        }
+
         [TestMethod]
         public void GetValue_ErrorCodeToolTip()
         {

src/Integration.Vsix/CFamily/CFamilyIssueToAnalysisIssueConverter.cs

@@ -122,11 +122,11 @@ public IAnalysisIssue Convert(Message cFamilyIssue, string sqLanguage, ICFamilyR
             // Look up default severity and type
             var defaultSeverity = ruleMetaData.DefaultSeverity;
             var defaultType = ruleMetaData.Type;
-            SoftwareQualitySeverity? highestSoftwareQualitySeverity = null;
+            Impact highestImpact = null;
 
             if (ruleMetaData.Type != IssueType.SecurityHotspot && connectedModeFeaturesConfiguration.IsNewCctAvailable())
             {
-                highestSoftwareQualitySeverity = GetHighestSoftwareQualitySeverity(ruleMetaData);
+                highestImpact = GetHighestImpact(ruleMetaData);
             }
 
             var fileContents = GetFileContentsOfReportedFiles(cFamilyIssue);
@@ -143,7 +143,7 @@ public IAnalysisIssue Convert(Message cFamilyIssue, string sqLanguage, ICFamilyR
 
             var flows = locations.Any() ? new[] { new AnalysisIssueFlow(locations) } : null;
 
-            var result = ToAnalysisIssue(cFamilyIssue, sqLanguage, defaultSeverity, defaultType, flows, fileContents, highestSoftwareQualitySeverity);
+            var result = ToAnalysisIssue(cFamilyIssue, sqLanguage, defaultSeverity, defaultType, flows, fileContents, highestImpact);
 
             CodeMarkers.Instance.CFamilyConvertIssueStop();
 
@@ -193,15 +193,15 @@ private IAnalysisIssue ToAnalysisIssue(Message cFamilyIssue,
             IssueType defaultType,
             IReadOnlyList<IAnalysisIssueFlow> flows,
             IReadOnlyDictionary<string, ITextDocument> fileContents,
-            SoftwareQualitySeverity? highestSoftwareQualitySeverity)
+            Impact highestImpact)
         {
             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),
-                highestSoftwareQualitySeverity,
+                highestImpact,
                 primaryLocation: ToAnalysisIssueLocation(cFamilyIssue, fileContents),
                 flows: flows,
                 fixes: ToQuickFixes(cFamilyIssue)
@@ -312,7 +312,15 @@ private AnalysisIssueLocation ToAnalysisIssueLocation(MessagePart cFamilyIssueLo
             }
         }
 
-        internal /* for testing */ static SoftwareQualitySeverity? GetHighestSoftwareQualitySeverity(RuleMetadata ruleMetadata)
-            => ruleMetadata.Code?.Impacts?.Count > 0 ? (SoftwareQualitySeverity?)ruleMetadata.Code.Impacts.Max(r => r.Value) : null;
+        internal /* for testing */ static Impact GetHighestImpact(RuleMetadata ruleMetadata)
+        {
+            if (ruleMetadata?.Code?.Impacts == null || ruleMetadata.Code.Impacts.Count == 0)
+            {
+                return null;
+            }
+
+            var highestImpact = ruleMetadata.Code.Impacts.OrderByDescending(kvp => kvp.Value).ThenByDescending(kvp => kvp.Key).First();
+            return new Impact(highestImpact.Key, highestImpact.Value);
+        }
     }
 }

src/Integration.Vsix/ErrorList/IssuesSnapshot.cs

@@ -227,7 +227,7 @@ public override bool TryGetValue(int index, string keyName, out object content)
                     return true;
 
                 case StandardTableKeyNames.ErrorCategory:
-                    content = $"{issue.Severity} {ToString(issue.Type)}";
+                    content = GetErrorCategory(issue);
                     return true;
 
                 case StandardTableKeyNames.ErrorCodeToolTip:
@@ -262,6 +262,15 @@ public override bool TryGetValue(int index, string keyName, out object content)
             }
         }
 
+        private string GetErrorCategory(IAnalysisIssue issue)
+        {
+            if (issue.HighestImpact != null)
+            {
+                return $"{issue.HighestImpact.Severity} {issue.HighestImpact.Quality}";
+            }
+            return $"{issue.Severity} {ToString(issue.Type)}";
+        }
+
         /// <summary>
         /// Returns true/false if the ErrorList should hide the requested issue.
         /// </summary>

src/IssueViz.UnitTests/AnalysisIssueVisualizationConverterTests.cs

@@ -272,7 +272,7 @@ private IAnalysisIssue CreateIssue(params IQuickFix[] quickFixes)
                 Guid.NewGuid().ToString(),
                 AnalysisIssueSeverity.Blocker,
                 AnalysisIssueType.Bug,
-                SoftwareQualitySeverity.High,
+                new Impact(SoftwareQuality.Maintainability, SoftwareQualitySeverity.High),
                 CreateLocation(Guid.NewGuid().ToString()),
                 null,
                 quickFixes
@@ -288,7 +288,7 @@ private IAnalysisIssue CreateIssue(string filePath, params IAnalysisIssueFlow[]
                 Guid.NewGuid().ToString(),
                 AnalysisIssueSeverity.Blocker,
                 AnalysisIssueType.Bug,
-                SoftwareQualitySeverity.High,
+                new Impact(SoftwareQuality.Maintainability, SoftwareQualitySeverity.High),
                 CreateLocation(filePath),
                 flows
             );

src/IssueViz.UnitTests/Helpers/AnalysisSeverityToVsSeverityConverterTests.cs

@@ -90,7 +90,7 @@ public void GetVsSeverity_IssueWithNewCct_UsesNewCctConverter()
 
             converter.Object.GetVsSeverity(new DummyAnalysisIssue
             {
-                Severity = AnalysisIssueSeverity.Major, HighestSoftwareQualitySeverity = SoftwareQualitySeverity.High
+                Severity = AnalysisIssueSeverity.Major, HighestImpact = new Impact(SoftwareQuality.Maintainability, SoftwareQualitySeverity.High)
             });
 
             converter.Verify(x => x.ConvertFromCct(SoftwareQualitySeverity.High), Times.Once);

src/IssueViz/Helpers/IAnalysisSeverityToVsSeverityConverter.cs

@@ -94,8 +94,8 @@ public static class AnalysisSeverityToVsSeverityConverterExtensions
         public static __VSERRORCATEGORY GetVsSeverity(
             this IAnalysisSeverityToVsSeverityConverter converter,
             IAnalysisIssue issue) =>
-            issue.HighestSoftwareQualitySeverity.HasValue
-                ? converter.ConvertFromCct(issue.HighestSoftwareQualitySeverity.Value)
+            issue.HighestImpact?.Severity != null
+                ? converter.ConvertFromCct(issue.HighestImpact.Severity)
                 : converter.Convert(issue.Severity);
     }
 }