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

Fix Mute command behavior for already muted issues #5107

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions src/ConnectedMode.UnitTests/Suppressions/ServerIssuesStoreTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,29 @@ public void MefCtor_Check_SameInstanceExported()
=> MefTestHelpers.CheckMultipleExportsReturnSameInstance<ServerIssuesStore, IServerIssuesStore, IServerIssuesStore>(
MefTestHelpers.CreateExport<ILogger>());

[TestMethod]
public void TryGetIssue_HasIssueWithSameKey_ReturnsTrue()
{
var issue = CreateIssue("1", false);

var testSubject = CreateTestSubject();
testSubject.AddIssues(new []{issue}, false);

testSubject.TryGetIssue(issue.IssueKey, out var storedIssue).Should().BeTrue();
storedIssue.Should().BeSameAs(issue);
}

[TestMethod]
public void TryGetIssue_NoMatchingIssue_ReturnsFalse()
{
var issue = CreateIssue("1", false);

var testSubject = CreateTestSubject();
testSubject.AddIssues(new []{issue}, false);

testSubject.TryGetIssue("NOTMATCHINGKEY", out var storedIssue).Should().BeFalse();
}

[TestMethod]
[DataRow(false)]
[DataRow(true)]
Expand Down
50 changes: 35 additions & 15 deletions src/ConnectedMode.UnitTests/Transition/MuteIssuesServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
using SonarLint.VisualStudio.Core;
using SonarLint.VisualStudio.Core.Binding;
using SonarLint.VisualStudio.Core.Transition;
using SonarLint.VisualStudio.Integration.TestInfrastructure.Helpers;
using SonarLint.VisualStudio.TestInfrastructure;
using SonarQube.Client;
using SonarQube.Client.Models;
Expand Down Expand Up @@ -56,24 +57,49 @@ public void MefCtor_CheckIsSingleton()
}

[TestMethod]
public async Task Mute_NotInConnectedMode_Logs()
public void CacheOutOfSyncResolvedIssue_ThrowsIfNotResolved()
{
var testSubject = CreateTestSubject();

Action act = () => testSubject.CacheOutOfSyncResolvedIssue(DummySonarQubeIssueFactory.CreateServerIssue());

act.Should().ThrowExactly<ArgumentException>();
}

[TestMethod]
public void CacheOutOfSyncResolvedIssue_SavesIssueToStore()
{
var sonarQubeIssue = DummySonarQubeIssueFactory.CreateServerIssue(true);
var storeMock = new Mock<IServerIssuesStoreWriter>();
var threadHandlingMock = new Mock<IThreadHandling>();

var testSubject = CreateTestSubject(serverIssuesStore:storeMock.Object, threadHandling:threadHandlingMock.Object);

testSubject.CacheOutOfSyncResolvedIssue(sonarQubeIssue);

storeMock.Verify(x => x.AddIssues(It.Is<IEnumerable<SonarQubeIssue>>(p => p.SequenceEqual(new[] { sonarQubeIssue })), false));
threadHandlingMock.Verify(x => x.ThrowIfOnUIThread());
}

[TestMethod]
public async Task ResolveIssueWithDialogAsyncNotInConnectedMode_Logs()
{
var threadHandling = CreateThreadHandling();
var activeSolutionBoundTracker = CreateActiveSolutionBoundTracker(false);
var logger = new Mock<ILogger>();

var testSubject = CreateTestSubject(activeSolutionBoundTracker: activeSolutionBoundTracker, logger: logger.Object, threadHandling: threadHandling.Object);

await testSubject.Mute(CreateServerIssue(), CancellationToken.None);
await testSubject.ResolveIssueWithDialogAsync(DummySonarQubeIssueFactory.CreateServerIssue(), CancellationToken.None);

logger.Verify(l => l.LogVerbose("[Transition]Issue muting is only supported in connected mode"), Times.Once);
threadHandling.Verify(t => t.ThrowIfOnUIThread(), Times.Once());
}

