Skip to content
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.

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

Add support for named argument with .NET Methods #21487

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d324032
Add parsing work for named arguments
jborean93 Mar 28, 2024
3d4953d
Expand tests and fix up remaining token parser
jborean93 Mar 28, 2024
bb92583
Pass through argument names through CallInfo
jborean93 Mar 29, 2024
282756a
Pass down argument names to best method specifier
jborean93 Apr 1, 2024
48b237c
Add check for duplicate labels in parsing
jborean93 Apr 2, 2024
ad405c2
Add check to ensure named arguments always appear after other named a…
jborean93 Apr 2, 2024
4e7d2ae
Add name logic for overload candidate selection
jborean93 Apr 12, 2024
0cf0bd6
Remap args after finding best overload
jborean93 Apr 15, 2024
15211c3
Add test cases
jborean93 Apr 15, 2024
98c9bab
Slowly fix params examples - support params and default MI
jborean93 Apr 16, 2024
19ab8b9
Use new OverloadArgument setup
jborean93 Apr 16, 2024
69f39e5
Use new OverloadCandidate rules
jborean93 Apr 16, 2024
afcbd0d
Cover more edge cases and expand docs
jborean93 Apr 17, 2024
d86617d
Cleanup before PR
jborean93 Apr 17, 2024
5b7569d
Fix build for Windows
jborean93 Apr 17, 2024
340ac59
Fix index check for CallInfo that is not set
jborean93 Apr 17, 2024
3d7592f
Fix support for null argument names
jborean93 Apr 17, 2024
39c8029
Avoid test class name collision
jborean93 Apr 17, 2024
44d52a1
Fix up CallInfo ArgumentName logic
jborean93 Apr 17, 2024
49c979c
Add tests for COM
jborean93 Apr 17, 2024
9036daf
Make .NET args case insensitive and fix collision fallback
jborean93 Apr 17, 2024
aea77d3
Re-add DebuggerDisplay
jborean93 May 19, 2024
4583628
Remove stray WriteLine and rename Ast to NamedMethodArgumentAst
jborean93 Jun 11, 2024
5b05faa
Implement logic for duplicate and unnamed args
jborean93 Jul 16, 2024
6e0f7df
Fix up minor comments
jborean93 Jul 16, 2024
d733208
Parser change suggestions
MartinGC94 Apr 19, 2024
8bb80d5
Address feedback
MartinGC94 Apr 19, 2024
2721192
Remove comma token variable
MartinGC94 Apr 20, 2024
faa9281
Renamed Ast from Martins commit
jborean93 Jul 16, 2024
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
2 changes: 1 addition & 1 deletion src/System.Management.Automation/engine/COM/ComUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ internal static ParameterInformation[] GetParameterInformation(COM.FUNCDESC func
}

bool fByRef = (ElementDescription.desc.paramdesc.wParamFlags & COM.PARAMFLAG.PARAMFLAG_FOUT) != 0;
parameters[i] = new ParameterInformation(type, fOptional, defaultvalue, fByRef);
parameters[i] = new ParameterInformation(string.Empty, type, fOptional, defaultvalue, fByRef);
}

return parameters;
Expand Down
573 changes: 381 additions & 192 deletions src/System.Management.Automation/engine/CoreAdapter.cs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/System.Management.Automation/engine/ExtraAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ protected override object MethodInvoke(PSMethod method, object[] arguments)

for (int i = 0; i < arguments.Length; i++)
{
parameters[i] = new ParameterInformation(typeof(object), false, null, false);
parameters[i] = new ParameterInformation(string.Empty, typeof(object), false, null, false);
}

MethodInformation[] methodInformation = new MethodInformation[1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ internal class WMIParameterInformation : ParameterInformation
{
public string Name { get; }

public WMIParameterInformation(string name, Type ty) : base(ty, true, null, false)
public WMIParameterInformation(string name, Type ty) : base(name, ty, true, null, false)
{
Name = name;
}
Expand Down
10 changes: 10 additions & 0 deletions src/System.Management.Automation/engine/parser/AstVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ public interface ICustomAstVisitor2 : ICustomAstVisitor

/// <summary/>
object? VisitPipelineChain(PipelineChainAst statementChainAst) => DefaultVisit(statementChainAst);

/// <summary/>
object? VisitNamedMethodArgumentExpression(NamedMethodArgumentAst namedMethodArgExpression) => DefaultVisit(namedMethodArgExpression);
}
#nullable restore

Expand Down Expand Up @@ -378,6 +381,8 @@ internal AstVisitAction CheckParent(Ast ast)
public override AstVisitAction VisitTernaryExpression(TernaryExpressionAst ast) => CheckParent(ast);

public override AstVisitAction VisitPipelineChain(PipelineChainAst ast) => CheckParent(ast);

public override AstVisitAction VisitNamedMethodArgumentExpression(NamedMethodArgumentAst ast) => CheckParent(ast);
}

