-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Implement keyword recommenders for allows ref struct
constraint
#73463
Implement keyword recommenders for allows ref struct
constraint
#73463
Conversation
|
||
namespace Microsoft.CodeAnalysis.CSharp.Completion.KeywordRecommenders; | ||
|
||
internal class AllowsKeywordRecommender : AbstractSyntacticSingleKeywordRecommender |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internal class AllowsKeywordRecommender : AbstractSyntacticSingleKeywordRecommender | |
internal sealed class AllowsKeywordRecommender : AbstractSyntacticSingleKeywordRecommender |
src/Features/CSharp/Portable/Completion/KeywordRecommenders/AllowsKeywordRecommender.cs
Show resolved
Hide resolved
if (token.Kind() == SyntaxKind.RefKeyword && | ||
token.Parent is RefStructConstraintSyntax refStructConstraint && refStructConstraint.RefKeyword == token && | ||
refStructConstraint.Parent is AllowsConstraintClauseSyntax allowsClause && | ||
allowsClause.Parent is TypeParameterConstraintClauseSyntax) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can trim off this last check. #WontFix
{ | ||
if (!constraintClause.Constraints | ||
.OfType<ClassOrStructConstraintSyntax>() | ||
.Any(c => c.ClassOrStructKeyword.Kind() == SyntaxKind.ClassKeyword)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'd be fine with this being dropped. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'd be fine with this being dropped.
Following what we do for new
constraint
public async Task TestAfterAllowsRefAfterClassTypeParameterConstraint() | ||
{ | ||
await VerifyKeywordAsync( | ||
@"class C<T> where T : class, allows ref $$"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could these all be raw strings plz. or indent 4 from await
. we're trying to move away entirely from tests that are 0-column aligned. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with minor nits, simplification suggestions, and requests about test formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with minor nits, simplification suggestions, and requests about test formatting.
class C1<T> | ||
where T : allows ref struct; | ||
class C2<T, S> | ||
where T : allows ref struct , where S : struct; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The space before the comma looks unexpected. Shouldn't it be:
where T : allows ref struct , where S : struct; | |
where T : allows ref struct, where S : struct; | |
``` #Resolved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The space before the comma looks unexpected.
It is a syntax error, a comma cannot follow allows ref struct
constraint
private static bool IsAllowsRefStructConstraintContext(CSharpSyntaxContext context) | ||
{ | ||
// where T : | | ||
if (context.SyntaxTree.IsTypeParameterConstraintStartContext(context.Position, context.LeftToken)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't expect this check to be inside method IsAllowsRefStructConstraintContext
, consider returning it where it was previously. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rename the method
03fb6ba
into
dotnet:features/RefStructInterfaces
No description provided.