[TestMethod]
public async Task Mute_WindowOK_CallService()
public async Task ResolveIssueWithDialogAsyncWindowOK_CallService()
{
var sonarQubeIssue = CreateServerIssue();
var sonarQubeIssue = DummySonarQubeIssueFactory.CreateServerIssue();
sonarQubeIssue.IsResolved.Should().BeFalse();

var threadHandling = CreateThreadHandling();
Expand All @@ -88,7 +114,7 @@ public async Task Mute_WindowOK_CallService()

var testSubject = CreateTestSubject(muteIssuesWindowService: muteIssuesWindowService.Object, sonarQubeService: sonarQubeService.Object, serverIssuesStore: serverIssuesStore.Object, threadHandling: threadHandling.Object);

await testSubject.Mute(sonarQubeIssue, CancellationToken.None);
await testSubject.ResolveIssueWithDialogAsync(sonarQubeIssue, CancellationToken.None);

muteIssuesWindowService.Verify(s => s.Show(), Times.Once);
sonarQubeService.Verify(s => s.TransitionIssueAsync(sonarQubeIssue.IssueKey, SonarQubeIssueTransition.FalsePositive, "some comment", CancellationToken.None), Times.Once);
Expand All @@ -98,15 +124,15 @@ public async Task Mute_WindowOK_CallService()
}

[TestMethod]
public async Task Mute_WindowCancel_DontCallService()
public async Task ResolveIssueWithDialogAsyncWindowCancel_DontCallService()
{
var muteIssuesWindowService = CreateMuteIssuesWindowService(false, SonarQubeIssueTransition.FalsePositive, "some comment");

var sonarQubeService = new Mock<ISonarQubeService>();

var testSubject = CreateTestSubject(muteIssuesWindowService: muteIssuesWindowService.Object, sonarQubeService: sonarQubeService.Object);

await testSubject.Mute(CreateServerIssue(), CancellationToken.None);
await testSubject.ResolveIssueWithDialogAsync(DummySonarQubeIssueFactory.CreateServerIssue(), CancellationToken.None);

muteIssuesWindowService.Verify(s => s.Show(), Times.Once);
sonarQubeService.VerifyNoOtherCalls();
Expand All @@ -116,7 +142,7 @@ public async Task Mute_WindowCancel_DontCallService()
[DataRow(SonarQubeIssueTransitionResult.FailedToTransition, "Unable to resolve the issue, please refer to the logs for more information.")]
[DataRow(SonarQubeIssueTransitionResult.CommentAdditionFailed, "Issue is resolved but an error occured while adding the comment, please refer to the logs for more information.")]
[TestMethod]
public async Task Mute_SQError_ShowsError(SonarQubeIssueTransitionResult result, string errorMessage)
public async Task ResolveIssueWithDialogAsyncSQError_ShowsError(SonarQubeIssueTransitionResult result, string errorMessage)
{
var messageBox = new Mock<IMessageBox>();
var muteIssuesWindowService = CreateMuteIssuesWindowService(true, SonarQubeIssueTransition.FalsePositive, "some comment");
Expand All @@ -127,7 +153,7 @@ public async Task Mute_SQError_ShowsError(SonarQubeIssueTransitionResult result,

var testSubject = CreateTestSubject(muteIssuesWindowService: muteIssuesWindowService.Object, sonarQubeService: sonarQubeService.Object, messageBox: messageBox.Object);

await testSubject.Mute(CreateServerIssue(), CancellationToken.None);
await testSubject.ResolveIssueWithDialogAsync(DummySonarQubeIssueFactory.CreateServerIssue(), CancellationToken.None);

messageBox.Verify(mb => mb.Show(errorMessage, "Error", MessageBoxButton.OK, MessageBoxImage.Error));
}
Expand Down Expand Up @@ -194,11 +220,5 @@ private MuteIssuesService CreateTestSubject(IActiveSolutionBoundTracker activeSo

return new MuteIssuesService(activeSolutionBoundTracker, logger, muteIssuesWindowService, sonarQubeService, serverIssuesStore, threadHandling, messageBox);
}

private static SonarQubeIssue CreateServerIssue()
{
return new SonarQubeIssue("testKey", "test", "test", "test", "test", "test", false, SonarQubeIssueSeverity.Info,
DateTimeOffset.MinValue, DateTimeOffset.MinValue, null, null);
}
}
}
5 changes: 5 additions & 0 deletions src/ConnectedMode/Suppressions/IServerIssuesStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ public interface IServerIssuesStore
/// Returns all issues in the store
/// </summary>
IEnumerable<SonarQubeIssue> Get();

