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-1693 Fix: issues set to empty after hotspots event #5886

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void HandleMessage_IssuesForAnalyzedFileAreNotIgnored(string fileNameInMe
context.IssueConverter.Invocations.Count.Should().Be(1);
context.IssueConsumer.Invocations.Count.Should().Be(1);

context.IssueConsumer.Verify(x => x.Set(analyzedFile, It.IsAny<IEnumerable<IAnalysisIssue>>()));
context.IssueConsumer.Verify(x => x.SetIssues(analyzedFile, It.IsAny<IEnumerable<IAnalysisIssue>>()));
}

[TestMethod]
Expand Down
2 changes: 1 addition & 1 deletion src/CFamily/Subprocess/MessageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private void HandleAnalysisIssue(Message message)
// TextBufferIssueTracker when the file was closed, but the TextBufferIssueTracker will
// still exist and handle the call.
// todo https://sonarsource.atlassian.net/browse/SLVS-1661
issueConsumer.Set(request.Context.File, new[] { issue });
issueConsumer.SetIssues(request.Context.File, new[] { issue });
}

internal /* for testing */ static bool IsIssueForActiveRule(Message message, ICFamilyRulesConfig rulesConfiguration)
Expand Down
38 changes: 0 additions & 38 deletions src/Core.UnitTests/Analysis/AnalysisServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,44 +113,6 @@ public void ScheduleAnalysis_NoEnvironmentSettings_DefaultTimeout()
scheduler.Received().Schedule("file/path", Arg.Any<Action<CancellationToken>>(), AnalysisService.DefaultAnalysisTimeoutMs);
}

[TestMethod]
public void PublishIssues_NoConsumerInStorage_DoesNothing()
{
var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(Guid.NewGuid(), null, false);
var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage);

var act = () => testSubject.PublishIssues("file/path", Guid.NewGuid(), Substitute.For<IEnumerable<IAnalysisIssue>>());

act.Should().NotThrow();
}

[TestMethod]
public void PublishIssues_DifferentAnalysisId_DoesNothing()
{
var analysisId = Guid.NewGuid();
var issueConsumer = Substitute.For<IIssueConsumer>();
var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(Guid.NewGuid(), issueConsumer, true);
var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage);

testSubject.PublishIssues("file/path", analysisId, Substitute.For<IEnumerable<IAnalysisIssue>>());

issueConsumer.DidNotReceiveWithAnyArgs().Set(default, default);
}

[TestMethod]
public void PublishIssues_MatchingConsumer_PublishesIssues()
{
var analysisId = Guid.NewGuid();
var issueConsumer = Substitute.For<IIssueConsumer>();
var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(analysisId, issueConsumer, true);
var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage);
var analysisIssues = Substitute.For<IEnumerable<IAnalysisIssue>>();

testSubject.PublishIssues("file/path", analysisId, analysisIssues);

issueConsumer.Received().Set("file/path", analysisIssues);
}

[DataRow(true)]
[DataRow(false)]
[DataTestMethod]
Expand Down
9 changes: 0 additions & 9 deletions src/Core/Analysis/AnalysisService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ public bool IsAnalysisSupported(IEnumerable<AnalysisLanguage> languages)
return analyzerController.IsAnalysisSupported(languages);
}

public void PublishIssues(string filePath, Guid analysisId, IEnumerable<IAnalysisIssue> issues)
{
if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer)
&& analysisId == currentAnalysisId)
{
issueConsumer.Set(filePath, issues);
}
}

public void ScheduleAnalysis(string filePath,
Guid analysisId,
IEnumerable<AnalysisLanguage> detectedLanguages,
Expand Down
38 changes: 38 additions & 0 deletions src/Core/Analysis/HotspotPublisher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SonarLint for Visual Studio
* Copyright (C) 2016-2024 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.
*/

using System.ComponentModel.Composition;

namespace SonarLint.VisualStudio.Core.Analysis;

[Export(typeof(IHotspotPublisher))]
[PartCreationPolicy(CreationPolicy.Shared)]
[method:ImportingConstructor]
internal class HotspotPublisher(IIssueConsumerStorage issueConsumerStorage) : IHotspotPublisher
{
public void Publish(string filePath, Guid analysisId, IEnumerable<IAnalysisIssue> issues)
{
if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer)
&& analysisId == currentAnalysisId)
{
issueConsumer.SetHotspots(filePath, issues);
}
}
}
17 changes: 12 additions & 5 deletions src/Core/Analysis/IAnalysisService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ public interface IAnalysisService
/// </summary>
bool IsAnalysisSupported(IEnumerable<AnalysisLanguage> languages);

/// <summary>
/// Handles analysis results
/// </summary>
void PublishIssues(string filePath, Guid analysisId, IEnumerable<IAnalysisIssue> issues);

/// <summary>
/// Starts analysis for <paramref name="filePath"/>
/// </summary>
Expand All @@ -49,3 +44,15 @@ void ScheduleAnalysis(string filePath,
/// </summary>
void CancelForFile(string filePath);
}

public interface IFindingsPublisher

Choose a reason for hiding this comment

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

Please move this interface into its own class.

