Skip to content

Fix parsing of lambda parameter named "scoped"#64085

Merged
jjonescz merged 4 commits intodotnet:mainfrom
jjonescz:scoped-param-name
Sep 21, 2022
Merged

Fix parsing of lambda parameter named "scoped"#64085
jjonescz merged 4 commits intodotnet:mainfrom
jjonescz:scoped-param-name

Conversation

@jjonescz
Copy link
Member

Fixes #63469

@ghost ghost added the Area-Compilers label Sep 16, 2022
@jjonescz jjonescz marked this pull request as ready for review September 16, 2022 15:35
@jjonescz jjonescz requested a review from a team as a code owner September 16, 2022 15:35
@AlekseyTs
Copy link
Contributor

@jjonescz The underlying issue is probably fixed in #64088. Could you confirm that? If so, then let's wait for that PR to go in and adjust this one to only add tests.

@jjonescz
Copy link
Member Author

jjonescz commented Sep 19, 2022

The underlying issue is probably fixed in #64088. Could you confirm that?

Yes, it's fixed, except the last of my tests (ScopedAsParameterName_06) does not pass, i.e., this code is not reporting any errors (I am not sure whether it should?):

(scoped scoped) => { }

The tree also does not contain ScopedKeyword; it looks like this:

N(SyntaxKind.ParenthesizedLambdaExpression);
{
    N(SyntaxKind.ParameterList);
    {
        N(SyntaxKind.OpenParenToken);
        N(SyntaxKind.Parameter);
        {
            N(SyntaxKind.IdentifierName);
            {
                N(SyntaxKind.IdentifierToken, "scoped");
            }
            N(SyntaxKind.IdentifierToken, "scoped");
        }
        N(SyntaxKind.CloseParenToken);
    }
    N(SyntaxKind.EqualsGreaterThanToken);
    N(SyntaxKind.Block);
    {
        N(SyntaxKind.OpenBraceToken);
        N(SyntaxKind.CloseBraceToken);
    }
}
EOF();

Also feel free to just copy my tests into your branch and close this PR if that would be simpler.

@AlekseyTs
Copy link
Contributor

this code is not reporting any errors

This matches my expectations and the syntax tree looks good to me a s well.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 4). This is a test only change, this means it can be merged with a single sign-off. Please squash commits and adjust the comment to accurately reflect the nature of the change (an addition of tests) while merging.

@jjonescz jjonescz merged commit 858272d into dotnet:main Sep 21, 2022
@jjonescz jjonescz deleted the scoped-param-name branch September 21, 2022 10:44
@ghost ghost added this to the Next milestone Sep 21, 2022
@Cosifne Cosifne modified the milestones: Next, 17.4 P3 Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Scoped" word is forbidden since MSBuild version 17.3.0+92e077650

3 participants