Skip to content

Commit

Permalink
Merge pull request #2 from MartinGC94/namedArgSuggestions
Browse files Browse the repository at this point in the history 
Parser change suggestions
  • Loading branch information
@jborean93
jborean93 committed Apr 19, 2024
2 parents 573a048 + b714074 commit 5f5a587
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 73 deletions.
51 changes: 12 additions & 39 deletions src/System.Management.Automation/engine/parser/Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7889,10 +7889,10 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
// G argument-label-expression:
// G simple-name ':'

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

bool oldDisableCommaOperator = _disableCommaOperator;
bool reportedError = false;
Expand All @@ -7919,8 +7919,11 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
argumentName.Extent.ToString(),
colon.Text);
reportedError = true;
lastValidToken = argNameToken;
break;
}

lastValidToken = colon;
}

ExpressionAst argument = ExpressionRule();
Expand All @@ -7930,23 +7933,10 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx

if (argumentName is not null)
{
if (comma is null)
{
comma = NextToken();
UngetToken(comma);
}

string nextTokenText = comma.Text;
if (nextTokenText.Length == 1 && char.IsControl(nextTokenText[0]))
{
nextTokenText = string.Format("\\u{0:x4}", (short)nextTokenText[0]);
}

ReportIncompleteInput(After(argumentName.Extent),
nameof(ParserStrings.MissingArgumentAfterLabel),
ParserStrings.MissingArgumentAfterLabel,
argumentName.Value,
nextTokenText);
ReportIncompleteInput(After(lastValidToken),
nameof(ParserStrings.MissingExpressionAfterToken),
ParserStrings.MissingExpressionAfterToken,
TokenKind.Colon.Text());
reportedError = true;
}
else if (comma != null)
Expand All @@ -7963,31 +7953,11 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx

if (argumentName is null)
{
if (existingArguments.Count > 0)
{
// ErrorRecovery: sync at closing paren or newline.

ReportError(argument.Extent,
nameof(ParserStrings.UnnamedArgumentAfterNamed),
ParserStrings.UnnamedArgumentAfterNamed);
reportedError = true;
break;
}

arguments.Add(argument);
}
else
{
IScriptExtent entryExtent = ExtentOf(argumentName, argument);
if (!existingArguments.Add(argumentName.Value))
{
ReportError(entryExtent,
nameof(ParserStrings.DuplicateArgumentLabel),
ParserStrings.DuplicateArgumentLabel,
argumentName.Value);
reportedError = true;
break;
}
arguments.Add(new LabeledExpressionAst(entryExtent, argumentName, argument));
}

Expand All @@ -7996,9 +7966,12 @@ private List<ExpressionAst> InvokeParamParenListRule(Token lParen, out IScriptEx
if (comma.Kind != TokenKind.Comma)
{
UngetToken(comma);
lastValidToken = null;
comma = null;
break;
}

lastValidToken = comma;
}

SkipNewlines();
Expand All @@ -8023,7 +7996,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
21 changes: 21 additions & 0 deletions src/System.Management.Automation/engine/parser/SemanticChecks.cs
View file
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 LabeledExpressionAst labeledExpression)
{
if (!existingArguments.Add(labeledExpression.Label.Value))
{
_parser.ReportError(labeledExpression.Label.Extent,
nameof(ParserStrings.DuplicateArgumentLabel),
ParserStrings.DuplicateArgumentLabel,
labeledExpression.Label.Value);
}
}
else if (existingArguments.Count > 0)
{
_parser.ReportError(argument.Extent, nameof(ParserStrings.UnnamedArgumentAfterNamed), ParserStrings.UnnamedArgumentAfterNamed);
}
}
}
return AstVisitAction.Continue;
}

Expand Down
3 changes: 0 additions & 3 deletions src/System.Management.Automation/resources/ParserStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,6 @@
<data name="MissingColonAfterArgumentLabel" xml:space="preserve">
<value>Expected colon after argument name identifier "{0}" but found "{1}".</value>
</data>
<data name="MissingArgumentAfterLabel" xml:space="preserve">
<value>Expected argument value after name identifier "{0}" but found "{1}".</value>
</data>
<data name="DuplicateArgumentLabel" xml:space="preserve">
<value>Found duplicate argument name label "{0}"</value>
</data>
Expand Down
51 changes: 20 additions & 31 deletions test/powershell/Language/Parser/Parser.Tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,7 @@ foo``u{2195}abc
}
}

