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 a Deconstruct method for syntax nodes #52784

Open
RikkiGibson opened this issue Apr 20, 2021 · 7 comments
Open

Add a Deconstruct method for syntax nodes #52784

RikkiGibson opened this issue Apr 20, 2021 · 7 comments
Labels
Area-Compilers Bug Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality
Milestone

Comments

@RikkiGibson
Copy link
Contributor

This made me realize a void Deconstruct(this SyntaxNode, out SyntaxKind) might be actually useful. Currently matching multi-kind nodes is kind of ugly..

is RecordDeclarationSyntax(SyntaxKind.RecordStructDeclaration) { ParameterList: not null }

Originally posted by @alrz in #52702 (comment)


I recall often being stymied by the inability to match the SyntaxKind when pattern matching against a syntax node. It might work nicely to introduce an internal extension which lets us deconstruct the SyntaxKind.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 20, 2021
@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Apr 20, 2021

Well, you can write this:

RecordDeclarationSyntax { RawKind: (int)SyntaxKind.RecordStructDeclaration, ParameterList: not null }

The drawback is that RawKind is typed as an int. We could have a CSharpKind property on the type that then exposes that as SyntaxKind which would make thigns nicer. It's totally unclear to me why we made Kind an extension method...

@RikkiGibson
Copy link
Contributor Author

It's totally unclear to me why we made Kind an extension method...

According to 93e9eaf#r11017500 back in 2015,

Consistency with extension methods of the same name.

@CyrusNajmabadi
Copy link
Member

Seems like it should have been a property. But alas here, we are. Maybe we might make it possible for pattern matching to support something like { Kind(): .RecordStructDeclaration } in the future.

@alrz
Copy link
Member

alrz commented Apr 21, 2021

RawKind

That's not that bad. But I'd still prefer Deconstruct cause it perfectly frame the type check part of the match. just my 2c.

Maybe we might make it possible for pattern matching to support something like { Kind(): .RecordStructDeclaration } in the future.

I think extension props is more likely than methods in patterns though.

@RikkiGibson
Copy link
Contributor Author

I have been thinking off and on about the ergonomics of this pattern, particularly about the fact that these forms would work:

if (node is (SyntaxKind kind) { }) { ... }
if (node is SyntaxNode(SyntaxKind kind)) { ... }
if (node is (SyntaxKind kind) _) { ... }

But this would not:

if (node is (SyntaxKind kind)) { ... }

I don't know if this is particularly solvable or not. But it feels "weird", similar to how before we had type patterns users had to do e.g. node switch { SyntaxNode _ => ... } instead of just node switch { SyntaxNode => ... }.

@alrz
Copy link
Member

alrz commented Jun 13, 2021

I don't think that matters, we probably wouldn't want to do any of those?

See some sample usages in #53553 (https://github.com/dotnet/roslyn/.../UseRecursivePatternsCodeRefactoringProvider.cs)
Also #53663

@jaredpar jaredpar added Bug Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 9, 2021
@jaredpar jaredpar added this to the Backlog milestone Jul 9, 2021
@RikkiGibson
Copy link
Contributor Author

It looks like this is already present in Workspaces/SharedUtilitiesAndExtensions, so this issue tracks making the API available in the compiler layer.

public static void Deconstruct(this SyntaxNode node, out SyntaxKind kind)
{
kind = node.Kind();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality
Projects
None yet
Development

No branches or pull requests

4 participants