Add mocks for configuration for testing, still need to fix some parameter tests

Author: JustinGrote

src/PowerShellEditorServices/Server/PsesLanguageServer.cs

@@ -86,7 +86,7 @@ public async Task StartAsync()
                         .AddLanguageProtocolLogging()
                         .SetMinimumLevel(_minimumLogLevel))
                     // TODO: Consider replacing all WithHandler with AddSingleton
-                    .WithConfigurationSection("powershell")
+                    .WithConfigurationSection("powershell.rename")
                     .WithHandler<PsesWorkspaceSymbolsHandler>()
                     .WithHandler<PsesTextDocumentHandler>()
                     .WithHandler<GetVersionHandler>()

src/PowerShellEditorServices/Services/PowerShell/Refactoring/IterativeVariableVisitor.cs

@@ -24,15 +24,15 @@ internal class IterativeVariableRename
         internal bool isParam;
         internal bool AliasSet;
         internal FunctionDefinitionAst TargetFunction;
-        internal RenameServiceOptions options;
+        internal bool CreateAlias;
 
-        public IterativeVariableRename(string NewName, int StartLineNumber, int StartColumnNumber, Ast ScriptAst, RenameServiceOptions options)
+        public IterativeVariableRename(string NewName, int StartLineNumber, int StartColumnNumber, Ast ScriptAst, bool CreateAlias)
         {
             this.NewName = NewName;
             this.StartLineNumber = StartLineNumber;
             this.StartColumnNumber = StartColumnNumber;
             this.ScriptAst = ScriptAst;
-            this.options = options;
+            this.CreateAlias = CreateAlias;
 
             VariableExpressionAst Node = (VariableExpressionAst)GetVariableTopAssignment(StartLineNumber, StartColumnNumber, ScriptAst);
             if (Node != null)
@@ -404,7 +404,7 @@ private void ProcessVariableExpressionAst(VariableExpressionAst variableExpressi
                     };
                     // If the variables parent is a parameterAst Add a modification
                     if (variableExpressionAst.Parent is ParameterAst paramAst && !AliasSet &&
-                        options.createVariableAlias)
+                        CreateAlias)
                     {
                         TextEdit aliasChange = NewParameterAliasChange(variableExpressionAst, paramAst);
                         Modifications.Add(aliasChange);

src/PowerShellEditorServices/Services/TextDocument/RenameService.cs

@@ -19,11 +19,14 @@
 
 namespace Microsoft.PowerShell.EditorServices.Services;
 
+/// <summary>
+/// Used with Configuration Bind to sync the settings to what is set on the client.
+/// </summary>
 public class RenameServiceOptions
 {
-    internal bool createFunctionAlias { get; set; }
-    internal bool createVariableAlias { get; set; }
-    internal bool acceptDisclaimer { get; set; }
+    public bool createFunctionAlias { get; set; }
+    public bool createVariableAlias { get; set; }
+    public bool acceptDisclaimer { get; set; }
 }
 
 public interface IRenameService
@@ -46,44 +49,44 @@ internal class RenameService(
     WorkspaceService workspaceService,
     ILanguageServerFacade lsp,
     ILanguageServerConfiguration config,
+    bool disclaimerDeclinedForSession = false,
+    bool disclaimerAcceptedForSession = false,
     string configSection = "powershell.rename"
 ) : IRenameService
 {
-    private bool disclaimerDeclined;
-    private bool disclaimerAccepted;
-
-    private readonly RenameServiceOptions settings = new();
 
-    internal void RefreshSettings() => config.GetSection(configSection).Bind(settings);
+    private async Task<RenameServiceOptions> GetScopedSettings(DocumentUri uri, CancellationToken cancellationToken = default)
+    {
+        IScopedConfiguration scopedConfig = await config.GetScopedConfiguration(uri, cancellationToken).ConfigureAwait(false);
+        return scopedConfig.GetSection(configSection).Get<RenameServiceOptions>() ?? new RenameServiceOptions();
+    }
 
     public async Task<RangeOrPlaceholderRange?> PrepareRenameSymbol(PrepareRenameParams request, CancellationToken cancellationToken)
     {
-
-        if (!await AcceptRenameDisclaimer(cancellationToken).ConfigureAwait(false)) { return null; }
-        ScriptFile scriptFile = workspaceService.GetFile(request.TextDocument.Uri);
-
-        // TODO: Is this too aggressive? We can still rename inside a var/function even if dotsourcing is in use in a file, we just need to be clear it's not supported to expect rename actions to propogate.
-        if (Utilities.AssertContainsDotSourced(scriptFile.ScriptAst))
+        RenameParams renameRequest = new()
         {
-            throw new HandlerErrorException("Dot Source detected, this is currently not supported");
-        }
-
-        // TODO: FindRenamableSymbol may create false positives for renaming, so we probably should go ahead and execute a full rename and return true if edits are found.
-
-        ScriptPositionAdapter position = request.Position;
-        Ast? target = FindRenamableSymbol(scriptFile, position);
+            NewName = "PREPARERENAMETEST", //A placeholder just to gather edits
+            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?
+        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.
-        RangeOrPlaceholderRange? renamable = target is null ? null : new RangeOrPlaceholderRange
-        (
-            new RenameDefaultBehavior() { DefaultBehavior = true }
-        );
-        return renamable;
+        return (renameResponse?.Changes?[request.TextDocument.Uri].ToArray().Length > 0)
+            ? new RangeOrPlaceholderRange
+            (
+                new RenameDefaultBehavior() { DefaultBehavior = true }
+            )
+            : null;
     }
 
     public async Task<WorkspaceEdit?> RenameSymbol(RenameParams request, CancellationToken cancellationToken)
     {
-        if (!await AcceptRenameDisclaimer(cancellationToken).ConfigureAwait(false)) { return null; }
+        // We want scoped settings because a workspace setting might be relevant here.
+        RenameServiceOptions options = await GetScopedSettings(request.TextDocument.Uri, cancellationToken).ConfigureAwait(false);
+
+        if (!await AcceptRenameDisclaimer(options.acceptDisclaimer, cancellationToken).ConfigureAwait(false)) { return null; }
 
         ScriptFile scriptFile = workspaceService.GetFile(request.TextDocument.Uri);
         ScriptPositionAdapter position = request.Position;
@@ -95,7 +98,7 @@ internal class RenameService(
         TextEdit[] changes = tokenToRename switch
         {
             FunctionDefinitionAst or CommandAst => RenameFunction(tokenToRename, scriptFile.ScriptAst, request),
-            VariableExpressionAst => RenameVariable(tokenToRename, scriptFile.ScriptAst, request),
+            VariableExpressionAst => RenameVariable(tokenToRename, scriptFile.ScriptAst, request, options.createVariableAlias),
             // FIXME: Only throw if capability is not prepareprovider
             _ => throw new InvalidOperationException("This should not happen as PrepareRename should have already checked for viability. File an issue if you see this.")
         };
@@ -122,17 +125,16 @@ internal static TextEdit[] RenameFunction(Ast target, Ast scriptAst, RenameParam
         return visitor.VisitAndGetEdits(scriptAst);
     }
 
-    internal TextEdit[] RenameVariable(Ast symbol, Ast scriptAst, RenameParams requestParams)
+    internal static TextEdit[] RenameVariable(Ast symbol, Ast scriptAst, RenameParams requestParams, bool createAlias)
     {
         if (symbol is VariableExpressionAst or ParameterAst or CommandParameterAst or StringConstantExpressionAst)
         {
-
             IterativeVariableRename visitor = new(
                 requestParams.NewName,
                 symbol.Extent.StartLineNumber,
                 symbol.Extent.StartColumnNumber,
                 scriptAst,
-                settings
+                createAlias
             );
             visitor.Visit(scriptAst);
             return visitor.Modifications.ToArray();
@@ -200,15 +202,10 @@ internal static ScriptExtentAdapter GetFunctionNameExtent(FunctionDefinitionAst
     /// Prompts the user to accept the rename disclaimer.
     /// </summary>
     /// <returns>true if accepted, false if rejected</returns>
-    private async Task<bool> AcceptRenameDisclaimer(CancellationToken cancellationToken)
+    private async Task<bool> AcceptRenameDisclaimer(bool acceptDisclaimerOption, CancellationToken cancellationToken)
     {
-        // Fetch the latest settings from the client, in case they have changed.
-        config.GetSection(configSection).Bind(settings);
-
-        // User has declined for the session so we don't want this popping up a bunch.
-        if (disclaimerDeclined) { return false; }
-
-        if (settings.acceptDisclaimer || disclaimerAccepted) { return true; }
+        if (disclaimerDeclinedForSession) { return false; }
+        if (acceptDisclaimerOption || disclaimerAcceptedForSession) { return true; }
 
         // TODO: Localization
         const string renameDisclaimer = "PowerShell rename functionality is only supported in a limited set of circumstances. Please review the notice and understand the limitations and risks.";
@@ -242,8 +239,8 @@ private async Task<bool> AcceptRenameDisclaimer(CancellationToken cancellationTo
                 Type = MessageType.Info
             };
             lsp.SendNotification(msgParams);
-            disclaimerDeclined = true;
-            return !disclaimerDeclined;
+            disclaimerDeclinedForSession = true;
+            return !disclaimerDeclinedForSession;
         }
         if (result.Title == acceptAnswer)
         {
@@ -255,8 +252,8 @@ private async Task<bool> AcceptRenameDisclaimer(CancellationToken cancellationTo
             };
             lsp.SendNotification(msgParams);
 
-            disclaimerAccepted = true;
-            return disclaimerAccepted;
+            disclaimerAcceptedForSession = true;
+            return disclaimerAcceptedForSession;
         }
         // if (result.Title == acceptWorkspaceAnswer)
         // {
@@ -514,7 +511,6 @@ public ScriptPositionAdapter(Position position) : this(position.Line + 1, positi
         scriptPosition.position.LineNumber - 1, scriptPosition.position.ColumnNumber - 1
     );
 
-
     public static implicit operator ScriptPositionAdapter(ScriptPosition position) => new(position);
     public static implicit operator ScriptPosition(ScriptPositionAdapter position) => position;
 

test/PowerShellEditorServices.Test/Refactoring/PrepareRenameHandlerTests.cs

@@ -9,7 +9,6 @@
 using MediatR;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging.Abstractions;
-using Microsoft.Extensions.Primitives;
 using Microsoft.PowerShell.EditorServices.Handlers;
 using Microsoft.PowerShell.EditorServices.Services;
 using Microsoft.PowerShell.EditorServices.Test.Shared;
@@ -44,7 +43,8 @@ public PrepareRenameHandlerTests()
             (
                 workspace,
                 new fakeLspSendMessageRequestFacade("I Accept"),
-                new fakeConfigurationService()
+                new EmptyConfiguration(),
+                disclaimerAcceptedForSession: true //Suppresses prompts
             )
         );
     }
@@ -134,18 +134,17 @@ public async Task<TResponse> SendRequest<TResponse>(IRequest<TResponse> request,
     public bool TryGetRequest(long id, out string method, out TaskCompletionSource<JToken> pendingTask) => throw new NotImplementedException();
 }
 
-public class fakeConfigurationService : ILanguageServerConfiguration
+
+
+public class EmptyConfiguration : ConfigurationRoot, ILanguageServerConfiguration, IScopedConfiguration
 {
-    public string this[string key] { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+    public EmptyConfiguration() : base([]) { }
 
     public bool IsSupported => throw new NotImplementedException();
 
     public ILanguageServerConfiguration AddConfigurationItems(IEnumerable<ConfigurationItem> configurationItems) => throw new NotImplementedException();
-    public IEnumerable<IConfigurationSection> GetChildren() => throw new NotImplementedException();
     public Task<IConfiguration> GetConfiguration(params ConfigurationItem[] items) => throw new NotImplementedException();
-    public IChangeToken GetReloadToken() => throw new NotImplementedException();
-    public Task<IScopedConfiguration> GetScopedConfiguration(DocumentUri scopeUri, CancellationToken cancellationToken) => throw new NotImplementedException();
-    public IConfigurationSection GetSection(string key) => throw new NotImplementedException();
+    public Task<IScopedConfiguration> GetScopedConfiguration(DocumentUri scopeUri, CancellationToken cancellationToken) => Task.FromResult((IScopedConfiguration)this);
     public ILanguageServerConfiguration RemoveConfigurationItems(IEnumerable<ConfigurationItem> configurationItems) => throw new NotImplementedException();
     public bool TryGetScopedConfiguration(DocumentUri scopeUri, out IScopedConfiguration configuration) => throw new NotImplementedException();
 }

test/PowerShellEditorServices.Test/Refactoring/RenameHandlerTests.cs

@@ -36,7 +36,8 @@ public RenameHandlerTests()
             (
                 workspace,
                 new fakeLspSendMessageRequestFacade("I Accept"),
-                new fakeConfigurationService()
+                new EmptyConfiguration(),
+                disclaimerAcceptedForSession: true //Disables UI prompts
             )
         );
     }