Context "Method invocation argument labels" -Tag Jordan {
Context "Method invocation argument labels" {
It "Parses single argument with label <Label.Trim()>" -TestCases @(
@{ Label = "simple" }
@{ Label = "_underscore" }
Expand Down Expand Up @@ -1431,31 +1431,6 @@ $c.Method(
$actual[3].Expression.Value | Should -Be test
}

It "Parses method invocation with label differing in case" {
$script = @'
$c.Method(
label: $arg1,
Label: $arg2)
'@

$errors = @()
$ast = [System.Management.Automation.Language.Parser]::ParseInput($script, [ref]$null, [ref]$errors)
$errors | Should -BeNullOrEmpty

$actual = $ast.EndBlock.Statements[0].PipelineElements[0].Expression.Arguments
$actual.Count | Should -Be 2

$actual[0] | Should -BeOfType ([System.Management.Automation.Language.LabeledExpressionAst])
$actual[0].Label | Should -BeExactly label
$actual[0].Expression | Should -BeOfType ([System.Management.Automation.Language.VariableExpressionAst])
$actual[0].Expression.VariablePath | Should -Be arg1

$actual[1] | Should -BeOfType ([System.Management.Automation.Language.LabeledExpressionAst])
$actual[1].Label | Should -BeExactly Label
$actual[1].Expression | Should -BeOfType ([System.Management.Automation.Language.VariableExpressionAst])
$actual[1].Expression.VariablePath | Should -Be arg2
}

It "Parses method invocation without label with string value" {
$script = '$c.Method(''value'')'

Expand Down Expand Up @@ -1486,17 +1461,31 @@ $c.Method(
$actual.VariablePath.UserPath | Should -Be 'provider:var'
}

It "Fails to parse method invocation with label differing in case" {
$err = { ExecuteCommand '$c.Method(Arg: $value, arg: $value)' } | Should -Throw -ErrorId ParseException -PassThru
$pe = $err.Exception.InnerException
$pe | Should -BeOfType ([System.Management.Automation.ParseException])

$pe.Errors[0].ErrorId | Should -Be DuplicateArgumentLabel
$pe.Errors[0].Message | Should -Be 'Found duplicate argument name label "arg"'
$pe.ErrorRecord.InvocationInfo.PositionMessage | Should -BeExactly (@(
'At line:1 char:24'
'+ $c.Method(Arg: $value, arg: $value)'
'+ ~~~'
) -join [Environment]::NewLine)
}

It "Fails to parse label with newline after separator" {
$err = { ExecuteCommand "`$c.Method(label:`n'value')" } | Should -Throw -ErrorId ParseException -PassThru
$pe = $err.Exception.InnerException
$pe | Should -BeOfType ([System.Management.Automation.ParseException])

$pe.Errors[0].ErrorId | Should -Be MissingArgumentAfterLabel
$pe.Errors[0].Message | Should -Be 'Expected argument value after name identifier "label" but found "\u000a".'
$pe.Errors[0].ErrorId | Should -Be MissingExpressionAfterToken
$pe.Errors[0].Message | Should -Be "Missing expression after ':'."
$pe.ErrorRecord.InvocationInfo.PositionMessage | Should -Be (@(
'At line:1 char:16'
'At line:1 char:17'
'+ $c.Method(label:'
'+ ~'
'+ ~'
) -join [Environment]::NewLine)
}

Expand Down Expand Up @@ -1655,7 +1644,7 @@ $c.Method(
$pe.ErrorRecord.InvocationInfo.PositionMessage | Should -Be (@(
'At line:1 char:24'
'+ $c.Method(arg: $value, arg: $value)'
'+ ~~~~~~~~~~~'
'+ ~~~'
) -join [Environment]::NewLine)
}

Expand Down

0 comments on commit 5f5a587

Please sign in to comment.