Add support for negative test cases

JustinGrote · JustinGrote · commit ff753551f5d9 · 2025-03-18T12:13:59.000-07:00
diff --git a/src/PowerShellEditorServices/Services/TextDocument/RenameService.cs b/src/PowerShellEditorServices/Services/TextDocument/RenameService.cs
@@ -63,7 +63,8 @@ ILanguageServerConfiguration config
             Position = request.Position,
             TextDocument = request.TextDocument
         };
-        // TODO: Should we cache these resuls and just fetch them on the actual rename, and move the bulk to an implementation method?
+
+        // TODO: As a performance optimization, should we cache these results and just fetch them on the actual rename, and move the bulk to an implementation method? Seems pretty fast right now but may slow down on large documents. Need to add a large document test example.
         WorkspaceEdit? renameResponse = await RenameSymbol(renameRequest, cancellationToken).ConfigureAwait(false);
 
         // Since LSP 3.16 we can simply basically return a DefaultBehavior true or null to signal to the client that the position is valid for rename and it should use its default selection criteria (which is probably the language semantic highlighting or grammar). For the current scope of the rename provider, this should be fine, but we have the option to supply the specific range in the future for special cases.
@@ -318,7 +319,7 @@ public AstVisitAction Visit(Ast ast)
             {
                 FunctionDefinitionAst f => f,
                 CommandAst command => CurrentDocument.FindFunctionDefinition(command)
-                    ?? throw new TargetSymbolNotFoundException("The command to rename does not have a function definition. Renaming a function is only supported when the function is defined within the same scope"),
+                    ?? throw new HandlerErrorException("The command to rename does not have a function definition. Renaming a function is only supported when the function is defined within the same scope"),
                 _ => throw new Exception($"Unsupported AST type {target.GetType()} encountered")
             };
         };
diff --git a/test/PowerShellEditorServices.Test.Shared/Refactoring/Functions/FunctionSameNameRenamed.ps1 b/test/PowerShellEditorServices.Test.Shared/Refactoring/Functions/FunctionSameNameRenamed.ps1
@@ -1,8 +1,8 @@
 function SameNameFunction {
     Write-Host "This is the outer function"
-    function RenamedSameNameFunction {
-        Write-Host "This is the inner function"
+    function Renamed {
+        Write-Host 'This is the inner function'
     }
-    RenamedSameNameFunction
+    Renamed
 }
 SameNameFunction
diff --git a/test/PowerShellEditorServices.Test.Shared/Refactoring/Functions/RefactorFunctionTestCases.cs b/test/PowerShellEditorServices.Test.Shared/Refactoring/Functions/RefactorFunctionTestCases.cs
@@ -19,7 +19,7 @@ public class RefactorFunctionTestCases
         new("FunctionMultipleOccurrences.ps1",         Line:  5, Column:  3 ),
         new("FunctionNestedRedefinition.ps1",          Line: 13, Column: 15 ),
         new("FunctionOuterHasNestedFunction.ps1",      Line:  1, Column: 10 ),
-        new("FunctionSameName.ps1",                    Line:  3, Column: 14 , "RenamedSameNameFunction"),
+        new("FunctionSameName.ps1",                    Line:  3, Column: 14 ),
         new("FunctionScriptblock.ps1",                 Line:  5, Column:  5 ),
         new("FunctionsSingle.ps1",                     Line:  1, Column: 11 ),
         new("FunctionWithInnerFunction.ps1",           Line:  5, Column:  5 ),
diff --git a/test/PowerShellEditorServices.Test.Shared/Refactoring/RenameTestTarget.cs b/test/PowerShellEditorServices.Test.Shared/Refactoring/RenameTestTarget.cs
@@ -27,18 +27,22 @@ public class RenameTestTarget
     /// </summary>
     public string NewName = "Renamed";
 
+    public bool ShouldFail;
+
     /// <param name="FileName">The test case file name e.g. testScript.ps1</param>
     /// <param name="Line">The line where the cursor should be positioned for the rename</param>
     /// <param name="Column">The column/character indent where ther cursor should be positioned for the rename</param>
     /// <param name="NewName">What the target symbol represented by the line and column should be renamed to. Defaults to "Renamed" if not specified</param>
-    public RenameTestTarget(string FileName, int Line, int Column, string NewName = "Renamed")
+    /// <param name="ShouldFail">This test case should not succeed and return either null or a handler error</param>
+    public RenameTestTarget(string FileName, int Line, int Column, string NewName = "Renamed", bool ShouldFail = false)
     {
         this.FileName = FileName;
         this.Line = Line;
         this.Column = Column;
         this.NewName = NewName;
+        this.ShouldFail = ShouldFail;
     }
     public RenameTestTarget() { }
 
-    public override string ToString() => $"{FileName.Substring(0, FileName.Length - 4)}";
+    public override string ToString() => $"{FileName} L{Line} C{Column} N:{NewName} F:{ShouldFail}";
 }
