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

Convert SuppressedIssueMatcher to IssueMatcher, return SonarQubeIssue instead of boolean #5043

Merged
merged 1 commit into from
Nov 23, 2023
Merged

Convert SuppressedIssueMatcher to IssueMatcher, return SonarQubeIssue instead of boolean #5043

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
24 changes: 20 additions & 4 deletions src/ConnectedMode.UnitTests/Suppressions/ClientSuppressionSynchronizerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
using System;
using System.Collections.Generic;
using SonarLint.VisualStudio.ConnectedMode.Suppressions;
using SonarLint.VisualStudio.ConnectedMode.Synchronization;
using SonarLint.VisualStudio.ConnectedMode.UnitTests.Helpers;
using SonarLint.VisualStudio.Core.Suppressions;
using SonarLint.VisualStudio.IssueVisualization.Editor.LocationTagging;
using SonarLint.VisualStudio.IssueVisualization.Models;
using SonarLint.VisualStudio.TestInfrastructure;
using SonarQube.Client.Models;

namespace SonarLint.VisualStudio.ConnectedMode.UnitTests.Suppressions
{
Expand All @@ -36,7 +39,7 @@ public void MefCtor_CheckIsExported()
{
MefTestHelpers.CheckTypeCanBeImported<ClientSuppressionSynchronizer, IClientSuppressionSynchronizer>(
MefTestHelpers.CreateExport<IIssueLocationStoreAggregator>(),
MefTestHelpers.CreateExport<ISuppressedIssueMatcher>());
MefTestHelpers.CreateExport<IIssueMatcher>());
}

[TestMethod]
Expand Down Expand Up @@ -184,13 +187,26 @@ private static IIssueLocationStoreAggregator CreateIssueStore(IEnumerable<IAnaly
return issuesStore.Object;
}

