Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLVS-1483 Implement apply fix suggestion #5726

Merged
merged 11 commits into from
Oct 7, 2024
Merged

SLVS-1483 Implement apply fix suggestion #5726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2f463d0
SLVS-1483 Add parameter that contains the info to apply a fix suggestion
gabriela-trutan-sonarsource Oct 2, 2024
3679763
SLVS-1483 Add functionality to calculate the span for a given line range
gabriela-trutan-sonarsource Oct 2, 2024
afd98a4
SLVS-1483 Add basic implementation for directly applying a fix sugges…
gabriela-trutan-sonarsource Oct 2, 2024
a8b68d0
SLVS-1483 Calculate the file's absolute path based on the config scope
vnaskos-sonar Oct 3, 2024
4934958
SLVS-1483 When applying change bring focus to changed line
vnaskos-sonar Oct 3, 2024
7f00766
SLVS-1483 When applying change bring window to front
vnaskos-sonar Oct 3, 2024
dc99c04
SLVS-1483 Fix CalculateSpan TODO with edge cases
vnaskos-sonar Oct 3, 2024
4dae51e
SLVS-1483 Remove unnecessary internal method
vnaskos-sonar Oct 3, 2024
6f62921
SLVS-1483 Cancel edit when exception occurs
vnaskos-sonar Oct 4, 2024
186552c
SLVS-1483 Process changes from bottom to top in order to avoid line m…
vnaskos-sonar Oct 4, 2024
78973a7
SLVS-1483 Move caret once, when all changes applied
vnaskos-sonar Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 72 additions & 4 deletions src/IssueViz.UnitTests/Editor/IssueSpanCalculatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@
* 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;
using SonarLint.VisualStudio.IssueVisualization.Editor;
using SonarLint.VisualStudio.TestInfrastructure;
using SonarQube.Client;

namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.Editor
Expand All @@ -34,6 +32,8 @@ public class IssueSpanCalculatorTests
private Mock<IChecksumCalculator> checksumCalculatorMock;

private IssueSpanCalculator testSubject;
private const int SnapshotLength = 10000;
private const int SnapshotLineCount = 10000;

[TestInitialize]
public void TestInitialize()
Expand Down Expand Up @@ -331,6 +331,60 @@ 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);
}

[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<ArgumentOutOfRangeException>();
}

private static Mock<ITextSnapshot> MockTextSnapshotForLines(ITextSnapshotLine startLine, ITextSnapshotLine endLine)
{
var textSnapshot = new Mock<ITextSnapshot>();
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);

return textSnapshot;
}

private class VSLineDescription
{
public int ZeroBasedLineNumber { get; set; }
Expand All @@ -339,7 +393,7 @@ private class VSLineDescription
public string Text { get; set; }
}

private static Mock<ITextSnapshot> CreateSnapshotMock(int bufferLineCount = 1000, int snapShotLength = 10000, params VSLineDescription[] lines)
private static Mock<ITextSnapshot> CreateSnapshotMock(int bufferLineCount = 1000, int snapShotLength = SnapshotLength, params VSLineDescription[] lines)
{
var textSnapshotMock = new Mock<ITextSnapshot>();

Expand Down Expand Up @@ -378,5 +432,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<ITextSnapshotLine>();
var textSnapshot = new Mock<ITextSnapshot>();

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;
}
}
}
227 changes: 224 additions & 3 deletions src/IssueViz.UnitTests/FixSuggestion/FixSuggestionHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,246 @@
* 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.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;
using FileEditDto = SonarLint.VisualStudio.SLCore.Listener.FixSuggestion.Models.FileEditDto;

namespace SonarLint.VisualStudio.IssueVisualization.UnitTests.FixSuggestion;