/// <param name="issueKey">Sonar Server issue key</param>
/// <param name="issue">Issue associated with the <paramref name="issueKey"/>, if present</param>
/// <returns>True if issue with the same key is present, False otherwise</returns>
bool TryGetIssue(string issueKey, out SonarQubeIssue issue);
}

/// <summary>
Expand Down
8 changes: 8 additions & 0 deletions src/ConnectedMode/Suppressions/ServerIssuesStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ public IEnumerable<SonarQubeIssue> Get()
}
}

public bool TryGetIssue(string issueKey, out SonarQubeIssue issue)
{
lock (serverIssuesLock)
{
return serverIssues.TryGetValue(issueKey, out issue);
}
}

#endregion IServerIssuesStore implementation

#region IServerIssueStoreWriter implementation
Expand Down
16 changes: 15 additions & 1 deletion src/ConnectedMode/Transition/MuteIssuesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System;
using System.ComponentModel.Composition;
using System.Resources;
using System.Threading;
Expand Down Expand Up @@ -70,7 +71,19 @@ internal MuteIssuesService(IActiveSolutionBoundTracker activeSolutionBoundTracke
resourceManager = new ResourceManager(typeof(Resources));
}

public async Task Mute(SonarQubeIssue issue, CancellationToken token)
public void CacheOutOfSyncResolvedIssue(SonarQubeIssue issue)
{
threadHandling.ThrowIfOnUIThread();

if (!issue.IsResolved)
{
throw new ArgumentException("Issue should be resolved.", nameof(issue));
}

serverIssuesStore.AddIssues(new []{ issue }, false);
}

