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.

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

SLVS-1597 Store SLCore issue id in AnalysisIssue #5823

Merged
9 changes: 7 additions & 2 deletions src/Core/Analysis/AnalysisIssue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class AnalysisIssue : IAnalysisIssue
private static readonly IReadOnlyList<IQuickFix> EmptyFixes = [];

public AnalysisIssue(
Guid? id,
string ruleKey,
AnalysisIssueSeverity? severity,
AnalysisIssueType? type,
Expand All @@ -36,6 +37,7 @@ public AnalysisIssue(
string context = null
)
{
Id = id;
RuleKey = ruleKey;
Severity = severity;
HighestSoftwareQualitySeverity = highestSoftwareQualitySeverity;
Expand All @@ -46,6 +48,8 @@ public AnalysisIssue(
RuleDescriptionContextKey = context;
}

public Guid? Id { get; }

public string RuleKey { get; }

public AnalysisIssueSeverity? Severity { get; }
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down
5 changes: 5 additions & 0 deletions src/Core/Analysis/IAnalysisIssue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 6 additions & 4 deletions src/IssueViz.Security.UnitTests/Hotspots/Models/HotspotTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
13 changes: 8 additions & 5 deletions src/IssueViz.Security.UnitTests/Taint/Models/TaintIssueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 &&
Expand Down
5 changes: 4 additions & 1 deletion src/IssueViz.Security/Hotspots/Models/IHotspot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 5 additions & 1 deletion src/IssueViz.Security/Taint/Models/ITaintIssue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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));
Expand All @@ -61,6 +64,7 @@ public TaintIssue(string issueKey,
}
}

public Guid? Id { get; }
public string IssueKey { get; }
public string RuleKey { get; }
public AnalysisIssueSeverity? Severity { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
{
}
}
Loading