Skip to content

Commit

Permalink
SLVS-1679 Enable issue streaming for SLCore analysis (#5876)
Browse files Browse the repository at this point in the history
  • Loading branch information
georgii-borovinskikh-sonarsource authored Dec 6, 2024
1 parent d1ed802 commit eafc02f
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 142 deletions.
2 changes: 1 addition & 1 deletion src/CFamily.UnitTests/Subprocess/MessageHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void HandleMessage_IssuesForAnalyzedFileAreNotIgnored(string fileNameInMe
context.IssueConverter.Invocations.Count.Should().Be(1);
context.IssueConsumer.Invocations.Count.Should().Be(1);

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

[TestMethod]
Expand Down
7 changes: 4 additions & 3 deletions src/CFamily/Subprocess/MessageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void HandleMessage(Message message)

// Rules that start with internal shouldn't be treated as issues.
// Some of them should be handled like `internal.fileDependency`. See: https://github.com/SonarSource/sonarlint-visualstudio/issues/2611
// Others should can simply ignored like `internal.z3RefutationRate`, which is used to log in the scanner how many issues are rejected by the Z3 solver
// Others should can simply ignored like `internal.z3RefutationRate`, which is used to log in the scanner how many issues are rejected by the Z3 solver
case string s when s.StartsWith("internal."):
break;

Expand Down Expand Up @@ -137,11 +137,12 @@ private void HandleAnalysisIssue(Message message)
IssueCount++;
var issue = issueConverter.Convert(message, request.Context.CFamilyLanguage, request.Context.RulesConfiguration);

// Note: the file being analyzed might have been closed by the time the analysis results are
// Note: the file being analyzed might have been closed by the time the analysis results are
// returned. This doesn't cause a crash; all active taggers will have been detached from the
// TextBufferIssueTracker when the file was closed, but the TextBufferIssueTracker will
// still exist and handle the call.
issueConsumer.Accept(request.Context.File, new[] { issue });
// todo https://sonarsource.atlassian.net/browse/SLVS-1661
issueConsumer.Set(request.Context.File, new[] { issue });
}

internal /* for testing */ static bool IsIssueForActiveRule(Message message, ICFamilyRulesConfig rulesConfiguration)
Expand Down
54 changes: 27 additions & 27 deletions src/Core.UnitTests/Analysis/AnalysisServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void MefCtor_CheckIsSingleton()
{
MefTestHelpers.CheckIsSingletonMefComponent<AnalysisService>();
}

[TestMethod]
public void ScheduleAnalysis_AnalysisScheduler_CachesIssueConsumer_And_RunsAnalyzerController()
{
Expand All @@ -54,15 +54,15 @@ public void ScheduleAnalysis_AnalysisScheduler_CachesIssueConsumer_And_RunsAnaly
var testSubject = CreateTestSubject(analyzerController, issueConsumerStorage, scheduler);

testSubject.ScheduleAnalysis("file/path", analysisId, detectedLanguages, issueConsumer, analyzerOptions);

Received.InOrder(() =>
{
scheduler.Schedule("file/path", Arg.Any<Action<CancellationToken>>(), Arg.Any<int>());
issueConsumerStorage.Set("file/path", analysisId, issueConsumer);
analyzerController.ExecuteAnalysis("file/path", analysisId, detectedLanguages, issueConsumer, analyzerOptions, Arg.Any<CancellationToken>());
});
}

[TestMethod]
public void ScheduleAnalysis_JobCancelledBeforeStarting_DoesNotExecute()
{
Expand All @@ -72,7 +72,7 @@ public void ScheduleAnalysis_JobCancelledBeforeStarting_DoesNotExecute()
var testSubject = CreateTestSubject(analyzerController, issueConsumerStorage, scheduler);

testSubject.ScheduleAnalysis("file/path", default, default, default, default);

scheduler.Received().Schedule("file/path", Arg.Any<Action<CancellationToken>>(), Arg.Any<int>());
issueConsumerStorage.DidNotReceiveWithAnyArgs().Set(default, default, default);
analyzerController.DidNotReceiveWithAnyArgs().ExecuteAnalysis(default, default, default, default, default, default);
Expand All @@ -90,26 +90,26 @@ public void ScheduleAnalysis_ProvidesCorrectTimeout(int envSettingsResponse, int
Environment.SetEnvironmentVariable(EnvironmentSettings.AnalysisTimeoutEnvVar, envSettingsResponse.ToString());
var scheduler = Substitute.For<IScheduler>();
var testSubject = CreateTestSubject(scheduler: scheduler);

testSubject.ScheduleAnalysis("file/path", default, default, default, default);

scheduler.Received().Schedule("file/path", Arg.Any<Action<CancellationToken>>(), expectedTimeout);
}
finally
{
Environment.SetEnvironmentVariable(EnvironmentSettings.AnalysisTimeoutEnvVar, null);
}

}

[TestMethod]
public void ScheduleAnalysis_NoEnvironmentSettings_DefaultTimeout()
{
var scheduler = Substitute.For<IScheduler>();
var testSubject = CreateTestSubject(scheduler: scheduler);

testSubject.ScheduleAnalysis("file/path", default, default, default, default);

scheduler.Received().Schedule("file/path", Arg.Any<Action<CancellationToken>>(), AnalysisService.DefaultAnalysisTimeoutMs);
}

Expand All @@ -118,25 +118,25 @@ public void PublishIssues_NoConsumerInStorage_DoesNothing()
{
var issueConsumerStorage = CreateIssueConsumerStorageWithStoredItem(Guid.NewGuid(), null, false);
var testSubject = CreateTestSubject(issueConsumerStorage:issueConsumerStorage);

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

act.Should().NotThrow();
}

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

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

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

[TestMethod]
public void PublishIssues_MatchingConsumer_PublishesIssues()
{
Expand All @@ -147,10 +147,10 @@ public void PublishIssues_MatchingConsumer_PublishesIssues()
var analysisIssues = Substitute.For<IEnumerable<IAnalysisIssue>>();

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

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

[DataRow(true)]
[DataRow(false)]
[DataTestMethod]
Expand All @@ -170,22 +170,22 @@ public void CancelForFile_JobCancelledBeforeStarting_DoesNotExecute()
var issueConsumerStorage = Substitute.For<IIssueConsumerStorage>();
var scheduler = CreateDefaultScheduler(true);
var testSubject = CreateTestSubject(issueConsumerStorage: issueConsumerStorage, scheduler: scheduler);

testSubject.CancelForFile("file/path");

scheduler.Received().Schedule("file/path", Arg.Any<Action<CancellationToken>>(), -1);
issueConsumerStorage.DidNotReceiveWithAnyArgs().Remove(default);
}

[TestMethod]
public void CancelForFile_RunsConsumerStorageClearAsScheduledJob()
{
var issueConsumerStorage = Substitute.For<IIssueConsumerStorage>();
var scheduler = CreateDefaultScheduler();
var testSubject = CreateTestSubject(issueConsumerStorage: issueConsumerStorage, scheduler: scheduler);

testSubject.CancelForFile("file/path");

Received.InOrder(() =>
{
scheduler.Schedule("file/path", Arg.Any<Action<CancellationToken>>(), -1);
Expand All @@ -202,7 +202,7 @@ private static IAnalysisService CreateTestSubject(IAnalyzerController analyzerCo
scheduler ??= Substitute.For<IScheduler>();
return new AnalysisService(analyzerController, issueConsumerStorage, scheduler);
}

private static IIssueConsumerStorage CreateIssueConsumerStorageWithStoredItem(Guid analysisId, IIssueConsumer issueConsumer, bool result)
{
var issueConsumerStorage = Substitute.For<IIssueConsumerStorage>();
Expand All @@ -214,7 +214,7 @@ private static IIssueConsumerStorage CreateIssueConsumerStorageWithStoredItem(Gu
});
return issueConsumerStorage;
}

private static IScheduler CreateDefaultScheduler(bool createCancelled = false)
{
var cancellationTokenSource = new CancellationTokenSource();
Expand Down
4 changes: 2 additions & 2 deletions src/Core/Analysis/AnalysisService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void PublishIssues(string filePath, Guid analysisId, IEnumerable<IAnalysi
if (issueConsumerStorage.TryGet(filePath, out var currentAnalysisId, out var issueConsumer)
&& analysisId == currentAnalysisId)
{
issueConsumer.Accept(filePath, issues);
issueConsumer.Set(filePath, issues);
}
}

Expand Down Expand Up @@ -84,7 +84,7 @@ public void CancelForFile(string filePath)
},
-1);
}

private static int GetAnalysisTimeoutInMilliseconds()
{
var environmentSettings = new EnvironmentSettings();
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Analysis/IIssueConsumer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ namespace SonarLint.VisualStudio.Core.Analysis;
/// </summary>
public interface IIssueConsumer
{
void Accept(string path, IEnumerable<IAnalysisIssue> issues);
void Set(string path, IEnumerable<IAnalysisIssue> issues);
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void Create_InitializedIssueConsumerReturned()
/* The empty issues list is passed as an argument here because
it's impossible to verify the actual pipeline due to the fact
that mocking ITextSnapshot in a way that then can be used by a SnapshotSpan takes a lot of effort */
consumer.Accept("analysisfile.txt", []);
consumer.Set("analysisfile.txt", []);

publishedIssuesSnapshot.Should().NotBeNull();
publishedIssuesSnapshot.AnalyzedFilePath.Should().Be("updatedfile.txt"); // filename should be updted by this point
Expand Down
2 changes: 1 addition & 1 deletion src/Integration.Vsix.UnitTests/ReferencesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class ReferencesTests
public void MicrosoftVisualStudioVCProjectEngine_EnsureCorrectVersion()
{
var codeAnalysisAssemblyVersion = AssemblyHelper.GetVersionOfReferencedAssembly(
typeof(SonarLint.VisualStudio.Integration.Vsix.AccumulatingIssueConsumer), // any type in the VSIX assembly will do
typeof(SonarLint.VisualStudio.Integration.Vsix.IssueConsumer), // any type in the VSIX assembly will do
"Microsoft.VisualStudio.VCProjectEngine");

AssertIsCorrectMajorVersion(codeAnalysisAssemblyVersion.Major);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,15 @@
* 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.TestTools.UnitTesting;
using Microsoft.VisualStudio.Text;
using Moq;
using SonarLint.VisualStudio.Core.Analysis;
using SonarLint.VisualStudio.Integration.Vsix;
using SonarLint.VisualStudio.IssueVisualization.Models;
using SonarLint.VisualStudio.TestInfrastructure;

namespace SonarLint.VisualStudio.Integration.UnitTests.SonarLintTagger
{
[TestClass]
public class AccumulatingIssueConsumerTests
public class IssueConsumerTests
{
private static readonly IAnalysisIssue ValidIssue = CreateIssue(startLine: 1, endLine: 1);
private static readonly ITextSnapshot ValidTextSnapshot = CreateSnapshot(lineCount: 10);
Expand All @@ -44,18 +37,18 @@ public class AccumulatingIssueConsumerTests
[TestMethod]
public void Ctor_InvalidArgs_Throws()
{
AccumulatingIssueConsumer.OnIssuesChanged validCallback = _ => { };
IssueConsumer.OnIssuesChanged validCallback = _ => { };

Action act = () => new AccumulatingIssueConsumer(null, ValidFilePath, validCallback, ValidConverter);
Action act = () => new IssueConsumer(null, ValidFilePath, validCallback, ValidConverter);
act.Should().ThrowExactly<ArgumentNullException>().And.ParamName.Should().Be("analysisSnapshot");

act = () => new AccumulatingIssueConsumer(ValidTextSnapshot, null, validCallback, ValidConverter);
act = () => new IssueConsumer(ValidTextSnapshot, null, validCallback, ValidConverter);
act.Should().ThrowExactly<ArgumentNullException>().And.ParamName.Should().Be("analysisFilePath");

act = () => new AccumulatingIssueConsumer(ValidTextSnapshot, ValidFilePath, null, ValidConverter);
act = () => new IssueConsumer(ValidTextSnapshot, ValidFilePath, null, ValidConverter);
act.Should().ThrowExactly<ArgumentNullException>().And.ParamName.Should().Be("onIssuesChangedCallback");

act = () => new AccumulatingIssueConsumer(ValidTextSnapshot, ValidFilePath, validCallback, null);
act = () => new IssueConsumer(ValidTextSnapshot, ValidFilePath, validCallback, null);
act.Should().ThrowExactly<ArgumentNullException>().And.ParamName.Should().Be("issueToIssueVisualizationConverter");
}

Expand All @@ -65,11 +58,11 @@ public void Accept_WrongFile_CallbackIsNotCalled()
var callbackSpy = new OnIssuesChangedCallbackSpy();
var issues = new IAnalysisIssue[] { ValidIssue };

var testSubject = new AccumulatingIssueConsumer(ValidTextSnapshot, "c:\\file1.txt", callbackSpy.Callback, ValidConverter);
var testSubject = new IssueConsumer(ValidTextSnapshot, "c:\\file1.txt", callbackSpy.Callback, ValidConverter);

using (new AssertIgnoreScope())
{
testSubject.Accept("wrong file", issues);
testSubject.Set("wrong file", issues);
}

callbackSpy.CallCount.Should().Be(0);
Expand All @@ -95,11 +88,11 @@ public void Accept_IssuesNotInSnapshotAreIgnored_CallbackIsCalledWithExpectedIss
var callbackSpy = new OnIssuesChangedCallbackSpy();
var converter = CreatePassthroughConverter();

var testSubject = new AccumulatingIssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);
var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);

using (new AssertIgnoreScope())
{
testSubject.Accept(ValidFilePath, issues);
testSubject.Set(ValidFilePath, issues);
}

callbackSpy.CallCount.Should().Be(1);
Expand All @@ -122,16 +115,16 @@ public void Accept_HasFileLevelIssues_NotIgnored()
var callbackSpy = new OnIssuesChangedCallbackSpy();
var converter = CreatePassthroughConverter();

var testSubject = new AccumulatingIssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);
var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);

testSubject.Accept(ValidFilePath, issues);
testSubject.Set(ValidFilePath, issues);

callbackSpy.CallCount.Should().Be(1);
callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(issues);
}

[TestMethod]
public void Accept_MultipleCallsToAccept_IssuesAreAccumulated()
public void Accept_MultipleCallsToAccept_IssuesAreReplaced()
{
var callbackSpy = new OnIssuesChangedCallbackSpy();
var firstSetOfIssues = new[]
Expand All @@ -147,19 +140,19 @@ public void Accept_MultipleCallsToAccept_IssuesAreAccumulated()
var snapshot = CreateSnapshot(lineCount: 10);
var converter = CreatePassthroughConverter();

var testSubject = new AccumulatingIssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);
var testSubject = new IssueConsumer(snapshot, ValidFilePath, callbackSpy.Callback, converter);

// 1. First call
testSubject.Accept(ValidFilePath, firstSetOfIssues);
testSubject.Set(ValidFilePath, firstSetOfIssues);

callbackSpy.CallCount.Should().Be(1);
callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(firstSetOfIssues);

// 2. Second call
testSubject.Accept(ValidFilePath, secondSetOfIssues);
testSubject.Set(ValidFilePath, secondSetOfIssues);

callbackSpy.CallCount.Should().Be(2);
callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(firstSetOfIssues.Union(secondSetOfIssues));
callbackSpy.LastSuppliedIssues.Should().BeEquivalentTo(secondSetOfIssues);
}

