From 2f463d064638f8beb8319e4cf14c07fe818c9e22 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 2 Oct 2024 17:11:59 +0200 Subject: [PATCH 01/11] SLVS-1483 Add parameter that contains the info to apply a fix suggestion --- .../FixSuggestion/FixSuggestionHandlerTests.cs | 2 +- src/IssueViz/FixSuggestion/FixSuggestionHandler.cs | 3 ++- src/IssueViz/FixSuggestion/IFixSuggestionHandler.cs | 4 +++- .../Implementation/ShowFixSuggestionListenerTests.cs | 2 +- .../Implementation/ShowFixSuggestionListener.cs | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 1f45b4abc..3cae7042f 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -37,6 +37,6 @@ public void ApplyFixSuggestion_ThrowsNotImplementedException() { var testSubject = new FixSuggestionHandler(); - Exceptions.Expect(() => testSubject.ApplyFixSuggestion()); + Exceptions.Expect(() => testSubject.ApplyFixSuggestion(null)); } } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 7a5553398..d3cd25ce0 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -19,6 +19,7 @@ */ using System.ComponentModel.Composition; +using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; @@ -26,7 +27,7 @@ namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; [PartCreationPolicy(CreationPolicy.Shared)] public class FixSuggestionHandler : IFixSuggestionHandler { - public void ApplyFixSuggestion() + public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) { throw new NotImplementedException(); } diff --git a/src/IssueViz/FixSuggestion/IFixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/IFixSuggestionHandler.cs index 68ff84548..1c332843d 100644 --- a/src/IssueViz/FixSuggestion/IFixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/IFixSuggestionHandler.cs @@ -18,9 +18,11 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; + namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; public interface IFixSuggestionHandler { - void ApplyFixSuggestion(); + void ApplyFixSuggestion(ShowFixSuggestionParams parameters); } diff --git a/src/SLCore.Listeners.UnitTests/Implementation/ShowFixSuggestionListenerTests.cs b/src/SLCore.Listeners.UnitTests/Implementation/ShowFixSuggestionListenerTests.cs index d730ffb6f..23b8c5ecc 100644 --- a/src/SLCore.Listeners.UnitTests/Implementation/ShowFixSuggestionListenerTests.cs +++ b/src/SLCore.Listeners.UnitTests/Implementation/ShowFixSuggestionListenerTests.cs @@ -65,6 +65,6 @@ public void ShowFixSuggestion_CallsHandler() testSubject.ShowFixSuggestion(parameters); - fixSuggestionHandler.Received(1).ApplyFixSuggestion(); + fixSuggestionHandler.Received(1).ApplyFixSuggestion(parameters); } } diff --git a/src/SLCore.Listeners/Implementation/ShowFixSuggestionListener.cs b/src/SLCore.Listeners/Implementation/ShowFixSuggestionListener.cs index 09c75a151..0a04d3ad1 100644 --- a/src/SLCore.Listeners/Implementation/ShowFixSuggestionListener.cs +++ b/src/SLCore.Listeners/Implementation/ShowFixSuggestionListener.cs @@ -39,6 +39,6 @@ public ShowFixSuggestionListener(IFixSuggestionHandler fixSuggestionHandler) public void ShowFixSuggestion(ShowFixSuggestionParams parameters) { - fixSuggestionHandler.ApplyFixSuggestion(); + fixSuggestionHandler.ApplyFixSuggestion(parameters); } } From 3679763387d3eb5f86d69c106ec56c6efb680819 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 2 Oct 2024 17:53:44 +0200 Subject: [PATCH 02/11] SLVS-1483 Add functionality to calculate the span for a given line range --- .../Editor/IssueSpanCalculatorTests.cs | 55 ++++++++++++++++++- src/IssueViz/Editor/IIssueSpanCalculator.cs | 11 ++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs b/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs index 8a0b32472..973c9f3ab 100644 --- a/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs +++ b/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using FluentAssertions; -using Microsoft.VisualStudio.TestTools.UnitTesting; using Microsoft.VisualStudio.Text; using Moq; using SonarLint.VisualStudio.TestInfrastructure; @@ -34,6 +32,7 @@ public class IssueSpanCalculatorTests private Mock checksumCalculatorMock; private IssueSpanCalculator testSubject; + private const int SnapshotLength = 10000; [TestInitialize] public void TestInitialize() @@ -331,6 +330,42 @@ public void CalculateSpan_TextRangeNull_ReturnsNull() checksumCalculatorMock.VerifyNoOtherCalls(); } + [TestMethod] + public void CalculateSpan_ForStartAndEndLines_GetsPositionOfCorrectLines() + { + var startLine = CreateLineMock(lineNumber: 66, startPos: 1, endPos: 2); + var endLine = CreateLineMock(lineNumber: 224, startPos: 13, endPos: 23); + var textSnapshotMock = MockTextSnapshotForLines(startLine, endLine); + + testSubject.CalculateSpan(textSnapshotMock.Object, startLine.LineNumber, endLine.LineNumber); + + textSnapshotMock.Verify(mock => mock.GetLineFromLineNumber(startLine.LineNumber - 1), Times.Once); + textSnapshotMock.Verify(mock => mock.GetLineFromLineNumber(endLine.LineNumber - 1), Times.Once); + } + + [TestMethod] + public void CalculateSpan_ForStartAndEndLines_ReturnsSnapshotSpanWithCorrectStartAndEnd() + { + var startLine = CreateLineMock(lineNumber:66, startPos:1, endPos:2); + var endLine = CreateLineMock(lineNumber:224, startPos:13, endPos:23); + var textSnapshotMock = MockTextSnapshotForLines(startLine, endLine); + + var snapshotSpan = testSubject.CalculateSpan(textSnapshotMock.Object, startLine.LineNumber, endLine.LineNumber); + + snapshotSpan.Start.Position.Should().Be(startLine.Start.Position); + snapshotSpan.End.Position.Should().Be(endLine.End.Position); + } + + private static Mock MockTextSnapshotForLines(ITextSnapshotLine startLine, ITextSnapshotLine endLine) + { + var textSnapshot = new Mock(); + textSnapshot.SetupGet(x => x.Length).Returns(SnapshotLength); + textSnapshot.Setup(x => x.GetLineFromLineNumber(startLine.LineNumber - 1)).Returns(startLine); + textSnapshot.Setup(x => x.GetLineFromLineNumber(endLine.LineNumber - 1)).Returns(endLine); + + return textSnapshot; + } + private class VSLineDescription { public int ZeroBasedLineNumber { get; set; } @@ -339,7 +374,7 @@ private class VSLineDescription public string Text { get; set; } } - private static Mock CreateSnapshotMock(int bufferLineCount = 1000, int snapShotLength = 10000, params VSLineDescription[] lines) + private static Mock CreateSnapshotMock(int bufferLineCount = 1000, int snapShotLength = SnapshotLength, params VSLineDescription[] lines) { var textSnapshotMock = new Mock(); @@ -378,5 +413,19 @@ private static ITextSnapshotLine CreateLineMock(ITextSnapshot textSnapshot, VSLi return startLineMock.Object; } + + private static ITextSnapshotLine CreateLineMock(int lineNumber, int startPos, int endPos) + { + var startLineMock = new Mock(); + var textSnapshot = new Mock(); + + textSnapshot.SetupGet(x => x.Length).Returns(() => endPos +1); + + startLineMock.SetupGet(x => x.LineNumber).Returns(() => lineNumber); + startLineMock.SetupGet(x => x.Start).Returns(() => new SnapshotPoint(textSnapshot.Object, startPos)); + startLineMock.SetupGet(x => x.End).Returns(() => new SnapshotPoint(textSnapshot.Object, endPos)); + + return startLineMock.Object; + } } } diff --git a/src/IssueViz/Editor/IIssueSpanCalculator.cs b/src/IssueViz/Editor/IIssueSpanCalculator.cs index cf1a6d4e6..95c44763a 100644 --- a/src/IssueViz/Editor/IIssueSpanCalculator.cs +++ b/src/IssueViz/Editor/IIssueSpanCalculator.cs @@ -34,6 +34,8 @@ public interface IIssueSpanCalculator /// Returns null if no textRange is passed /// SnapshotSpan? CalculateSpan(ITextRange range, ITextSnapshot currentSnapshot); + + SnapshotSpan CalculateSpan(ITextSnapshot snapshot, int startLine, int endLine); } [Export(typeof(IIssueSpanCalculator))] @@ -105,6 +107,15 @@ internal IssueSpanCalculator(IChecksumCalculator checksumCalculator) return snapshotSpan; } + public SnapshotSpan CalculateSpan(ITextSnapshot snapshot, int startLine, int endLine) + { + // TODO add into consideration all edge cases + var startPosition = snapshot.GetLineFromLineNumber(startLine - 1).Start.Position; + var endPosition = snapshot.GetLineFromLineNumber(endLine - 1).End.Position; + var span = Span.FromBounds(startPosition, endPosition); + return new SnapshotSpan(snapshot, span); + } + private static bool RangeHasHash(ITextRange range) => !string.IsNullOrEmpty(range.LineHash); From afd98a43ba4772f044d5534024336ff4e3c3ce7f Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 2 Oct 2024 17:55:27 +0200 Subject: [PATCH 03/11] SLVS-1483 Add basic implementation for directly applying a fix suggestion --- .../FixSuggestionHandlerTests.cs | 105 +++++++++++++- .../FixSuggestion/FixSuggestionHandler.cs | 71 +++++++++- .../FixSuggestionResources.Designer.cs | 90 ++++++++++++ .../FixSuggestion/FixSuggestionResources.resx | 129 ++++++++++++++++++ src/IssueViz/IssueViz.csproj | 11 ++ 5 files changed, 402 insertions(+), 4 deletions(-) create mode 100644 src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs create mode 100644 src/IssueViz/FixSuggestion/FixSuggestionResources.resx diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 3cae7042f..b2df6c20b 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -18,14 +18,43 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.VisualStudio.Text; +using Microsoft.VisualStudio.Text.Editor; +using NSubstitute.ExceptionExtensions; +using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.Infrastructure.VS; +using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.FixSuggestion; +using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; +using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; using SonarLint.VisualStudio.TestInfrastructure; +using FileEditDto = SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models.FileEditDto; namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.FixSuggestion; [TestClass] public class FixSuggestionHandlerTests { + private readonly FixSuggestionDto suggestionDto = new("id", "refactor", new FileEditDto(@"C:\myFile.cs", [new ChangesDto(new LineRangeDto(1, 2), "var a=1;", "")])); + private FixSuggestionHandler testSubject; + private IThreadHandling threadHandling; + private ILogger logger; + private IDocumentNavigator documentNavigator; + private ISpanTranslator spanTranslator; + private IIssueSpanCalculator issueSpanCalculator; + + [TestInitialize] + public void TestInitialize() + { + threadHandling = new NoOpThreadHandler(); + logger = Substitute.For(); + documentNavigator = Substitute.For(); + spanTranslator = Substitute.For(); + issueSpanCalculator = Substitute.For(); + + testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator); + } + [TestMethod] public void MefCtor_CheckIsSingleton() { @@ -33,10 +62,80 @@ public void MefCtor_CheckIsSingleton() } [TestMethod] - public void ApplyFixSuggestion_ThrowsNotImplementedException() + public void ApplyFixSuggestion_RunsOnUIThread() + { + var threadHandlingMock = Substitute.For(); + var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator); + + fixSuggestionHandler.ApplyFixSuggestion(CreateFixSuggestionParams()); + + threadHandlingMock.ReceivedWithAnyArgs().RunOnUIThread(default); + } + + [TestMethod] + public void ApplyFixSuggestion_OneChange_AppliesChange() + { + var suggestionParams = CreateFixSuggestionParams(); + var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; + var affectedSnapshot = MockCalculateSpan(suggestedChange); + var edit = Substitute.For(); + var textView = MockTextView(edit); + MockOpenDocument(suggestionParams, textView); + + testSubject.ApplyFixSuggestion(suggestionParams); + + Received.InOrder(() => + { + logger.WriteLine(FixSuggestionResources.ProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); + documentNavigator.Open(suggestionDto.fileEdit.idePath); + textView.TextBuffer.CreateEdit(); + issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); + spanTranslator.TranslateTo(affectedSnapshot, Arg.Any(), SpanTrackingMode.EdgeExclusive); + edit.Replace(Arg.Any(), suggestedChange.after); + edit.Apply(); + logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); + }); + } + + [TestMethod] + public void ApplyFixSuggestion_Throws_Logs() { - var testSubject = new FixSuggestionHandler(); + var suggestionParams = CreateFixSuggestionParams(); + var exceptionMsg = "error"; + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Throws(new Exception(exceptionMsg)); - Exceptions.Expect(() => testSubject.ApplyFixSuggestion(null)); + testSubject.ApplyFixSuggestion(suggestionParams); + + Received.InOrder(() => + { + logger.WriteLine(FixSuggestionResources.ProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); + logger.WriteLine(FixSuggestionResources.ProcessingRequestFailed, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId, exceptionMsg); + }); + logger.DidNotReceive().WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); + } + + private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) + { + var affectedSnapshot = new SnapshotSpan(); + issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine) + .Returns(affectedSnapshot); + return affectedSnapshot; + } + + private void MockOpenDocument(ShowFixSuggestionParams suggestionParams, ITextView textView) + { + documentNavigator.Open(suggestionParams.fixSuggestion.fileEdit.idePath).Returns(textView); + } + + private static ITextView MockTextView(ITextEdit edit) + { + var textView = Substitute.For(); + textView.TextBuffer.CreateEdit().Returns(edit); + return textView; + } + + private ShowFixSuggestionParams CreateFixSuggestionParams() + { + return new ShowFixSuggestionParams("scopeId", "key", suggestionDto); } } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index d3cd25ce0..1d3f99073 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -19,7 +19,13 @@ */ using System.ComponentModel.Composition; +using Microsoft.VisualStudio.Text; +using Microsoft.VisualStudio.Text.Editor; +using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.Infrastructure.VS; +using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; +using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; @@ -27,8 +33,71 @@ namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; [PartCreationPolicy(CreationPolicy.Shared)] public class FixSuggestionHandler : IFixSuggestionHandler { + private readonly IThreadHandling threadHandling; + private readonly ILogger logger; + private readonly IDocumentNavigator documentNavigator; + private readonly ISpanTranslator spanTranslator; + private readonly IIssueSpanCalculator issueSpanCalculator; + + [ImportingConstructor] + internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator) : + this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator) + { + } + + internal FixSuggestionHandler( + IThreadHandling threadHandling, + ILogger logger, + IDocumentNavigator documentNavigator, + ISpanTranslator spanTranslator, + IIssueSpanCalculator issueSpanCalculator) + { + this.threadHandling = threadHandling; + this.logger = logger; + this.documentNavigator = documentNavigator; + this.spanTranslator = spanTranslator; + this.issueSpanCalculator = issueSpanCalculator; + } + public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) { - throw new NotImplementedException(); + threadHandling.RunOnUIThread(() => + { + ApplyFixSuggestionInternal(parameters); + }); + } + + private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) + { + try + { + logger.WriteLine(FixSuggestionResources.ProcessingRequest, parameters.configurationScopeId, parameters.fixSuggestion.suggestionId); + + var textView = documentNavigator.Open(parameters.fixSuggestion.fileEdit.idePath); + ApplySuggestedChanges(textView, parameters.fixSuggestion); + + logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, parameters.configurationScopeId, parameters.fixSuggestion.suggestionId); + } + catch (Exception exception) when (!ErrorHandler.IsCriticalException(exception)) + { + logger.WriteLine(FixSuggestionResources.ProcessingRequestFailed, parameters.configurationScopeId, parameters.fixSuggestion.suggestionId, exception.Message); + } + } + + private void ApplySuggestedChanges(ITextView textView, FixSuggestionDto fixSuggestion) + { + var textEdit = textView.TextBuffer.CreateEdit(); + foreach (var changeDto in fixSuggestion.fileEdit.changes) + { + var updatedSpan = CalculateSnapshotSpan(textView, changeDto); + textEdit.Replace(updatedSpan, changeDto.after); + } + textEdit.Apply(); + } + + private SnapshotSpan CalculateSnapshotSpan(ITextView textView, ChangesDto changeDto) + { + var snapshotSpan = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); + return spanTranslator.TranslateTo(snapshotSpan, textView.TextBuffer.CurrentSnapshot, SpanTrackingMode.EdgeExclusive); } } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs b/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs new file mode 100644 index 000000000..8724e01fc --- /dev/null +++ b/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs @@ -0,0 +1,90 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion { + using System; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class FixSuggestionResources { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal FixSuggestionResources() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("SonarLint.VisualStudio.IssueVisualization.FixSuggestion.FixSuggestionResources", typeof(FixSuggestionResources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to [Fix Suggestion in IDE] Done processing request. Configuration scope: {0}, SuggestionId: {1}. + /// + internal static string DoneProcessingRequest { + get { + return ResourceManager.GetString("DoneProcessingRequest", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to [Fix Suggestion in IDE] Processing request. Configuration scope: {0}, SuggestionId: {1}. + /// + internal static string ProcessingRequest { + get { + return ResourceManager.GetString("ProcessingRequest", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to [Fix Suggestion in IDE] Fail to process request for configuration scope: {0}, SuggestionId: {1} due to {2}.. + /// + internal static string ProcessingRequestFailed { + get { + return ResourceManager.GetString("ProcessingRequestFailed", resourceCulture); + } + } + } +} diff --git a/src/IssueViz/FixSuggestion/FixSuggestionResources.resx b/src/IssueViz/FixSuggestion/FixSuggestionResources.resx new file mode 100644 index 000000000..b6828d985 --- /dev/null +++ b/src/IssueViz/FixSuggestion/FixSuggestionResources.resx @@ -0,0 +1,129 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + [Fix Suggestion in IDE] Done processing request. Configuration scope: {0}, SuggestionId: {1} + + + [Fix Suggestion in IDE] Processing request. Configuration scope: {0}, SuggestionId: {1} + + + [Fix Suggestion in IDE] Fail to process request for configuration scope: {0}, SuggestionId: {1} due to {2}. + + \ No newline at end of file diff --git a/src/IssueViz/IssueViz.csproj b/src/IssueViz/IssueViz.csproj index 2a27e1cca..5a20da8e8 100644 --- a/src/IssueViz/IssueViz.csproj +++ b/src/IssueViz/IssueViz.csproj @@ -53,12 +53,23 @@ Never + + True + True + FixSuggestionResources.resx + + True True Resources.resx + + ResXFileCodeGenerator + FixSuggestionResources.Designer.cs + + ResXFileCodeGenerator Resources.Designer.cs From a8b68d04fdd077d2230358b3926e2376334bfd4d Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Thu, 3 Oct 2024 15:50:28 +0200 Subject: [PATCH 04/11] SLVS-1483 Calculate the file's absolute path based on the config scope --- .../FixSuggestionHandlerTests.cs | 70 ++++++++++++++----- .../FixSuggestion/FixSuggestionHandler.cs | 36 +++++++--- .../FixSuggestionResources.Designer.cs | 10 ++- .../FixSuggestion/FixSuggestionResources.resx | 3 + 4 files changed, 90 insertions(+), 29 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index b2df6c20b..60ef89402 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -25,6 +25,7 @@ using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.FixSuggestion; +using SonarLint.VisualStudio.IssueVisualization.OpenInIde; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; using SonarLint.VisualStudio.TestInfrastructure; @@ -35,13 +36,17 @@ namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.FixSuggestion; [TestClass] public class FixSuggestionHandlerTests { - private readonly FixSuggestionDto suggestionDto = new("id", "refactor", new FileEditDto(@"C:\myFile.cs", [new ChangesDto(new LineRangeDto(1, 2), "var a=1;", "")])); + private const string ConfigurationScopeRoot = @"C:\"; + + private readonly FixSuggestionDto suggestionDto = new("scopeId", "refactor", new FileEditDto(@"myFile.cs", [new ChangesDto(new LineRangeDto(1, 2), "var a=1;", "")])); + private FixSuggestionHandler testSubject; private IThreadHandling threadHandling; private ILogger logger; private IDocumentNavigator documentNavigator; private ISpanTranslator spanTranslator; private IIssueSpanCalculator issueSpanCalculator; + private IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; [TestInitialize] public void TestInitialize() @@ -51,8 +56,9 @@ public void TestInitialize() documentNavigator = Substitute.For(); spanTranslator = Substitute.For(); issueSpanCalculator = Substitute.For(); + openInIdeConfigScopeValidator = Substitute.For(); - testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator); + testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator); } [TestMethod] @@ -64,8 +70,9 @@ public void MefCtor_CheckIsSingleton() [TestMethod] public void ApplyFixSuggestion_RunsOnUIThread() { + MockConfigScopeRoot(); var threadHandlingMock = Substitute.For(); - var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator); + var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator); fixSuggestionHandler.ApplyFixSuggestion(CreateFixSuggestionParams()); @@ -78,16 +85,17 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() var suggestionParams = CreateFixSuggestionParams(); var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; var affectedSnapshot = MockCalculateSpan(suggestedChange); + var textView = MockOpenFile(); var edit = Substitute.For(); - var textView = MockTextView(edit); - MockOpenDocument(suggestionParams, textView); + textView.TextBuffer.CreateEdit().Returns(edit); + MockConfigScopeRoot(); testSubject.ApplyFixSuggestion(suggestionParams); Received.InOrder(() => { logger.WriteLine(FixSuggestionResources.ProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); - documentNavigator.Open(suggestionDto.fileEdit.idePath); + documentNavigator.Open(@"C:\myFile.cs"); textView.TextBuffer.CreateEdit(); issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); spanTranslator.TranslateTo(affectedSnapshot, Arg.Any(), SpanTrackingMode.EdgeExclusive); @@ -102,6 +110,7 @@ public void ApplyFixSuggestion_Throws_Logs() { var suggestionParams = CreateFixSuggestionParams(); var exceptionMsg = "error"; + MockConfigScopeRoot(); issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()).Throws(new Exception(exceptionMsg)); testSubject.ApplyFixSuggestion(suggestionParams); @@ -114,28 +123,55 @@ public void ApplyFixSuggestion_Throws_Logs() logger.DidNotReceive().WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); } - private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) + [TestMethod] + public void ApplyFixSuggestion_WhenConfigRootScopeNotFound_ShouldLogFailure() { - var affectedSnapshot = new SnapshotSpan(); - issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine) - .Returns(affectedSnapshot); - return affectedSnapshot; + MockFailedConfigScopeRoot("Scope not found"); + var suggestionParams = CreateFixSuggestionParams("SpecificConfigScopeId"); + + testSubject.ApplyFixSuggestion(suggestionParams); + + logger.Received().WriteLine(FixSuggestionResources.GetConfigScopeRootPathFailed, "SpecificConfigScopeId", "Scope not found"); } - private void MockOpenDocument(ShowFixSuggestionParams suggestionParams, ITextView textView) + 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) { - documentNavigator.Open(suggestionParams.fixSuggestion.fileEdit.idePath).Returns(textView); + openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(Arg.Any(), out Arg.Any(), out Arg.Any()).Returns( + x => + { + x[2] = failureReason; + return false; + }); } - private static ITextView MockTextView(ITextEdit edit) + private ITextView MockOpenFile() { var textView = Substitute.For(); - textView.TextBuffer.CreateEdit().Returns(edit); + documentNavigator.Open(Arg.Any()).Returns(textView); return textView; } - private ShowFixSuggestionParams CreateFixSuggestionParams() + private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) + { + var affectedSnapshot = new SnapshotSpan(); + issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine) + .Returns(affectedSnapshot); + return affectedSnapshot; + } + + private ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId") { - return new ShowFixSuggestionParams("scopeId", "key", suggestionDto); + return new ShowFixSuggestionParams(scopeId, "key", suggestionDto); } } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 1d3f99073..154d9c162 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -19,11 +19,13 @@ */ using System.ComponentModel.Composition; +using System.IO; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.IssueVisualization.Editor; +using SonarLint.VisualStudio.IssueVisualization.OpenInIde; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion; using SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models; @@ -33,6 +35,7 @@ namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; [PartCreationPolicy(CreationPolicy.Shared)] public class FixSuggestionHandler : IFixSuggestionHandler { + private readonly IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; private readonly IThreadHandling threadHandling; private readonly ILogger logger; private readonly IDocumentNavigator documentNavigator; @@ -40,8 +43,8 @@ public class FixSuggestionHandler : IFixSuggestionHandler private readonly IIssueSpanCalculator issueSpanCalculator; [ImportingConstructor] - internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator) : - this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator) + internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator) : + this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator, openInIdeConfigScopeValidator) { } @@ -50,31 +53,41 @@ internal FixSuggestionHandler( ILogger logger, IDocumentNavigator documentNavigator, ISpanTranslator spanTranslator, - IIssueSpanCalculator issueSpanCalculator) + IIssueSpanCalculator issueSpanCalculator, + IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator) { this.threadHandling = threadHandling; this.logger = logger; this.documentNavigator = documentNavigator; this.spanTranslator = spanTranslator; this.issueSpanCalculator = issueSpanCalculator; + this.openInIdeConfigScopeValidator = openInIdeConfigScopeValidator; } public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) { - threadHandling.RunOnUIThread(() => - { - ApplyFixSuggestionInternal(parameters); - }); + ApplyFixSuggestionInternal(parameters); } + private bool ValidateConfiguration(string configurationScopeId, out string configurationScopeRoot, out string failureReason) + { + return openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(configurationScopeId, out configurationScopeRoot, out failureReason); + } + private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) { + if (!ValidateConfiguration(parameters.configurationScopeId, out var configurationScopeRoot, out var failureReason)) + { + logger.WriteLine(FixSuggestionResources.GetConfigScopeRootPathFailed, parameters.configurationScopeId, failureReason); + return; + } + try { logger.WriteLine(FixSuggestionResources.ProcessingRequest, parameters.configurationScopeId, parameters.fixSuggestion.suggestionId); - var textView = documentNavigator.Open(parameters.fixSuggestion.fileEdit.idePath); - ApplySuggestedChanges(textView, parameters.fixSuggestion); + var absoluteFilePath = Path.Combine(configurationScopeRoot, parameters.fixSuggestion.fileEdit.idePath); + threadHandling.RunOnUIThread(() => ApplySuggestedChanges(absoluteFilePath, parameters.fixSuggestion.fileEdit.changes)); logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, parameters.configurationScopeId, parameters.fixSuggestion.suggestionId); } @@ -84,10 +97,11 @@ private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) } } - private void ApplySuggestedChanges(ITextView textView, FixSuggestionDto fixSuggestion) + private void ApplySuggestedChanges(string absoluteFilePath, List changes) { + var textView = documentNavigator.Open(absoluteFilePath); var textEdit = textView.TextBuffer.CreateEdit(); - foreach (var changeDto in fixSuggestion.fileEdit.changes) + foreach (var changeDto in changes) { var updatedSpan = CalculateSnapshotSpan(textView, changeDto); textEdit.Replace(updatedSpan, changeDto.after); diff --git a/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs b/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs index 8724e01fc..c17207be0 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionResources.Designer.cs @@ -1,7 +1,6 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -69,6 +68,15 @@ internal static string DoneProcessingRequest { } } + /// + /// Looks up a localized string similar to [Fix Suggestion in IDE] Could not determine configuration scope root path for scope [{0}] due to: {1}. + /// + internal static string GetConfigScopeRootPathFailed { + get { + return ResourceManager.GetString("GetConfigScopeRootPathFailed", resourceCulture); + } + } + /// /// Looks up a localized string similar to [Fix Suggestion in IDE] Processing request. Configuration scope: {0}, SuggestionId: {1}. /// diff --git a/src/IssueViz/FixSuggestion/FixSuggestionResources.resx b/src/IssueViz/FixSuggestion/FixSuggestionResources.resx index b6828d985..0d2089283 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionResources.resx +++ b/src/IssueViz/FixSuggestion/FixSuggestionResources.resx @@ -126,4 +126,7 @@ [Fix Suggestion in IDE] Fail to process request for configuration scope: {0}, SuggestionId: {1} due to {2}. + + [Fix Suggestion in IDE] Could not determine configuration scope root path for scope [{0}] due to: {1} + \ No newline at end of file From 49349586fd03f018afb7cf34661f91da7e7d5fe3 Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Thu, 3 Oct 2024 16:12:54 +0200 Subject: [PATCH 05/11] SLVS-1483 When applying change bring focus to changed line --- .../FixSuggestion/FixSuggestionHandlerTests.cs | 14 ++++++++++++++ src/IssueViz/FixSuggestion/FixSuggestionHandler.cs | 10 ++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 60ef89402..65e97e2b2 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -105,6 +105,20 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() }); } + [TestMethod] + public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToChangedLines() + { + var suggestionParams = CreateFixSuggestionParams(); + var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; + var affectedSnapshot = MockCalculateSpan(suggestedChange); + var textView = MockOpenFile(); + MockConfigScopeRoot(); + + testSubject.ApplyFixSuggestion(suggestionParams); + + textView.ViewScroller.Received().EnsureSpanVisible(affectedSnapshot, EnsureSpanVisibleOptions.AlwaysCenter); + } + [TestMethod] public void ApplyFixSuggestion_Throws_Logs() { diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 154d9c162..fc2c32abc 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -103,15 +103,17 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha var textEdit = textView.TextBuffer.CreateEdit(); foreach (var changeDto in changes) { - var updatedSpan = CalculateSnapshotSpan(textView, changeDto); - textEdit.Replace(updatedSpan, changeDto.after); + var spanToUpdate = CalculateSnapshotSpan(textView, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); + textView.Caret.MoveTo(spanToUpdate.Start); + textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); + textEdit.Replace(spanToUpdate, changeDto.after); } textEdit.Apply(); } - private SnapshotSpan CalculateSnapshotSpan(ITextView textView, ChangesDto changeDto) + private SnapshotSpan CalculateSnapshotSpan(ITextView textView, int startLine, int endLine) { - var snapshotSpan = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); + var snapshotSpan = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, startLine, endLine); return spanTranslator.TranslateTo(snapshotSpan, textView.TextBuffer.CurrentSnapshot, SpanTrackingMode.EdgeExclusive); } } From 7f007666258159b4857e98d7c89b96cab522b833 Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Thu, 3 Oct 2024 16:23:36 +0200 Subject: [PATCH 06/11] SLVS-1483 When applying change bring window to front --- .../FixSuggestion/FixSuggestionHandlerTests.cs | 17 +++++++++++++++-- .../FixSuggestion/FixSuggestionHandler.cs | 10 +++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 65e97e2b2..6d965cbef 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -47,6 +47,7 @@ public class FixSuggestionHandlerTests private ISpanTranslator spanTranslator; private IIssueSpanCalculator issueSpanCalculator; private IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; + private IIDEWindowService ideWindowService; [TestInitialize] public void TestInitialize() @@ -57,8 +58,9 @@ public void TestInitialize() spanTranslator = Substitute.For(); issueSpanCalculator = Substitute.For(); openInIdeConfigScopeValidator = Substitute.For(); + ideWindowService = Substitute.For(); - testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator); + testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); } [TestMethod] @@ -72,7 +74,7 @@ public void ApplyFixSuggestion_RunsOnUIThread() { MockConfigScopeRoot(); var threadHandlingMock = Substitute.For(); - var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator); + var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); fixSuggestionHandler.ApplyFixSuggestion(CreateFixSuggestionParams()); @@ -104,6 +106,17 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); }); } + + [TestMethod] + public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront() + { + var suggestionParams = CreateFixSuggestionParams(); + MockConfigScopeRoot(); + + testSubject.ApplyFixSuggestion(suggestionParams); + + ideWindowService.Received().BringToFront(); + } [TestMethod] public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToChangedLines() diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index fc2c32abc..d75fea576 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -36,6 +36,7 @@ namespace SonarLint.VisualStudio.IssueVisualization.FixSuggestion; public class FixSuggestionHandler : IFixSuggestionHandler { private readonly IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; + private readonly IIDEWindowService ideWindowService; private readonly IThreadHandling threadHandling; private readonly ILogger logger; private readonly IDocumentNavigator documentNavigator; @@ -43,8 +44,8 @@ public class FixSuggestionHandler : IFixSuggestionHandler private readonly IIssueSpanCalculator issueSpanCalculator; [ImportingConstructor] - internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator) : - this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator, openInIdeConfigScopeValidator) + internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, IIDEWindowService ideWindowService) : + this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService) { } @@ -54,7 +55,8 @@ internal FixSuggestionHandler( IDocumentNavigator documentNavigator, ISpanTranslator spanTranslator, IIssueSpanCalculator issueSpanCalculator, - IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator) + IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, + IIDEWindowService ideWindowService) { this.threadHandling = threadHandling; this.logger = logger; @@ -62,6 +64,7 @@ internal FixSuggestionHandler( this.spanTranslator = spanTranslator; this.issueSpanCalculator = issueSpanCalculator; this.openInIdeConfigScopeValidator = openInIdeConfigScopeValidator; + this.ideWindowService = ideWindowService; } public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) @@ -99,6 +102,7 @@ private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) private void ApplySuggestedChanges(string absoluteFilePath, List changes) { + ideWindowService.BringToFront(); var textView = documentNavigator.Open(absoluteFilePath); var textEdit = textView.TextBuffer.CreateEdit(); foreach (var changeDto in changes) From dc99c04f232b2d65d72932d72caf475b0d1be2eb Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Thu, 3 Oct 2024 19:58:47 +0200 Subject: [PATCH 07/11] SLVS-1483 Fix CalculateSpan TODO with edge cases --- .../Editor/IssueSpanCalculatorTests.cs | 25 +++++++++++-- .../FixSuggestionHandlerTests.cs | 35 +++++++++++++------ src/IssueViz/Editor/IIssueSpanCalculator.cs | 6 ++-- .../FixSuggestion/FixSuggestionHandler.cs | 14 ++------ 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs b/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs index 973c9f3ab..4cb0c891a 100644 --- a/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs +++ b/src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs @@ -20,8 +20,8 @@ using Microsoft.VisualStudio.Text; using Moq; -using SonarLint.VisualStudio.TestInfrastructure; using SonarLint.VisualStudio.IssueVisualization.Editor; +using SonarLint.VisualStudio.TestInfrastructure; using SonarQube.Client; namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.Editor @@ -33,6 +33,7 @@ public class IssueSpanCalculatorTests private IssueSpanCalculator testSubject; private const int SnapshotLength = 10000; + private const int SnapshotLineCount = 10000; [TestInitialize] public void TestInitialize() @@ -351,15 +352,33 @@ public void CalculateSpan_ForStartAndEndLines_ReturnsSnapshotSpanWithCorrectStar var textSnapshotMock = MockTextSnapshotForLines(startLine, endLine); var snapshotSpan = testSubject.CalculateSpan(textSnapshotMock.Object, startLine.LineNumber, endLine.LineNumber); - + snapshotSpan.Start.Position.Should().Be(startLine.Start.Position); snapshotSpan.End.Position.Should().Be(endLine.End.Position); } + + [TestMethod] + [DataRow(0, 1)] + [DataRow(1, 0)] + [DataRow(SnapshotLineCount + 1, 1)] + [DataRow(1, SnapshotLineCount + 1)] + [DataRow(4, 1)] + public void CalculateSpan_ForInvalidStartAndEndLines_ThrowsException(int startLineNumber, int endLineNumber) + { + var startLine = CreateLineMock(lineNumber:startLineNumber, startPos:1, endPos:2); + var endLine = CreateLineMock(lineNumber:endLineNumber, startPos:13, endPos:23); + var textSnapshotMock = MockTextSnapshotForLines(startLine, endLine); + + Action act = () => testSubject.CalculateSpan(textSnapshotMock.Object, startLine.LineNumber, endLine.LineNumber); + + act.Should().Throw(); + } private static Mock MockTextSnapshotForLines(ITextSnapshotLine startLine, ITextSnapshotLine endLine) { var textSnapshot = new Mock(); - textSnapshot.SetupGet(x => x.Length).Returns(SnapshotLength); + textSnapshot.SetupGet(x => x.LineCount).Returns(SnapshotLineCount); + textSnapshot.SetupGet(x => x.Length).Returns(SnapshotLength); textSnapshot.Setup(x => x.GetLineFromLineNumber(startLine.LineNumber - 1)).Returns(startLine); textSnapshot.Setup(x => x.GetLineFromLineNumber(endLine.LineNumber - 1)).Returns(endLine); diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 6d965cbef..f78a1a3c3 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -22,7 +22,6 @@ using Microsoft.VisualStudio.Text.Editor; using NSubstitute.ExceptionExtensions; using SonarLint.VisualStudio.Core; -using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.FixSuggestion; using SonarLint.VisualStudio.IssueVisualization.OpenInIde; @@ -38,13 +37,10 @@ public class FixSuggestionHandlerTests { private const string ConfigurationScopeRoot = @"C:\"; - private readonly FixSuggestionDto suggestionDto = new("scopeId", "refactor", new FileEditDto(@"myFile.cs", [new ChangesDto(new LineRangeDto(1, 2), "var a=1;", "")])); - private FixSuggestionHandler testSubject; private IThreadHandling threadHandling; private ILogger logger; private IDocumentNavigator documentNavigator; - private ISpanTranslator spanTranslator; private IIssueSpanCalculator issueSpanCalculator; private IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator; private IIDEWindowService ideWindowService; @@ -55,12 +51,11 @@ public void TestInitialize() threadHandling = new NoOpThreadHandler(); logger = Substitute.For(); documentNavigator = Substitute.For(); - spanTranslator = Substitute.For(); issueSpanCalculator = Substitute.For(); openInIdeConfigScopeValidator = Substitute.For(); ideWindowService = Substitute.For(); - testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); + testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); } [TestMethod] @@ -74,7 +69,7 @@ public void ApplyFixSuggestion_RunsOnUIThread() { MockConfigScopeRoot(); var threadHandlingMock = Substitute.For(); - var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, spanTranslator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); + var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService); fixSuggestionHandler.ApplyFixSuggestion(CreateFixSuggestionParams()); @@ -86,7 +81,7 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() { var suggestionParams = CreateFixSuggestionParams(); var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; - var affectedSnapshot = MockCalculateSpan(suggestedChange); + MockCalculateSpan(suggestedChange); var textView = MockOpenFile(); var edit = Substitute.For(); textView.TextBuffer.CreateEdit().Returns(edit); @@ -100,7 +95,6 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() documentNavigator.Open(@"C:\myFile.cs"); textView.TextBuffer.CreateEdit(); issueSpanCalculator.CalculateSpan(Arg.Any(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine); - spanTranslator.TranslateTo(affectedSnapshot, Arg.Any(), SpanTrackingMode.EdgeExclusive); edit.Replace(Arg.Any(), suggestedChange.after); edit.Apply(); logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); @@ -160,6 +154,23 @@ public void ApplyFixSuggestion_WhenConfigRootScopeNotFound_ShouldLogFailure() logger.Received().WriteLine(FixSuggestionResources.GetConfigScopeRootPathFailed, "SpecificConfigScopeId", "Scope not found"); } + + [TestMethod] + public void ApplyFixSuggestion_WhenLineNumbersDoNotMatch_ShouldLogFailure() + { + MockConfigScopeRoot(); + var edit = Substitute.For(); + var textView = MockOpenFile(); + textView.TextBuffer.CreateEdit().Returns(edit); + var suggestionWithWrongLineNumbers = CreateFixSuggestionParams(scopeId: "AScopeId", suggestionKey: "key", startLine: 0, endLine: 0); + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()) + .Throws(new Exception("Line numbers do not match")); + + testSubject.ApplyFixSuggestion(suggestionWithWrongLineNumbers); + + edit.DidNotReceiveWithAnyArgs().Replace(default, default); + logger.Received().WriteLine(FixSuggestionResources.ProcessingRequestFailed, "AScopeId", "key", "Line numbers do not match"); + } private void MockConfigScopeRoot() { @@ -197,8 +208,10 @@ private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) return affectedSnapshot; } - private ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId") + private static ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId", string suggestionKey = "suggestionKey", string idePath = @"myFile.cs", int startLine = 1, int endLine = 2) { - return new ShowFixSuggestionParams(scopeId, "key", suggestionDto); + var fixSuggestion = new FixSuggestionDto(suggestionKey, "refactor", new FileEditDto(idePath, [new ChangesDto(new LineRangeDto(startLine, endLine), "var a=1;", "")])); + var suggestionParams = new ShowFixSuggestionParams(scopeId, "key", fixSuggestion); + return suggestionParams; } } diff --git a/src/IssueViz/Editor/IIssueSpanCalculator.cs b/src/IssueViz/Editor/IIssueSpanCalculator.cs index 95c44763a..d81070b7e 100644 --- a/src/IssueViz/Editor/IIssueSpanCalculator.cs +++ b/src/IssueViz/Editor/IIssueSpanCalculator.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; using System.ComponentModel.Composition; using Microsoft.VisualStudio.Text; using SonarLint.VisualStudio.Core.Analysis; @@ -109,7 +108,10 @@ internal IssueSpanCalculator(IChecksumCalculator checksumCalculator) public SnapshotSpan CalculateSpan(ITextSnapshot snapshot, int startLine, int endLine) { - // TODO add into consideration all edge cases + if (startLine < 1 || endLine < 1 || startLine > snapshot.LineCount || endLine > snapshot.LineCount || startLine > endLine) + { + throw new ArgumentOutOfRangeException(nameof(startLine), nameof(endLine)); + } var startPosition = snapshot.GetLineFromLineNumber(startLine - 1).Start.Position; var endPosition = snapshot.GetLineFromLineNumber(endLine - 1).End.Position; var span = Span.FromBounds(startPosition, endPosition); diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index d75fea576..c4f67ef4a 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -20,7 +20,6 @@ using System.ComponentModel.Composition; using System.IO; -using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Infrastructure.VS; @@ -40,12 +39,11 @@ public class FixSuggestionHandler : IFixSuggestionHandler private readonly IThreadHandling threadHandling; private readonly ILogger logger; private readonly IDocumentNavigator documentNavigator; - private readonly ISpanTranslator spanTranslator; private readonly IIssueSpanCalculator issueSpanCalculator; [ImportingConstructor] internal FixSuggestionHandler(ILogger logger, IDocumentNavigator documentNavigator, IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, IIDEWindowService ideWindowService) : - this(ThreadHandling.Instance, logger, documentNavigator, new SpanTranslator(), issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService) + this(ThreadHandling.Instance, logger, documentNavigator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService) { } @@ -53,7 +51,6 @@ internal FixSuggestionHandler( IThreadHandling threadHandling, ILogger logger, IDocumentNavigator documentNavigator, - ISpanTranslator spanTranslator, IIssueSpanCalculator issueSpanCalculator, IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator, IIDEWindowService ideWindowService) @@ -61,7 +58,6 @@ internal FixSuggestionHandler( this.threadHandling = threadHandling; this.logger = logger; this.documentNavigator = documentNavigator; - this.spanTranslator = spanTranslator; this.issueSpanCalculator = issueSpanCalculator; this.openInIdeConfigScopeValidator = openInIdeConfigScopeValidator; this.ideWindowService = ideWindowService; @@ -107,17 +103,11 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha var textEdit = textView.TextBuffer.CreateEdit(); foreach (var changeDto in changes) { - var spanToUpdate = CalculateSnapshotSpan(textView, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); + var spanToUpdate = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); textView.Caret.MoveTo(spanToUpdate.Start); textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); textEdit.Replace(spanToUpdate, changeDto.after); } textEdit.Apply(); } - - private SnapshotSpan CalculateSnapshotSpan(ITextView textView, int startLine, int endLine) - { - var snapshotSpan = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, startLine, endLine); - return spanTranslator.TranslateTo(snapshotSpan, textView.TextBuffer.CurrentSnapshot, SpanTrackingMode.EdgeExclusive); - } } From 4dae51ecdf991c82672c841ecca33f7697311e88 Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Thu, 3 Oct 2024 20:00:04 +0200 Subject: [PATCH 08/11] SLVS-1483 Remove unnecessary internal method --- .../FixSuggestion/FixSuggestionHandler.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index c4f67ef4a..11c066342 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -64,16 +64,6 @@ internal FixSuggestionHandler( } public void ApplyFixSuggestion(ShowFixSuggestionParams parameters) - { - ApplyFixSuggestionInternal(parameters); - } - - private bool ValidateConfiguration(string configurationScopeId, out string configurationScopeRoot, out string failureReason) - { - return openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(configurationScopeId, out configurationScopeRoot, out failureReason); - } - - private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) { if (!ValidateConfiguration(parameters.configurationScopeId, out var configurationScopeRoot, out var failureReason)) { @@ -96,6 +86,11 @@ private void ApplyFixSuggestionInternal(ShowFixSuggestionParams parameters) } } + private bool ValidateConfiguration(string configurationScopeId, out string configurationScopeRoot, out string failureReason) + { + return openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(configurationScopeId, out configurationScopeRoot, out failureReason); + } + private void ApplySuggestedChanges(string absoluteFilePath, List changes) { ideWindowService.BringToFront(); From 6f6292188e259d1a7b220c19d4f69e1bde02fab8 Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Fri, 4 Oct 2024 15:52:46 +0200 Subject: [PATCH 09/11] SLVS-1483 Cancel edit when exception occurs --- .../FixSuggestionHandlerTests.cs | 51 +++++++++++++------ .../FixSuggestion/FixSuggestionHandler.cs | 20 +++++--- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index f78a1a3c3..569ff788f 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -100,7 +100,7 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId); }); } - + [TestMethod] public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront() { @@ -158,23 +158,39 @@ public void ApplyFixSuggestion_WhenConfigRootScopeNotFound_ShouldLogFailure() [TestMethod] public void ApplyFixSuggestion_WhenLineNumbersDoNotMatch_ShouldLogFailure() { - MockConfigScopeRoot(); - var edit = Substitute.For(); - var textView = MockOpenFile(); - textView.TextBuffer.CreateEdit().Returns(edit); - var suggestionWithWrongLineNumbers = CreateFixSuggestionParams(scopeId: "AScopeId", suggestionKey: "key", startLine: 0, endLine: 0); - issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()) - .Throws(new Exception("Line numbers do not match")); + FailWhenApplyingEdit(out var suggestionWithWrongLineNumbers, "Line numbers do not match"); testSubject.ApplyFixSuggestion(suggestionWithWrongLineNumbers); - edit.DidNotReceiveWithAnyArgs().Replace(default, default); logger.Received().WriteLine(FixSuggestionResources.ProcessingRequestFailed, "AScopeId", "key", "Line numbers do not match"); } + + [TestMethod] + public void ApplyFixSuggestion_WhenApplyingChangeAndExceptionIsThrown_ShouldCancelEdit() + { + var edit = FailWhenApplyingEdit(out var suggestionWithWrongLineNumbers); + + testSubject.ApplyFixSuggestion(suggestionWithWrongLineNumbers); + + edit.DidNotReceiveWithAnyArgs().Replace(default, default); + edit.Received().Cancel(); + } + + private static ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId", string suggestionKey = "suggestionKey", string idePath = @"myFile.cs", List changes = null) + { + changes ??= [CreateChangesDto()]; + var fixSuggestion = new FixSuggestionDto(suggestionKey, "refactor", new FileEditDto(idePath, changes)); + var suggestionParams = new ShowFixSuggestionParams(scopeId, "key", fixSuggestion); + return suggestionParams; + } + + private static ChangesDto CreateChangesDto(int startLine = 1, int endLine = 2) + { + return new ChangesDto(new LineRangeDto(startLine, endLine), "var a=1;", ""); + } private void MockConfigScopeRoot() { - openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(Arg.Any(), out Arg.Any(), out Arg.Any()).Returns( x => { @@ -207,11 +223,16 @@ private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange) .Returns(affectedSnapshot); return affectedSnapshot; } - - private static ShowFixSuggestionParams CreateFixSuggestionParams(string scopeId = "scopeId", string suggestionKey = "suggestionKey", string idePath = @"myFile.cs", int startLine = 1, int endLine = 2) + + private ITextEdit FailWhenApplyingEdit(out ShowFixSuggestionParams suggestionWithWrongLineNumbers, string reason = "") { - var fixSuggestion = new FixSuggestionDto(suggestionKey, "refactor", new FileEditDto(idePath, [new ChangesDto(new LineRangeDto(startLine, endLine), "var a=1;", "")])); - var suggestionParams = new ShowFixSuggestionParams(scopeId, "key", fixSuggestion); - return suggestionParams; + MockConfigScopeRoot(); + var edit = Substitute.For(); + var textView = MockOpenFile(); + textView.TextBuffer.CreateEdit().Returns(edit); + suggestionWithWrongLineNumbers = CreateFixSuggestionParams(scopeId: "AScopeId", suggestionKey: "key"); + issueSpanCalculator.CalculateSpan(Arg.Any(), Arg.Any(), Arg.Any()) + .Throws(new Exception(reason)); + return edit; } } diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 11c066342..19b8f5af8 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -95,14 +95,22 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha { ideWindowService.BringToFront(); var textView = documentNavigator.Open(absoluteFilePath); - var textEdit = textView.TextBuffer.CreateEdit(); foreach (var changeDto in changes) { - var spanToUpdate = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); - textView.Caret.MoveTo(spanToUpdate.Start); - textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); - textEdit.Replace(spanToUpdate, changeDto.after); + var textEdit = textView.TextBuffer.CreateEdit(); + try + { + var spanToUpdate = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); + textView.Caret.MoveTo(spanToUpdate.Start); + textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); + textEdit.Replace(spanToUpdate, changeDto.after); + textEdit.Apply(); + } + catch (Exception) + { + textEdit.Cancel(); + throw; + } } - textEdit.Apply(); } } From 186552ca4ecd2af7ab63fcc4902186b2dffe4d7a Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Fri, 4 Oct 2024 15:54:20 +0200 Subject: [PATCH 10/11] SLVS-1483 Process changes from bottom to top in order to avoid line matching issues --- .../FixSuggestionHandlerTests.cs | 23 +++++++++++++++++++ .../FixSuggestion/FixSuggestionHandler.cs | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index 569ff788f..e234ca4b3 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -101,6 +101,29 @@ public void ApplyFixSuggestion_OneChange_AppliesChange() }); } + /// + /// 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() + { + MockConfigScopeRoot(); + MockOpenFile(); + List changes = [CreateChangesDto(1, 1), CreateChangesDto(3, 3)]; + var suggestionParams = CreateFixSuggestionParams(changes: changes); + + testSubject.ApplyFixSuggestion(suggestionParams); + + Received.InOrder(() => + { + issueSpanCalculator.CalculateSpan(Arg.Any(), 3, 3); + issueSpanCalculator.CalculateSpan(Arg.Any(), 1, 1); + }); + } + [TestMethod] public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront() { diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 19b8f5af8..037c62643 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -95,8 +95,9 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha { ideWindowService.BringToFront(); var textView = documentNavigator.Open(absoluteFilePath); - foreach (var changeDto in changes) + for (var i = changes.Count - 1; i >= 0; i--) { + var changeDto = changes[i]; var textEdit = textView.TextBuffer.CreateEdit(); try { From 78973a7358b4394d720bc76b2ae8f4a9aa259ed7 Mon Sep 17 00:00:00 2001 From: Vasileios Naskos Date: Mon, 7 Oct 2024 11:11:31 +0200 Subject: [PATCH 11/11] SLVS-1483 Move caret once, when all changes applied --- .../FixSuggestion/FixSuggestionHandlerTests.cs | 12 +++++++----- src/IssueViz/FixSuggestion/FixSuggestionHandler.cs | 13 +++++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs index e234ca4b3..3b9928944 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs @@ -136,17 +136,19 @@ public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront() } [TestMethod] - public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToChangedLines() + public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToFirstChangedLines() { - var suggestionParams = CreateFixSuggestionParams(); - var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; - var affectedSnapshot = MockCalculateSpan(suggestedChange); + List changes = [CreateChangesDto(1, 1), CreateChangesDto(3, 3)]; + var suggestionParams = CreateFixSuggestionParams(changes: changes); + var firstSuggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0]; + var firstAffectedSnapshot = MockCalculateSpan(firstSuggestedChange); var textView = MockOpenFile(); MockConfigScopeRoot(); testSubject.ApplyFixSuggestion(suggestionParams); - textView.ViewScroller.Received().EnsureSpanVisible(affectedSnapshot, EnsureSpanVisibleOptions.AlwaysCenter); + textView.ViewScroller.ReceivedWithAnyArgs(1).EnsureSpanVisible(Arg.Any(), default); + textView.ViewScroller.Received().EnsureSpanVisible(firstAffectedSnapshot, EnsureSpanVisibleOptions.AlwaysCenter); } [TestMethod] diff --git a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs index 037c62643..bb5b2232c 100644 --- a/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs +++ b/src/IssueViz/FixSuggestion/FixSuggestionHandler.cs @@ -95,17 +95,20 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha { ideWindowService.BringToFront(); var textView = documentNavigator.Open(absoluteFilePath); + var textEdit = textView.TextBuffer.CreateEdit(); for (var i = changes.Count - 1; i >= 0; i--) { var changeDto = changes[i]; - var textEdit = textView.TextBuffer.CreateEdit(); + try { var spanToUpdate = issueSpanCalculator.CalculateSpan(textView.TextSnapshot, changeDto.beforeLineRange.startLine, changeDto.beforeLineRange.endLine); - textView.Caret.MoveTo(spanToUpdate.Start); - textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); + if (i == 0) + { + textView.Caret.MoveTo(spanToUpdate.Start); + textView.ViewScroller.EnsureSpanVisible(spanToUpdate, EnsureSpanVisibleOptions.AlwaysCenter); + } textEdit.Replace(spanToUpdate, changeDto.after); - textEdit.Apply(); } catch (Exception) { @@ -113,5 +116,7 @@ private void ApplySuggestedChanges(string absoluteFilePath, List cha throw; } } + + textEdit.Apply(); } }