/// <summary>
Expand Down Expand Up @@ -614,6 +619,8 @@ protected AstVisitAction CheckScriptBlock(Ast ast)
public override AstVisitAction VisitTernaryExpression(TernaryExpressionAst ast) { return Check(ast); }

public override AstVisitAction VisitPipelineChain(PipelineChainAst ast) { return Check(ast); }

public override AstVisitAction VisitNamedMethodArgumentExpression(NamedMethodArgumentAst ast) { return Check(ast); }
}

/// <summary>
Expand Down Expand Up @@ -818,5 +825,8 @@ public abstract class DefaultCustomAstVisitor2 : DefaultCustomAstVisitor, ICusto

/// <summary/>
public virtual object VisitPipelineChain(PipelineChainAst statementChainAst) => DefaultVisit(statementChainAst);

/// <summary/>
public virtual object VisitNamedMethodArgumentExpression(NamedMethodArgumentAst argumentWithLabelAst) => DefaultVisit(argumentWithLabelAst);
}
}
61 changes: 46 additions & 15 deletions src/System.Management.Automation/engine/parser/Compiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -960,20 +960,31 @@ internal Expression CompileExpressionOperand(ExpressionAst exprAst)
return result;
}

private IEnumerable<Expression> CompileInvocationArguments(IReadOnlyList<ExpressionAst> arguments)
private (string[], IEnumerable<Expression>) CompileInvocationArguments(IReadOnlyList<ExpressionAst> arguments)
{
if (arguments is null || arguments.Count == 0)
{
return Array.Empty<Expression>();
return (Array.Empty<string>(), Array.Empty<Expression>());
}

var result = new Expression[arguments.Count];
for (int i = 0; i < result.Length; i++)
// argNames when provided to the CallInfo in left to right order
// with the last name corresponding to the last argument.
var argNames = new List<string>(arguments.Count);
var argExpressions = new Expression[arguments.Count];
for (int i = 0; i < argExpressions.Length; i++)
{
result[i] = CompileExpressionOperand(arguments[i]);
ExpressionAst value = arguments[i];

if (value is NamedMethodArgumentAst labelAst)
{
argNames.Add(labelAst.Label.Value);
value = labelAst.Expression;
}

argExpressions[i] = CompileExpressionOperand(value);
}

return result;
return (argNames.ToArray(), argExpressions);
}

internal Expression ReduceAssignment(ISupportsAssignment left, TokenKind tokenKind, Expression right)
Expand Down Expand Up @@ -3426,9 +3437,9 @@ public object VisitFunctionMember(FunctionMemberAst functionMemberAst)
public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst)
{
var target = CompileExpressionOperand(baseCtorInvokeMemberExpressionAst.Expression);
var args = CompileInvocationArguments(baseCtorInvokeMemberExpressionAst.Arguments);
(var argNames, var args) = CompileInvocationArguments(baseCtorInvokeMemberExpressionAst.Arguments);
var baseCtorCallConstraints = GetInvokeMemberConstraints(baseCtorInvokeMemberExpressionAst);
return InvokeBaseCtorMethod(baseCtorCallConstraints, target, args);
return InvokeBaseCtorMethod(baseCtorCallConstraints, target, argNames, args);
}