public async Task ResolveIssueWithDialogAsync(SonarQubeIssue issue, CancellationToken token)
{
threadHandling.ThrowIfOnUIThread();

Expand All @@ -96,6 +109,7 @@ public async Task Mute(SonarQubeIssue issue, CancellationToken token)

if (serviceResult != SonarQubeIssueTransitionResult.Success)
{
// ideally, message box invocation should be moved to Mute command
messageBox.Show(resourceManager.GetString($"MuteIssuesService_Error_{serviceResult}"), Resources.MuteIssuesService_Error_Caption, MessageBoxButton.OK, MessageBoxImage.Error);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Core/Transition/IMuteIssuesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace SonarLint.VisualStudio.Core.Transition
{
public interface IMuteIssuesService
{
Task Mute(SonarQubeIssue issue, CancellationToken token);
void CacheOutOfSyncResolvedIssue(SonarQubeIssue issue);

Task ResolveIssueWithDialogAsync(SonarQubeIssue issue, CancellationToken token);
}
}
48 changes: 48 additions & 0 deletions src/Infrastructure.VS.UnitTests/ErrorListHelperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,54 @@ public void TryGetRuleId_FromHandle_NotSonarLintIssue()
errorCode.Should().BeNull();
}

[TestMethod]
public void TryGetRuleIdAndSuppressionStateFromSelectedRow_NoSuppressionState_ReturnsIsNotSuppressed()
{
// Arrange
var issueHandle = CreateIssueHandle(111, new Dictionary<string, object>
{
{ StandardTableKeyNames.BuildTool, "SonarLint" },
{ StandardTableKeyNames.ErrorCode, "cpp:S222" }
});

var errorList = CreateErrorList(issueHandle);
var serviceProvider = CreateServiceOperation(errorList);

// Act
var testSubject = new ErrorListHelper(serviceProvider);
bool result = testSubject.TryGetRuleIdAndSuppressionStateFromSelectedRow(out var ruleId, out var isSuppressed);

// Assert
result.Should().BeTrue();
isSuppressed.Should().BeFalse();
}

[DataTestMethod]
[DataRow(Infrastructure.VS.SuppressionState.SuppressedEnumValue, true)]
[DataRow(Infrastructure.VS.SuppressionState.NotApplicableEnumValue, false)]
[DataRow(Infrastructure.VS.SuppressionState.ActiveEnumValue, false)]
public void TryGetRuleIdAndSuppressionStateFromSelectedRow_NoSuppressionState_ReturnsIsNotSuppressed(int suppressionState, bool expectedSuppression)
{
// Arrange
var issueHandle = CreateIssueHandle(111, new Dictionary<string, object>
{
{ StandardTableKeyNames.BuildTool, "SonarLint" },
{ StandardTableKeyNames.ErrorCode, "cpp:S222" },
{ Infrastructure.VS.SuppressionState.ColumnName, suppressionState },
});

var errorList = CreateErrorList(issueHandle);
var serviceProvider = CreateServiceOperation(errorList);

// Act
var testSubject = new ErrorListHelper(serviceProvider);
bool result = testSubject.TryGetRuleIdAndSuppressionStateFromSelectedRow(out var ruleId, out var isSuppressed);

// Assert
result.Should().BeTrue();
isSuppressed.Should().Be(expectedSuppression);
}


private IVsUIServiceOperation CreateServiceOperation(IErrorList svcToPassToCallback)
{
Expand Down
29 changes: 29 additions & 0 deletions src/Infrastructure.VS/ErrorListHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,28 @@ public bool TryGetRuleId(ITableEntryHandle handle, out SonarCompositeRuleId rule
return SonarCompositeRuleId.TryParse(errorCode, out ruleId);
}

public bool TryGetRuleIdAndSuppressionStateFromSelectedRow(out SonarCompositeRuleId ruleId, out bool isSuppressed)
{
SonarCompositeRuleId ruleIdOut = null;
var isSuppressedOut = false;
var result = vSServiceOperation.Execute<SVsErrorList, IErrorList, bool>(errorList =>
{
if (!TryGetSelectedTableEntry(errorList, out var handle) || !TryGetRuleId(handle, out ruleIdOut))
{
return false;
}

isSuppressedOut = IsSuppressed(handle);
return true;

});

ruleId = ruleIdOut;
isSuppressed = isSuppressedOut;

return result;
}

public bool TryGetIssueFromSelectedRow(out IFilterableIssue issue)
{
IFilterableIssue issueOut = null;
Expand Down Expand Up @@ -99,6 +121,13 @@ public bool TryGetRoslynIssueFromSelectedRow(out IFilterableRoslynIssue filterab
return result;
}

private static bool IsSuppressed(ITableEntryHandle handle)
{
return handle.TryGetSnapshot(out var snapshot, out var index)
&& TryGetValue(snapshot, index, Infrastructure.VS.SuppressionState.ColumnName, out int suppressionState)
&& suppressionState == Infrastructure.VS.SuppressionState.SuppressedEnumValue;
}

private static string FindErrorCodeForEntry(ITableEntriesSnapshot snapshot, int index)
{
if (TryGetValue(snapshot, index, StandardTableKeyNames.BuildTool, out string buildTool))
Expand Down
4 changes: 4 additions & 0 deletions src/Infrastructure.VS/IErrorListHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,9 @@ public interface IErrorListHelper
/// <returns>True if error code is of sonar issue, line number and file path are available. False otherwise or if multiple rows selected</returns>
bool TryGetRoslynIssueFromSelectedRow(out IFilterableRoslynIssue filterableRoslynIssue);

/// <summary>
/// Similarly to <see cref="TryGetRuleIdFromSelectedRow"/>, this method extracts the rule id, however, it also fetches suppression state from the same row
/// </summary>
bool TryGetRuleIdAndSuppressionStateFromSelectedRow(out SonarCompositeRuleId ruleId, out bool isSuppressed);
}
}
32 changes: 32 additions & 0 deletions src/Infrastructure.VS/SuppressionState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* SonarLint for Visual Studio
* Copyright (C) 2016-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarLint.VisualStudio.Infrastructure.VS
{
// This class duplicates SuppressionState enum and StandardTableKeyNames.SuppressionState because it's not available in vs2019 assembly (even though it is available in the ide)
internal static class SuppressionState
{
public const string ColumnName = "suppression";

public const int ActiveEnumValue = 0;
public const int SuppressedEnumValue = 1;
public const int NotApplicableEnumValue = 2;
}
}
Loading