From f7ae4229dc5886d2739ad1937d9f8d11ff246ace Mon Sep 17 00:00:00 2001 From: MartinGC94 <42123497+MartinGC94@users.noreply.github.com> Date: Fri, 7 Mar 2025 05:26:03 +0100 Subject: [PATCH] Fix parameter completion when script requirements fail (#17687) --- .../PseudoParameterBinder.cs | 2 +- .../engine/CommandDiscovery.cs | 39 ++++++++++++------- .../engine/ExecutionContext.cs | 6 +-- .../TabCompletion/TabCompletion.Tests.ps1 | 30 ++++++++++++++ 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/PseudoParameterBinder.cs b/src/System.Management.Automation/engine/CommandCompletion/PseudoParameterBinder.cs index facb77b637..2e7457cd81 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/PseudoParameterBinder.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/PseudoParameterBinder.cs @@ -1198,7 +1198,7 @@ namespace System.Management.Automation.Language string commandName = null; try { - processor = PrepareFromAst(context, out commandName) ?? context.CreateCommand(commandName, dotSource); + processor = PrepareFromAst(context, out commandName) ?? context.CreateCommand(commandName, dotSource, forCompletion:true); } catch (RuntimeException) { diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 561a33ccba..cf4f561c98 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -262,6 +262,9 @@ namespace System.Management.Automation /// False if not. Null if command discovery should default to something reasonable /// for the command discovered. /// + /// + /// True if this for parameter completion and script requirements should be ignored. + /// /// /// /// @@ -271,14 +274,15 @@ namespace System.Management.Automation /// If the security manager is preventing the command from running. /// internal CommandProcessorBase LookupCommandProcessor(string commandName, - CommandOrigin commandOrigin, bool? useLocalScope) + CommandOrigin commandOrigin, bool? useLocalScope, bool forCompletion = false) { CommandProcessorBase processor = null; CommandInfo commandInfo = LookupCommandInfo(commandName, commandOrigin); if (commandInfo != null) { - processor = LookupCommandProcessor(commandInfo, commandOrigin, useLocalScope, null); + processor = LookupCommandProcessor(commandInfo, commandOrigin, useLocalScope, null, forCompletion); + // commandInfo.Name might be different than commandName - restore the original invocation name processor.Command.MyInvocation.InvocationName = commandName; } @@ -286,7 +290,7 @@ namespace System.Management.Automation return processor; } - internal static void VerifyRequiredModules(ExternalScriptInfo scriptInfo, ExecutionContext context) + internal static void VerifyRequiredModules(ExternalScriptInfo scriptInfo, ExecutionContext context, bool forCompletion = false) { // Check Required Modules if (scriptInfo.RequiresModules != null) @@ -301,7 +305,7 @@ namespace System.Management.Automation moduleManifestPath: null, manifestProcessingFlags: ModuleCmdletBase.ManifestProcessingFlags.LoadElements | ModuleCmdletBase.ManifestProcessingFlags.WriteErrors, error: out error); - if (error != null) + if (!forCompletion && error is not null) { ScriptRequiresException scriptRequiresException = new ScriptRequiresException( @@ -316,9 +320,9 @@ namespace System.Management.Automation } } - private CommandProcessorBase CreateScriptProcessorForSingleShell(ExternalScriptInfo scriptInfo, ExecutionContext context, bool useLocalScope, SessionStateInternal sessionState) + private CommandProcessorBase CreateScriptProcessorForSingleShell(ExternalScriptInfo scriptInfo, ExecutionContext context, bool useLocalScope, SessionStateInternal sessionState, bool forCompletion = false) { - VerifyScriptRequirements(scriptInfo, Context); + VerifyScriptRequirements(scriptInfo, Context, forCompletion); if (!string.IsNullOrEmpty(scriptInfo.RequiresApplicationID)) { @@ -340,12 +344,18 @@ namespace System.Management.Automation // #Requires -PSVersion // #Requires -PSEdition // #Requires -Module - internal static void VerifyScriptRequirements(ExternalScriptInfo scriptInfo, ExecutionContext context) + internal static void VerifyScriptRequirements(ExternalScriptInfo scriptInfo, ExecutionContext context, bool forCompletion = false) { - VerifyElevatedPrivileges(scriptInfo); - VerifyPSVersion(scriptInfo); - VerifyPSEdition(scriptInfo); - VerifyRequiredModules(scriptInfo, context); + // When completing script parameters we don't care if these requirements are met. + // VerifyRequiredModules will attempt to load the required modules which is useful for completion (so the correct types are loaded). + if (!forCompletion) + { + VerifyElevatedPrivileges(scriptInfo); + VerifyPSVersion(scriptInfo); + VerifyPSEdition(scriptInfo); + } + + VerifyRequiredModules(scriptInfo, context, forCompletion); } internal static void VerifyPSVersion(ExternalScriptInfo scriptInfo) @@ -426,6 +436,9 @@ namespace System.Management.Automation /// False if not. Null if command discovery should default to something reasonable /// for the command discovered. /// + /// + /// True if this for parameter completion and script requirements should be ignored. + /// /// The session state the commandInfo should be run in. /// /// @@ -436,7 +449,7 @@ namespace System.Management.Automation /// If the security manager is preventing the command from running. /// internal CommandProcessorBase LookupCommandProcessor(CommandInfo commandInfo, - CommandOrigin commandOrigin, bool? useLocalScope, SessionStateInternal sessionState) + CommandOrigin commandOrigin, bool? useLocalScope, SessionStateInternal sessionState, bool forCompletion = false) { CommandProcessorBase processor = null; @@ -482,7 +495,7 @@ namespace System.Management.Automation scriptInfo.SignatureChecked = true; try { - processor = CreateScriptProcessorForSingleShell(scriptInfo, Context, useLocalScope ?? true, sessionState); + processor = CreateScriptProcessorForSingleShell(scriptInfo, Context, useLocalScope ?? true, sessionState, forCompletion); } catch (ScriptRequiresSyntaxException reqSyntaxException) { diff --git a/src/System.Management.Automation/engine/ExecutionContext.cs b/src/System.Management.Automation/engine/ExecutionContext.cs index 56f64c1a5c..16f2dbcc21 100644 --- a/src/System.Management.Automation/engine/ExecutionContext.cs +++ b/src/System.Management.Automation/engine/ExecutionContext.cs @@ -477,7 +477,6 @@ namespace System.Management.Automation /// The assemblies that have been loaded for this runspace. /// internal Dictionary AssemblyCache { get; private set; } - #endregion Properties #region Engine State @@ -634,12 +633,13 @@ namespace System.Management.Automation /// /// The name of the command to lookup. /// + /// /// The command processor object. - internal CommandProcessorBase CreateCommand(string command, bool dotSource) + internal CommandProcessorBase CreateCommand(string command, bool dotSource, bool forCompletion = false) { CommandOrigin commandOrigin = this.EngineSessionState.CurrentScope.ScopeOrigin; CommandProcessorBase commandProcessor = - CommandDiscovery.LookupCommandProcessor(command, commandOrigin, !dotSource); + CommandDiscovery.LookupCommandProcessor(command, commandOrigin, !dotSource, forCompletion); // Reset the command origin for script commands... // BUGBUG - dotting can get around command origin checks??? if (commandProcessor != null && commandProcessor is ScriptCommandProcessorBase) { diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 301fccc031..f07de8b14a 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -1783,6 +1783,36 @@ class InheritedClassTest : System.Attribute } } + Context "Script parameter completion" { + BeforeAll { + Setup -File -Path 'ModuleReqTest.ps1' -Content @' +#requires -Modules ThisModuleDoesNotExist +param ($Param1) +'@ + Setup -File -Path 'AdminReqTest.ps1' -Content @' +#requires -RunAsAdministrator +param ($Param1) +'@ + Push-Location ${TestDrive}\ + } + + AfterAll { + Pop-Location + } + + It "Input should successfully complete script parameter for script with failed script requirements" { + $res = TabExpansion2 -inputScript '.\ModuleReqTest.ps1 -' + $res.CompletionMatches.Count | Should -BeGreaterThan 0 + $res.CompletionMatches[0].CompletionText | Should -BeExactly '-Param1' + } + + It "Input should successfully complete script parameter for admin script while not elevated" { + $res = TabExpansion2 -inputScript '.\AdminReqTest.ps1 -' + $res.CompletionMatches.Count | Should -BeGreaterThan 0 + $res.CompletionMatches[0].CompletionText | Should -BeExactly '-Param1' + } + } + Context "File name completion" { BeforeAll { $tempDir = Join-Path -Path $TestDrive -ChildPath "baseDir"