From 7f5b8276c387c8133f569a07568292a304748131 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 10:28:59 +0100 Subject: [PATCH 1/6] SLVS-1745 Rename interface --- src/IssueViz/FixSuggestion/DiffView/DiffViewToolWindowPane.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/IssueViz/FixSuggestion/DiffView/DiffViewToolWindowPane.cs b/src/IssueViz/FixSuggestion/DiffView/DiffViewToolWindowPane.cs index 2d6524c011..b276e8dad9 100644 --- a/src/IssueViz/FixSuggestion/DiffView/DiffViewToolWindowPane.cs +++ b/src/IssueViz/FixSuggestion/DiffView/DiffViewToolWindowPane.cs @@ -27,14 +27,14 @@ namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; -public interface IDiffViewWindow +public interface IDiffViewToolWindowPane { bool ShowDiff(FixSuggestionDetails fixSuggestionDetails, ITextBuffer before, ITextBuffer after); } [ExcludeFromCodeCoverage] [Guid(DiffViewToolWindowPaneId)] -public class DiffViewToolWindowPane : ToolWindowPane, IDiffViewWindow +public class DiffViewToolWindowPane : ToolWindowPane, IDiffViewToolWindowPane { private readonly IDifferenceBufferFactoryService differenceBufferFactoryService; private readonly IWpfDifferenceViewerFactoryService differenceViewerFactoryService; From 6e1bdfdf9cea38d54ca96e26dfa765fc0675ce65 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 11:04:50 +0100 Subject: [PATCH 2/6] SLVS-1745 Introduce DiffViewService to encapsulate all logic related to the UI and for better testability --- .../DiffView/DiffViewServiceTests.cs | 96 +++++++++++++++++++ .../FixSuggestion/DiffView/DiffViewService.cs | 53 ++++++++++ .../DiffView/FixSuggestionDetails.cs | 7 +- 3 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 src/IssueViz.UnitTests/FixSuggestion/DiffView/DiffViewServiceTests.cs create mode 100644 src/IssueViz/FixSuggestion/DiffView/DiffViewService.cs diff --git a/src/IssueViz.UnitTests/FixSuggestion/DiffView/DiffViewServiceTests.cs b/src/IssueViz.UnitTests/FixSuggestion/DiffView/DiffViewServiceTests.cs new file mode 100644 index 0000000000..aef5e16bd9 --- /dev/null +++ b/src/IssueViz.UnitTests/FixSuggestion/DiffView/DiffViewServiceTests.cs @@ -0,0 +1,96 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 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 Microsoft.VisualStudio.Text; +using Microsoft.VisualStudio.Utilities; +using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; +using SonarLint.VisualStudio.TestInfrastructure; + +namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.FixSuggestion.DiffView; + +[TestClass] +public class DiffViewServiceTests +{ + private readonly FixSuggestionDetails fixSuggestionDetails = new(1, 3, "C://somePath/myFile.cs"); + private IDiffViewToolWindowPane diffViewToolWindowPane; + private DiffViewService testSubject; + private ITextBufferFactoryService textBufferFactoryService; + private IToolWindowService toolWindowService; + + [TestInitialize] + public void TestInitialize() + { + toolWindowService = Substitute.For(); + textBufferFactoryService = Substitute.For(); + diffViewToolWindowPane = Substitute.For(); + toolWindowService.GetToolWindow().Returns(diffViewToolWindowPane); + + testSubject = new DiffViewService( + toolWindowService, + textBufferFactoryService); + } + + [TestMethod] + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported( + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport()); + + [TestMethod] + public void MefCtor_CheckIsNonShared() => MefTestHelpers.CheckIsNonSharedMefComponent(); + + [TestMethod] + public void Ctor_InstantiatesDiffViewWindowToolPane() => toolWindowService.Received(1).GetToolWindow(); + + [TestMethod] + public void ShowDiffView_CallsShowDiffWithCorrectParameters() + { + var before = CreateChangeModel("int a=1;"); + var after = CreateChangeModel("var a=1;"); + var expectedBeforeTextBuffer = MockTextBuffer(before); + var expectedAfterTextBuffer = MockTextBuffer(after); + + testSubject.ShowDiffView(fixSuggestionDetails, before, after); + + diffViewToolWindowPane.Received(1).ShowDiff(fixSuggestionDetails, expectedBeforeTextBuffer, expectedAfterTextBuffer); + } + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public void ShowDiffView_ReturnsResultFromToolWindowPane(bool expectedResult) + { + diffViewToolWindowPane.ShowDiff(fixSuggestionDetails, Arg.Any(), Arg.Any()).Returns(expectedResult); + + var applied = testSubject.ShowDiffView(fixSuggestionDetails, CreateChangeModel(string.Empty), CreateChangeModel(";")); + + applied.Should().Be(expectedResult); + } + + private ITextBuffer MockTextBuffer(ChangeModel change) + { + var textBuffer = Substitute.For(); + textBufferFactoryService.CreateTextBuffer(change.Text, change.ContentType).Returns(textBuffer); + return textBuffer; + } + + private static ChangeModel CreateChangeModel(string text) => new(text, Substitute.For()); +} diff --git a/src/IssueViz/FixSuggestion/DiffView/DiffViewService.cs b/src/IssueViz/FixSuggestion/DiffView/DiffViewService.cs new file mode 100644 index 0000000000..95fc88e31c --- /dev/null +++ b/src/IssueViz/FixSuggestion/DiffView/DiffViewService.cs @@ -0,0 +1,53 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 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; +using Microsoft.VisualStudio.Text; +using SonarLint.VisualStudio.Core; + +namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; + +public interface IDiffViewService +{ + bool ShowDiffView(FixSuggestionDetails fixSuggestionDetails, ChangeModel before, ChangeModel after); +} + +[Export(typeof(IDiffViewService))] +[PartCreationPolicy(CreationPolicy.NonShared)] +public class DiffViewService : IDiffViewService +{ + private readonly ITextBufferFactoryService textBufferFactoryService; + private readonly IDiffViewToolWindowPane diffViewToolWindowPane; + + [ImportingConstructor] + internal DiffViewService( + IToolWindowService toolWindowService, + ITextBufferFactoryService textBufferFactoryService) + { + this.textBufferFactoryService = textBufferFactoryService; + + diffViewToolWindowPane = toolWindowService.GetToolWindow(); + } + + public bool ShowDiffView(FixSuggestionDetails fixSuggestionDetails, ChangeModel before, ChangeModel after) => + diffViewToolWindowPane.ShowDiff(fixSuggestionDetails, CreateTextBuffer(before), CreateTextBuffer(after)); + + private ITextBuffer CreateTextBuffer(ChangeModel change) => textBufferFactoryService.CreateTextBuffer(change.Text, change.ContentType); +} diff --git a/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs b/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs index f7ce624e14..6b9ae232cd 100644 --- a/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs +++ b/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs @@ -18,11 +18,14 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Diagnostics.CodeAnalysis; +using Microsoft.VisualStudio.Utilities; namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; -[ExcludeFromCodeCoverage] // TODO by SLVS-1745: remove the code coverage exclusion public record FixSuggestionDetails(int ChangeIndex, int TotalChangesFixes, string FileName) { } + +public record ChangeModel(string Text, IContentType ContentType) +{ +} From 983881de61dba3b52d336f635fc1af086f75d3f4 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 12:42:20 +0100 Subject: [PATCH 3/6] SLVS-1745 Show DiffToolWindow for each suggestion --- .../FixSuggestionHandlerTests.cs | 164 ++++++++++++++---- .../FixSuggestion/FixSuggestionHandler.cs | 23 ++- 2 files changed, 146 insertions(+), 41 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index a75c081f10..cade8f6945 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -21,10 +21,12 @@ using System.IO; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; +using Microsoft.VisualStudio.Utilities; using NSubstitute.ExceptionExtensions; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.FixSuggestion; +using SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; using SonarLint.VisualStudio.IssueVisualization.OpenInIde; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; @@ -38,14 +40,15 @@ public class FixSuggestionHandlerTests { private const string ConfigurationScopeRoot = @"C:\"; private readonly ShowFixSuggestionParams suggestionWithOneChange = CreateFixSuggestionParams(changes: CreateChangesDto(1, 1, "var a=1;")); - private FixSuggestionHandler testSubject; - private IThreadHandling threadHandling; - private ILogger logger; + private IDiffViewService diffViewService; private IDocumentNavigator documentNavigator; + private IFixSuggestionNotification fixSuggestionNotification; + private IIDEWindowService ideWindowService; private IIssueSpanCalculator issueSpanCalculator; + private ILogger logger; private IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; - private IIDEWindowService ideWindowService; - private IFixSuggestionNotification fixSuggestionNotification; + private FixSuggestionHandler testSubject; + private IThreadHandling threadHandling; [TestInitialize] public void TestInitialize() @@ -57,6 +60,7 @@ public void TestInitialize() openInIdeConfigScopeValidator = Substitute.For(); ideWindowService = Substitute.For(); fixSuggestionNotification = Substitute.For(); + diffViewService = Substitute.For(); testSubject = new FixSuggestionHandler( threadHandling, @@ -65,16 +69,14 @@ public void TestInitialize() issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService, - fixSuggestionNotification); + fixSuggestionNotification, + diffViewService); MockConfigScopeRoot(); issueSpanCalculator.IsSameHash(Arg.Any(), Arg.Any()).Returns(true); } [TestMethod] - public void MefCtor_CheckIsSingleton() - { - MefTestHelpers.CheckIsSingletonMefComponent(); - } + public void MefCtor_CheckIsSingleton() => MefTestHelpers.CheckIsSingletonMefComponent(); [TestMethod] public void ApplyFixSuggestion_RunsOnUIThread() @@ -87,7 +89,8 @@ public void ApplyFixSuggestion_RunsOnUIThread() issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService, - fixSuggestionNotification); + fixSuggestionNotification, + diffViewService); fixSuggestionHandler.ApplyFixSuggestion(suggestionWithOneChange); @@ -95,12 +98,13 @@ public void ApplyFixSuggestion_RunsOnUIThread() } [TestMethod] - public void ApplyFixSuggestion_OneChange_AppliesChange() + public void ApplyFixSuggestion_OneChangeAccepted_AppliesChange() { var suggestedChange = suggestionWithOneChange.fixSuggestion.fileEdit.changes[0]; MockCalculateSpan(suggestedChange); var textView = MockOpenFile(); var edit = MockTextEdit(textView); + MockChangeAccepted(); testSubject.ApplyFixSuggestion(suggestionWithOneChange); @@ -118,13 +122,40 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() }); } + [TestMethod] + public void ApplyFixSuggestion_OneChangeNotAccepted_DoesNotApplyChange() + { + var suggestedChange = suggestionWithOneChange.fixSuggestion.fileEdit.changes[0]; + MockCalculateSpan(suggestedChange); + var textView = MockOpenFile(); + var edit = MockTextEdit(textView); + MockChangeNotAccepted(); + + testSubject.ApplyFixSuggestion(suggestionWithOneChange); + + Received.InOrder(() => + { + logger.WriteLine(FixSuggestionResources.ProcessingRequest, suggestionWithOneChange.configurationScopeId, suggestionWithOneChange.fixSuggestion.suggestionId); + ideWindowService.BringToFront(); + fixSuggestionNotification.Clear(); + documentNavigator.Open(@"C:\myFile.cs"); + textView.TextBuffer.CreateEdit(); + issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); + edit.Apply(); + logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionWithOneChange.configurationScopeId, suggestionWithOneChange.fixSuggestion.suggestionId); + }); + + edit.DidNotReceive().Replace(Arg.Any(), suggestedChange.after); + } + [TestMethod] public void ApplyFixSuggestion_TwoChanges_AppliesChangeOnce() { - issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(new SnapshotSpan()); + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => CreateMockedSnapshotSpan(string.Empty)); var suggestionWithTwoChanges = CreateFixSuggestionParams(changes: [CreateChangesDto(1, 1, "var a=1;"), CreateChangesDto(2, 2, "var b=0;")]); var textView = MockOpenFile(); var edit = MockTextEdit(textView); + MockChangeAccepted(); testSubject.ApplyFixSuggestion(suggestionWithTwoChanges); @@ -134,18 +165,18 @@ public void ApplyFixSuggestion_TwoChanges_AppliesChangeOnce() } /// - /// The changes are applied from bottom to top to avoid changing the line numbers - /// of the changes that are below the current change. - /// - /// This is important when the change is more lines than the original line range. + /// The changes are applied from bottom to top to avoid changing the line numbers + /// of the changes that are below the current change. + /// This is important when the change is more lines than the original line range. /// [TestMethod] public void ApplyFixSuggestion_WhenMoreThanOneFixes_ApplyThemFromBottomToTop() { - issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(new SnapshotSpan()); + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => CreateMockedSnapshotSpan(string.Empty)); MockOpenFile(); ChangesDto[] changes = [CreateChangesDto(1, 1, "var a=1;"), CreateChangesDto(3, 3, "var b=0;")]; var suggestionParams = CreateFixSuggestionParams(changes: changes); + MockChangeAccepted(); testSubject.ApplyFixSuggestion(suggestionParams); @@ -164,17 +195,17 @@ public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront() ideWindowService.Received().BringToFront(); } - [TestMethod] public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToFirstChangedLines() { - issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(new SnapshotSpan()); + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Returns(_ => CreateMockedSnapshotSpan(string.Empty)); ChangesDto[] changes = [CreateChangesDto(1, 1, "var a=1;"), CreateChangesDto(3, 3, "var a=1;")]; var suggestionParams = CreateFixSuggestionParams(changes: changes); var firstSuggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; var firstAffectedSnapshot = MockCalculateSpan(firstSuggestedChange); var textView = MockOpenFile(); MockConfigScopeRoot(); + MockChangeAccepted(); testSubject.ApplyFixSuggestion(suggestionParams); @@ -290,25 +321,68 @@ public void ApplyFixSuggestion_TwoChanges_OneIssueCanNotBeLocated_ShowsNotificat VerifyFixSuggestionNotApplied(edit); } - private void MockConfigScopeRoot() + [TestMethod] + public void ApplyFixSuggestion_TwoChanges_ShowsCorrectDiffView() { + var suggestionWithTwoChanges = CreateFixSuggestionParams(changes: [CreateChangesDto(1, 1, "int a=1;", "var a=1;"), CreateChangesDto(2, 2, "int b=0;", "var b=0;")]); + MockCalculateSpan(suggestionWithTwoChanges.fixSuggestion.fileEdit.changes[0]); + MockCalculateSpan(suggestionWithTwoChanges.fixSuggestion.fileEdit.changes[1]); + var textView = MockOpenFile(); + var edit = MockTextEdit(textView); + MockChangeAccepted(); + + testSubject.ApplyFixSuggestion(suggestionWithTwoChanges); + + Received.InOrder(() => + { + diffViewService.ShowDiffView(Arg.Is(x => IsExpectedFixSuggestionDetails(x, 1, 2)), + Arg.Any(), // difficult to mock the GetText of the SnapshotSpan + Arg.Is(x => IsExpectedChangeModel(x, "var b=0;", edit.Snapshot.ContentType))); + diffViewService.ShowDiffView(Arg.Is(x => IsExpectedFixSuggestionDetails(x, 2, 2)), + Arg.Any(), + Arg.Is(x => IsExpectedChangeModel(x, "var a=1;", edit.Snapshot.ContentType))); + }); + } + + [TestMethod] + public void ApplyFixSuggestion_TwoChangesAndJustOneAccepted_ReplacesJustOne() + { + var suggestionWithTwoChanges = CreateFixSuggestionParams(changes: [CreateChangesDto(1, 1, "int a=1;", "var a=1;"), CreateChangesDto(2, 2, "int b=0;", "var b=0;")]); + MockCalculateSpan(suggestionWithTwoChanges.fixSuggestion.fileEdit.changes[0]); + MockCalculateSpan(suggestionWithTwoChanges.fixSuggestion.fileEdit.changes[1]); + var textView = MockOpenFile(); + var edit = MockTextEdit(textView); + MockDiffView(1, "var b=0;", true); + MockDiffView(2, "var a=1;", false); + + testSubject.ApplyFixSuggestion(suggestionWithTwoChanges); + + edit.Received(1).Replace(Arg.Any(), "var b=0;"); + edit.DidNotReceive().Replace(Arg.Any(), "var a=1;"); + } + + private bool IsExpectedFixSuggestionDetails(FixSuggestionDetails fixSuggestionDetails, int expectedIndex, int expectedTotal) => + fixSuggestionDetails.ChangeIndex == expectedIndex && + fixSuggestionDetails.TotalChangesFixes == expectedTotal && + fixSuggestionDetails.FileName == @"C:\myFile.cs"; + + private bool IsExpectedChangeModel(ChangeModel changeModel, string expectedText, IContentType contentType) => changeModel.Text == expectedText && changeModel.ContentType == contentType; + + private void MockConfigScopeRoot() => openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(Arg.Any(), out Arg.Any(), out Arg.Any()).Returns( x => { x[1] = ConfigurationScopeRoot; return true; }); - } - private void MockFailedConfigScopeRoot(string failureReason) - { + private void MockFailedConfigScopeRoot(string failureReason) => openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(Arg.Any(), out Arg.Any(), out Arg.Any()).Returns( x => { x[2] = failureReason; return false; }); - } private ITextView MockOpenFile() { @@ -317,19 +391,24 @@ private ITextView MockOpenFile() return textView; } - private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) - { - return MockCalculateSpan(suggestedChange.before, suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); - } + private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) => MockCalculateSpan(suggestedChange.before, suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); private SnapshotSpan MockCalculateSpan(string text, int startLine, int endLine) { - var affectedSnapshot = new SnapshotSpan(); + var affectedSnapshot = CreateMockedSnapshotSpan(text); issueSpanCalculator.CalculateSpan(Arg.Any(), startLine, endLine).Returns(affectedSnapshot); issueSpanCalculator.IsSameHash(affectedSnapshot, text).Returns(true); return affectedSnapshot; } + private static SnapshotSpan CreateMockedSnapshotSpan(string text) + { + var mockTextSnapshot = Substitute.For(); + mockTextSnapshot.Length.Returns(text.Length + 9999); + + return new SnapshotSpan(mockTextSnapshot, new Span(0, text.Length)); + } + private ITextEdit FailWhenApplyingEdit(string reason = "") { var edit = Substitute.For(); @@ -340,7 +419,8 @@ private ITextEdit FailWhenApplyingEdit(string reason = "") return edit; } - private static ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId", + private static ShowFixSuggestionParams CreateFixSuggestionParams( + string scopeId = "scopeId", string suggestionKey = "suggestionKey", string idePath = @"myFile.cs", params ChangesDto[] changes) @@ -350,13 +430,14 @@ private static ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId return suggestionParams; } - private static ChangesDto CreateChangesDto(int startLine, int endLine, string before) - { - return new ChangesDto(new LineRangeDto(startLine, endLine), before, ""); - } + private static ChangesDto CreateChangesDto( + int startLine, + int endLine, + string before, + string after = "") => + new(new LineRangeDto(startLine, endLine), before, after); - private static string GetAbsolutePathOfFile(ShowFixSuggestionParams suggestionParams) => - Path.Combine(ConfigurationScopeRoot, suggestionParams.fixSuggestion.fileEdit.idePath); + private static string GetAbsolutePathOfFile(ShowFixSuggestionParams suggestionParams) => Path.Combine(ConfigurationScopeRoot, suggestionParams.fixSuggestion.fileEdit.idePath); private ITextEdit MockTextEdit(ITextView textView = null) { @@ -375,4 +456,13 @@ private void VerifyFixSuggestionNotApplied(ITextEdit edit) }); edit.DidNotReceive().Apply(); } + + private void MockChangeAccepted() => MockDiffView(true); + + private void MockChangeNotAccepted() => MockDiffView(false); + + private void MockDiffView(bool accepted) => diffViewService.ShowDiffView(Arg.Any(), Arg.Any(), Arg.Any()).Returns(accepted); + + private void MockDiffView(int index, string after, bool accepted) => + diffViewService.ShowDiffView(Arg.Is(x => x.ChangeIndex == index), Arg.Any(), Arg.Is(x => x.Text == after)).Returns(accepted); } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 079acb2768..a628e41501 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -25,6 +25,7 @@ using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.IssueVisualization.Editor; +using SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; using SonarLint.VisualStudio.IssueVisualization.OpenInIde; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; @@ -38,6 +39,7 @@ public class FixSuggestionHandler : IFixSuggestionHandler private readonly IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; private readonly IIDEWindowService ideWindowService; private readonly IFixSuggestionNotification fixSuggestionNotification; + private readonly IDiffViewService diffViewService; private readonly IThreadHandling threadHandling; private readonly ILogger logger; private readonly IDocumentNavigator documentNavigator; @@ -50,7 +52,8 @@ internal FixSuggestionHandler( IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, IIDEWindowService ideWindowService, - IFixSuggestionNotification fixSuggestionNotification) : + IFixSuggestionNotification fixSuggestionNotification, + IDiffViewService diffViewService) : this( ThreadHandling.Instance, logger, @@ -58,8 +61,10 @@ internal FixSuggestionHandler( issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService, - fixSuggestionNotification) + fixSuggestionNotification, + diffViewService) { + this.diffViewService = diffViewService; } internal FixSuggestionHandler( @@ -69,7 +74,8 @@ internal FixSuggestionHandler( IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, IIDEWindowService ideWindowService, - IFixSuggestionNotification fixSuggestionNotification) + IFixSuggestionNotification fixSuggestionNotification, + IDiffViewService diffViewService) { this.threadHandling = threadHandling; this.logger = logger; @@ -78,6 +84,7 @@ internal FixSuggestionHandler( this.openInIdeConfigScopeValidator = openInIdeConfigScopeValidator; this.ideWindowService = ideWindowService; this.fixSuggestionNotification = fixSuggestionNotification; + this.diffViewService = diffViewService; } public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) @@ -140,7 +147,15 @@ private void ApplySuggestedChanges(ITextView textView, List changes, textView.Caret.MoveTo(spanToUpdate.Value.Start); textView.ViewScroller.EnsureSpanVisible(spanToUpdate.Value, EnsureSpanVisibleOptions.AlwaysCenter); } - textEdit.Replace(spanToUpdate.Value, changeDto.after); + + var suggestionDetails = new FixSuggestionDetails(changes.Count - i, changes.Count, filePath); + var applied = diffViewService.ShowDiffView(suggestionDetails, + new ChangeModel(spanToUpdate.Value.GetText(), textEdit.Snapshot.ContentType), + new ChangeModel(changeDto.after, textEdit.Snapshot.ContentType)); + if (applied) + { + textEdit.Replace(spanToUpdate.Value, changeDto.after); + } } textEdit.Apply(); } From 49e7e7f8f6d42f82a2764c7dc26688361b6162e4 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 14:04:37 +0100 Subject: [PATCH 4/6] SLVS-1745 Fix silent exception called by edit.Cancel after applying a suggetsion by using the dispose method instead --- .../FixSuggestion/FixSuggestionHandlerTests.cs | 6 +++--- src/IssueViz/FixSuggestion/FixSuggestionHandler.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index cade8f6945..53b3790ae6 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -255,14 +255,14 @@ public void ApplyFixSuggestion_WhenLineNumbersDoNotMatch_ShouldLogFailure() } [TestMethod] - public void ApplyFixSuggestion_WhenApplyingChangeAndExceptionIsThrown_ShouldCancelEdit() + public void ApplyFixSuggestion_WhenApplyingChangeAndExceptionIsThrown_ShouldDisposeEdit() { var edit = FailWhenApplyingEdit(); testSubject.ApplyFixSuggestion(suggestionWithOneChange); edit.DidNotReceiveWithAnyArgs().Replace(default, default); - edit.Received().Cancel(); + edit.Received().Dispose(); } [TestMethod] @@ -452,7 +452,7 @@ private void VerifyFixSuggestionNotApplied(ITextEdit edit) Received.InOrder(() => { fixSuggestionNotification.UnableToLocateIssue(Arg.Is(msg => msg == GetAbsolutePathOfFile(suggestionWithOneChange))); - edit.Cancel(); + edit.Dispose(); }); edit.DidNotReceive().Apply(); } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index a628e41501..ad5fea9467 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -161,7 +161,7 @@ private void ApplySuggestedChanges(ITextView textView, List changes, } finally { - textEdit.Cancel(); + textEdit.Dispose(); } } From 5f6200e0ad710828a89c70a6ef7b8d12bb0baec2 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 14:06:37 +0100 Subject: [PATCH 5/6] SLVS-1745 Remove not needed initialization --- src/IssueViz/FixSuggestion/FixSuggestionHandler.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index ad5fea9467..b9774cc400 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -64,7 +64,6 @@ internal FixSuggestionHandler( fixSuggestionNotification, diffViewService) { - this.diffViewService = diffViewService; } internal FixSuggestionHandler( @@ -111,10 +110,8 @@ public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) } } - private bool ValidateConfiguration(string configurationScopeId, out string configurationScopeRoot, out string failureReason) - { - return openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(configurationScopeId, out configurationScopeRoot, out failureReason); - } + private bool ValidateConfiguration(string configurationScopeId, out string configurationScopeRoot, out string failureReason) => + openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(configurationScopeId, out configurationScopeRoot, out failureReason); private void ApplyAndShowAppliedFixSuggestions(ShowFixSuggestionParams parameters, string configurationScopeRoot) { From 060fd193c4879d7c5669a8a2bbc2bea19994ba56 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Thu, 23 Jan 2025 15:56:49 +0100 Subject: [PATCH 6/6] SLVS-1745 Move model into its own file --- .../FixSuggestion/DiffView/ChangeModel.cs | 27 +++++++++++++++++++ .../DiffView/FixSuggestionDetails.cs | 6 ----- 2 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 src/IssueViz/FixSuggestion/DiffView/ChangeModel.cs diff --git a/src/IssueViz/FixSuggestion/DiffView/ChangeModel.cs b/src/IssueViz/FixSuggestion/DiffView/ChangeModel.cs new file mode 100644 index 0000000000..41be0456e3 --- /dev/null +++ b/src/IssueViz/FixSuggestion/DiffView/ChangeModel.cs @@ -0,0 +1,27 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 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 Microsoft.VisualStudio.Utilities; + +namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; + +public record ChangeModel(string Text, IContentType ContentType) +{ +} diff --git a/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs b/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs index 6b9ae232cd..9ab2b9bbfe 100644 --- a/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs +++ b/src/IssueViz/FixSuggestion/DiffView/FixSuggestionDetails.cs @@ -18,14 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.VisualStudio.Utilities; - namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion.DiffView; public record FixSuggestionDetails(int ChangeIndex, int TotalChangesFixes, string FileName) { } - -public record ChangeModel(string Text, IContentType ContentType) -{ -}