public object VisitUsingStatement(UsingStatementAst usingStatementAst)
Expand Down Expand Up @@ -6393,11 +6404,12 @@ internal static PSMethodInvocationConstraints GetInvokeMemberConstraints(BaseCto
PSMethodInvocationConstraints constraints,
Expression target,
IEnumerable<Expression> args,
IEnumerable<string> argNames,
bool @static,
bool propertySet,
bool nullConditional = false)
{
var callInfo = new CallInfo(args.Count());
var callInfo = new CallInfo(args.Count(), argNames);
var classScope = _memberFunctionType?.Type;
var binder = name.Equals("new", StringComparison.OrdinalIgnoreCase) && @static
? (CallSiteBinder)PSCreateInstanceBinder.Get(callInfo, constraints, publicTypeOnly: true)
Expand All @@ -6408,9 +6420,13 @@ internal static PSMethodInvocationConstraints GetInvokeMemberConstraints(BaseCto
return nullConditional ? GetNullConditionalWrappedExpression(target, dynamicExprFromBinder) : dynamicExprFromBinder;
}

private static Expression InvokeBaseCtorMethod(PSMethodInvocationConstraints constraints, Expression target, IEnumerable<Expression> args)
private static Expression InvokeBaseCtorMethod(
PSMethodInvocationConstraints constraints,
Expression target,
IEnumerable<string> argNames,
IEnumerable<Expression> args)
{
var callInfo = new CallInfo(args.Count());
var callInfo = new CallInfo(args.Count(), argNames);
var binder = PSInvokeBaseCtorBinder.Get(callInfo, constraints);
return DynamicExpression.Dynamic(binder, typeof(object), args.Prepend(target));
}
Expand All @@ -6435,7 +6451,7 @@ public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMember
var constraints = GetInvokeMemberConstraints(invokeMemberExpressionAst);

var target = CompileExpressionOperand(invokeMemberExpressionAst.Expression);
var args = CompileInvocationArguments(invokeMemberExpressionAst.Arguments);
(var argNames, var args) = CompileInvocationArguments(invokeMemberExpressionAst.Arguments);

var memberNameAst = invokeMemberExpressionAst.Member as StringConstantExpressionAst;
if (memberNameAst != null)
Expand All @@ -6445,9 +6461,10 @@ public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMember
constraints,
target,
args,
argNames,
invokeMemberExpressionAst.Static,
propertySet: false,
invokeMemberExpressionAst.NullConditional);
nullConditional: invokeMemberExpressionAst.NullConditional);
}

var memberNameExpr = Compile(invokeMemberExpressionAst.Member);
Expand Down Expand Up @@ -6767,7 +6784,14 @@ public Expression GetValue(Compiler compiler, List<Expression> exprs, List<Param
var memberNameAst = InvokeMemberExpressionAst.Member as StringConstantExpressionAst;
if (memberNameAst != null)
{
return compiler.InvokeMember(memberNameAst.Value, constraints, _targetExprTemp, _argExprTemps, @static: false, propertySet: false);
return compiler.InvokeMember(
memberNameAst.Value,
constraints,
_targetExprTemp,
_argExprTemps,
Array.Empty<string>(),
@static: false,
propertySet: false);
}

var memberNameExpr = GetMemberNameExpr(compiler);
Expand All @@ -6788,7 +6812,14 @@ public Expression SetValue(Compiler compiler, Expression rhs)
var args = GetArgumentExprs(compiler);
if (memberNameAst != null)
{
return compiler.InvokeMember(memberNameAst.Value, constraints, target, args.Append(rhs), @static: false, propertySet: true);
return compiler.InvokeMember(
memberNameAst.Value,
constraints,
target,
args.Append(rhs),
Array.Empty<string>(),
@static: false,
propertySet: true);
}

var memberNameExpr = GetMemberNameExpr(compiler);
Expand Down
68 changes: 54 additions & 14 deletions src/System.Management.Automation/engine/parser/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7891,16 +7891,18 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
// G argument-list: '(' is passed in lParen
// G argument-expression-list:opt new-lines:opt ')'
// G argument-expression-list:
// G argument-expression
// G argument-expression new-lines:opt ',' argument-expression-list
// G argument-label-expression:opt argument-expression
// G argument-label-expression:opt argument-expression new-lines:opt ',' argument-expression-list
// G argument-expression:
// G See grammar for expression - the only difference is that an
// G array-literal-expression is not allowed - the comma is used
// G to separate argument-expressions.
// G argument-label-expression:
// G simple-name ':'

List<ExpressionAst> arguments = new List<ExpressionAst>();
Token comma = null;
Token rParen = null;
Token lastValidToken = null;

bool oldDisableCommaOperator = _disableCommaOperator;
bool reportedError = false;
Expand All @@ -7912,31 +7914,60 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
while (true)
{
SkipNewlines();

StringConstantExpressionAst argumentName = SimpleNameRule(out Token argNameToken);
if (argumentName is not null)
{
Token colon = NextToken();
if (colon.Kind != TokenKind.Colon)
{
UngetToken(colon);

ReportIncompleteInput(After(argNameToken),
nameof(ParserStrings.MissingColonAfterArgumentLabel),
ParserStrings.MissingColonAfterArgumentLabel,
argumentName.Extent.ToString(),
colon.Text);
reportedError = true;
lastValidToken = argNameToken;
break;
}

lastValidToken = colon;
}

ExpressionAst argument = ExpressionRule();
if (argument == null)
{
if (comma != null)
// ErrorRecovery: sync at closing paren or newline.
if (lastValidToken is not null)
{
// ErrorRecovery: sync at closing paren or newline.

ReportIncompleteInput(After(comma),
ReportIncompleteInput(After(lastValidToken),
nameof(ParserStrings.MissingExpressionAfterToken),
ParserStrings.MissingExpressionAfterToken,
TokenKind.Comma.Text());
lastValidToken.Text);
reportedError = true;
}

break;
}

arguments.Add(argument);
if (argumentName is null)
{
arguments.Add(argument);
}
else
{
IScriptExtent entryExtent = ExtentOf(argumentName, argument);
arguments.Add(new NamedMethodArgumentAst(entryExtent, argumentName, argument));
}

SkipNewlines();
comma = NextToken();
if (comma.Kind != TokenKind.Comma)
lastValidToken = NextToken();
if (lastValidToken.Kind != TokenKind.Comma)
{
UngetToken(comma);
comma = null;
UngetToken(lastValidToken);
lastValidToken = null;
break;
}
}
Expand All @@ -7963,7 +7994,7 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
_disableCommaOperator = oldDisableCommaOperator;
}