[TestClass]
public class FixSuggestionHandlerTests
{
private const string ConfigurationScopeRoot = @"C:\";

private FixSuggestionHandler testSubject;
private IThreadHandling threadHandling;
private ILogger logger;
private IDocumentNavigator documentNavigator;
private IIssueSpanCalculator issueSpanCalculator;
private IOpenInIdeConfigScopeValidator openInIdeConfigScopeValidator;
private IIDEWindowService ideWindowService;

[TestInitialize]
public void TestInitialize()
{
threadHandling = new NoOpThreadHandler();
logger = Substitute.For<ILogger>();
documentNavigator = Substitute.For<IDocumentNavigator>();
issueSpanCalculator = Substitute.For<IIssueSpanCalculator>();
openInIdeConfigScopeValidator = Substitute.For<IOpenInIdeConfigScopeValidator>();
ideWindowService = Substitute.For<IIDEWindowService>();

testSubject = new FixSuggestionHandler(threadHandling, logger, documentNavigator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService);
}

[TestMethod]
public void MefCtor_CheckIsSingleton()
{
MefTestHelpers.CheckIsSingletonMefComponent<FixSuggestionHandler>();
}

[TestMethod]
public void ApplyFixSuggestion_ThrowsNotImplementedException()
public void ApplyFixSuggestion_RunsOnUIThread()
{
MockConfigScopeRoot();
var threadHandlingMock = Substitute.For<IThreadHandling>();
var fixSuggestionHandler = new FixSuggestionHandler(threadHandlingMock, logger, documentNavigator, issueSpanCalculator, openInIdeConfigScopeValidator, ideWindowService);

fixSuggestionHandler.ApplyFixSuggestion(CreateFixSuggestionParams());

threadHandlingMock.ReceivedWithAnyArgs().RunOnUIThread(default);
}

[TestMethod]
public void ApplyFixSuggestion_OneChange_AppliesChange()
{
var suggestionParams = CreateFixSuggestionParams();
var suggestedChange = suggestionParams.fixSuggestion.fileEdit.changes[0];
MockCalculateSpan(suggestedChange);
var textView = MockOpenFile();
var edit = Substitute.For<ITextEdit>();
textView.TextBuffer.CreateEdit().Returns(edit);
MockConfigScopeRoot();

testSubject.ApplyFixSuggestion(suggestionParams);

Received.InOrder(() =>
{
logger.WriteLine(FixSuggestionResources.ProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId);
documentNavigator.Open(@"C:\myFile.cs");
textView.TextBuffer.CreateEdit();
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine);
edit.Replace(Arg.Any<Span>(), suggestedChange.after);
edit.Apply();
logger.WriteLine(FixSuggestionResources.DoneProcessingRequest, suggestionParams.configurationScopeId, suggestionParams.fixSuggestion.suggestionId);
});
}

/// <summary>
/// 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.
/// </summary>
[TestMethod]
public void ApplyFixSuggestion_WhenMoreThanOneFixes_ApplyThemFromBottomToTop()
{
MockConfigScopeRoot();
MockOpenFile();
List<ChangesDto> changes = [CreateChangesDto(1, 1), CreateChangesDto(3, 3)];
var suggestionParams = CreateFixSuggestionParams(changes: changes);

testSubject.ApplyFixSuggestion(suggestionParams);

Received.InOrder(() =>
{
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), 3, 3);
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), 1, 1);
});
}

[TestMethod]
public void ApplyFixSuggestion_WhenApplyingChange_BringWindowToFront()
{
var suggestionParams = CreateFixSuggestionParams();
MockConfigScopeRoot();

testSubject.ApplyFixSuggestion(suggestionParams);

ideWindowService.Received().BringToFront();
}

[TestMethod]
public void ApplyFixSuggestion_WhenApplyingChange_BringFocusToFirstChangedLines()
{
List<ChangesDto> 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.ReceivedWithAnyArgs(1).EnsureSpanVisible(Arg.Any<SnapshotSpan>(), default);
textView.ViewScroller.Received().EnsureSpanVisible(firstAffectedSnapshot, EnsureSpanVisibleOptions.AlwaysCenter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it should also be Received(1).EnsureSpanVisible to ensure it is received once and not any amount of times (Received() will not check for the amount of times it is called)

}

[TestMethod]
public void ApplyFixSuggestion_Throws_Logs()
{
var testSubject = new FixSuggestionHandler();
var suggestionParams = CreateFixSuggestionParams();
var exceptionMsg = "error";
MockConfigScopeRoot();
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), Arg.Any<int>(), Arg.Any<int>()).Throws(new Exception(exceptionMsg));

