-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Offer 'field' when within a property accessor. #74956
Offer 'field' when within a property accessor. #74956
Conversation
Add test Add test
@@ -7,222 +7,419 @@ | |||
using Microsoft.CodeAnalysis.Test.Utilities; | |||
using Xunit; | |||
|
|||
namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations | |||
namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations; |
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.
view with whitespace off.
// version check here. We do not want to interfere with users trying to use/learn this feature. The user will | ||
// get a clear message if they're not on the right lang version telling them about the issue, and offering to | ||
// upgrade their project if they way. | ||
if (context.IsAnyExpressionContext || context.IsStatementContext) |
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.
💭 Is there a case where IsAnyExpressionContext
is false but IsStatementContext
is true?
=> context.IsMemberAttributeContext(s_validTypeDeclarations, cancellationToken); | ||
private static bool IsInPropertyAccessor(SyntaxToken targetToken) | ||
{ | ||
for (var node = targetToken.Parent; node != null; node = node.Parent) |
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.
❔ Does this intentionally walk out of local functions? Even if so, it seems like static local functions should be skipped.
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.
yes. it intentionally walks out of local functions.
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.
Even if so, it seems like static local functions should be skipped.
field
is allowed ina static local/lambda if hte containing property is also static. We could consdier adding the logic to check for that. but i'm in the not-caring poitn currently.
08b50e8
into
dotnet:features/field-keyword
No description provided.