diff --git a/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs b/src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs index a4c7fa3aa..0acf246c8 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 306769203..c6b18b8ce 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 d543a6cd0..775a24556 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.UnitTests/Analysis/HotspotsPublisherTests.cs b/src/Core.UnitTests/Analysis/HotspotsPublisherTests.cs new file mode 100644 index 000000000..346672f84 --- /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 000000000..237d18789 --- /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/Core/Analysis/AnalysisService.cs b/src/Core/Analysis/AnalysisService.cs index 07b70e93d..8258aaec6 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 000000000..9cbe88f6a --- /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 findings) + { + if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) + && analysisId == currentAnalysisId) + { + issueConsumer.SetHotspots(filePath, findings); + } + } +} diff --git a/src/Core/Analysis/IAnalysisService.cs b/src/Core/Analysis/IAnalysisService.cs index 99ac54d4a..492a24654 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 /// diff --git a/src/Core/Analysis/IFindingsPublisher.cs b/src/Core/Analysis/IFindingsPublisher.cs new file mode 100644 index 000000000..89bb3e620 --- /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/IIssueConsumer.cs b/src/Core/Analysis/IIssueConsumer.cs index 228de739c..a10d445af 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 000000000..df09fbe22 --- /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 findings) + { + if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer) + && analysisId == currentAnalysisId) + { + issueConsumer.SetIssues(filePath, findings); + } + } +} diff --git a/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs b/src/Integration.Vsix.UnitTests/Analysis/IssueConsumerFactoryTests.cs index d3a12d3d0..e313367a1 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 f121f2dfa..5c67f22dd 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; @@ -42,16 +35,14 @@ namespace SonarLint.VisualStudio.Integration.UnitTests.Analysis public class IssueHandlerTests { [TestMethod] - public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() + public void HandleNewIssues_UpdatedSnapshot() { 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(); @@ -59,7 +50,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); @@ -70,10 +61,9 @@ 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}); + notificationHandler.UpdatedSnapshot.Issues.Should().BeEquivalentTo(issue); notificationHandler.UpdatedSnapshot.TryGetValue(0, StandardTableKeyNames.ProjectName, out var actualProjectName).Should().BeTrue(); actualProjectName.Should().Be(expectedProjectName); @@ -85,6 +75,34 @@ public void HandleNewIssues_UpdatedSnapshotAndHotspotStoreHaveExpectedValues() actualFilePath.Should().Be(expectedFilePath); notificationHandler.UpdatedSnapshot.AnalyzedFilePath.Should().Be(expectedFilePath); + hotspotStoreMock.VerifyNoOtherCalls(); + } + + [TestMethod] + public void HandleNewHotspots_HotspotStoreHaveExpectedValues() + { + 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, [hotspot]); } [TestMethod] @@ -125,14 +143,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 +171,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 +238,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 +318,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 +346,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 268003c86..b20773bd8 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 472ce93da..258919b4c 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 7f9b06712..60aa3cbd8 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 e8b41e5fc..0d85d43ca 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 f7e09675f..83401e9e2 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 false; + } + return true; } /// diff --git a/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs b/src/Integration.Vsix/SonarLintTagger/VsAwareAnalysisService.cs index 7b831d0a0..1a5436f2b 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 de8867cbf..8a79ee451 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 1aea5df67..8c3f6ebb3 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 f431dd08c..c487732c0 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 1be9ce63a..53e93b6c2 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() ?? []; - } }