From 63cf0c330c27c58b272ae0721359af725282d517 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 4 Aug 2020 22:59:53 -0700 Subject: [PATCH] Allow explicitly specified named parameter to supersede the same one from hashtable splatting (#13162) Allow explicitly specified named parameter to supersede the same one from hashtable splatting. The work is done in parameter binder, so that parameters can be resolved to cover a parameter's official name, alias name, and unambiguous partial prefix name. The changes covers covers Hashtable splatting in 3 scenarios: - Cmdlet or advanced script invocation; - Simple function invocation; - ScriptBlock.GetPowerShell(...), where the script block contains command invocation only and uses Hashtable splatting. Some code refactoring is done to ParameterBinderController to avoid redundant code being duplicated in CmdletParameterBinderController and ScriptParameterBinderController. --- .../engine/CmdletParameterBinderController.cs | 175 ++------- .../engine/CommandParameter.cs | 20 +- .../MinishellParameterBinderController.cs | 18 - .../engine/ParameterBinderController.cs | 150 ++++++- .../engine/hostifaces/PSCommand.cs | 20 + .../engine/hostifaces/Parameter.cs | 71 ++-- .../engine/hostifaces/PowerShell.cs | 18 + .../engine/parser/Compiler.cs | 3 +- .../engine/runtime/Operations/MiscOps.cs | 11 +- .../engine/runtime/ScriptBlockToPowerShell.cs | 2 +- .../engine/scriptparameterbindercontroller.cs | 79 +--- .../ParameterBinding/Splatting.Tests.ps1 | 370 ++++++++++++++++++ 12 files changed, 646 insertions(+), 291 deletions(-) create mode 100644 test/powershell/engine/ParameterBinding/Splatting.Tests.ps1 diff --git a/src/System.Management.Automation/engine/CmdletParameterBinderController.cs b/src/System.Management.Automation/engine/CmdletParameterBinderController.cs index 069201589a..4e28630243 100644 --- a/src/System.Management.Automation/engine/CmdletParameterBinderController.cs +++ b/src/System.Management.Automation/engine/CmdletParameterBinderController.cs @@ -207,13 +207,7 @@ namespace System.Management.Automation psCompiledScriptCmdlet.PrepareForBinding(this.CommandLineParameters); } - // Add the passed in arguments to the unboundArguments collection - - foreach (CommandParameterInternal argument in arguments) - { - UnboundArguments.Add(argument); - } - + InitUnboundArguments(arguments); CommandMetadata cmdletMetadata = _commandMetadata; // Clear the warningSet at the beginning. _warningSet.Clear(); @@ -232,7 +226,7 @@ namespace System.Management.Automation _commandMetadata.Name)) { // Bind the actual arguments - UnboundArguments = BindParameters(_currentParameterSetFlag, this.UnboundArguments); + UnboundArguments = BindNamedParameters(_currentParameterSetFlag, this.UnboundArguments); } ParameterBindingException reportedBindingException; @@ -1064,134 +1058,56 @@ namespace System.Management.Automation } /// - /// Binds the actual arguments to only the formal parameters - /// for only the parameters in the specified parameter set. + /// Validate the given named parameter against the specified parameter set, + /// and then bind the argument to the parameter. /// - /// - /// The parameter set used to bind the arguments. - /// - /// - /// The arguments that should be attempted to bind to the parameters of the specified - /// parameter binder. - /// - /// - /// if multiple parameters are found matching the name. - /// or - /// if no match could be found. - /// or - /// If argument transformation fails. - /// or - /// The argument could not be coerced to the appropriate type for the parameter. - /// or - /// The parameter argument transformation, prerequisite, or validation failed. - /// or - /// If the binding to the parameter fails. - /// - private Collection BindParameters(uint parameterSets, Collection arguments) + protected override void BindNamedParameter( + uint parameterSets, + CommandParameterInternal argument, + MergedCompiledCommandParameter parameter) { - Collection result = new Collection(); - - foreach (CommandParameterInternal argument in arguments) + if ((parameter.Parameter.ParameterSetFlags & parameterSets) == 0 && + !parameter.Parameter.IsInAllSets) { - if (!argument.ParameterNameSpecified) - { - result.Add(argument); - continue; - } + string parameterSetName = BindableParameters.GetParameterSetName(parameterSets); - // We don't want to throw an exception yet because - // the parameter might be a positional argument or it - // might match up to a dynamic parameter - - MergedCompiledCommandParameter parameter = - BindableParameters.GetMatchingParameter( + ParameterBindingException bindingException = + new ParameterBindingException( + ErrorCategory.InvalidArgument, + this.Command.MyInvocation, + errorPosition: null, argument.ParameterName, - false, true, - new InvocationInfo(this.InvocationInfo.MyCommand, argument.ParameterExtent)); + parameterType: null, + typeSpecified: null, + ParameterBinderStrings.ParameterNotInParameterSet, + "ParameterNotInParameterSet", + parameterSetName); - // If the parameter is not in the specified parameter set, - // throw a binding exception - - if (parameter != null) + // Might be caused by default parameter binding + if (!DefaultParameterBindingInUse) { - // Now check to make sure it hasn't already been - // bound by looking in the boundParameters collection - - if (BoundParameters.ContainsKey(parameter.Parameter.Name)) - { - ParameterBindingException bindingException = - new ParameterBindingException( - ErrorCategory.InvalidArgument, - this.InvocationInfo, - GetParameterErrorExtent(argument), - argument.ParameterName, - null, - null, - ParameterBinderStrings.ParameterAlreadyBound, - nameof(ParameterBinderStrings.ParameterAlreadyBound)); - - // Multiple values assigned to the same parameter. - // Not caused by default parameter binding - throw bindingException; - } - - if ((parameter.Parameter.ParameterSetFlags & parameterSets) == 0 && - !parameter.Parameter.IsInAllSets) - { - string parameterSetName = BindableParameters.GetParameterSetName(parameterSets); - - ParameterBindingException bindingException = - new ParameterBindingException( - ErrorCategory.InvalidArgument, - this.Command.MyInvocation, - null, - argument.ParameterName, - null, - null, - ParameterBinderStrings.ParameterNotInParameterSet, - "ParameterNotInParameterSet", - parameterSetName); - - // Might be caused by default parameter binding - if (!DefaultParameterBindingInUse) - { - throw bindingException; - } - else - { - ThrowElaboratedBindingException(bindingException); - } - } - - try - { - BindParameter(parameterSets, argument, parameter, - ParameterBindingFlags.ShouldCoerceType | ParameterBindingFlags.DelayBindScriptBlock); - } - catch (ParameterBindingException pbex) - { - if (!DefaultParameterBindingInUse) - { - throw; - } - - ThrowElaboratedBindingException(pbex); - } - } - else if (argument.ParameterName.Equals(Parser.VERBATIM_PARAMETERNAME, StringComparison.Ordinal)) - { - // We sometimes send a magic parameter from a remote machine with the values referenced via - // a using expression ($using:x). We then access these values via PSBoundParameters, so - // "bind" them here. - DefaultParameterBinder.CommandLineParameters.SetImplicitUsingParameters(argument.ArgumentValue); + throw bindingException; } else { - result.Add(argument); + ThrowElaboratedBindingException(bindingException); } } - return result; + try + { + BindParameter(parameterSets, argument, parameter, + ParameterBindingFlags.ShouldCoerceType | ParameterBindingFlags.DelayBindScriptBlock); + } + catch (ParameterBindingException pbex) + { + if (!DefaultParameterBindingInUse) + { + throw; + } + + ThrowElaboratedBindingException(pbex); + } } /// @@ -1259,17 +1175,6 @@ namespace System.Management.Automation return result; } - /// - /// Binds the specified parameters to the cmdlet. - /// - /// - /// The parameters to bind. - /// - internal override Collection BindParameters(Collection parameters) - { - return BindParameters(uint.MaxValue, parameters); - } - /// /// Binds the specified argument to the specified parameter using the appropriate /// parameter binder. If the argument is of type ScriptBlock and the parameter takes @@ -1815,7 +1720,7 @@ namespace System.Management.Automation ReparseUnboundArguments(); - UnboundArguments = BindParameters(_currentParameterSetFlag, UnboundArguments); + UnboundArguments = BindNamedParameters(_currentParameterSetFlag, UnboundArguments); } using (ParameterBinderBase.bindingTracer.TraceScope( diff --git a/src/System.Management.Automation/engine/CommandParameter.cs b/src/System.Management.Automation/engine/CommandParameter.cs index 0ee5f7855f..b13cc94ad4 100644 --- a/src/System.Management.Automation/engine/CommandParameter.cs +++ b/src/System.Management.Automation/engine/CommandParameter.cs @@ -29,14 +29,17 @@ namespace System.Management.Automation private Parameter _parameter; private Argument _argument; private bool _spaceAfterParameter; + private bool _fromHashtableSplatting; - internal bool SpaceAfterParameter { get { return _spaceAfterParameter; } } + internal bool SpaceAfterParameter => _spaceAfterParameter; - internal bool ParameterNameSpecified { get { return _parameter != null; } } + internal bool ParameterNameSpecified => _parameter != null; - internal bool ArgumentSpecified { get { return _argument != null; } } + internal bool ArgumentSpecified => _argument != null; - internal bool ParameterAndArgumentSpecified { get { return ParameterNameSpecified && ArgumentSpecified; } } + internal bool ParameterAndArgumentSpecified => ParameterNameSpecified && ArgumentSpecified; + + internal bool FromHashtableSplatting => _fromHashtableSplatting; /// /// Gets and sets the string that represents parameter name, which does not include the '-' (dash). @@ -111,7 +114,7 @@ namespace System.Management.Automation /// /// If an argument was specified and is to be splatted, returns true, otherwise false. /// - internal bool ArgumentSplatted + internal bool ArgumentToBeSplatted { get { return _argument != null ? _argument.splatted : false; } } @@ -201,19 +204,22 @@ namespace System.Management.Automation /// The ast of the argument value in the script. /// The argument value. /// Used in native commands to correctly handle -foo:bar vs. -foo: bar. + /// Indicate if this parameter-argument pair comes from splatting. internal static CommandParameterInternal CreateParameterWithArgument( Ast parameterAst, string parameterName, string parameterText, Ast argumentAst, object value, - bool spaceAfterParameter) + bool spaceAfterParameter, + bool fromSplatting = false) { return new CommandParameterInternal { _parameter = new Parameter { ast = parameterAst, parameterName = parameterName, parameterText = parameterText }, _argument = new Argument { ast = argumentAst, value = value }, - _spaceAfterParameter = spaceAfterParameter + _spaceAfterParameter = spaceAfterParameter, + _fromHashtableSplatting = fromSplatting, }; } diff --git a/src/System.Management.Automation/engine/MinishellParameterBinderController.cs b/src/System.Management.Automation/engine/MinishellParameterBinderController.cs index a85b462abb..4bf375e7c9 100644 --- a/src/System.Management.Automation/engine/MinishellParameterBinderController.cs +++ b/src/System.Management.Automation/engine/MinishellParameterBinderController.cs @@ -37,24 +37,6 @@ namespace System.Management.Automation #endregion ctor - /// - /// Override of parent class which should not be used. - /// - /// - /// The parameters to bind. - /// - /// - /// For any parameters that do not have a name, they are added to the command - /// line arguments for the command - /// - internal override - Collection - BindParameters(Collection parameters) - { - Dbg.Assert(false, "this method should be used"); - return null; - } - /// /// Value of input format. This property should be read after binding of parameters. /// diff --git a/src/System.Management.Automation/engine/ParameterBinderController.cs b/src/System.Management.Automation/engine/ParameterBinderController.cs index a68192c7a9..17e3f07656 100644 --- a/src/System.Management.Automation/engine/ParameterBinderController.cs +++ b/src/System.Management.Automation/engine/ParameterBinderController.cs @@ -130,7 +130,7 @@ namespace System.Management.Automation /// Or /// The name of the argument matches more than one parameter. /// - internal void ReparseUnboundArguments() + protected void ReparseUnboundArguments() { Collection result = new Collection(); @@ -260,6 +260,34 @@ namespace System.Management.Automation UnboundArguments = result; } + protected void InitUnboundArguments(Collection arguments) + { + // Add the passed in arguments to the unboundArguments collection + Collection paramsFromSplatting = null; + foreach (CommandParameterInternal argument in arguments) + { + if (argument.FromHashtableSplatting) + { + paramsFromSplatting ??= new Collection(); + paramsFromSplatting.Add(argument); + } + else + { + UnboundArguments.Add(argument); + } + } + + // Move the arguments from hashtable splatting to the end of the unbound args list, so that + // the explicitly specified named arguments can supersede those from a hashtable splatting. + if (paramsFromSplatting != null) + { + foreach (CommandParameterInternal argument in paramsFromSplatting) + { + UnboundArguments.Add(argument); + } + } + } + private static bool IsSwitchAndSetValue( string argumentName, CommandParameterInternal argument, @@ -447,7 +475,10 @@ namespace System.Management.Automation /// /// The arguments which are still not bound. /// - internal abstract Collection BindParameters(Collection parameters); + internal virtual Collection BindParameters(Collection parameters) + { + throw new NotImplementedException(); + } /// /// Bind the argument to the specified parameter. @@ -514,6 +545,121 @@ namespace System.Management.Automation return result; } + /// + /// This is used by to validate and bind a given named parameter. + /// + protected virtual void BindNamedParameter( + uint parameterSets, + CommandParameterInternal argument, + MergedCompiledCommandParameter parameter) + { + BindParameter(parameterSets, argument, parameter, ParameterBindingFlags.ShouldCoerceType); + } + + /// + /// Bind the named parameters from the specified argument collection, + /// for only the parameters in the specified parameter set. + /// + /// + /// The parameter set used to bind the arguments. + /// + /// + /// The arguments that should be attempted to bind to the parameters of the specified parameter binder. + /// + /// + /// if multiple parameters are found matching the name. + /// or + /// if no match could be found. + /// or + /// If argument transformation fails. + /// or + /// The argument could not be coerced to the appropriate type for the parameter. + /// or + /// The parameter argument transformation, prerequisite, or validation failed. + /// or + /// If the binding to the parameter fails. + /// + protected Collection BindNamedParameters(uint parameterSets, Collection arguments) + { + Collection result = new Collection(); + HashSet boundExplicitNamedParams = null; + + foreach (CommandParameterInternal argument in arguments) + { + if (!argument.ParameterNameSpecified) + { + result.Add(argument); + continue; + } + + // We don't want to throw an exception yet because the parameter might be a positional argument, + // or in case of a cmdlet or an advanced function, it might match up to a dynamic parameter. + MergedCompiledCommandParameter parameter = + BindableParameters.GetMatchingParameter( + name: argument.ParameterName, + throwOnParameterNotFound: false, + tryExactMatching: true, + invocationInfo: new InvocationInfo(this.InvocationInfo.MyCommand, argument.ParameterExtent)); + + // If the parameter is not in the specified parameter set, throw a binding exception + if (parameter != null) + { + string formalParamName = parameter.Parameter.Name; + + if (argument.FromHashtableSplatting) + { + boundExplicitNamedParams ??= new HashSet( + BoundParameters.Keys, + StringComparer.OrdinalIgnoreCase); + + if (boundExplicitNamedParams.Contains(formalParamName)) + { + // This named parameter from splatting is also explicitly specified by the user, + // which was successfully bound, so we ignore the one from splatting because it + // is superceded by the explicit one. For example: + // $splat = @{ Path = $path1 } + // dir @splat -Path $path2 + continue; + } + } + + // Now check to make sure it hasn't already been + // bound by looking in the boundParameters collection + + if (BoundParameters.ContainsKey(formalParamName)) + { + ParameterBindingException bindingException = + new ParameterBindingException( + ErrorCategory.InvalidArgument, + this.InvocationInfo, + GetParameterErrorExtent(argument), + argument.ParameterName, + null, + null, + ParameterBinderStrings.ParameterAlreadyBound, + nameof(ParameterBinderStrings.ParameterAlreadyBound)); + + throw bindingException; + } + + BindNamedParameter(parameterSets, argument, parameter); + } + else if (argument.ParameterName.Equals(Parser.VERBATIM_PARAMETERNAME, StringComparison.Ordinal)) + { + // We sometimes send a magic parameter from a remote machine with the values referenced via + // a using expression ($using:x). We then access these values via PSBoundParameters, so + // "bind" them here. + DefaultParameterBinder.CommandLineParameters.SetImplicitUsingParameters(argument.ArgumentValue); + } + else + { + result.Add(argument); + } + } + + return result; + } + /// /// Binds the unbound arguments to positional parameters. /// diff --git a/src/System.Management.Automation/engine/hostifaces/PSCommand.cs b/src/System.Management.Automation/engine/hostifaces/PSCommand.cs index 781e53eaf5..77bf704f38 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSCommand.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSCommand.cs @@ -359,6 +359,26 @@ namespace System.Management.Automation return this; } + /// + /// Adds a instance to the last added command. + /// + internal PSCommand AddParameter(CommandParameter parameter) + { + if (_currentCommand == null) + { + throw PSTraceSource.NewInvalidOperationException(PSCommandStrings.ParameterRequiresCommand, + new object[] { "PSCommand" }); + } + + if (_owner != null) + { + _owner.AssertChangesAreAccepted(); + } + + _currentCommand.Parameters.Add(parameter); + return this; + } + /// /// Adds an argument to the last added command. /// For example, to construct a command string "get-process | select-object name" diff --git a/src/System.Management.Automation/engine/hostifaces/Parameter.cs b/src/System.Management.Automation/engine/hostifaces/Parameter.cs index 3f7d072318..725cc93e02 100644 --- a/src/System.Management.Automation/engine/hostifaces/Parameter.cs +++ b/src/System.Management.Automation/engine/hostifaces/Parameter.cs @@ -80,9 +80,10 @@ namespace System.Management.Automation.Runspaces #endregion Public properties - #region Private Fields - - #endregion Private Fields + /// + /// Gets whether the parameter was from splatting a Hashtable. + /// + private bool FromHashtableSplatting { get; set; } #region Conversion from and to CommandParameterInternal @@ -108,17 +109,14 @@ namespace System.Management.Automation.Runspaces Diagnostics.Assert(name.Trim().Length != 1, "Parameter name has to have some non-whitespace characters in it"); } - if (internalParameter.ParameterAndArgumentSpecified) - { - return new CommandParameter(name, internalParameter.ArgumentValue); - } + CommandParameter result = internalParameter.ParameterAndArgumentSpecified + ? new CommandParameter(name, internalParameter.ArgumentValue) + : name != null + ? new CommandParameter(name) + : new CommandParameter(name: null, internalParameter.ArgumentValue); - if (name != null) // either a switch parameter or first part of parameter+argument - { - return new CommandParameter(name); - } - // either a positional argument or second part of parameter+argument - return new CommandParameter(null, internalParameter.ArgumentValue); + result.FromHashtableSplatting = internalParameter.FromHashtableSplatting; + return result; } internal static CommandParameterInternal ToCommandParameterInternal(CommandParameter publicParameter, bool forNativeCommand) @@ -143,9 +141,12 @@ namespace System.Management.Automation.Runspaces { parameterText = forNativeCommand ? name : "-" + name; return CommandParameterInternal.CreateParameterWithArgument( - /*parameterAst*/null, name, parameterText, - /*argumentAst*/null, value, - true); + parameterAst: null, + parameterName: name, + parameterText: parameterText, + argumentAst: null, + value: value, + spaceAfterParameter: true); } // if first character of name is '-', then we try to fake the original token @@ -184,9 +185,13 @@ namespace System.Management.Automation.Runspaces // name+value pair return CommandParameterInternal.CreateParameterWithArgument( - /*parameterAst*/null, parameterName, parameterText, - /*argumentAst*/null, value, - spaceAfterParameter); + parameterAst: null, + parameterName, + parameterText, + argumentAst: null, + value, + spaceAfterParameter, + publicParameter.FromHashtableSplatting); } #endregion @@ -233,34 +238,6 @@ namespace System.Management.Automation.Runspaces } #endregion - - #region Win Blue Extensions - -#if !CORECLR // PSMI Not Supported On CSS - internal CimInstance ToCimInstance() - { - CimInstance c = InternalMISerializer.CreateCimInstance("PS_Parameter"); - CimProperty nameProperty = InternalMISerializer.CreateCimProperty("Name", this.Name, - Microsoft.Management.Infrastructure.CimType.String); - c.CimInstanceProperties.Add(nameProperty); - Microsoft.Management.Infrastructure.CimType cimType = CimConverter.GetCimType(this.Value.GetType()); - CimProperty valueProperty; - if (cimType == Microsoft.Management.Infrastructure.CimType.Unknown) - { - valueProperty = InternalMISerializer.CreateCimProperty("Value", (object)PSMISerializer.Serialize(this.Value), - Microsoft.Management.Infrastructure.CimType.Instance); - } - else - { - valueProperty = InternalMISerializer.CreateCimProperty("Value", this.Value, cimType); - } - - c.CimInstanceProperties.Add(valueProperty); - return c; - } -#endif - - #endregion Win Blue Extensions } /// diff --git a/src/System.Management.Automation/engine/hostifaces/PowerShell.cs b/src/System.Management.Automation/engine/hostifaces/PowerShell.cs index ba559a5e1f..e5015a010a 100644 --- a/src/System.Management.Automation/engine/hostifaces/PowerShell.cs +++ b/src/System.Management.Automation/engine/hostifaces/PowerShell.cs @@ -1266,6 +1266,24 @@ namespace System.Management.Automation } } + /// + /// Adds a instance to the last added command. + /// + internal PowerShell AddParameter(CommandParameter parameter) + { + lock (_syncObject) + { + if (_psCommand.Commands.Count == 0) + { + throw PSTraceSource.NewInvalidOperationException(PowerShellStrings.ParameterRequiresCommand); + } + + AssertChangesAreAccepted(); + _psCommand.AddParameter(parameter); + return this; + } + } + /// /// Adds a set of parameters to the last added command. /// diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index c2a4bb5f16..3a25e8c5d9 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -4241,7 +4241,8 @@ namespace System.Management.Automation.Language Expression.Constant(errorPos.Text), Expression.Constant(arg), Expression.Convert(GetCommandArgumentExpression(arg), typeof(object)), - ExpressionCache.Constant(spaceAfterParameter)); + ExpressionCache.Constant(spaceAfterParameter), + ExpressionCache.Constant(false)); } return Expression.Call( diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index edc7e6797d..a8f58d9dee 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -174,7 +174,7 @@ namespace System.Management.Automation } } - if (cpi.ArgumentSplatted) + if (cpi.ArgumentToBeSplatted) { foreach (var splattedCpi in Splat(cpi.ArgumentValue, cpi.ArgumentAst)) { @@ -338,8 +338,13 @@ namespace System.Management.Automation } yield return CommandParameterInternal.CreateParameterWithArgument( - splatAst, parameterName, parameterText, - splatAst, parameterValue, false); + parameterAst: splatAst, + parameterName: parameterName, + parameterText: parameterText, + argumentAst: splatAst, + value: parameterValue, + spaceAfterParameter: false, + fromSplatting: true); } } else diff --git a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs index e2409eba62..4e6d81e920 100644 --- a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs +++ b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs @@ -748,7 +748,7 @@ namespace System.Management.Automation foreach (var splattedParameter in PipelineOps.Splat(splattedValue, variableAst)) { CommandParameter publicParameter = CommandParameter.FromCommandParameterInternal(splattedParameter); - _powershell.AddParameter(publicParameter.Name, publicParameter.Value); + _powershell.AddParameter(publicParameter); } } diff --git a/src/System.Management.Automation/engine/scriptparameterbindercontroller.cs b/src/System.Management.Automation/engine/scriptparameterbindercontroller.cs index 9a63b1969a..9f45fa4227 100644 --- a/src/System.Management.Automation/engine/scriptparameterbindercontroller.cs +++ b/src/System.Management.Automation/engine/scriptparameterbindercontroller.cs @@ -77,16 +77,10 @@ namespace System.Management.Automation internal void BindCommandLineParameters(Collection arguments) { // Add the passed in arguments to the unboundArguments collection - - foreach (CommandParameterInternal argument in arguments) - { - UnboundArguments.Add(argument); - } - + InitUnboundArguments(arguments); ReparseUnboundArguments(); - // To support named parameters you just have un-comment the following line - UnboundArguments = BindParameters(UnboundArguments); + UnboundArguments = BindNamedParameters(uint.MaxValue, UnboundArguments); ParameterBindingException parameterBindingError; UnboundArguments = @@ -136,75 +130,6 @@ namespace System.Management.Automation return true; } - /// - /// Binds the specified parameters to the shell function. - /// - /// - /// The arguments to bind. - /// - internal override Collection BindParameters(Collection arguments) - { - Collection result = new Collection(); - - foreach (CommandParameterInternal argument in arguments) - { - if (!argument.ParameterNameSpecified) - { - result.Add(argument); - continue; - } - - // We don't want to throw an exception yet because - // the parameter might be a positional argument - - MergedCompiledCommandParameter parameter = - BindableParameters.GetMatchingParameter( - argument.ParameterName, - false, true, - new InvocationInfo(this.InvocationInfo.MyCommand, argument.ParameterExtent)); - - // If the parameter is not in the specified parameter set, - // throw a binding exception - - if (parameter != null) - { - // Now check to make sure it hasn't already been - // bound by looking in the boundParameters collection - - if (BoundParameters.ContainsKey(parameter.Parameter.Name)) - { - ParameterBindingException bindingException = - new ParameterBindingException( - ErrorCategory.InvalidArgument, - this.InvocationInfo, - GetParameterErrorExtent(argument), - argument.ParameterName, - null, - null, - ParameterBinderStrings.ParameterAlreadyBound, - nameof(ParameterBinderStrings.ParameterAlreadyBound)); - - throw bindingException; - } - - BindParameter(uint.MaxValue, argument, parameter, ParameterBindingFlags.ShouldCoerceType); - } - else if (argument.ParameterName.Equals(Language.Parser.VERBATIM_PARAMETERNAME, StringComparison.Ordinal)) - { - // We sometimes send a magic parameter from a remote machine with the values referenced via - // a using expression ($using:x). We then access these values via PSBoundParameters, so - // "bind" them here. - DefaultParameterBinder.CommandLineParameters.SetImplicitUsingParameters(argument.ArgumentValue); - } - else - { - result.Add(argument); - } - } - - return result; - } - /// /// Takes the remaining arguments that haven't been bound, and binds /// them to $args. diff --git a/test/powershell/engine/ParameterBinding/Splatting.Tests.ps1 b/test/powershell/engine/ParameterBinding/Splatting.Tests.ps1 new file mode 100644 index 0000000000..51e8be6755 --- /dev/null +++ b/test/powershell/engine/ParameterBinding/Splatting.Tests.ps1 @@ -0,0 +1,370 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe "Hashtable Splatting Parameter Binding Tests" -Tags "CI" { + + BeforeAll { + function SimpleTest { + param( + [Alias('Key')] + $Name, + $Path + ) + + "Key: $Name; Path: $Path; Args: $args" + } + } + + Context "Basic Hashtable Splatting" { + + It "works on cmdlet" { + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + Get-Verb @hash > $null + $zoo | Should -BeOfType System.Management.Automation.VerbInfo + $zoo.Verb | Should -BeExactly 'Get' + } + + It "works on simple function" { + $hash = @{ Name = "Hello"; Blah = "World" } + SimpleTest @hash | Should -BeExactly 'Key: Hello; Path: ; Args: -Blah: World' + + $hash = @{ Name = "Hello"; Path = "World" } + SimpleTest @hash | Should -BeExactly 'Key: Hello; Path: World; Args: ' + + $hash = @{ Name = "Hello" } + SimpleTest @hash -Path "Yeah" | Should -BeExactly 'Key: Hello; Path: Yeah; Args: ' + SimpleTest -Path "Yeah" @hash | Should -BeExactly 'Key: Hello; Path: Yeah; Args: ' + + $hash = @{ Key = "Hello" } + SimpleTest @hash | Should -BeExactly 'Key: Hello; Path: ; Args: ' + } + + It "works on ScriptBlock.GetPowerShell" { + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + $ps = { param($hash) Get-Verb @hash; Get-Variable zoo }.GetPowerShell($hash) + + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Get' + } finally { + $ps.Dispose() + } + } + + It "works on steppable pipeline" { + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + $sp = { Get-Verb @hash }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Get' + $zoo | Should -BeOfType System.Management.Automation.VerbInfo + $zoo.Verb | Should -BeExactly 'Get' + + $sp.End() + } finally { + $sp.Dispose() + } + } + } + + Context "Explicitly specified named parameter supersedes the same one in Hashtable splatting" { + + It "works with the same parameter name" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + Get-Verb @hash -Verb "Send" > $null + $zoo | Should -BeOfType System.Management.Automation.VerbInfo + $zoo.Verb | Should -BeExactly "Send" + + $zoo = $null + Get-Verb -Verb "Send" @hash > $null + $zoo | Should -BeOfType System.Management.Automation.VerbInfo + $zoo.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send"; Get-Variable zoo }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $zoo = $null + $sp = { Get-Verb @hash -Verb "Send" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeOfType System.Management.Automation.VerbInfo + $zoo.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ Name = "Hello"; Path = "World" } + SimpleTest @hash -Path "Yeah" | Should -BeExactly 'Key: Hello; Path: Yeah; Args: ' + SimpleTest -Path "Yeah" @hash | Should -BeExactly 'Key: Hello; Path: Yeah; Args: ' + + $hash = @{ Name = "Hello"; Blah = "World" } + SimpleTest @hash -Name "Yeah" | Should -BeExactly 'Key: Yeah; Path: ; Args: -Blah: World' + } + + It "works with the same alias name" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; ov = "zoo" } + Get-Verb @hash -Verb "Send" -ov "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -ov "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -ov "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ key = "Hello"; Path = "World" } + SimpleTest @hash -Key "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + + It "works with parameter name and alias name mixed" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + Get-Verb @hash -Verb "Send" -ov "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -ov "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -ov "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ Name = "Hello"; Path = "World" } + SimpleTest @hash -Key "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + + It "works with unambiguous prefix and parameter name mixed - prefix explicitly specified" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; OutVariable = "zoo" } + Get-Verb @hash -Verb "Send" -outv "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -outv "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -outv "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ Name = "Hello"; Path = "World" } + SimpleTest @hash -n "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + + It "works with unambiguous prefix and parameter name mixed - prefix in splatting hashtable" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; outv = "zoo" } + Get-Verb @hash -Verb "Send" -OutVariable "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -OutVariable "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -OutVariable "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ n = "Hello"; Path = "World" } + SimpleTest @hash -Name "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + + It "works with unambiguous prefix and alias name mixed - prefix explicitly specified" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; ov = "zoo" } + Get-Verb @hash -Verb "Send" -outv "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -outv "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -outv "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ key = "Hello"; Path = "World" } + SimpleTest @hash -n "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + + It "works with unambiguous prefix and alias name mixed - prefix in splatting hashtable" { + ## Regular use with cmdlet + $hash = @{ Verb = "Get"; outv = "zoo" } + Get-Verb @hash -Verb "Send" -ov "bar" > $null + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly "Send" + + ## GetPowerShell + $ps = { param($hash) Get-Verb @hash -Verb "Send" -ov "bar"; Get-Variable bar }.GetPowerShell($hash) + try { + $result = $ps.Invoke() + $result[0] | Should -BeOfType System.Management.Automation.VerbInfo + $result[0].Verb | Should -BeExactly 'Send' + } finally { + $ps.Dispose() + } + + ## Steppable pipeline + $bar = $null + $sp = { Get-Verb @hash -Verb "Send" -ov "bar" }.GetSteppablePipeline() + try { + $sp.Begin($false) + + $result = $sp.Process() + $result | Should -BeOfType System.Management.Automation.VerbInfo + $result.Verb | Should -BeExactly 'Send' + $zoo | Should -BeNullOrEmpty + $bar | Should -BeOfType System.Management.Automation.VerbInfo + $bar.Verb | Should -BeExactly 'Send' + + $sp.End() + } finally { + $sp.Dispose() + } + + ## Regular use with simple function + $hash = @{ n = "Hello"; Path = "World" } + SimpleTest @hash -key "Yeah" | Should -BeExactly 'Key: Yeah; Path: World; Args: ' + } + } +}