lastExtent = ExtentFromFirstOf(rParen, comma, arguments.LastOrDefault(), lParen);
lastExtent = ExtentFromFirstOf(rParen, lastValidToken, arguments.LastOrDefault(), lParen);
return arguments;
}

Expand Down Expand Up @@ -8127,6 +8158,15 @@ internal bool ReportIncompleteInput(IScriptExtent extent, string errorId, string
return incompleteInput;
}

internal bool ReportIncompleteInput(IScriptExtent extent, string errorId, string errorMsg, params object[] args)
{
// If the error position isn't at the end of the input, then we don't want to mark the error
// as incomplete input.
bool incompleteInput = _tokenizer.IsAtEndOfScript(extent, checkCommentsAndWhitespace: true);
SaveError(extent, errorId, errorMsg, incompleteInput, args);
return incompleteInput;
}

internal bool ReportIncompleteInput(IScriptExtent errorPosition,
IScriptExtent errorDetectedPosition,
string errorId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ public abstract class AstVisitor2 : AstVisitor

/// <summary/>
public virtual AstVisitAction VisitPipelineChain(PipelineChainAst statementChain) => DefaultVisit(statementChain);

/// <summary/>
public virtual AstVisitAction VisitNamedMethodArgumentExpression(NamedMethodArgumentAst argumentWithLabelAst) => DefaultVisit(argumentWithLabelAst);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,27 @@ public override AstVisitAction VisitMemberExpression(MemberExpressionAst memberE
public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst memberExpressionAst)
{
CheckMemberAccess(memberExpressionAst);
if (memberExpressionAst.Arguments is not null)
{
var existingArguments = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (ExpressionAst argument in memberExpressionAst.Arguments)
{
if (argument is NamedMethodArgumentAst namedArgumentExpression)
{
if (!existingArguments.Add(namedArgumentExpression.Label.Value))
{
_parser.ReportError(namedArgumentExpression.Label.Extent,
nameof(ParserStrings.DuplicateArgumentLabel),
ParserStrings.DuplicateArgumentLabel,
namedArgumentExpression.Label.Value);
}
}
else if (existingArguments.Count > 0)
{
_parser.ReportError(argument.Extent, nameof(ParserStrings.UnnamedArgumentAfterNamed), ParserStrings.UnnamedArgumentAfterNamed);
}
}
}
return AstVisitAction.Continue;
}

Expand Down
Loading