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

Support init accessor in CSharpSyntaxFacts #48137

Merged
merged 8 commits into from
Oct 14, 2020
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,7 @@ private static bool IsMemberDeclaration(SyntaxNode node)
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
Copy link
Member Author

Choose a reason for hiding this comment

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

@davidwengier I removed both GetAccessorDeclaration and InitAccessorDeclaration in a draft. There are no test failures. So the current tests, unfortunately, doesn't cover those.

The usages that depend on this method are in:

  • CodeLens\CodeLensReferencesService.cs (there is a call to syntaxFactsService.IsDeclaration(node))
  • CodeRefactorings\SyncNamespace\AbstractChangeNamespaceService.cs (there is a call to .DescendantNodes(n => !syntaxFacts.IsDeclaration(n)))

Unlike the other change, I'm not able to guess the effect of this change.
I'll try to change the method to only return false in the draft in a hope to see test failures (i.e. other switch arms are tested).

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's a good idea to update this one without knowing the change it introduce.

This whole method isn't covered in the tests. See how #48158 is green.

Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
case SyntaxKind.EventDeclaration:
case SyntaxKind.EventFieldDeclaration:
case SyntaxKind.AddAccessorDeclaration:
Expand Down Expand Up @@ -1756,6 +1757,7 @@ public override bool CanHaveAccessibility(SyntaxNode declaration)
case SyntaxKind.EventFieldDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
return true;
Expand Down Expand Up @@ -2006,7 +2008,7 @@ public override DeclarationKind GetDeclarationKind(SyntaxNode declaration)
return DeclarationKind.Attribute;
}
break;

// Add InitAccessor once https://github.com/dotnet/roslyn/issues/48136 is resolved
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
case SyntaxKind.GetAccessorDeclaration:
return DeclarationKind.GetAccessor;
case SyntaxKind.SetAccessorDeclaration:
Expand Down