{
/// <summary>
/// Handles analysis results
/// </summary>
void Publish(string filePath, Guid analysisId, IEnumerable<IAnalysisIssue> issues);
}

public interface IIssuePublisher : IFindingsPublisher;

public interface IHotspotPublisher : IFindingsPublisher;
3 changes: 2 additions & 1 deletion src/Core/Analysis/IIssueConsumer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ namespace SonarLint.VisualStudio.Core.Analysis;
/// </summary>
public interface IIssueConsumer
{
void Set(string path, IEnumerable<IAnalysisIssue> issues);
void SetIssues(string path, IEnumerable<IAnalysisIssue> issues);
void SetHotspots(string path, IEnumerable<IAnalysisIssue> hotspots);
}
38 changes: 38 additions & 0 deletions src/Core/Analysis/IssuePublisher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SonarLint for Visual Studio
* Copyright (C) 2016-2024 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.
*/

using System.ComponentModel.Composition;

namespace SonarLint.VisualStudio.Core.Analysis;

[Export(typeof(IIssuePublisher))]
[PartCreationPolicy(CreationPolicy.Shared)]
[method:ImportingConstructor]
internal class IssuePublisher(IIssueConsumerStorage issueConsumerStorage) : IIssuePublisher
{
public void Publish(string filePath, Guid analysisId, IEnumerable<IAnalysisIssue> issues)
{
if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer)
&& analysisId == currentAnalysisId)
{
issueConsumer.SetIssues(filePath, issues);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void Create_InitializedIssueConsumerReturned()
/* The empty issues list is passed as an argument here because
it's impossible to verify the actual pipeline due to the fact
that mocking ITextSnapshot in a way that then can be used by a SnapshotSpan takes a lot of effort */
consumer.Set("analysisfile.txt", []);
consumer.SetIssues("analysisfile.txt", []);

publishedIssuesSnapshot.Should().NotBeNull();
publishedIssuesSnapshot.AnalyzedFilePath.Should().Be("updatedfile.txt"); // filename should be updted by this point
Expand Down
18 changes: 7 additions & 11 deletions src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues()
var expectedGuid = Guid.NewGuid();
const string expectedProjectName = "my project name";
const string expectedFilePath = "c:\\aaa\\file.txt";

var testSubject = CreateTestSubject(notificationHandler.OnSnapshotChanged,
expectedProjectName, expectedGuid, expectedFilePath, localHotspotsStoreUpdater:hotspotStoreMock.Object);

Expand All @@ -71,7 +71,7 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues()

// Check the updated issues
VerifyHotspotsAdded(hotspotStoreMock, expectedFilePath, new []{ hotspot });

notificationHandler.UpdatedSnapshot.Issues.Count().Should().Be(1);
notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(new []{issue});

Expand Down Expand Up @@ -125,14 +125,12 @@ public void HandleNewIssues_SpansAreTranslated()

var inputIssues = new[]
{
CreateIssue("xxx", startLine: 1, endLine: 1),
CreateIssue("xxxx", startLine: 3, endLine: 3, isHotspot: true)
CreateIssue("xxx", startLine: 1, endLine: 1)
};

var issuesToReturnFromTranslator = new[]
{
CreateIssue("yyy", startLine: 2, endLine: 2),
CreateIssue("yyyy", startLine: 4, endLine: 4, isHotspot: true)
CreateIssue("yyy", startLine: 2, endLine: 2)
};

var notificationHandler = new SnapshotChangeHandler();
Expand All @@ -155,8 +153,6 @@ public void HandleNewIssues_SpansAreTranslated()
translator.VerifyAll();
notificationHandler.InvocationCount.Should().Be(1);
notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(issuesToReturnFromTranslator.First());

VerifyHotspotsAdded(hotspotStoreMock, filePath, new []{ issuesToReturnFromTranslator.Last()});
}

[TestMethod]
Expand Down Expand Up @@ -224,7 +220,7 @@ public void HandleNewIssues_SomeSuppressedIssues_IssuesGetMarkedCorrectly()
issue3.IsSuppressed.Should().BeFalse();
issue4.IsSuppressed.Should().BeTrue();
}

private static void VerifyHotspotsAdded(Mock<ILocalHotspotsStoreUpdater> hotspotStoreMock, string filePath,
IAnalysisIssueVisualization[] expectedHotspots)
{
Expand Down Expand Up @@ -304,7 +300,7 @@ private static ITextDocument CreateValidTextDocument(string filePath)
}

private static IssueHandler CreateTestSubject(SnapshotChangedHandler notificationHandler,
ISuppressedIssueMatcher suppressedIssueMatcher = null,
ISuppressedIssueMatcher suppressedIssueMatcher = null,
TranslateSpans translator = null,
ITextDocument textDocument = null,
ILocalHotspotsStoreUpdater localHotspotsStoreUpdater = null)
Expand Down Expand Up @@ -332,7 +328,7 @@ private static IssueHandler CreateTestSubject(SnapshotChangedHandler notificatio
var testSubject = new IssueHandler(
textDocument,
projectName,
projectGuid,
projectGuid,
suppressedIssueMatcher,
notificationHandler,
localHotspotsStoreUpdater ?? Mock.Of<ILocalHotspotsStoreUpdater>(),
Expand Down
Loading
Loading