diff --git a/test/PowerShellEditorServices.Test.Shared/Refactoring/Variables/RefactorVariableTestCases.cs b/test/PowerShellEditorServices.Test.Shared/Refactoring/Variables/RefactorVariableTestCases.cs
@@ -6,6 +6,8 @@ public class RefactorVariableTestCases
     public static RenameTestTarget[] TestCases =
     [
         new ("SimpleVariableAssignment.ps1",                   Line:  1, Column:  1),
+        new ("SimpleVariableAssignment.ps1",                   Line:  1, Column:  1, NewName: "$Renamed"),
+        new ("SimpleVariableAssignment.ps1",                   Line:  2, Column:  1, NewName: "Wrong", ShouldFail: true),
         new ("VariableCommandParameter.ps1",                   Line:  3, Column: 17),
         new ("VariableCommandParameter.ps1",                   Line: 10, Column: 10),
         new ("VariableCommandParameterSplatted.ps1",           Line:  3, Column: 19 ),
diff --git a/test/PowerShellEditorServices.Test/Refactoring/PrepareRenameHandlerTests.cs b/test/PowerShellEditorServices.Test/Refactoring/PrepareRenameHandlerTests.cs
@@ -67,7 +67,21 @@ public async Task FindsFunction(RenameTestTarget s)
     {
         PrepareRenameParams testParams = s.ToPrepareRenameParams("Functions");
 
-        RangeOrPlaceholderRange? result = await testHandler.Handle(testParams, CancellationToken.None);
+        RangeOrPlaceholderRange? result;
+        try
+        {
+            result = await testHandler.Handle(testParams, CancellationToken.None);
+        }
+        catch (HandlerErrorException)
+        {
+            Assert.True(s.ShouldFail);
+            return;
+        }
+        if (s.ShouldFail)
+        {
+            Assert.Null(result);
+            return;
+        }
 
         Assert.NotNull(result);
         Assert.True(result?.DefaultBehavior?.DefaultBehavior);
@@ -79,7 +93,21 @@ public async Task FindsVariable(RenameTestTarget s)
     {
         PrepareRenameParams testParams = s.ToPrepareRenameParams("Variables");
 
-        RangeOrPlaceholderRange? result = await testHandler.Handle(testParams, CancellationToken.None);
+        RangeOrPlaceholderRange? result;
+        try
+        {
+            result = await testHandler.Handle(testParams, CancellationToken.None);
+        }
+        catch (HandlerErrorException)
+        {
+            Assert.True(s.ShouldFail);
+            return;
+        }
+        if (s.ShouldFail)
+        {
+            Assert.Null(result);
+            return;
+        }
 
         Assert.NotNull(result);
         Assert.True(result?.DefaultBehavior?.DefaultBehavior);
@@ -184,6 +212,7 @@ public void Serialize(IXunitSerializationInfo info)
         info.AddValue(nameof(Line), Line);
         info.AddValue(nameof(Column), Column);
         info.AddValue(nameof(NewName), NewName);
+        info.AddValue(nameof(ShouldFail), ShouldFail);
     }
 
     public void Deserialize(IXunitSerializationInfo info)
@@ -192,6 +221,7 @@ public void Deserialize(IXunitSerializationInfo info)
         Line = info.GetValue<int>(nameof(Line));
         Column = info.GetValue<int>(nameof(Column));
         NewName = info.GetValue<string>(nameof(NewName));
+        ShouldFail = info.GetValue<bool>(nameof(ShouldFail));
     }
 
     public static RenameTestTargetSerializable FromRenameTestTarget(RenameTestTarget t)
@@ -200,6 +230,7 @@ public static RenameTestTargetSerializable FromRenameTestTarget(RenameTestTarget
             FileName = t.FileName,
             Column = t.Column,
             Line = t.Line,
-            NewName = t.NewName
+            NewName = t.NewName,
+            ShouldFail = t.ShouldFail
         };
 }
