Skip to content

Allow explicitly specified named parameter to supersede the same one from hashtable splatting #13162

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

Merged
iSazonov merged 10 commits into from
Aug 5, 2020
Merged

Allow explicitly specified named parameter to supersede the same one from hashtable splatting #13162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
72a9a3a
Splatting initial commit
daxian-dbw Jul 4, 2020
252bd05
Make explicitly specified named parameter supersede the name paramete…
daxian-dbw Jul 4, 2020
1281217
Generalize to simple functions
daxian-dbw Jul 4, 2020
b06caf8
Make it work for hashtable splatting that happens in 'GetPowerShell'
daxian-dbw Jul 10, 2020
7a3e789
Add tests
daxian-dbw Jul 12, 2020
8adc77e
Add more tests for steppable pipeline
daxian-dbw Aug 2, 2020
4af4617
Address Rob's comment
daxian-dbw Aug 3, 2020
40dc192
Address Paul's comments
daxian-dbw Aug 3, 2020
1ed1df4
Address Ilya's comments
daxian-dbw Aug 4, 2020
31e431a
Address Ilya's comment on '-BeOffType'
daxian-dbw Aug 4, 2020
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
175 changes: 40 additions & 135 deletions src/System.Management.Automation/engine/CmdletParameterBinderController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,7 @@ internal void BindCommandLineParametersNoValidation(Collection<CommandParameterI
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();
Expand All @@ -232,7 +226,7 @@ internal void BindCommandLineParametersNoValidation(Collection<CommandParameterI
_commandMetadata.Name))
{
// Bind the actual arguments
UnboundArguments = BindParameters(_currentParameterSetFlag, this.UnboundArguments);
UnboundArguments = BindNamedParameters(_currentParameterSetFlag, this.UnboundArguments);
}

ParameterBindingException reportedBindingException;
Expand Down Expand Up @@ -1064,134 +1058,56 @@ private bool RestoreParameter(CommandParameterInternal argumentToBind, MergedCom
}

/// <summary>
/// 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.
/// </summary>
/// <param name="parameterSets">
/// The parameter set used to bind the arguments.
/// </param>
/// <param name="arguments">
/// The arguments that should be attempted to bind to the parameters of the specified
/// parameter binder.
/// </param>
/// <exception cref="ParameterBindingException">
/// 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.
/// </exception>
private Collection<CommandParameterInternal> BindParameters(uint parameterSets, Collection<CommandParameterInternal> arguments)
protected override void BindNamedParameter(
Comment on lines +1062 to +1064
Copy link
Collaborator

@iSazonov iSazonov Aug 5, 2020

Choose a reason for hiding this comment

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

(As side note, there is a full mess with 'bind the argument' vs 'bind the parameter' in comments and method names.)

Copy link
Member Author

@daxian-dbw daxian-dbw Aug 5, 2020

Choose a reason for hiding this comment

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

When talking about parameter binding, it should be bind an argument to a parameter, an argument is bound (to a parameter), a parameter is bound (by an argument).

uint parameterSets,
CommandParameterInternal argument,
MergedCompiledCommandParameter parameter)
{
Collection<CommandParameterInternal> result = new Collection<CommandParameterInternal>();

foreach (CommandParameterInternal argument in arguments)
if ((parameter.Parameter.ParameterSetFlags & parameterSets) == 0 &&
!parameter.Parameter.IsInAllSets)
{
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 it
// might match up to a dynamic parameter
string parameterSetName = BindableParameters.GetParameterSetName(parameterSets);

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));

// 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));

// 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;
}
parameterType: null,
typeSpecified: null,
ParameterBinderStrings.ParameterNotInParameterSet,
"ParameterNotInParameterSet",
parameterSetName);

ThrowElaboratedBindingException(pbex);
}
}
else if (argument.ParameterName.Equals(Parser.VERBATIM_PARAMETERNAME, StringComparison.Ordinal))
// Might be caused by default parameter binding
if (!DefaultParameterBindingInUse)
{
// 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);
}
}

/// <summary>
Expand Down Expand Up @@ -1259,17 +1175,6 @@ private static bool IsParameterScriptBlockBindable(MergedCompiledCommandParamete
return result;
}

/// <summary>
/// Binds the specified parameters to the cmdlet.
/// </summary>
/// <param name="parameters">
/// The parameters to bind.
/// </param>
internal override Collection<CommandParameterInternal> BindParameters(Collection<CommandParameterInternal> parameters)
{
return BindParameters(uint.MaxValue, parameters);
}

/// <summary>
/// Binds the specified argument to the specified parameter using the appropriate
/// parameter binder. If the argument is of type ScriptBlock and the parameter takes
Expand Down Expand Up @@ -1815,7 +1720,7 @@ private void HandleCommandLineDynamicParameters(out ParameterBindingException ou

ReparseUnboundArguments();

UnboundArguments = BindParameters(_currentParameterSetFlag, UnboundArguments);
UnboundArguments = BindNamedParameters(_currentParameterSetFlag, UnboundArguments);
}

using (ParameterBinderBase.bindingTracer.TraceScope(
Expand Down
20 changes: 13 additions & 7 deletions src/System.Management.Automation/engine/CommandParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ private class Argument
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;

/// <summary>
/// Gets and sets the string that represents parameter name, which does not include the '-' (dash).
Expand Down Expand Up @@ -111,7 +114,7 @@ internal object ArgumentValue
/// <summary>
/// If an argument was specified and is to be splatted, returns true, otherwise false.
/// </summary>
internal bool ArgumentSplatted
internal bool ArgumentToBeSplatted
{
get { return _argument != null ? _argument.splatted : false; }
}
Expand Down Expand Up @@ -201,19 +204,22 @@ internal static CommandParameterInternal CreateArgument(
/// <param name="argumentAst">The ast of the argument value in the script.</param>
/// <param name="value">The argument value.</param>
/// <param name="spaceAfterParameter">Used in native commands to correctly handle -foo:bar vs. -foo: bar.</param>
/// <param name="fromSplatting">Indicate if this parameter-argument pair comes from splatting.</param>
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,
};
}

Expand Down
18 changes: 0 additions & 18 deletions src/System.Management.Automation/engine/MinishellParameterBinderController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,6 @@ internal MinishellParameterBinderController(

#endregion ctor

/// <summary>
/// Override of parent class which should not be used.
/// </summary>
/// <param name="parameters">
/// The parameters to bind.
/// </param>
/// <remarks>
/// For any parameters that do not have a name, they are added to the command
/// line arguments for the command
/// </remarks>
internal override
Collection<CommandParameterInternal>
BindParameters(Collection<CommandParameterInternal> parameters)
{
Dbg.Assert(false, "this method should be used");
return null;
}

/// <summary>
/// Value of input format. This property should be read after binding of parameters.
/// </summary>
Expand Down
Loading