private class OnIssuesChangedCallbackSpy
Expand Down Expand Up @@ -217,7 +210,7 @@ private static IAnalysisIssueVisualizationConverter CreatePassthroughConverter()
mockIssueConverter
.Setup(x => x.Convert(It.IsAny<IAnalysisIssue>(), It.IsAny<ITextSnapshot>()))
.Returns<IAnalysisIssue, ITextSnapshot>((issue, snapshot) => CreateIssueViz(issue, new SnapshotSpan()));

return mockIssueConverter.Object;

IAnalysisIssueVisualization CreateIssueViz(IAnalysisIssue issue, SnapshotSpan snapshotSpan)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public void RequestAnalysis_ClearsErrorListAndSchedulesAnalysisOnBackgroundThrea
Received.InOrder(() =>
{
threadHandling.RunOnBackgroundThread(Arg.Any<Func<Task<int>>>());
issueConsumer.Accept(analysisFilePath, []);
issueConsumer.Set(analysisFilePath, []);
analysisService.ScheduleAnalysis(analysisFilePath,
Arg.Any<Guid>(),
Arg.Any<IEnumerable<AnalysisLanguage>>(),
Expand Down
2 changes: 1 addition & 1 deletion src/Integration.Vsix/Analysis/IssueConsumerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public IIssueConsumer Create(ITextDocument textDocument,
SnapshotChangedHandler onSnapshotChanged)
{
var issueHandler = new IssueHandler(textDocument, projectName, projectGuid, suppressedIssueMatcher, onSnapshotChanged, localHotspotsStore);
var issueConsumer = new AccumulatingIssueConsumer(analysisSnapshot, analysisFilePath, issueHandler.HandleNewIssues, converter);
var issueConsumer = new IssueConsumer(analysisSnapshot, analysisFilePath, issueHandler.HandleNewIssues, converter);

return issueConsumer;
}
Expand Down
Loading

0 comments on commit eafc02f

Please sign in to comment.