diff --git a/test/PowerShellEditorServices.Test/Refactoring/RenameHandlerTests.cs b/test/PowerShellEditorServices.Test/Refactoring/RenameHandlerTests.cs
@@ -56,7 +56,22 @@ public static TheoryData<RenameTestTargetSerializable> FunctionTestCases()
     public async void RenamedFunction(RenameTestTarget s)
     {
         RenameParams request = s.ToRenameParams("Functions");
-        WorkspaceEdit response = await testHandler.Handle(request, CancellationToken.None);
+        WorkspaceEdit response;
+        try
+        {
+            response = await testHandler.Handle(request, CancellationToken.None);
+        }
+        catch (HandlerErrorException)
+        {
+            Assert.True(s.ShouldFail);
+            return;
+        }
+        if (s.ShouldFail)
+        {
+            Assert.Null(response);
+            return;
+        }
+
         DocumentUri testScriptUri = request.TextDocument.Uri;
 
         string expected = workspace.GetFile
@@ -78,7 +93,21 @@ public async void RenamedFunction(RenameTestTarget s)
     public async void RenamedVariable(RenameTestTarget s)
     {
         RenameParams request = s.ToRenameParams("Variables");
-        WorkspaceEdit response = await testHandler.Handle(request, CancellationToken.None);
+        WorkspaceEdit response;
+        try
+        {
+            response = await testHandler.Handle(request, CancellationToken.None);
+        }
+        catch (HandlerErrorException)
+        {
+            Assert.True(s.ShouldFail);
+            return;
+        }
+        if (s.ShouldFail)
+        {
+            Assert.Null(response);
+            return;
+        }
         DocumentUri testScriptUri = request.TextDocument.Uri;
 
         string expected = workspace.GetFile

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`	`1`	`function SameNameFunction {`
`2`	`2`	`Write-Host "This is the outer function"`
`3`		`- function RenamedSameNameFunction {`
`4`		`- Write-Host "This is the inner function"`
	`3`	`+ function Renamed {`
	`4`	`+ Write-Host 'This is the inner function'`
`5`	`5`	`}`
`6`		`- RenamedSameNameFunction`
	`6`	`+ Renamed`
`7`	`7`	`}`
`8`	`8`	`SameNameFunction`
Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,8 @@ public class RefactorVariableTestCases`
`6`	`6`	`public static RenameTestTarget[] TestCases =`
`7`	`7`	`[`
`8`	`8`	`new ("SimpleVariableAssignment.ps1", Line: 1, Column: 1),`
	`9`	`+ new ("SimpleVariableAssignment.ps1", Line: 1, Column: 1, NewName: "$Renamed"),`
	`10`	`+ new ("SimpleVariableAssignment.ps1", Line: 2, Column: 1, NewName: "Wrong", ShouldFail: true),`
`9`	`11`	`new ("VariableCommandParameter.ps1", Line: 3, Column: 17),`
`10`	`12`	`new ("VariableCommandParameter.ps1", Line: 10, Column: 10),`
`11`	`13`	`new ("VariableCommandParameterSplatted.ps1", Line: 3, Column: 19 ),`