From 8f52629067d83126513c1ccb2a9c1170310af2dd Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Wed, 11 Dec 2024 11:14:27 +0100 Subject: [PATCH 1/6] SLVS-1693 Fix: issues set to empty after hotspots event --- .../Subprocess/MessageHandlerTests.cs | 2 +- src/CFamily/Subprocess/MessageHandler.cs | 2 +- .../Analysis/AnalysisServiceTests.cs | 38 ---- src/Core/Analysis/AnalysisService.cs | 9 - src/Core/Analysis/HotspotPublisher.cs | 38 ++++ src/Core/Analysis/IAnalysisService.cs | 17 +- src/Core/Analysis/IIssueConsumer.cs | 3 +- src/Core/Analysis/IssuePublisher.cs | 38 ++++ .../Analysis/IssueConsumerFactoryTests.cs | 2 +- .../Analysis/IssueHandlerTests.cs | 18 +- .../SonarLintTagger/IssueConsumerTests.cs | 177 +++++++++++++----- .../VsAwareAnalysisServiceTests.cs | 3 +- .../Analysis/IssueConsumerFactory.cs | 2 +- .../IssueConsumerFactory_IssueHandler.cs | 45 ++--- .../SonarLintTagger/IssueConsumer.cs | 48 +++-- .../SonarLintTagger/VsAwareAnalysisService.cs | 7 +- .../Analysis/AnalysisListenerTests.cs | 24 ++- .../Analysis/RaisedFindingProcessorTests.cs | 69 +++---- .../Analysis/AnalysisListener.cs | 37 ++-- .../Analysis/RaisedFindingProcessor.cs | 51 ++--- 20 files changed, 371 insertions(+), 259 deletions(-) create mode 100644 src/Core/Analysis/HotspotPublisher.cs create mode 100644 src/Core/Analysis/IssuePublisher.cs diff --git a/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs b/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs index a4c7fa3aa7..0acf246c84 100644 --- a/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs +++ b/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs @@ -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>())); + context.IssueConsumer.Verify(x => x.SetIssues(analyzedFile, It.IsAny>())); } [TestMethod] diff --git a/src/CFamily/Subprocess/MessageHandler.cs b/src/CFamily/Subprocess/MessageHandler.cs index 306769203b..c6b18b8ce9 100644 --- a/src/CFamily/Subprocess/MessageHandler.cs +++ b/src/CFamily/Subprocess/MessageHandler.cs @@ -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) diff --git a/src/Core.UnitTests/Analysis/AnalysisServiceTests.cs b/src/Core.UnitTests/Analysis/AnalysisServiceTests.cs index d543a6cd0c..775a24556b 100644 --- a/src/Core.UnitTests/Analysis/AnalysisServiceTests.cs +++ b/src/Core.UnitTests/Analysis/AnalysisServiceTests.cs @@ -113,44 +113,6 @@ public void ScheduleAnalysis_NoEnvironmentSettings_DefaultTimeout() scheduler.Received().Schedule("file/path", Arg.Any>(), 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>()); - - act.Should().NotThrow(); - } - - [TestMethod] - public void PublishIssues_DifferentAnalysisId_DoesNothing() - { - var analysisId = Guid.NewGuid(); - var issueConsumer = Substitute.For(); - var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(Guid.NewGuid(), issueConsumer, true); - var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage); - - testSubject.PublishIssues("file/path", analysisId, Substitute.For>()); - - issueConsumer.DidNotReceiveWithAnyArgs().Set(default, default); - } - - [TestMethod] - public void PublishIssues_MatchingConsumer_PublishesIssues() - { - var analysisId = Guid.NewGuid(); - var issueConsumer = Substitute.For(); - var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(analysisId, issueConsumer, true); - var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage); - var analysisIssues = Substitute.For>(); - - testSubject.PublishIssues("file/path", analysisId, analysisIssues); - - issueConsumer.Received().Set("file/path", analysisIssues); - } - [DataRow(true)] [DataRow(false)] [DataTestMethod] diff --git a/src/Core/Analysis/AnalysisService.cs b/src/Core/Analysis/AnalysisService.cs index 07b70e93da..8258aaec67 100644 --- a/src/Core/Analysis/AnalysisService.cs +++ b/src/Core/Analysis/AnalysisService.cs @@ -45,15 +45,6 @@ public bool IsAnalysisSupported(IEnumerable languages) return analyzerController.IsAnalysisSupported(languages); } - public void PublishIssues(string filePath, Guid analysisId, IEnumerable 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 detectedLanguages, diff --git a/src/Core/Analysis/HotspotPublisher.cs b/src/Core/Analysis/HotspotPublisher.cs new file mode 100644 index 0000000000..3b26a01e59 --- /dev/null +++ b/src/Core/Analysis/HotspotPublisher.cs @@ -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 issues) + { + if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) + && analysisId == currentAnalysisId) + { + issueConsumer.SetHotspots(filePath, issues); + } + } +} diff --git a/src/Core/Analysis/IAnalysisService.cs b/src/Core/Analysis/IAnalysisService.cs index 99ac54d4a0..717c960ec1 100644 --- a/src/Core/Analysis/IAnalysisService.cs +++ b/src/Core/Analysis/IAnalysisService.cs @@ -30,11 +30,6 @@ public interface IAnalysisService /// bool IsAnalysisSupported(IEnumerable languages); - /// - /// Handles analysis results - /// - void PublishIssues(string filePath, Guid analysisId, IEnumerable issues); - /// /// Starts analysis for /// @@ -49,3 +44,15 @@ void ScheduleAnalysis(string filePath, /// void CancelForFile(string filePath); } + +public interface IFindingsPublisher +{ + /// + /// Handles analysis results + /// + void Publish(string filePath, Guid analysisId, IEnumerable issues); +} + +public interface IIssuePublisher : IFindingsPublisher; + +public interface IHotspotPublisher : IFindingsPublisher; diff --git a/src/Core/Analysis/IIssueConsumer.cs b/src/Core/Analysis/IIssueConsumer.cs index 228de739c1..a10d445af3 100644 --- a/src/Core/Analysis/IIssueConsumer.cs +++ b/src/Core/Analysis/IIssueConsumer.cs @@ -25,5 +25,6 @@ namespace SonarLint.VisualStudio.Core.Analysis; /// public interface IIssueConsumer { - void Set(string path, IEnumerable issues); + void SetIssues(string path, IEnumerable issues); + void SetHotspots(string path, IEnumerable hotspots); } diff --git a/src/Core/Analysis/IssuePublisher.cs b/src/Core/Analysis/IssuePublisher.cs new file mode 100644 index 0000000000..0fb61f08cb --- /dev/null +++ b/src/Core/Analysis/IssuePublisher.cs @@ -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 issues) + { + if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) + && analysisId == currentAnalysisId) + { + issueConsumer.SetIssues(filePath, issues); + } + } +} diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs index d3a12d3d04..e313367a1c 100644 --- a/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs +++ b/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs @@ -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 diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs index f121f2dfaa..effcc6fefb 100644 --- a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs +++ b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs @@ -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); @@ -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}); @@ -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(); @@ -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] @@ -224,7 +220,7 @@ public void HandleNewIssues_SomeSuppressedIssues_IssuesGetMarkedCorrectly() issue3.IsSuppressed.Should().BeFalse(); issue4.IsSuppressed.Should().BeTrue(); } - + private static void VerifyHotspotsAdded(Mock hotspotStoreMock, string filePath, IAnalysisIssueVisualization[] expectedHotspots) { @@ -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) @@ -332,7 +328,7 @@ private static IssueHandler CreateTestSubject(SnapshotChangedHandler notificatio var testSubject = new IssueHandler( textDocument, projectName, - projectGuid, + projectGuid, suppressedIssueMatcher, notificationHandler, localHotspotsStoreUpdater ?? Mock.Of(), diff --git a/src/Integration.Vsix.UnitTests/SonarLintTagger/IssueConsumerTests.cs b/src/Integration.Vsix.UnitTests/SonarLintTagger/IssueConsumerTests.cs index 268003c867..b20773bd8a 100644 --- a/src/Integration.Vsix.UnitTests/SonarLintTagger/IssueConsumerTests.cs +++ b/src/Integration.Vsix.UnitTests/SonarLintTagger/IssueConsumerTests.cs @@ -21,6 +21,7 @@ using Microsoft.VisualStudio.Text; using SonarLint.VisualStudio.Core.Analysis; using SonarLint.VisualStudio.Integration.Vsix; +using SonarLint.VisualStudio.Integration.Vsix.Analysis; using SonarLint.VisualStudio.IssueVisualization.Models; namespace SonarLint.VisualStudio.Integration.UnitTests.SonarLintTagger @@ -28,44 +29,66 @@ namespace SonarLint.VisualStudio.Integration.UnitTests.SonarLintTagger [TestClass] public class IssueConsumerTests { + private IssueConsumerFactory.IIssueHandler issueHandler; private static readonly IAnalysisIssue ValidIssue = CreateIssue(startLine: 1, endLine: 1); private static readonly ITextSnapshot ValidTextSnapshot = CreateSnapshot(lineCount: 10); private static readonly IAnalysisIssueVisualizationConverter ValidConverter = Mock.Of(); private const string ValidFilePath = "c:\\myfile.txt"; + [TestInitialize] + public void TestInitialize() + { + issueHandler = Substitute.For(); + } + [TestMethod] public void Ctor_InvalidArgs_Throws() { - IssueConsumer.OnIssuesChanged validCallback = _ => { }; - Action act = () => new IssueConsumer(null, ValidFilePath, validCallback, ValidConverter); + Action act = () => new IssueConsumer(null, ValidFilePath, issueHandler, ValidConverter); act.Should().ThrowExactly().And.ParamName.Should().Be("analysisSnapshot"); - act = () => new IssueConsumer(ValidTextSnapshot, null, validCallback, ValidConverter); + act = () => new IssueConsumer(ValidTextSnapshot, null, issueHandler, ValidConverter); act.Should().ThrowExactly().And.ParamName.Should().Be("analysisFilePath"); act = () => new IssueConsumer(ValidTextSnapshot, ValidFilePath, null, ValidConverter); - act.Should().ThrowExactly().And.ParamName.Should().Be("onIssuesChangedCallback"); + act.Should().ThrowExactly().And.ParamName.Should().Be("issueHandler"); - act = () => new IssueConsumer(ValidTextSnapshot, ValidFilePath, validCallback, null); + act = () => new IssueConsumer(ValidTextSnapshot, ValidFilePath, issueHandler, null); act.Should().ThrowExactly().And.ParamName.Should().Be("issueToIssueVisualizationConverter"); } [TestMethod] - public void Accept_WrongFile_CallbackIsNotCalled() + public void SetIssues_WrongFile_CallbackIsNotCalled() + { + var issues = new IAnalysisIssue[] { ValidIssue }; + + var testSubject = new IssueConsumer(ValidTextSnapshot, "c:\\file1.txt", issueHandler, ValidConverter); + + using (new AssertIgnoreScope()) + { + testSubject.SetIssues("wrong file", issues); + } + + issueHandler.DidNotReceiveWithAnyArgs().HandleNewIssues(default); + issueHandler.DidNotReceiveWithAnyArgs().HandleNewHotspots(default); + } + + [TestMethod] + public void SetHotspots_WrongFile_CallbackIsNotCalled() { - var callbackSpy = new OnIssuesChangedCallbackSpy(); var issues = new IAnalysisIssue[] { ValidIssue }; - var testSubject = new IssueConsumer(ValidTextSnapshot, "c:\\file1.txt", callbackSpy.Callback, ValidConverter); + var testSubject = new IssueConsumer(ValidTextSnapshot, "c:\\file1.txt", issueHandler, ValidConverter); using (new AssertIgnoreScope()) { - testSubject.Set("wrong file", issues); + testSubject.SetHotspots("wrong file", issues); } - callbackSpy.CallCount.Should().Be(0); + issueHandler.DidNotReceiveWithAnyArgs().HandleNewIssues(default); + issueHandler.DidNotReceiveWithAnyArgs().HandleNewHotspots(default); } [TestMethod] @@ -77,56 +100,84 @@ public void Accept_WrongFile_CallbackIsNotCalled() [DataRow(10, 10, true)] // end is in last line of snapshot [DataRow(10, 11, false)] // end is outside snapshot [DataRow(11, 11, false)] // end is outside snapshot - public void Accept_IssuesNotInSnapshotAreIgnored_CallbackIsCalledWithExpectedIssues(int issueStartLine, int issueEndLine, bool isMappableToSnapshot) + public void SetIssues_IssuesNotInSnapshotAreIgnored_CallbackIsCalledWithExpectedIssues(int issueStartLine, int issueEndLine, bool isMappableToSnapshot) { // Issues are 1-based. // Snapshots are 0-based so last line = index 9 const int LinesInSnapshot = 10; var snapshot = CreateSnapshot(LinesInSnapshot); var issues = new[] { CreateIssue(issueStartLine, issueEndLine) }; - - var callbackSpy = new OnIssuesChangedCallbackSpy(); var converter = CreatePassthroughConverter(); - var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter); + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); using (new AssertIgnoreScope()) { - testSubject.Set(ValidFilePath, issues); + testSubject.SetIssues(ValidFilePath, issues); } - callbackSpy.CallCount.Should().Be(1); - if (isMappableToSnapshot) - { - callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(issues); - } - else + ValidateReceivedIssues(isMappableToSnapshot ? issues : []); + } + + [TestMethod] + [DataRow(-1, 1, false)] // start line < 1 + [DataRow(0, 0, false)] // file-level issue, can't be mapped to snapshot + [DataRow(0, 1, false)] // illegal i.e. shouldn't happen, but should be ignored if it does + [DataRow(1, 1, true)] // starts in first line of snapshot + [DataRow(9, 10, true)] // in snapshot + [DataRow(10, 10, true)] // end is in last line of snapshot + [DataRow(10, 11, false)] // end is outside snapshot + [DataRow(11, 11, false)] // end is outside snapshot + public void SetHotspots_IssuesNotInSnapshotAreIgnored_CallbackIsCalledWithExpectedIssues(int issueStartLine, int issueEndLine, bool isMappableToSnapshot) + { + // Issues are 1-based. + // Snapshots are 0-based so last line = index 9 + const int LinesInSnapshot = 10; + var snapshot = CreateSnapshot(LinesInSnapshot); + var hotspots = new[] { CreateIssue(issueStartLine, issueEndLine) }; + var converter = CreatePassthroughConverter(); + + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); + + using (new AssertIgnoreScope()) { - callbackSpy.LastSuppliedIssueVisualizations.Should().BeEmpty(); + testSubject.SetHotspots(ValidFilePath, hotspots); } + + ValidateReceivedHotspots(isMappableToSnapshot ? hotspots : []); } [TestMethod] - public void Accept_HasFileLevelIssues_NotIgnored() + public void SetIssues_HasFileLevelIssues_NotIgnored() { var snapshot = CreateSnapshot(10); var issues = new[] { CreateFileLevelIssue() }; + var converter = CreatePassthroughConverter(); + + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); + + testSubject.SetIssues(ValidFilePath, issues); - var callbackSpy = new OnIssuesChangedCallbackSpy(); + ValidateReceivedIssues(issues); + } + + [TestMethod] + public void SetHotspots_HasFileLevelIssues_NotIgnored() + { + var snapshot = CreateSnapshot(10); + var hotspots = new[] { CreateFileLevelIssue() }; var converter = CreatePassthroughConverter(); - var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter); + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); - testSubject.Set(ValidFilePath, issues); + testSubject.SetHotspots(ValidFilePath, hotspots); - callbackSpy.CallCount.Should().Be(1); - callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(issues); + ValidateReceivedHotspots(hotspots); } [TestMethod] - public void Accept_MultipleCallsToAccept_IssuesAreReplaced() + public void SetIssues_MultipleCallsToAccept_IssuesAreReplaced() { - var callbackSpy = new OnIssuesChangedCallbackSpy(); var firstSetOfIssues = new[] { CreateIssue(1, 1), CreateIssue(2, 2) @@ -140,38 +191,64 @@ public void Accept_MultipleCallsToAccept_IssuesAreReplaced() var snapshot = CreateSnapshot(lineCount: 10); var converter = CreatePassthroughConverter(); - var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter); + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); // 1. First call - testSubject.Set(ValidFilePath, firstSetOfIssues); + testSubject.SetIssues(ValidFilePath, firstSetOfIssues); - callbackSpy.CallCount.Should().Be(1); - callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(firstSetOfIssues); + ValidateReceivedIssues(firstSetOfIssues); + issueHandler.ClearReceivedCalls(); // 2. Second call - testSubject.Set(ValidFilePath, secondSetOfIssues); + testSubject.SetIssues(ValidFilePath, secondSetOfIssues); - callbackSpy.CallCount.Should().Be(2); - callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(secondSetOfIssues); + ValidateReceivedIssues(secondSetOfIssues); } - private class OnIssuesChangedCallbackSpy + [TestMethod] + public void SetHotspots_MultipleCallsToAccept_IssuesAreReplaced() { - public int CallCount { get; private set; } - public IList LastSuppliedIssueVisualizations { get; private set; } - public IList LastSuppliedIssues + var firstSetOfHotspots = new[] { - get - { - return LastSuppliedIssueVisualizations?.Select(x => x.Issue).ToList(); - } - } + CreateIssue(1, 1), CreateIssue(2, 2) + }; - public void Callback(IEnumerable issues) + var secondSetOfHotspots = new[] { - CallCount++; - LastSuppliedIssueVisualizations = issues?.ToList(); - } + CreateIssue(3,3), CreateIssue(4,4) + }; + + var snapshot = CreateSnapshot(lineCount: 10); + var converter = CreatePassthroughConverter(); + + var testSubject = new IssueConsumer(snapshot, ValidFilePath, issueHandler, converter); + + // 1. First call + testSubject.SetHotspots(ValidFilePath, firstSetOfHotspots); + + ValidateReceivedHotspots(firstSetOfHotspots); + issueHandler.ClearReceivedCalls(); + + // 2. Second call + testSubject.SetHotspots(ValidFilePath, secondSetOfHotspots); + + ValidateReceivedHotspots(secondSetOfHotspots); + } + + private void ValidateReceivedIssues(IAnalysisIssue[] issues) + { + issueHandler.ReceivedWithAnyArgs(1).HandleNewIssues(default); + issueHandler.DidNotReceiveWithAnyArgs().HandleNewHotspots(default); + var analysisIssues = issueHandler.ReceivedCalls().Single().GetArguments()[0] as IEnumerable; + analysisIssues.Select(x => x.Issue).Should().BeEquivalentTo(issues); + } + + private void ValidateReceivedHotspots(IAnalysisIssue[] hotspots) + { + issueHandler.ReceivedWithAnyArgs(1).HandleNewHotspots(default); + issueHandler.DidNotReceiveWithAnyArgs().HandleNewIssues(default); + var analysisIssues = issueHandler.ReceivedCalls().Single().GetArguments()[0] as IEnumerable; + analysisIssues.Select(x => x.Issue).Should().BeEquivalentTo(hotspots); } private static ITextSnapshot CreateSnapshot(int lineCount) diff --git a/src/Integration.Vsix.UnitTests/SonarLintTagger/VsAwareAnalysisServiceTests.cs b/src/Integration.Vsix.UnitTests/SonarLintTagger/VsAwareAnalysisServiceTests.cs index 472ce93daa..258919b4ce 100644 --- a/src/Integration.Vsix.UnitTests/SonarLintTagger/VsAwareAnalysisServiceTests.cs +++ b/src/Integration.Vsix.UnitTests/SonarLintTagger/VsAwareAnalysisServiceTests.cs @@ -125,7 +125,8 @@ public void RequestAnalysis_ClearsErrorListAndSchedulesAnalysisOnBackgroundThrea Received.InOrder(() => { threadHandling.RunOnBackgroundThread(Arg.Any>>()); - issueConsumer.Set(analysisFilePath, []); + issueConsumer.SetIssues(analysisFilePath, []); + issueConsumer.SetHotspots(analysisFilePath, []); analysisService.ScheduleAnalysis(analysisFilePath, Arg.Any(), Arg.Any>(), diff --git a/src/Integration.Vsix/Analysis/IssueConsumerFactory.cs b/src/Integration.Vsix/Analysis/IssueConsumerFactory.cs index 7f9b06712c..60aa3cbd8b 100644 --- a/src/Integration.Vsix/Analysis/IssueConsumerFactory.cs +++ b/src/Integration.Vsix/Analysis/IssueConsumerFactory.cs @@ -74,7 +74,7 @@ public IIssueConsumer Create(ITextDocument textDocument, SnapshotChangedHandler onSnapshotChanged) { var issueHandler = new IssueHandler(textDocument, projectName, projectGuid, suppressedIssueMatcher, onSnapshotChanged, localHotspotsStore); - var issueConsumer = new IssueConsumer(analysisSnapshot, analysisFilePath, issueHandler.HandleNewIssues, converter); + var issueConsumer = new IssueConsumer(analysisSnapshot, analysisFilePath, issueHandler, converter); return issueConsumer; } diff --git a/src/Integration.Vsix/Analysis/IssueConsumerFactory_IssueHandler.cs b/src/Integration.Vsix/Analysis/IssueConsumerFactory_IssueHandler.cs index e8b41e5fcb..0d85d43ca3 100644 --- a/src/Integration.Vsix/Analysis/IssueConsumerFactory_IssueHandler.cs +++ b/src/Integration.Vsix/Analysis/IssueConsumerFactory_IssueHandler.cs @@ -18,12 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Collections.Generic; -using System.Linq; using Microsoft.VisualStudio.Text; using SonarLint.VisualStudio.ConnectedMode.Suppressions; -using SonarLint.VisualStudio.Core.Analysis; using SonarLint.VisualStudio.IssueVisualization.Models; using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; @@ -31,7 +27,14 @@ namespace SonarLint.VisualStudio.Integration.Vsix.Analysis { partial class IssueConsumerFactory { - internal class IssueHandler + internal interface IIssueHandler + { + void HandleNewIssues(IEnumerable issues); + + void HandleNewHotspots(IEnumerable hotspots); + } + + internal class IssueHandler : IIssueHandler { // We can't mock the span translation code (to difficult to mock), // so we use this delegate to by-passes it in tests. @@ -71,11 +74,24 @@ public IssueHandler(ITextDocument textDocument, this.suppressedIssueMatcher = suppressedIssueMatcher; this.onSnapshotChanged = onSnapshotChanged; this.localHotspotsStore = localHotspotsStore; - + this.translateSpans = translateSpans; } - internal /* for testing */ void HandleNewIssues(IEnumerable issues) + public void HandleNewIssues(IEnumerable issues) + { + var translatedIssues = PrepareIssues(issues); + var newSnapshot = new IssuesSnapshot(projectName, projectGuid, textDocument.FilePath, translatedIssues); + onSnapshotChanged(newSnapshot); + } + + public void HandleNewHotspots(IEnumerable hotspots) + { + var translatedIssues = PrepareIssues(hotspots); + localHotspotsStore.UpdateForFile(textDocument.FilePath, translatedIssues); + } + + private IAnalysisIssueVisualization[] PrepareIssues(IEnumerable issues) { MarkSuppressedIssues(issues); @@ -83,20 +99,7 @@ public IssueHandler(ITextDocument textDocument, // all issues to the current snapshot. // See bug #1487: https://github.com/SonarSource/sonarlint-visualstudio/issues/1487 var translatedIssues = translateSpans(issues, textDocument.TextBuffer.CurrentSnapshot); - - localHotspotsStore.UpdateForFile(textDocument.FilePath, - translatedIssues - .Where(issue => (issue.Issue as IAnalysisIssue)?.Type == AnalysisIssueType.SecurityHotspot)); - - var newSnapshot = new IssuesSnapshot(projectName, - projectGuid, - textDocument.FilePath, - translatedIssues.Where(issue => - { - var analysisIssueType = (issue.Issue as IAnalysisIssue)?.Type; - return analysisIssueType != AnalysisIssueType.SecurityHotspot; - })); - onSnapshotChanged(newSnapshot); + return translatedIssues; } private void MarkSuppressedIssues(IEnumerable issues) diff --git a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs index f7e09675f2..980d26d6e4 100644 --- a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs +++ b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs @@ -20,6 +20,7 @@ using Microsoft.VisualStudio.Text; using SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.Integration.Vsix.Analysis; using SonarLint.VisualStudio.IssueVisualization.Models; /* @@ -62,33 +63,56 @@ internal class IssueConsumer : IIssueConsumer private readonly ITextSnapshot analysisSnapshot; private readonly IAnalysisIssueVisualizationConverter issueToIssueVisualizationConverter; private readonly string analysisFilePath; - private readonly OnIssuesChanged onIssuesChanged; + private readonly IssueConsumerFactory.IIssueHandler issueHandler; - public delegate void OnIssuesChanged(IEnumerable issues); - - public IssueConsumer(ITextSnapshot analysisSnapshot, string analysisFilePath, OnIssuesChanged onIssuesChangedCallback, IAnalysisIssueVisualizationConverter issueToIssueVisualizationConverter) + public IssueConsumer(ITextSnapshot analysisSnapshot, string analysisFilePath, IssueConsumerFactory.IIssueHandler issueHandler, IAnalysisIssueVisualizationConverter issueToIssueVisualizationConverter) { this.analysisSnapshot = analysisSnapshot ?? throw new ArgumentNullException(nameof(analysisSnapshot)); this.analysisFilePath = analysisFilePath ?? throw new ArgumentNullException(nameof(analysisFilePath)); - this.onIssuesChanged = onIssuesChangedCallback ?? throw new ArgumentNullException(nameof(onIssuesChangedCallback)); + this.issueHandler = issueHandler ?? throw new ArgumentNullException(nameof(issueHandler)); this.issueToIssueVisualizationConverter = issueToIssueVisualizationConverter ?? throw new ArgumentNullException(nameof(issueToIssueVisualizationConverter)); } - public void Set(string path, IEnumerable issues) + public void SetIssues(string path, IEnumerable issues) { - // Callback from the daemon when new results are available - if (path != analysisFilePath) + if (ValidatePath(path)) + { + return; + } + + issueHandler.HandleNewIssues(PrepareFindings(issues)); + } + + public void SetHotspots(string path, IEnumerable hotspots) + { + if (ValidatePath(path)) { - Debug.Fail("Issues returned for an unexpected file path"); return; } - Debug.Assert(issues.All(IsIssueFileLevelOrInAnalysisSnapshot), "Not all reported issues could be mapped to the analysis snapshot"); + issueHandler.HandleNewHotspots(PrepareFindings(hotspots)); + } + + private List PrepareFindings(IEnumerable findings) + { + Debug.Assert(findings.All(IsIssueFileLevelOrInAnalysisSnapshot), "Not all reported findings could be mapped to the analysis snapshot"); - onIssuesChanged.Invoke(issues + var analysisIssueVisualizations = findings .Where(IsIssueFileLevelOrInAnalysisSnapshot) .Select(x => issueToIssueVisualizationConverter.Convert(x, analysisSnapshot)) - .ToList()); + .ToList(); + return analysisIssueVisualizations; + } + + private bool ValidatePath(string path) + { + // Callback from the daemon when new results are available + if (path != analysisFilePath) + { + Debug.Fail("Findings returned for an unexpected file path"); + return true; + } + return false; } /// diff --git a/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs b/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs index 7b831d0a06..1a5436f2b9 100644 --- a/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs +++ b/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs @@ -104,6 +104,9 @@ await threadHandling.RunOnBackgroundThread(() => }); } - private static void ClearErrorList(string filePath, IIssueConsumer issueConsumer) => - issueConsumer.Set(filePath, []); + private static void ClearErrorList(string filePath, IIssueConsumer issueConsumer) + { + issueConsumer.SetIssues(filePath, []); + issueConsumer.SetHotspots(filePath, []); + } } diff --git a/src/SLCore.Listeners.UnitTests/Implementation/Analysis/AnalysisListenerTests.cs b/src/SLCore.Listeners.UnitTests/Implementation/Analysis/AnalysisListenerTests.cs index de8867cbf3..8a79ee4513 100644 --- a/src/SLCore.Listeners.UnitTests/Implementation/Analysis/AnalysisListenerTests.cs +++ b/src/SLCore.Listeners.UnitTests/Implementation/Analysis/AnalysisListenerTests.cs @@ -38,6 +38,8 @@ public void MefCtor_CheckIsExported() MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); } @@ -88,33 +90,39 @@ public void RaiseIssues_RaisesFinding() { var raiseIssueParams = new RaiseFindingParams(default, default, default, default); var raisedFindingProcessor = Substitute.For(); - var testSubject = CreateTestSubject(raisedFindingProcessor: raisedFindingProcessor); + var issuePublisher = Substitute.For(); + var testSubject = CreateTestSubject(raisedFindingProcessor: raisedFindingProcessor, issuePublisher: issuePublisher); testSubject.RaiseIssues(raiseIssueParams); - - raisedFindingProcessor.Received().RaiseFinding(raiseIssueParams); + + raisedFindingProcessor.Received().RaiseFinding(raiseIssueParams, issuePublisher); } - + [TestMethod] public void RaiseHotspots_RaisesFinding() { var raiseIssueParams = new RaiseHotspotParams(default, default, default, default); var raisedFindingProcessor = Substitute.For(); - var testSubject = CreateTestSubject(raisedFindingProcessor: raisedFindingProcessor); + var hotspotPublisher = Substitute.For(); + var testSubject = CreateTestSubject(raisedFindingProcessor: raisedFindingProcessor, hotspotPublisher: hotspotPublisher); testSubject.RaiseHotspots(raiseIssueParams); - - raisedFindingProcessor.Received().RaiseFinding(raiseIssueParams); + + raisedFindingProcessor.Received().RaiseFinding(raiseIssueParams, hotspotPublisher); } private AnalysisListener CreateTestSubject(IActiveConfigScopeTracker activeConfigScopeTracker = null, IAnalysisRequester analysisRequester = null, IRaisedFindingProcessor raisedFindingProcessor = null, + IIssuePublisher issuePublisher = null, + IHotspotPublisher hotspotPublisher = null, ILogger logger = null) => new(activeConfigScopeTracker ?? Substitute.For(), analysisRequester ?? Substitute.For(), raisedFindingProcessor ?? Substitute.For(), + issuePublisher ?? Substitute.For(), + hotspotPublisher ?? Substitute.For(), logger ?? new TestLogger()); - + } diff --git a/src/SLCore.Listeners.UnitTests/Implementation/Analysis/RaisedFindingProcessorTests.cs b/src/SLCore.Listeners.UnitTests/Implementation/Analysis/RaisedFindingProcessorTests.cs index 1aea5df67e..8c3f6ebb3f 100644 --- a/src/SLCore.Listeners.UnitTests/Implementation/Analysis/RaisedFindingProcessorTests.cs +++ b/src/SLCore.Listeners.UnitTests/Implementation/Analysis/RaisedFindingProcessorTests.cs @@ -41,7 +41,6 @@ public class RaisedFindingProcessorTests public void MefCtor_CheckIsExported() => MefTestHelpers.CheckTypeCanBeImported( MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); @@ -54,16 +53,14 @@ public void MefCtor_CheckIsSingleton() => public void RaiseFindings_AnalysisIdIsNull_Ignores() { var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", new Dictionary>(), false, null); - - var analysisService = Substitute.For(); + var publisher = Substitute.For(); var raiseFindingParamsToAnalysisIssueConverter = Substitute.For(); - var testSubject = CreateTestSubject(analysisService: analysisService, - raiseFindingToAnalysisIssueConverter: raiseFindingParamsToAnalysisIssueConverter); + var testSubject = CreateTestSubject(raiseFindingToAnalysisIssueConverter: raiseFindingParamsToAnalysisIssueConverter); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); - analysisService.DidNotReceive().PublishIssues(Arg.Any(), Arg.Any(), Arg.Any>()); + publisher.DidNotReceiveWithAnyArgs().Publish(default, default, default); raiseFindingParamsToAnalysisIssueConverter.DidNotReceive().GetAnalysisIssues(Arg.Any(), Arg.Any>()); } @@ -75,17 +72,15 @@ public void RaiseFindings_NoFindings_Ignores() var findingsByFileUri = new Dictionary> { { fileUri, [] } }; var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", findingsByFileUri, false, analysisId); - - var analysisService = Substitute.For(); + var publisher = Substitute.For(); var raiseFindingParamsToAnalysisIssueConverter = CreateConverter(findingsByFileUri.Single().Key, [], []); - var testSubject = CreateTestSubject(analysisService: analysisService, - raiseFindingToAnalysisIssueConverter: raiseFindingParamsToAnalysisIssueConverter); + var testSubject = CreateTestSubject(raiseFindingToAnalysisIssueConverter: raiseFindingParamsToAnalysisIssueConverter); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); raiseFindingParamsToAnalysisIssueConverter.Received().GetAnalysisIssues(fileUri, Arg.Is>(x => !x.Any())); - analysisService.Received().PublishIssues(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); + publisher.Received().Publish(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); } [TestMethod] @@ -97,21 +92,19 @@ public void RaiseFindings_NoSupportedLanguages_PublishesEmpty() { { fileUri, [CreateTestFinding("csharpsquid:S100"), CreateTestFinding("csharpsquid:S101")] } }; var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", findingsByFileUri, false, analysisId); - - var analysisService = Substitute.For(); + var publisher = Substitute.For(); IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter = CreateConverter(findingsByFileUri.Single().Key, [], []); var analysisStatusNotifierFactory = CreateAnalysisStatusNotifierFactory(out var analysisStatusNotifier, fileUri.LocalPath, analysisId); - var testSubject = CreateTestSubject(analysisService: analysisService, - raiseFindingToAnalysisIssueConverter: raiseFindingToAnalysisIssueConverter, + var testSubject = CreateTestSubject(raiseFindingToAnalysisIssueConverter: raiseFindingToAnalysisIssueConverter, analysisStatusNotifierFactory: analysisStatusNotifierFactory, slCoreConstantsProvider: CreateConstantsProviderWithLanguages([])); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); raiseFindingToAnalysisIssueConverter.Received().GetAnalysisIssues(fileUri, Arg.Is>(x => !x.Any())); - analysisService.Received().PublishIssues(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); + publisher.Received().Publish(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); analysisStatusNotifier.AnalysisFinished(0, TimeSpan.Zero); } @@ -124,21 +117,20 @@ public void RaiseFindings_NoKnownLanguages_PublishesEmpty() { { fileUri, [CreateTestFinding("csharpsquid:S100"), CreateTestFinding("csharpsquid:S101")] } }; var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", findingsByFileUri, false, analysisId); - - var analysisService = Substitute.For(); + var publisher = Substitute.For(); IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter = CreateConverter(findingsByFileUri.Single().Key, [], []); var analysisStatusNotifierFactory = CreateAnalysisStatusNotifierFactory(out var analysisStatusNotifier, fileUri.LocalPath, analysisId); - var testSubject = CreateTestSubject(analysisService: analysisService, + var testSubject = CreateTestSubject( raiseFindingToAnalysisIssueConverter: raiseFindingToAnalysisIssueConverter, analysisStatusNotifierFactory: analysisStatusNotifierFactory, slCoreConstantsProvider: CreateConstantsProviderWithLanguages([SloopLanguage.JAVA])); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); raiseFindingToAnalysisIssueConverter.Received().GetAnalysisIssues(fileUri, Arg.Is>(x => !x.Any())); - analysisService.Received().PublishIssues(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); + publisher.Received().Publish(fileUri.LocalPath, analysisId, Arg.Is>(x => !x.Any())); analysisStatusNotifier.AnalysisFinished(0, TimeSpan.Zero); } @@ -147,15 +139,15 @@ public void RaiseFindings_HasNoFileUri_FinishesAnalysis() { var analysisId = Guid.NewGuid(); var analysisStatusNotifierFactory = CreateAnalysisStatusNotifierFactory(out var analysisStatusNotifier, null, analysisId); - var analysisService = Substitute.For(); - var testSubject = CreateTestSubject(analysisService: analysisService, analysisStatusNotifierFactory: analysisStatusNotifierFactory); + var publisher = Substitute.For(); + var testSubject = CreateTestSubject(analysisStatusNotifierFactory: analysisStatusNotifierFactory); var act = () => testSubject.RaiseFinding(new RaiseFindingParams("CONFIGURATION_ID", new Dictionary>(), false, - analysisId)); + analysisId), publisher); act.Should().NotThrow(); - analysisService.ReceivedCalls().Should().BeEmpty(); + publisher.ReceivedCalls().Should().BeEmpty(); analysisStatusNotifier.ReceivedCalls().Should().BeEmpty(); } @@ -177,21 +169,20 @@ public void RaiseFindings_HasIssuesNotIntermediate_PublishFindings() var findingsByFileUri = new Dictionary> { { fileUri, raisedFindings } }; var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", findingsByFileUri, false, analysisId); - - var analysisService = Substitute.For(); + var publisher = Substitute.For(); IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter = CreateConverter(findingsByFileUri.Single().Key, filteredRaisedFindings, filteredIssues); var analysisStatusNotifierFactory = CreateAnalysisStatusNotifierFactory(out var analysisStatusNotifier, fileUri.LocalPath, analysisId); - var testSubject = CreateTestSubject(analysisService: analysisService, + var testSubject = CreateTestSubject( raiseFindingToAnalysisIssueConverter: raiseFindingToAnalysisIssueConverter, analysisStatusNotifierFactory: analysisStatusNotifierFactory, slCoreConstantsProvider: CreateConstantsProviderWithLanguages(SloopLanguage.SECRETS, SloopLanguage.CS)); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); - analysisService.Received(1).PublishIssues(fileUri.LocalPath, analysisId, filteredIssues); + publisher.Received(1).Publish(fileUri.LocalPath, analysisId, filteredIssues); raiseFindingToAnalysisIssueConverter.Received(1).GetAnalysisIssues(findingsByFileUri.Single().Key, Arg.Is>( x => x.SequenceEqual(filteredRaisedFindings))); @@ -217,22 +208,22 @@ public void RaiseFindings_MultipleFiles_PublishFindingsForEachFile(bool isInterm var raiseFindingParams = new RaiseFindingParams("CONFIGURATION_ID", findingsByFileUri, isIntermediate, analysisId); - var analysisService = Substitute.For(); + var publisher = Substitute.For(); var raiseFindingParamsToAnalysisIssueConverter = Substitute.For(); raiseFindingParamsToAnalysisIssueConverter.GetAnalysisIssues(fileUri1, Arg.Any>()).Returns([analysisIssue1]); raiseFindingParamsToAnalysisIssueConverter.GetAnalysisIssues(fileUri2, Arg.Any>()).Returns([analysisIssue2]); var analysisStatusNotifierFactory = Substitute.For(); - var testSubject = CreateTestSubject(analysisService: analysisService, + var testSubject = CreateTestSubject( raiseFindingToAnalysisIssueConverter: raiseFindingParamsToAnalysisIssueConverter, analysisStatusNotifierFactory: analysisStatusNotifierFactory); - testSubject.RaiseFinding(raiseFindingParams); + testSubject.RaiseFinding(raiseFindingParams, publisher); - analysisService.Received(1).PublishIssues(fileUri1.LocalPath, analysisId, + publisher.Received(1).Publish(fileUri1.LocalPath, analysisId, Arg.Is>(x => x.SequenceEqual(new List { analysisIssue1 }))); - analysisService.Received(1).PublishIssues(fileUri2.LocalPath, analysisId, + publisher.Received(1).Publish(fileUri2.LocalPath, analysisId, Arg.Is>(x => x.SequenceEqual(new List { analysisIssue2 }))); analysisStatusNotifierFactory.Received(1).Create("SLCoreAnalyzer", fileUri1.LocalPath, analysisId); @@ -240,7 +231,6 @@ public void RaiseFindings_MultipleFiles_PublishFindingsForEachFile(bool isInterm } private RaisedFindingProcessor CreateTestSubject( - IAnalysisService analysisService = null, IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter = null, IAnalysisStatusNotifierFactory analysisStatusNotifierFactory = null, ILogger logger = null, @@ -248,7 +238,6 @@ private RaisedFindingProcessor CreateTestSubject( => new( slCoreConstantsProvider ?? CreateConstantsProviderWithLanguages(SloopLanguage.SECRETS, SloopLanguage.JS, SloopLanguage.TS, SloopLanguage.CSS), - analysisService ?? Substitute.For(), raiseFindingToAnalysisIssueConverter ?? Substitute.For(), analysisStatusNotifierFactory ?? Substitute.For(), logger ?? new TestLogger()); diff --git a/src/SLCore.Listeners/Implementation/Analysis/AnalysisListener.cs b/src/SLCore.Listeners/Implementation/Analysis/AnalysisListener.cs index f431dd08cc..c487732c0c 100644 --- a/src/SLCore.Listeners/Implementation/Analysis/AnalysisListener.cs +++ b/src/SLCore.Listeners/Implementation/Analysis/AnalysisListener.cs @@ -31,25 +31,16 @@ namespace SonarLint.VisualStudio.SLCore.Listeners.Implementation.Analysis; [Export(typeof(ISLCoreListener))] [PartCreationPolicy(CreationPolicy.Shared)] -internal class AnalysisListener : IAnalysisListener +[method: ImportingConstructor] +internal class AnalysisListener( + IActiveConfigScopeTracker activeConfigScopeTracker, + IAnalysisRequester analysisRequester, + IRaisedFindingProcessor raisedFindingProcessor, + IIssuePublisher issuePublisher, + IHotspotPublisher hotspotPublisher, + ILogger logger) + : IAnalysisListener { - private readonly IActiveConfigScopeTracker activeConfigScopeTracker; - private readonly IAnalysisRequester analysisRequester; - private readonly IRaisedFindingProcessor raisedFindingProcessor; - private readonly ILogger logger; - - [ImportingConstructor] - public AnalysisListener( - IActiveConfigScopeTracker activeConfigScopeTracker, - IAnalysisRequester analysisRequester, - IRaisedFindingProcessor raisedFindingProcessor, - ILogger logger) - { - this.activeConfigScopeTracker = activeConfigScopeTracker; - this.analysisRequester = analysisRequester; - this.logger = logger; - this.raisedFindingProcessor = raisedFindingProcessor; - } public void DidChangeAnalysisReadiness(DidChangeAnalysisReadinessParams parameters) { @@ -65,13 +56,13 @@ public void DidChangeAnalysisReadiness(DidChangeAnalysisReadinessParams paramete } else { - logger.WriteLine(SLCoreStrings.AnalysisReadinessUpdate, SLCoreStrings.ConfigScopeConflict); + logger.WriteLine(SLCoreStrings.AnalysisReadinessUpdate, SLCoreStrings.ConfigScopeConflict); } } - public void RaiseIssues(RaiseFindingParams parameters) - => raisedFindingProcessor.RaiseFinding(parameters); + public void RaiseIssues(RaiseFindingParams parameters) + => raisedFindingProcessor.RaiseFinding(parameters, issuePublisher); - public void RaiseHotspots(RaiseHotspotParams parameters) - => raisedFindingProcessor.RaiseFinding(parameters); + public void RaiseHotspots(RaiseHotspotParams parameters) + => raisedFindingProcessor.RaiseFinding(parameters, hotspotPublisher); } diff --git a/src/SLCore.Listeners/Implementation/Analysis/RaisedFindingProcessor.cs b/src/SLCore.Listeners/Implementation/Analysis/RaisedFindingProcessor.cs index 1be9ce63a4..53e93b6c2e 100644 --- a/src/SLCore.Listeners/Implementation/Analysis/RaisedFindingProcessor.cs +++ b/src/SLCore.Listeners/Implementation/Analysis/RaisedFindingProcessor.cs @@ -31,42 +31,29 @@ namespace SonarLint.VisualStudio.SLCore.Listeners.Implementation.Analysis; internal interface IRaisedFindingProcessor { - void RaiseFinding(RaiseFindingParams parameters) where T : RaisedFindingDto; + void RaiseFinding(RaiseFindingParams parameters, IFindingsPublisher findingsPublisher) where T : RaisedFindingDto; } [Export(typeof(IRaisedFindingProcessor))] [PartCreationPolicy(CreationPolicy.Shared)] -internal class RaisedFindingProcessor : IRaisedFindingProcessor +[method: ImportingConstructor] +internal class RaisedFindingProcessor( + ISLCoreConstantsProvider slCoreConstantsProvider, + IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter, + IAnalysisStatusNotifierFactory analysisStatusNotifierFactory, + ILogger logger) + : IRaisedFindingProcessor { - private readonly IAnalysisService analysisService; - private readonly IAnalysisStatusNotifierFactory analysisStatusNotifierFactory; - private readonly List analyzableLanguagesRuleKeyPrefixes; - private readonly ILogger logger; - private readonly IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter; + private readonly List analyzableLanguagesRuleKeyPrefixes = CalculateAnalyzableRulePrefixes(slCoreConstantsProvider); - [ImportingConstructor] - public RaisedFindingProcessor(ISLCoreConstantsProvider slCoreConstantsProvider, - IAnalysisService analysisService, - IRaiseFindingToAnalysisIssueConverter raiseFindingToAnalysisIssueConverter, - IAnalysisStatusNotifierFactory analysisStatusNotifierFactory, - ILogger logger) - { - this.analysisService = analysisService; - this.raiseFindingToAnalysisIssueConverter = raiseFindingToAnalysisIssueConverter; - this.analysisStatusNotifierFactory = analysisStatusNotifierFactory; - this.logger = logger; - - analyzableLanguagesRuleKeyPrefixes = CalculateAnalyzableRulePrefixes(slCoreConstantsProvider); - } - - public void RaiseFinding(RaiseFindingParams parameters) where T : RaisedFindingDto + public void RaiseFinding(RaiseFindingParams parameters, IFindingsPublisher findingsPublisher) where T : RaisedFindingDto { if (!IsValid(parameters)) { return; } - PublishFindings(parameters); + PublishFindings(parameters, findingsPublisher); } private bool IsValid(RaiseFindingParams parameters) where T : RaisedFindingDto @@ -87,7 +74,7 @@ private bool IsValid(RaiseFindingParams parameters) where T : RaisedFindin return true; } - private void PublishFindings(RaiseFindingParams parameters) where T : RaisedFindingDto + private void PublishFindings(RaiseFindingParams parameters, IFindingsPublisher findingsPublisher) where T : RaisedFindingDto { foreach (var fileAndIssues in parameters.issuesByFileUri) { @@ -95,24 +82,20 @@ private void PublishFindings(RaiseFindingParams parameters) where T : Rais var localPath = fileUri.LocalPath; var analysisStatusNotifier = analysisStatusNotifierFactory.Create(nameof(SLCoreAnalyzer), localPath, parameters.analysisId); var supportedRaisedIssues = GetSupportedLanguageFindings(fileAndIssues.Value ?? []); - analysisService.PublishIssues(localPath, + findingsPublisher.Publish(localPath, parameters.analysisId!.Value, raiseFindingToAnalysisIssueConverter.GetAnalysisIssues(fileUri, supportedRaisedIssues)); analysisStatusNotifier.AnalysisFinished(supportedRaisedIssues.Length, TimeSpan.Zero); } } - private T[] GetSupportedLanguageFindings(IEnumerable findings) where T : RaisedFindingDto - { - return findings.Where(i => analyzableLanguagesRuleKeyPrefixes.Exists(languageRepo => i.ruleKey.StartsWith(languageRepo))).ToArray(); - } + private T[] GetSupportedLanguageFindings(IEnumerable findings) where T : RaisedFindingDto => + findings.Where(i => analyzableLanguagesRuleKeyPrefixes.Exists(languageRepo => i.ruleKey.StartsWith(languageRepo))).ToArray(); - private static List CalculateAnalyzableRulePrefixes(ISLCoreConstantsProvider slCoreConstantsProvider) - { - return slCoreConstantsProvider.SLCoreAnalyzableLanguages? + private static List CalculateAnalyzableRulePrefixes(ISLCoreConstantsProvider slCoreConstantsProvider) => + slCoreConstantsProvider.SLCoreAnalyzableLanguages? .Select(x => x.ConvertToCoreLanguage()) .Select(Language.GetSonarRepoKeyFromLanguage) .Where(r => r is not null) .ToList() ?? []; - } } From 601c4a001fc41b7bbd06c8530ca86e3ed5d38a18 Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Wed, 11 Dec 2024 13:18:24 +0100 Subject: [PATCH 2/6] fix test --- .../Analysis/IssueHandlerTests.cs | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs index effcc6fefb..18a1272d11 100644 --- a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs +++ b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs @@ -18,19 +18,12 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Collections.Generic; -using System.Linq; -using FluentAssertions; using Microsoft.VisualStudio.Shell.TableManager; -using Microsoft.VisualStudio.TestTools.UnitTesting; using Microsoft.VisualStudio.Text; -using Moq; using SonarLint.VisualStudio.ConnectedMode.Suppressions; using SonarLint.VisualStudio.Core.Analysis; using SonarLint.VisualStudio.Integration.Vsix; using SonarLint.VisualStudio.Integration.Vsix.Analysis; -using SonarLint.VisualStudio.TestInfrastructure; using SonarLint.VisualStudio.IssueVisualization.Models; using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; using static SonarLint.VisualStudio.Integration.Vsix.Analysis.IssueConsumerFactory; @@ -46,12 +39,10 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() { var hotspotStoreMock = new Mock(); - var hotspot = CreateIssue("S112", startLine: 1, endLine: 1, isHotspot: true); var issue = CreateIssue("S111", startLine: 1, endLine: 1); var inputIssues = new[] { issue, - hotspot, }; var notificationHandler = new SnapshotChangeHandler(); @@ -70,7 +61,6 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() notificationHandler.InvocationCount.Should().Be(1); // Check the updated issues - VerifyHotspotsAdded(hotspotStoreMock, expectedFilePath, new []{ hotspot }); notificationHandler.UpdatedSnapshot.Issues.Count().Should().Be(1); notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(new []{issue}); @@ -87,6 +77,33 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() notificationHandler.UpdatedSnapshot.AnalyzedFilePath.Should().Be(expectedFilePath); } + [TestMethod] + public void HandleNewHotspots_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() + { + var hotspotStoreMock = new Mock(); + + var hotspot = CreateIssue("S112", startLine: 1, endLine: 1, isHotspot: true); + var inputIssues = new[] + { + hotspot, + }; + + var notificationHandler = new SnapshotChangeHandler(); + + 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); + + // Act + testSubject.HandleNewHotspots(inputIssues); + + // Assert + VerifyHotspotsAdded(hotspotStoreMock, expectedFilePath, new []{ hotspot }); + } + [TestMethod] public void HandleNewIssues_IssuesGetMatchesIsCalled() { From 5a87bb1ae588e396378b418873de05a12e406945 Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Wed, 11 Dec 2024 14:59:44 +0100 Subject: [PATCH 3/6] tests --- .../Analysis/HotspotsPublisherTests.cs | 96 +++++++++++++++++++ .../Analysis/IssuePublisherTests.cs | 96 +++++++++++++++++++ .../Analysis/IssueHandlerTests.cs | 4 +- 3 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 src/Core.UnitTests/Analysis/HotspotsPublisherTests.cs create mode 100644 src/Core.UnitTests/Analysis/IssuePublisherTests.cs diff --git a/src/Core.UnitTests/Analysis/HotspotsPublisherTests.cs b/src/Core.UnitTests/Analysis/HotspotsPublisherTests.cs new file mode 100644 index 0000000000..346672f84f --- /dev/null +++ b/src/Core.UnitTests/Analysis/HotspotsPublisherTests.cs @@ -0,0 +1,96 @@ +/* + * 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 SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.TestInfrastructure; + +namespace SonarLint.VisualStudio.Core.UnitTests.Analysis; + +[TestClass] +public class HotspotPublisherTests +{ + private IIssueConsumerStorage issueConsumerStorage; + private IIssueConsumer issueConsumer; + private HotspotPublisher testSubject; + + [TestMethod] + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported(MefTestHelpers.CreateExport()); + + [TestMethod] + public void MefCtor_CheckIsSingleton() => + MefTestHelpers.CheckIsSingletonMefComponent(); + + [TestInitialize] + public void TestInitialize() + { + issueConsumerStorage = Substitute.For(); + issueConsumer = Substitute.For(); + testSubject = new HotspotPublisher(issueConsumerStorage); + } + + [TestMethod] + public void PublishHotspots_NoConsumerInStorage_DoesNothing() + { + issueConsumerStorage.TryGet(default, out _, out _).ReturnsForAnyArgs(false); + + var act = () => testSubject.Publish("file/path", Guid.NewGuid(), Substitute.For>()); + + act.Should().NotThrow(); + issueConsumer.DidNotReceiveWithAnyArgs().SetIssues(default, default); + issueConsumer.DidNotReceiveWithAnyArgs().SetHotspots(default, default); + } + + [TestMethod] + public void PublishHotspots_DifferentAnalysisId_DoesNothing() + { + issueConsumerStorage.TryGet("file/path", out Arg.Any(), out Arg.Any()) + .Returns(info => + { + info[1] = Guid.NewGuid(); + info[2] = issueConsumer; + return true; + }); + + testSubject.Publish("file/path", Guid.NewGuid(), Substitute.For>()); + + issueConsumer.DidNotReceiveWithAnyArgs().SetIssues(default, default); + issueConsumer.DidNotReceiveWithAnyArgs().SetHotspots(default, default); + } + + [TestMethod] + public void PublishHotspots_MatchingConsumer_PublishesHotspots() + { + var analysisId = Guid.NewGuid(); + var analysisIssues = Substitute.For>(); + issueConsumerStorage.TryGet("file/path", out Arg.Any(), out Arg.Any()) + .Returns(info => + { + info[1] = analysisId; + info[2] = issueConsumer; + return true; + }); + + testSubject.Publish("file/path", analysisId, analysisIssues); + + issueConsumer.Received().SetHotspots("file/path", analysisIssues); + issueConsumer.DidNotReceiveWithAnyArgs().SetIssues(default, default); + } +} diff --git a/src/Core.UnitTests/Analysis/IssuePublisherTests.cs b/src/Core.UnitTests/Analysis/IssuePublisherTests.cs new file mode 100644 index 0000000000..237d187890 --- /dev/null +++ b/src/Core.UnitTests/Analysis/IssuePublisherTests.cs @@ -0,0 +1,96 @@ +/* + * 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 SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.TestInfrastructure; + +namespace SonarLint.VisualStudio.Core.UnitTests.Analysis; + +[TestClass] +public class IssuePublisherTests +{ + private IIssueConsumerStorage issueConsumerStorage; + private IIssueConsumer issueConsumer; + private IssuePublisher testSubject; + + [TestMethod] + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported(MefTestHelpers.CreateExport()); + + [TestMethod] + public void MefCtor_CheckIsSingleton() => + MefTestHelpers.CheckIsSingletonMefComponent(); + + [TestInitialize] + public void TestInitialize() + { + issueConsumerStorage = Substitute.For(); + issueConsumer = Substitute.For(); + testSubject = new IssuePublisher(issueConsumerStorage); + } + + [TestMethod] + public void PublishIssues_NoConsumerInStorage_DoesNothing() + { + issueConsumerStorage.TryGet(default, out _, out _).ReturnsForAnyArgs(false); + + var act = () => testSubject.Publish("file/path", Guid.NewGuid(), Substitute.For>()); + + act.Should().NotThrow(); + issueConsumer.DidNotReceiveWithAnyArgs().SetIssues(default, default); + issueConsumer.DidNotReceiveWithAnyArgs().SetHotspots(default, default); + } + + [TestMethod] + public void PublishIssues_DifferentAnalysisId_DoesNothing() + { + issueConsumerStorage.TryGet("file/path", out Arg.Any(), out Arg.Any()) + .Returns(info => + { + info[1] = Guid.NewGuid(); + info[2] = issueConsumer; + return true; + }); + + testSubject.Publish("file/path", Guid.NewGuid(), Substitute.For>()); + + issueConsumer.DidNotReceiveWithAnyArgs().SetIssues(default, default); + issueConsumer.DidNotReceiveWithAnyArgs().SetHotspots(default, default); + } + + [TestMethod] + public void PublishIssues_MatchingConsumer_PublishesIssues() + { + var analysisId = Guid.NewGuid(); + var analysisIssues = Substitute.For>(); + issueConsumerStorage.TryGet("file/path", out Arg.Any(), out Arg.Any()) + .Returns(info => + { + info[1] = analysisId; + info[2] = issueConsumer; + return true; + }); + + testSubject.Publish("file/path", analysisId, analysisIssues); + + issueConsumer.Received().SetIssues("file/path", analysisIssues); + issueConsumer.DidNotReceiveWithAnyArgs().SetHotspots(default, default); + } +} diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs index 18a1272d11..4ec31ef281 100644 --- a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs +++ b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs @@ -63,7 +63,7 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() // Check the updated issues notificationHandler.UpdatedSnapshot.Issues.Count().Should().Be(1); - notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(new []{issue}); + notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(issue); notificationHandler.UpdatedSnapshot.TryGetValue(0, StandardTableKeyNames.ProjectName, out var actualProjectName).Should().BeTrue(); actualProjectName.Should().Be(expectedProjectName); @@ -101,7 +101,7 @@ public void HandleNewHotspots_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() testSubject.HandleNewHotspots(inputIssues); // Assert - VerifyHotspotsAdded(hotspotStoreMock, expectedFilePath, new []{ hotspot }); + VerifyHotspotsAdded(hotspotStoreMock, expectedFilePath, [hotspot]); } [TestMethod] From 90d97b1bd6e3e2e352f2110a65e6d68ea5ff7947 Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Mon, 16 Dec 2024 10:10:48 +0100 Subject: [PATCH 4/6] Fix CR issues --- src/Core/Analysis/HotspotPublisher.cs | 4 +-- src/Core/Analysis/IAnalysisService.cs | 12 ------- src/Core/Analysis/IFindingsPublisher.cs | 33 +++++++++++++++++++ src/Core/Analysis/IssuePublisher.cs | 4 +-- .../Analysis/IssueHandlerTests.cs | 5 +-- .../SonarLintTagger/IssueConsumer.cs | 4 +-- 6 files changed, 42 insertions(+), 20 deletions(-) create mode 100644 src/Core/Analysis/IFindingsPublisher.cs diff --git a/src/Core/Analysis/HotspotPublisher.cs b/src/Core/Analysis/HotspotPublisher.cs index 3b26a01e59..9cbe88f6af 100644 --- a/src/Core/Analysis/HotspotPublisher.cs +++ b/src/Core/Analysis/HotspotPublisher.cs @@ -27,12 +27,12 @@ namespace SonarLint.VisualStudio.Core.Analysis; [method:ImportingConstructor] internal class HotspotPublisher(IIssueConsumerStorage issueConsumerStorage) : IHotspotPublisher { - public void Publish(string filePath, Guid analysisId, IEnumerable issues) + public void Publish(string filePath, Guid analysisId, IEnumerable findings) { if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) && analysisId == currentAnalysisId) { - issueConsumer.SetHotspots(filePath, issues); + issueConsumer.SetHotspots(filePath, findings); } } } diff --git a/src/Core/Analysis/IAnalysisService.cs b/src/Core/Analysis/IAnalysisService.cs index 717c960ec1..492a24654a 100644 --- a/src/Core/Analysis/IAnalysisService.cs +++ b/src/Core/Analysis/IAnalysisService.cs @@ -44,15 +44,3 @@ void ScheduleAnalysis(string filePath, /// void CancelForFile(string filePath); } - -public interface IFindingsPublisher -{ - /// - /// Handles analysis results - /// - void Publish(string filePath, Guid analysisId, IEnumerable issues); -} - -public interface IIssuePublisher : IFindingsPublisher; - -public interface IHotspotPublisher : IFindingsPublisher; diff --git a/src/Core/Analysis/IFindingsPublisher.cs b/src/Core/Analysis/IFindingsPublisher.cs new file mode 100644 index 0000000000..89bb3e6203 --- /dev/null +++ b/src/Core/Analysis/IFindingsPublisher.cs @@ -0,0 +1,33 @@ +/* + * 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. + */ + +namespace SonarLint.VisualStudio.Core.Analysis; + +public interface IFindingsPublisher +{ + /// + /// Handles analysis results + /// + void Publish(string filePath, Guid analysisId, IEnumerable findings); +} + +public interface IIssuePublisher : IFindingsPublisher; + +public interface IHotspotPublisher : IFindingsPublisher; diff --git a/src/Core/Analysis/IssuePublisher.cs b/src/Core/Analysis/IssuePublisher.cs index 0fb61f08cb..df09fbe228 100644 --- a/src/Core/Analysis/IssuePublisher.cs +++ b/src/Core/Analysis/IssuePublisher.cs @@ -27,12 +27,12 @@ namespace SonarLint.VisualStudio.Core.Analysis; [method:ImportingConstructor] internal class IssuePublisher(IIssueConsumerStorage issueConsumerStorage) : IIssuePublisher { - public void Publish(string filePath, Guid analysisId, IEnumerable issues) + public void Publish(string filePath, Guid analysisId, IEnumerable findings) { if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) && analysisId == currentAnalysisId) { - issueConsumer.SetIssues(filePath, issues); + issueConsumer.SetIssues(filePath, findings); } } } diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs index 4ec31ef281..5c67f22dd8 100644 --- a/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs +++ b/src/Integration.Vsix.UnitTests/Analysis/IssueHandlerTests.cs @@ -35,7 +35,7 @@ namespace SonarLint.VisualStudio.Integration.UnitTests.Analysis public class IssueHandlerTests { [TestMethod] - public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() + public void HandleNewIssues_UpdatedSnapshot() { var hotspotStoreMock = new Mock(); @@ -75,10 +75,11 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() actualFilePath.Should().Be(expectedFilePath); notificationHandler.UpdatedSnapshot.AnalyzedFilePath.Should().Be(expectedFilePath); + hotspotStoreMock.VerifyNoOtherCalls(); } [TestMethod] - public void HandleNewHotspots_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() + public void HandleNewHotspots_HotspotStoreHaveExpectedValues() { var hotspotStoreMock = new Mock(); diff --git a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs index 980d26d6e4..92bcd877f7 100644 --- a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs +++ b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs @@ -110,9 +110,9 @@ private bool ValidatePath(string path) if (path != analysisFilePath) { Debug.Fail("Findings returned for an unexpected file path"); - return true; + return false; } - return false; + return true; } /// From 369ce72d156317b5e3bc8ce3134faade65860aac Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Mon, 16 Dec 2024 10:22:38 +0100 Subject: [PATCH 5/6] fix --- src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs index 92bcd877f7..d1f91bf25e 100644 --- a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs +++ b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs @@ -75,7 +75,7 @@ public IssueConsumer(ITextSnapshot analysisSnapshot, string analysisFilePath, Is public void SetIssues(string path, IEnumerable issues) { - if (ValidatePath(path)) + if (!ValidatePath(path)) { return; } From 8be080de397de7a41cb145a9aedd7242c295a8f1 Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh Date: Mon, 16 Dec 2024 10:43:46 +0100 Subject: [PATCH 6/6] fix again --- src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs index d1f91bf25e..83401e9e2b 100644 --- a/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs +++ b/src/Integration.Vsix/SonarLintTagger/IssueConsumer.cs @@ -85,7 +85,7 @@ public void SetIssues(string path, IEnumerable issues) public void SetHotspots(string path, IEnumerable hotspots) { - if (ValidatePath(path)) + if (!ValidatePath(path)) { return; }