Adjust CheckedKeywordRecommender for checked user-defined operators.#60183
Conversation
| { | ||
| if (targetToken.GetPreviousToken(includeSkipped: true).IsLastTokenOfNode<TypeSyntax>()) | ||
| { | ||
| return true; |
There was a problem hiding this comment.
nit: consider adding comments to illustrate the situation (here and below)
There was a problem hiding this comment.
note: i would prefer this as well. but it's not a problem to not have it.
| @"ref int x = ref $$")); | ||
| } | ||
|
|
||
| [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] |
There was a problem hiding this comment.
Consider testing the recommendation in the declaration of an existing/complete operator (scenario is you copy/paste one operator, add checked, then tweak code)
|
@CyrusNajmabadi Please review changes past the first commit. |
|
@CyrusNajmabadi Please review changes past the first commit. |
|
@CyrusNajmabadi Please review changes made after the first commit. |
1 similar comment
|
@CyrusNajmabadi Please review changes made after the first commit. |
|
|
||
| if (firstSpecifierToken.GetPreviousToken(includeSkipped: true).IsLastTokenOfNode<TypeSyntax>()) | ||
| { | ||
| return true; |
There was a problem hiding this comment.
so 'checked' is allowed, in an explicit-impl, even without having 'explicit' in the sig?
There was a problem hiding this comment.
so 'checked' is allowed, in an explicit-impl, even without having 'explicit' in the sig?
An 'explicit' keyword has nothing to do with explicit interface implementation, it is part of a conversion operator declaration
There was a problem hiding this comment.
If we have a type before the specifier, this is not a conversion operator declaration
There was a problem hiding this comment.
sorry, what i meant here is that this code checks if we're after an explicit-interface specifier, and then checks that right before that is a type-syntax. But it does not do the previousToken.Kind() == SyntaxKind.ExplicitKeyword check below. I'm trying to understand if that's ok and desirable behavior. I commented on the test that seems to be hitting this, and i was trying to understand if this is a legal language construct and what it means.
There was a problem hiding this comment.
A conversion operator cannot have a type between 'explicit' and 'operator', its type goes before parameters. For example:
public static explicit operator checked TypeToConvertTo(TypeToConvertFrom x) {}
| { | ||
| await VerifyKeywordAsync( | ||
| @"class Goo { | ||
| public static int I1.operator $$"); |
There was a problem hiding this comment.
so this is legal from teh lang perspective? it doesn't need to be marked as 'explicit'?
There was a problem hiding this comment.
so this is legal from teh lang perspective? it doesn't need to be marked as 'explicit'?
If we have a type before the specifier, this is not a conversion operator declaration. The 'explicit' keyword is used only for conversion operators.
Test plan #59458