Skip to content

Introduce simpler pattern for matching nodes#53663

Merged
CyrusNajmabadi merged 1 commit intodotnet:mainfrom
CyrusNajmabadi:nodePattern
May 25, 2021
Merged

Introduce simpler pattern for matching nodes#53663
CyrusNajmabadi merged 1 commit intodotnet:mainfrom
CyrusNajmabadi:nodePattern

Conversation

@CyrusNajmabadi
Copy link
Contributor

Introduced by @alrz in #53553

Currently pattern matching is limiated because we have to go through fields/props, which means we can only access RawKind (not Kind()). And as RawKind is typed as an int, it hurts ergonomics.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 25, 2021 04:49
@ghost ghost added the Area-IDE label May 25, 2021
PrefixUnaryExpressionSyntax prefix => GetKind(prefix.Operand),
// Treat `expr!` the same as `expr` (i.e. treat `!` as if it's just trivia).
PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } postfix => GetKind(postfix.Operand),
PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression) postfix => GetKind(postfix.Operand),
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the formatting here should be to have a space before open paren.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting idea. I can see the appeal of making this not look like a method call, but also its kind of a method call ¯\_(ツ)_/¯

OperatorDeclarationSyntax op => op.ReturnType,
BasePropertyDeclarationSyntax property => property.Type,
AccessorDeclarationSyntax { RawKind: (int)SyntaxKind.GetAccessorDeclaration, Parent: AccessorListSyntax { Parent: BasePropertyDeclarationSyntax baseProperty } } accessor => baseProperty.Type,
AccessorDeclarationSyntax(SyntaxKind.GetAccessorDeclaration) { Parent: AccessorListSyntax { Parent: BasePropertyDeclarationSyntax baseProperty } } accessor => baseProperty.Type,
Copy link
Member

Choose a reason for hiding this comment

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

My initial reaction to this was confusion, but its a new language feature so thats not super surprising. Now that I know what this is, I like it, but I do wonder if we can do something in the IDE to help here. Go To Def on the open parens goes to the Deconstruct method? Perhaps a mention that its a deconstruct pattern in Quick Info? Not sure.

None of that helps with a GitHub review of course, for that its just a matter of this old dog learning a new trick 😀

PrefixUnaryExpressionSyntax prefix => GetKind(prefix.Operand),
// Treat `expr!` the same as `expr` (i.e. treat `!` as if it's just trivia).
PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } postfix => GetKind(postfix.Operand),
PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression) postfix => GetKind(postfix.Operand),
Copy link
Member

Choose a reason for hiding this comment

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

Interesting idea. I can see the appeal of making this not look like a method call, but also its kind of a method call ¯\_(ツ)_/¯

@CyrusNajmabadi CyrusNajmabadi merged commit 789d03c into dotnet:main May 25, 2021
@ghost ghost added this to the Next milestone May 25, 2021
@CyrusNajmabadi
Copy link
Contributor Author

Interesting idea. I can see the appeal of making this not look like a method call, but also its kind of a method call

Also, it's intentionl that we want construction/deconstruction/matching to look the same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants