Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CCT severity to Vs Severity mapping #4805

Merged
merged 1 commit into from
Aug 30, 2023
Merged

Add CCT severity to Vs Severity mapping #4805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Integration.Vsix/ErrorList/IssuesSnapshot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public override bool TryGetValue(int index, string keyName, out object content)
return true;

case StandardTableKeyNames.ErrorSeverity:
content = toVsSeverityConverter.Convert(issue.Severity);
content = toVsSeverityConverter.GetVsSeverity(issue);
return true;

case StandardTableKeyNames.BuildTool:
Expand Down
4 changes: 2 additions & 2 deletions src/IssueViz.UnitTests/AnalysisIssueVisualizationConverterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ private IAnalysisIssue CreateIssue(params IQuickFix[] quickFixes)
Guid.NewGuid().ToString(),
AnalysisIssueSeverity.Blocker,
AnalysisIssueType.Bug,
null,
SoftwareQualitySeverity.High,
CreateLocation(Guid.NewGuid().ToString()),
null,
quickFixes
Expand All @@ -286,7 +286,7 @@ private IAnalysisIssue CreateIssue(string filePath, params IAnalysisIssueFlow[]
Guid.NewGuid().ToString(),
AnalysisIssueSeverity.Blocker,
AnalysisIssueType.Bug,
null,
SoftwareQualitySeverity.High,
CreateLocation(filePath),
flows
);
Expand Down
44 changes: 44 additions & 0 deletions src/IssueViz.UnitTests/Helpers/AnalysisSeverityToVsSeverityConverterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using SonarLint.VisualStudio.Core;
using SonarLint.VisualStudio.Core.Analysis;
using SonarLint.VisualStudio.IssueVisualization.Helpers;
using SonarLint.VisualStudio.TestInfrastructure;

namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.Helpers
{
Expand Down Expand Up @@ -60,11 +61,54 @@ public void Convert_Blocker_CorrectlyMapped(bool shouldTreatBlockerAsError, __VS

testSubject.Convert(AnalysisIssueSeverity.Blocker).Should().Be(expectedVsErrorCategory);
}

[TestMethod]
[DataRow(SoftwareQualitySeverity.High, __VSERRORCATEGORY.EC_WARNING)]
[DataRow(SoftwareQualitySeverity.Medium, __VSERRORCATEGORY.EC_WARNING)]
[DataRow(SoftwareQualitySeverity.Low, __VSERRORCATEGORY.EC_MESSAGE)]
public void ConvertFromCct_CorrectlyMapped(SoftwareQualitySeverity severity, __VSERRORCATEGORY expectedVsErrorCategory)
{
testSubject.ConvertFromCct(severity).Should().Be(expectedVsErrorCategory);
}

[TestMethod]
public void ConvertFromCct_InvalidCctSeverity_DoesNotThrow()
{
testSubject.ConvertFromCct((SoftwareQualitySeverity)(-999)).Should().Be(__VSERRORCATEGORY.EC_MESSAGE);
}

[TestMethod]
public void Convert_InvalidDaemonSeverity_DoesNotThrow()
{
testSubject.Convert((AnalysisIssueSeverity)(-999)).Should().Be(__VSERRORCATEGORY.EC_MESSAGE);
}

[TestMethod]
public void GetVsSeverity_IssueWithNewCct_UsesNewCctConverter()
{
var converter = new Mock<IAnalysisSeverityToVsSeverityConverter>();

converter.Object.GetVsSeverity(new DummyAnalysisIssue
{
Severity = AnalysisIssueSeverity.Major, HighestSoftwareQualitySeverity = SoftwareQualitySeverity.High
});

converter.Verify(x => x.ConvertFromCct(SoftwareQualitySeverity.High), Times.Once);
converter.Invocations.Should().HaveCount(1);
}

[TestMethod]
public void GetVsSeverity_IssueWithoutNewCct_UsesOldSeverityConverter()
{
var converter = new Mock<IAnalysisSeverityToVsSeverityConverter>();

converter.Object.GetVsSeverity(new DummyAnalysisIssue
{
Severity = AnalysisIssueSeverity.Major
});

converter.Verify(x => x.Convert(AnalysisIssueSeverity.Major), Times.Once);
converter.Invocations.Should().HaveCount(1);
}
}
}
30 changes: 30 additions & 0 deletions src/IssueViz/Helpers/IAnalysisSeverityToVsSeverityConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace SonarLint.VisualStudio.IssueVisualization.Helpers
public interface IAnalysisSeverityToVsSeverityConverter
{
__VSERRORCATEGORY Convert(AnalysisIssueSeverity severity);
__VSERRORCATEGORY ConvertFromCct(SoftwareQualitySeverity severity);
}

public class AnalysisSeverityToVsSeverityConverter : IAnalysisSeverityToVsSeverityConverter
Expand All @@ -43,6 +44,25 @@ internal AnalysisSeverityToVsSeverityConverter(IEnvironmentSettings environmentS
this.environmentSettings = environmentSettings;
}

public __VSERRORCATEGORY ConvertFromCct(SoftwareQualitySeverity severity)
{
switch (severity)
{
case SoftwareQualitySeverity.Medium:
case SoftwareQualitySeverity.High:
return __VSERRORCATEGORY.EC_WARNING;

case SoftwareQualitySeverity.Low:
return __VSERRORCATEGORY.EC_MESSAGE;

default:
// We don't want to throw here - we're being called by VS to populate
// the columns in the error list, and if we're on a UI thread then
// we'll crash VS
return __VSERRORCATEGORY.EC_MESSAGE;
}
}

public __VSERRORCATEGORY Convert(AnalysisIssueSeverity severity)
{
switch (severity)
Expand All @@ -66,4 +86,14 @@ public __VSERRORCATEGORY Convert(AnalysisIssueSeverity severity)
}
}
}

public static class AnalysisSeverityToVsSeverityConverterExtensions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this as a extension method wouldn't it make more sense to add it as public method to the IAnalysisSeverityToVsSeverityConverter and make the other two private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see your point, but I would need to change all of the tests. If you think it's worth it I'll do it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep it for now but create a hardening ticket for the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4806

{
public static __VSERRORCATEGORY GetVsSeverity(
this IAnalysisSeverityToVsSeverityConverter converter,
IAnalysisIssue issue) =>
issue.HighestSoftwareQualitySeverity.HasValue
? converter.ConvertFromCct(issue.HighestSoftwareQualitySeverity.Value)
: converter.Convert(issue.Severity);
}
}