private static ISuppressedIssueMatcher CreateSuppressedIssueMatcher(params IFilterableIssue[] matches)
private static IIssueMatcher CreateSuppressedIssueMatcher(params IFilterableIssue[] matches)
{
var suppressedIssuesMatcher = new Mock<ISuppressedIssueMatcher>();
var suppressedIssuesMatcher = new Mock<IIssueMatcher>();

foreach (var match in matches)
{
suppressedIssuesMatcher.Setup(x => x.SuppressionExists(match)).Returns(true);
suppressedIssuesMatcher
.Setup(x => x.Match(match))
.Returns(new SonarQubeIssue(default,
default,
default,
default,
default,
default,
true, // suppressed issue
default,
default,
default,
default,
default));
}

return suppressedIssuesMatcher.Object;
Expand Down
62 changes: 33 additions & 29 deletions ...ppressions/SuppressedIssueMatcherTests.cs → ...ronization/SuppressedIssueMatcherTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@

using System;
using SonarLint.VisualStudio.ConnectedMode.Suppressions;
using SonarLint.VisualStudio.ConnectedMode.Synchronization;
using SonarLint.VisualStudio.Core.Suppressions;
using SonarLint.VisualStudio.TestInfrastructure;
using SonarQube.Client.Models;

namespace SonarLint.VisualStudio.ConnectedMode.UnitTests.Suppressions
namespace SonarLint.VisualStudio.ConnectedMode.UnitTests.Synchronization
{
[TestClass]
public class SuppressedIssueMatcherTests
Expand All @@ -42,14 +43,14 @@ public void TestInitialize()
[TestMethod]
public void MefCtor_CheckIsExported()
{
MefTestHelpers.CheckTypeCanBeImported<SuppressedIssueMatcher, ISuppressedIssueMatcher>(
MefTestHelpers.CheckTypeCanBeImported<SuppressedIssueMatcher, IIssueMatcher>(
MefTestHelpers.CreateExport<IServerIssuesStore>());
}

[TestMethod]
public void MatchExists_NullIssue_Throws()
{
Action act = () => testSubject.SuppressionExists(null);
Action act = () => testSubject.Match(null);
act.Should().ThrowExactly<ArgumentNullException>().And.ParamName.Should().Be("issue");
}

Expand All @@ -65,10 +66,11 @@ public void MatchExists_NullIssue_Throws()
public void MatchExists_LocalNonFileIssue_SingleServerIssue(string serverRuleId, int? serverIssueLine, string serverHash, bool expectedResult)
{
var issueToMatch = CreateIssueToMatch("CorrectRuleId", 1, "CorrectHash");
ConfigureServerIssues(CreateServerIssue(serverRuleId, serverIssueLine, serverHash, isSuppressed: true));
var serverIssue = CreateServerIssue(serverRuleId, serverIssueLine, serverHash);
ConfigureServerIssues(serverIssue);

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().Be(expectedResult);
testSubject.Match(issueToMatch).Should().Be(expectedResult ? serverIssue : null);
}

[DataTestMethod]
Expand All @@ -81,10 +83,11 @@ public void MatchExists_LocalFileIssue_SingleServerIssue(string serverRuleId, in
{
// File issues have line number of 0 and an empty hash
var issueToMatch = CreateIssueToMatch("CorrectRuleId", null, null);
ConfigureServerIssues(CreateServerIssue(serverRuleId, serverIssueLine, serverHash, expectedResult));
var serverIssue = CreateServerIssue(serverRuleId, serverIssueLine, serverHash);
ConfigureServerIssues(serverIssue);

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().Be(expectedResult);
testSubject.Match(issueToMatch).Should().Be(expectedResult ? serverIssue : null);
}

[TestMethod]
Expand All @@ -95,39 +98,41 @@ public void MatchExists_NoServerIssues_ReturnsFalse()
ConfigureServerIssues(Array.Empty<SonarQubeIssue>());

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().BeFalse();
testSubject.Match(issueToMatch).Should().BeNull();
}

[DataTestMethod]
[DataRow("aaa", 222, "aaa hash", true)]
[DataRow("bbb", 333, "bbb hash", true)]
[DataRow("ccc", 444, "ccc hash", true)]
[DataRow("xxx", 111, "xxx hash", false)]
public void MatchExists_MultipleServerIssues(string localRuleId, int localIssueLine, string localHash, bool expectedResult)
[DataRow("aaa", 222, "aaa hash", 0)]
[DataRow("bbb", 333, "bbb hash", 1)]
[DataRow("ccc", 444, "ccc hash", 2)]
[DataRow("xxx", 111, "xxx hash", -1)]
public void MatchExists_MultipleServerIssues(string localRuleId, int localIssueLine, string localHash, int expectedServerIssue)
{
// Arrange
var issueToMatch = CreateIssueToMatch(localRuleId, localIssueLine, localHash);

ConfigureServerIssues(
CreateServerIssue("aaa", 222, "aaa hash", isSuppressed: true),
CreateServerIssue("bbb", 333, "bbb hash", isSuppressed: true),
CreateServerIssue("ccc", 444, "ccc hash", isSuppressed: true));
var serverIssues = new []{CreateServerIssue("aaa", 222, "aaa hash"),
CreateServerIssue("bbb", 333, "bbb hash"),
CreateServerIssue("ccc", 444, "ccc hash")};

ConfigureServerIssues(serverIssues);

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().Be(expectedResult);
testSubject.Match(issueToMatch).Should().Be(expectedServerIssue >= 0 ? serverIssues[expectedServerIssue] : null);
}

[DataTestMethod]
[DataRow("CorrectRuleId", null, null, true, true)]
[DataRow("CorrectRuleId", null, null, false, false)]
public void MatchExists_ResultDependsOnSuppressionState(string serverRuleId, int? serverIssueLine, string serverHash, bool isSuppressed, bool expectedResult)
[DataRow("CorrectRuleId", null, null, true)]
[DataRow("CorrectRuleId", null, null, false)]
public void MatchExists_ResultDependsOnSuppressionState(string serverRuleId, int? serverIssueLine, string serverHash, bool expectedResult)
{
// File issues have line number of 0 and an empty hash
var issueToMatch = CreateIssueToMatch("CorrectRuleId", null, null);
ConfigureServerIssues(CreateServerIssue(serverRuleId, serverIssueLine, serverHash, isSuppressed));
var serverIssue = CreateServerIssue(serverRuleId, serverIssueLine, serverHash);
ConfigureServerIssues(serverIssue);

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().Be(expectedResult);
testSubject.Match(issueToMatch).Should().Be(serverIssue);
}

[TestMethod]
Expand Down Expand Up @@ -157,10 +162,11 @@ public void SuppressionExists_CheckFileComparisons(string serverFilePath, string
{
var issueToMatch = CreateIssueToMatch("111", 0, "hash", filePath: localFilePath);

ConfigureServerIssues(CreateServerIssue("111", 0, "hash", true, filePath: serverFilePath));
var serverIssue = CreateServerIssue("111", 0, "hash", filePath: serverFilePath);
ConfigureServerIssues(serverIssue);

// Act and assert
testSubject.SuppressionExists(issueToMatch).Should().Be(expected);
testSubject.Match(issueToMatch).Should().Be(expected ? serverIssue : null);
}

private IFilterableIssue CreateIssueToMatch(string ruleId, int? startLine, string lineHash,
Expand All @@ -173,18 +179,16 @@ private IFilterableIssue CreateIssueToMatch(string ruleId, int? startLine, strin
FilePath = filePath
};

private SonarQubeIssue CreateServerIssue(string ruleId, int? startLine, string lineHash, bool isSuppressed,
private SonarQubeIssue CreateServerIssue(string ruleId, int? startLine, string lineHash,
string filePath = null)
{
var sonarQubeIssue = new SonarQubeIssue(null, filePath, lineHash, null, null, ruleId, false, SonarQubeIssueSeverity.Info,
var sonarQubeIssue = new SonarQubeIssue(null, filePath, lineHash, null, null, ruleId, default, SonarQubeIssueSeverity.Info,
DateTimeOffset.MinValue, DateTimeOffset.MinValue,
startLine.HasValue
? new IssueTextRange(startLine.Value, 1, 1, 1)
: null,
flows: null);

sonarQubeIssue.IsResolved = isSuppressed;

return sonarQubeIssue;
}

Expand Down
9 changes: 5 additions & 4 deletions src/ConnectedMode/Suppressions/ClientSuppressionSynchronizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using SonarLint.VisualStudio.IssueVisualization.Models;
using System.Collections.Generic;
using System;
using SonarLint.VisualStudio.ConnectedMode.Synchronization;
using SonarLint.VisualStudio.IssueVisualization.Editor.LocationTagging;

namespace SonarLint.VisualStudio.ConnectedMode.Suppressions
Expand All @@ -32,15 +33,15 @@ namespace SonarLint.VisualStudio.ConnectedMode.Suppressions
internal class ClientSuppressionSynchronizer : IClientSuppressionSynchronizer
{
private readonly IIssueLocationStoreAggregator issuesStore;
private readonly ISuppressedIssueMatcher suppressedIssueMatcher;
private readonly IIssueMatcher issueMatcher;

public event EventHandler<LocalSuppressionsChangedEventArgs> LocalSuppressionsChanged;

[ImportingConstructor]
public ClientSuppressionSynchronizer(IIssueLocationStoreAggregator issuesStore, ISuppressedIssueMatcher suppressedIssueMatcher)
public ClientSuppressionSynchronizer(IIssueLocationStoreAggregator issuesStore, IIssueMatcher issueMatcher)
{
this.issuesStore = issuesStore;
this.suppressedIssueMatcher = suppressedIssueMatcher;
this.issueMatcher = issueMatcher;
}

public void SynchronizeSuppressedIssues()
Expand All @@ -54,7 +55,7 @@ public void SynchronizeSuppressedIssues()
var issueViz = issue as IAnalysisIssueVisualization;

// If the object was matched then it is suppressed on the server
var newIsSuppressedValue = suppressedIssueMatcher.SuppressionExists(issueViz);
var newIsSuppressedValue = issueMatcher.Match(issueViz)?.IsResolved ?? false;

if (issueViz.IsSuppressed != newIsSuppressedValue)
{
Expand Down
19 changes: 9 additions & 10 deletions ...de/Suppressions/SuppressedIssueMatcher.cs → ...Synchronization/SuppressedIssueMatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,20 @@
using System;
using System.ComponentModel.Composition;
using System.Linq;
using SonarLint.VisualStudio.ConnectedMode.Suppressions;
using SonarLint.VisualStudio.Core.Helpers;
using SonarLint.VisualStudio.Core.Suppressions;
using SonarQube.Client.Models;

namespace SonarLint.VisualStudio.ConnectedMode.Suppressions
namespace SonarLint.VisualStudio.ConnectedMode.Synchronization
{
public interface ISuppressedIssueMatcher
public interface IIssueMatcher
{
bool SuppressionExists(IFilterableIssue issue);
}
SonarQubeIssue Match(IFilterableIssue issue);
}

[Export(typeof(ISuppressedIssueMatcher))]
public class SuppressedIssueMatcher : ISuppressedIssueMatcher
[Export(typeof(IIssueMatcher))]
public class SuppressedIssueMatcher : IIssueMatcher
{
private readonly IServerIssuesStore serverIssuesStore;

Expand All @@ -43,7 +44,7 @@ public SuppressedIssueMatcher(IServerIssuesStore serverIssuesStore)
this.serverIssuesStore = serverIssuesStore;
}

public bool SuppressionExists(IFilterableIssue issue)
public SonarQubeIssue Match(IFilterableIssue issue)
{
if (issue == null)
{
Expand All @@ -62,9 +63,7 @@ public bool SuppressionExists(IFilterableIssue issue)
var serverIssues = serverIssuesStore.Get();

// Try to find an issue with the same ID and either the same line number or some line hash
bool isSuppressed = serverIssues.Any(s => s.IsResolved && IsMatch(issue, s));

return isSuppressed;
return serverIssues.FirstOrDefault(s => IsMatch(issue, s));
}

private static bool IsMatch(IFilterableIssue issue, SonarQubeIssue serverIssue)
Expand Down
5 changes: 3 additions & 2 deletions src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using Moq;
using SonarLint.VisualStudio.Core.Analysis;
using SonarLint.VisualStudio.ConnectedMode.Suppressions;
using SonarLint.VisualStudio.ConnectedMode.Synchronization;
using SonarLint.VisualStudio.Integration.Vsix;
using SonarLint.VisualStudio.Integration.Vsix.Analysis;
using SonarLint.VisualStudio.IssueVisualization.Models;
Expand All @@ -40,15 +41,15 @@ public class IssueConsumerFactoryTests
public void MefCtor_CheckIsExported()
{
MefTestHelpers.CheckTypeCanBeImported<IssueConsumerFactory, IIssueConsumerFactory>(
MefTestHelpers.CreateExport<ISuppressedIssueMatcher>(),
MefTestHelpers.CreateExport<IIssueMatcher>(),
MefTestHelpers.CreateExport<IAnalysisIssueVisualizationConverter>(),
MefTestHelpers.CreateExport<ILocalHotspotsStoreUpdater>());
}

[TestMethod]
public void Create_InitializedIssueConsumerReturned()
{
var testSubject = new IssueConsumerFactory(Mock.Of<ISuppressedIssueMatcher>(), Mock.Of<IAnalysisIssueVisualizationConverter>(), Mock.Of<ILocalHotspotsStoreUpdater>());
var testSubject = new IssueConsumerFactory(Mock.Of<IIssueMatcher>(), Mock.Of<IAnalysisIssueVisualizationConverter>(), Mock.Of<ILocalHotspotsStoreUpdater>());

IIssuesSnapshot publishedSnaphot = null;
var consumer = testSubject.Create(CreateValidTextDocument("file.txt"), "project name", Guid.NewGuid(), x => { publishedSnaphot = x; });
Expand Down
Loading