Exceptions.Expect<NotImplementedException>(() => testSubject.ApplyFixSuggestion());
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);
}

[TestMethod]
public void ApplyFixSuggestion_WhenConfigRootScopeNotFound_ShouldLogFailure()
{
MockFailedConfigScopeRoot("Scope not found");
var suggestionParams = CreateFixSuggestionParams("SpecificConfigScopeId");

testSubject.ApplyFixSuggestion(suggestionParams);

logger.Received().WriteLine(FixSuggestionResources.GetConfigScopeRootPathFailed, "SpecificConfigScopeId", "Scope not found");
}

[TestMethod]
public void ApplyFixSuggestion_WhenLineNumbersDoNotMatch_ShouldLogFailure()
{
FailWhenApplyingEdit(out var suggestionWithWrongLineNumbers, "Line numbers do not match");

testSubject.ApplyFixSuggestion(suggestionWithWrongLineNumbers);

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<ChangesDto> 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<string>(), out Arg.Any<string>(), out Arg.Any<string>()).Returns(
x =>
{
x[1] = ConfigurationScopeRoot;
return true;
});
}

private void MockFailedConfigScopeRoot(string failureReason)
{
openInIdeConfigScopeValidator.TryGetConfigurationScopeRoot(Arg.Any<string>(), out Arg.Any<string>(), out Arg.Any<string>()).Returns(
x =>
{
x[2] = failureReason;
return false;
});
}

private ITextView MockOpenFile()
{
var textView = Substitute.For<ITextView>();
documentNavigator.Open(Arg.Any<string>()).Returns(textView);
return textView;
}

private SnapshotSpan MockCalculateSpan(ChangesDto suggestedChange)
{
var affectedSnapshot = new SnapshotSpan();
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), suggestedChange.beforeLineRange.startLine, suggestedChange.beforeLineRange.endLine)
.Returns(affectedSnapshot);
return affectedSnapshot;
}

private ITextEdit FailWhenApplyingEdit(out ShowFixSuggestionParams suggestionWithWrongLineNumbers, string reason = "")
{
MockConfigScopeRoot();
var edit = Substitute.For<ITextEdit>();
var textView = MockOpenFile();
textView.TextBuffer.CreateEdit().Returns(edit);
suggestionWithWrongLineNumbers = CreateFixSuggestionParams(scopeId: "AScopeId", suggestionKey: "key");
issueSpanCalculator.CalculateSpan(Arg.Any<ITextSnapshot>(), Arg.Any<int>(), Arg.Any<int>())
.Throws(new Exception(reason));
return edit;
}
}
15 changes: 14 additions & 1 deletion src/IssueViz/Editor/IIssueSpanCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +33,8 @@ public interface IIssueSpanCalculator
/// Returns null if no textRange is passed
/// </summary>
SnapshotSpan? CalculateSpan(ITextRange range, ITextSnapshot currentSnapshot);

SnapshotSpan CalculateSpan(ITextSnapshot snapshot, int startLine, int endLine);
}

[Export(typeof(IIssueSpanCalculator))]
Expand Down Expand Up @@ -105,6 +106,18 @@ internal IssueSpanCalculator(IChecksumCalculator checksumCalculator)
return snapshotSpan;
}

public SnapshotSpan CalculateSpan(ITextSnapshot snapshot, int startLine, int endLine)
{
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);
return new SnapshotSpan(snapshot, span);
}

private static bool RangeHasHash(ITextRange range)
=> !string.IsNullOrEmpty(range.LineHash);

Expand Down
Loading