Fix the parsing code for method generic arguments (#16937)

This commit is contained in:
Dongbo Wang
2022-03-03 09:21:21 -08:00
committed by GitHub
parent 9b2d97e3e2
commit 102782fbb0
3 changed files with 117 additions and 104 deletions
@@ -7745,7 +7745,6 @@ namespace System.Management.Automation.Language
// On entry, we've verified that operatorToken is not preceded by whitespace.
CommandElementAst member = MemberNameRule();
List<ITypeName> genericTypeArguments = null;
if (member == null)
{
@@ -7760,9 +7759,10 @@ namespace System.Management.Automation.Language
}
else if (_ungotToken == null)
{
// Member name may be an incomplete token like `$a.$(Command-Name`; we do not look for generic args or
// invocation token(s) if the member name token is recognisably incomplete.
genericTypeArguments = GenericMethodArgumentsRule(out Token rBracket);
// Member name may be an incomplete token like `$a.$(Command-Name`, in which case, '_ungotToken != null'.
// We do not look for generic args or invocation token if the member name token is recognisably incomplete.
int resyncIndex = _tokenizer.GetRestorePoint();
List<ITypeName> genericTypeArguments = GenericMethodArgumentsRule(resyncIndex, out Token rBracket);
Token lParen = NextInvokeMemberToken();
if (lParen != null)
@@ -7778,6 +7778,13 @@ namespace System.Management.Automation.Language
"member and paren must be adjacent when the method is not generic");
return MemberInvokeRule(targetExpr, lParen, operatorToken, member, genericTypeArguments);
}
else if (rBracket != null)
{
// We had a legit section of generic arguments but no 'lParen' following that, so this is not a method
// invocation, but an invalid indexing operation. Resync the tokenizer back to before the generic arg
// parsing and then continue.
Resync(resyncIndex);
}
}
return new MemberExpressionAst(
@@ -7785,15 +7792,13 @@ namespace System.Management.Automation.Language
targetExpr,
member,
@static: operatorToken.Kind == TokenKind.ColonColon,
nullConditional: operatorToken.Kind == TokenKind.QuestionDot,
genericTypeArguments);
nullConditional: operatorToken.Kind == TokenKind.QuestionDot);
}
private List<ITypeName> GenericMethodArgumentsRule(out Token rBracketToken)
private List<ITypeName> GenericMethodArgumentsRule(int resyncIndex, out Token rBracketToken)
{
List<ITypeName> genericTypes = null;
int resyncIndex = _tokenizer.GetRestorePoint();
Token lBracket = NextToken();
rBracketToken = null;
@@ -7817,7 +7822,23 @@ namespace System.Management.Automation.Language
SkipNewlines();
Token firstToken = NextToken();
if (firstToken.Kind == TokenKind.Identifier || firstToken.Kind == TokenKind.LBracket)
// For method generic arguments, we only support the syntax `$var.Method[TypeName1 <, TypeName2 ...>]`,
// not the syntax `$var.Method[[TypeName1] <, [TypeName2] ...>]`.
// The latter syntax has been supported for type expression since the beginning, but it's ambiguous in
// this scenario because we could be looking at an indexing operation on a property like:
// `$var.Property[<expression>]`
// and the `<expression>` could start with a type expression like `[TypeName]::Method()`, or even just
// a single type expression acting as a key to a hashtable property. Such cases will cause ambiguities.
//
// It could be possible to write code that sorts out the ambiguity and continue to support the latter
// syntax for method generic arguments, and thus to allow assembly-qualified type names. But we choose
// not to do so because:
// 1. that will definitely increase the complexity of the parsing code and also make it fragile;
// 2. the latter syntax hurts readability a lot due to the number of opening/closing brackets.
// The downside is that the assembly-qualified type names won't be supported for method generic args,
// but that's likely not a problem in practice, and we can revisit if it turns out otherwise.
if (firstToken.Kind == TokenKind.Identifier)
{
resyncIndex = -1;
genericTypes = GenericTypeArgumentsRule(firstToken, out rBracketToken);
@@ -7836,11 +7857,11 @@ namespace System.Management.Automation.Language
finally
{
SetTokenizerMode(oldTokenizerMode);
}
if (resyncIndex > 0)
{
Resync(resyncIndex);
if (resyncIndex > 0)
{
Resync(resyncIndex);
}
}
return genericTypes;
@@ -8012,16 +8012,10 @@ namespace System.Management.Automation.Language
/// <param name="static">True if the '::' operator was used, false if '.' is used.
/// True if the member access is for a static member, using '::', false if accessing a member on an instance using '.'.
/// </param>
/// <param name="genericTypes">The generic type arguments passed to the member.</param>
/// <exception cref="PSArgumentNullException">
/// If <paramref name="extent"/>, <paramref name="expression"/>, or <paramref name="member"/> is null.
/// </exception>
public MemberExpressionAst(
IScriptExtent extent,
ExpressionAst expression,
CommandElementAst member,
bool @static,
IList<ITypeName> genericTypes)
public MemberExpressionAst(IScriptExtent extent, ExpressionAst expression, CommandElementAst member, bool @static)
: base(extent)
{
if (expression == null || member == null)
@@ -8034,35 +8028,6 @@ namespace System.Management.Automation.Language
this.Member = member;
SetParent(member);
this.Static = @static;
if (genericTypes is not null && genericTypes.Count > 0)
{
this.GenericTypeArguments = new ReadOnlyCollection<ITypeName>(genericTypes);
}
}
/// <summary>
/// Initializes a new instance of the <see cref="MemberExpressionAst"/> class.
/// </summary>
/// <param name="extent">
/// The extent of the expression, starting with the expression before the operator '.' or '::' and ending after
/// membername or expression naming the member.
/// </param>
/// <param name="expression">The expression before the member access operator '.' or '::'.</param>
/// <param name="member">The name or expression naming the member to access.</param>
/// <param name="static">True if the '::' operator was used, false if '.' is used.
/// True if the member access is for a static member, using '::', false if accessing a member on an instance using '.'.
/// </param>
/// <exception cref="PSArgumentNullException">
/// If <paramref name="extent"/>, <paramref name="expression"/>, or <paramref name="member"/> is null.
/// </exception>
public MemberExpressionAst(
IScriptExtent extent,
ExpressionAst expression,
CommandElementAst member,
bool @static)
: this(extent, expression, member, @static, genericTypes: null)
{
}
/// <summary>
@@ -8076,46 +8041,15 @@ namespace System.Management.Automation.Language
/// <param name="member">The name or expression naming the member to access.</param>
/// <param name="static">True if the '::' operator was used, false if '.' or '?.' is used.</param>
/// <param name="nullConditional">True if '?.' used.</param>
/// <param name="genericTypes">The generic type arguments passed to the member.</param>
/// <exception cref="PSArgumentNullException">
/// If <paramref name="extent"/>, <paramref name="expression"/>, or <paramref name="member"/> is null.
/// </exception>
public MemberExpressionAst(
IScriptExtent extent,
ExpressionAst expression,
CommandElementAst member,
bool @static,
bool nullConditional,
IList<ITypeName> genericTypes)
: this(extent, expression, member, @static, genericTypes)
public MemberExpressionAst(IScriptExtent extent, ExpressionAst expression, CommandElementAst member, bool @static, bool nullConditional)
: this(extent, expression, member, @static)
{
this.NullConditional = nullConditional;
}
/// <summary>
/// Initializes a new instance of the <see cref="MemberExpressionAst"/> class.
/// </summary>
/// <param name="extent">
/// The extent of the expression, starting with the expression before the operator '.', '::' or '?.' and ending after
/// membername or expression naming the member.
/// </param>
/// <param name="expression">The expression before the member access operator '.', '::' or '?.'.</param>
/// <param name="member">The name or expression naming the member to access.</param>
/// <param name="static">True if the '::' operator was used, false if '.' or '?.' is used.</param>
/// <param name="nullConditional">True if '?.' used.</param>
/// <exception cref="PSArgumentNullException">
/// If <paramref name="extent"/>, <paramref name="expression"/>, or <paramref name="member"/> is null.
/// </exception>
public MemberExpressionAst(
IScriptExtent extent,
ExpressionAst expression,
CommandElementAst member,
bool @static,
bool nullConditional)
: this(extent, expression, member, @static, nullConditional, genericTypes: null)
{
}
/// <summary>
/// The expression that produces the value to retrieve the member from. This property is never null.
/// </summary>
@@ -8136,11 +8070,6 @@ namespace System.Management.Automation.Language
/// </summary>
public bool NullConditional { get; protected set; }
/// <summary>
/// Gets a list of generic type arguments passed to this member.
/// </summary>
public ReadOnlyCollection<ITypeName> GenericTypeArguments { get; }
/// <summary>
/// Copy the MemberExpressionAst instance.
/// </summary>
@@ -8154,8 +8083,7 @@ namespace System.Management.Automation.Language
newExpression,
newMember,
this.Static,
this.NullConditional,
this.GenericTypeArguments);
this.NullConditional);
}
#region Visitors
@@ -8214,13 +8142,18 @@ namespace System.Management.Automation.Language
IEnumerable<ExpressionAst> arguments,
bool @static,
IList<ITypeName> genericTypes)
: base(extent, expression, method, @static, genericTypes)
: base(extent, expression, method, @static)
{
if (arguments != null && arguments.Any())
{
this.Arguments = new ReadOnlyCollection<ExpressionAst>(arguments.ToArray());
SetParents(Arguments);
}
if (genericTypes != null && genericTypes.Count > 0)
{
this.GenericTypeArguments = new ReadOnlyCollection<ITypeName>(genericTypes);
}
}
/// <summary>
@@ -8308,6 +8241,11 @@ namespace System.Management.Automation.Language
{
}
/// <summary>
/// Gets a list of generic type arguments passed to this method invocation.
/// </summary>
public ReadOnlyCollection<ITypeName> GenericTypeArguments { get; }
/// <summary>
/// The non-empty collection of arguments to pass when invoking the method, or null if no arguments were specified.
/// </summary>
@@ -13,9 +13,40 @@ Describe 'Generic Method invocation' -Tags 'CI' {
Script = '[Array]::Empty[System.Collections.Generic.Dictionary[System.Numerics.BigInteger, System.Collections.Generic.List[string[,]]]]()'
ExpectedType = [System.Collections.Generic.Dictionary[System.Numerics.BigInteger, System.Collections.Generic.List[string[, ]]][]]
}
)
$IndexingAProperty = @(
@{
Script = '[Array]::$("Empty")[[System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.Numerics.BigInteger, System.Runtime.Numerics]], System.Private.CoreLib]]()'
ExpectedType = [System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib], [System.Numerics.BigInteger, System.Runtime.Numerics]][], System.Private.CoreLib]
Script = '[object]::Property[[type]]'
IndexType = 'System.Management.Automation.Language.TypeExpressionAst'
IndexString = '[type]'
}
@{
Script = '$object.IPSubnet[[Array]::IndexOf($_.IPAddress, $_.IPAddress[0])]'
IndexType = 'System.Management.Automation.Language.InvokeMemberExpressionAst'
IndexString = '[Array]::IndexOf($_.IPAddress, $_.IPAddress[0])'
}
@{
Script = @'
[IPAddress]::Parse(
$_.IPSubnet[
[Array]::IndexOf($_.IPAddress, $_.IPAddress[0])
]
)
'@
IndexType = 'System.Management.Automation.Language.InvokeMemberExpressionAst'
IndexString = '[Array]::IndexOf($_.IPAddress, $_.IPAddress[0])'
}
@{
Script = @'
[IPAddress]::Parse(
$_.IPSubnet[
([Array]::IndexOf($_.IPAddress, $_.IPAddress[0]))
]
)
'@
IndexType = 'System.Management.Automation.Language.ParenExpressionAst'
IndexString = '([Array]::IndexOf($_.IPAddress, $_.IPAddress[0]))'
}
)
@@ -37,8 +68,8 @@ Describe 'Generic Method invocation' -Tags 'CI' {
}
@{
Script = '[array]::empty[type]]()'
ExpectedErrors = @('UnexpectedToken', 'ExpectedExpression')
ErrorCount = 2
ExpectedErrors = @('MissingArrayIndexExpression', 'UnexpectedToken', 'ExpectedExpression')
ErrorCount = 3
}
@{
Script = '$object.Method[type,]()'
@@ -70,6 +101,21 @@ Describe 'Generic Method invocation' -Tags 'CI' {
ExpectedErrors = @('EndSquareBracketExpectedAtEndOfType', 'UnexpectedToken')
ErrorCount = 2
}
@{
Script = '$object.Method[[type]]()'
ExpectedErrors = @('UnexpectedToken', 'ExpectedExpression')
ErrorCount = 2
}
@{
Script = '[Array]::Empty[[type]]()'
ExpectedErrors = @('UnexpectedToken', 'ExpectedExpression')
ErrorCount = 2
}
@{
Script = '$object.Property[type]'
ExpectedErrors = @('MissingArrayIndexExpression', 'UnexpectedToken')
ErrorCount = 2
}
)
}
@@ -79,6 +125,24 @@ Describe 'Generic Method invocation' -Tags 'CI' {
{ [scriptblock]::Create($script) } | Should -Not -Throw
}
It "parses fine for indexing a property: <Script>" -TestCases $IndexingAProperty {
param($Script, $IndexType, $IndexString)
$parseErrors = $null
$ast = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$null, [ref]$parseErrors)
$parseErrors | Should -BeNullOrEmpty
$cmdExpr = $ast.EndBlock.Statements[0].PipelineElements[0]
$cmdExpr | Should -BeOfType 'System.Management.Automation.Language.CommandExpressionAst'
$indexExpr = $cmdExpr.Expression -is [System.Management.Automation.Language.InvokeMemberExpressionAst] ? $cmdExpr.Expression.Arguments[0] : $cmdExpr.Expression
$indexExpr | Should -BeOfType 'System.Management.Automation.Language.IndexExpressionAst'
$indexExpr.Index | Should -BeOfType $IndexType
$indexExpr.Index.ToString() | Should -BeExactly $IndexString
}
It 'reports a parse error for "<Script>"' -TestCases $ExpectedParseErrors {
param($Script, $ExpectedErrors, $ErrorCount)
@@ -164,16 +228,6 @@ Describe 'Generic Method invocation' -Tags 'CI' {
}
}
It 'lists the same method group information with and without type parameters' {
$dict = [System.Collections.Concurrent.ConcurrentDictionary[string, int]]::new()
$noTypeParams = $dict.AddOrUpdate
$typeParams = $dict.AddOrUpdate[string]
$extraTypeParams = $dict.AddOrUpdate[string, int]
$typeParams.OverloadDefinitions | Should -BeExactly $noTypeParams.OverloadDefinitions
$extraTypeParams.OverloadDefinitions | Should -BeExactly $typeParams.OverloadDefinitions
}
It 'successfully invokes common Linq generic methods' {
[System.Collections.Generic.List[int]]$list = @( 1, 2, 3, 4, 5 )
$result = [System.Linq.Enumerable]::Select[int, float](