Recommend 'this' inside local functions #27690
Conversation
| while (enclosingSymbol is IMethodSymbol method && method.MethodKind == MethodKind.LocalFunction) | ||
| { | ||
| // It is allowed to reference the instance (`this`) within a local function, as long as the containing method allows it | ||
| enclosingSymbol = method.ContainingSymbol; |
There was a problem hiding this comment.
i find this fix surprising. Is a local function treated as static (even though it can use 'this')?
There was a problem hiding this comment.
Sounds wrong to me. If they're emitted as static methods, that's just a codegen decision.
There was a problem hiding this comment.
From discussion with @agocke and @AlekseyTs, there are two bugs:
- the compiler
LocalFunctionSymbol.IsStaticshould always returnfalse, but currently always returnstrue. - given that, the IDE should not consider
LocalFunctionSymbol.IsStaticto decide whetherthiscan be used in the current context.
Given that, I will revert this PR to my original fix.
There was a problem hiding this comment.
the compiler LocalFunctionSymbol.IsStatic should always return false, but currently always returns true.
Not disagreeing... but can an explanation be provided as to why?
This seems similar to props+accessors. Accessors can't have static/non-static info on them. They just 'inherit' from the containing prop. Seems similar to local-functions/containers. And it would seem to make the most sense. It would also tie into one of the main concepts of Static/Instance (namely, whether or not you can use 'this'..). Thoughts?
There was a problem hiding this comment.
It looks like there is some discussion going on for the compiler side (maybe it should always be true after all) ;-)
jcouv
added
the
PR For Personal Review Only
jcouv
force-pushed
the
recommend-this
branch
2 times, most recently
from
82d915d to
3a06d09
Compare
jcouv
removed
the
PR For Personal Review Only
|
@dotnet/roslyn-ide for a tiny review. Thanks |
|
Thanks! |
|
Approved to merge for 15.8.Preview4 |
| @@ -1771,6 +1771,13 @@ public static bool IsInstanceContext(this SyntaxTree syntaxTree, SyntaxToken tar | |||
| #endif | |||
|
|
|||
| var enclosingSymbol = semanticModel.GetEnclosingSymbol(targetToken.SpanStart, cancellationToken); | |||
|
|
|||
| while (enclosingSymbol is IMethodSymbol method && method.MethodKind == MethodKind.LocalFunction) | |||
There was a problem hiding this comment.
what about MethodKind.AnonymousFunction?
There was a problem hiding this comment.
it seems this is always offered inside them regardless of whether the containing method is static
Customer scenario
Try to type
thisinside a local function that is inside an instance method.thisshould be offered as a completion.Bugs this fixes
Fixes #27028
Workarounds, if any
Type
thisin full.Risk
Performance impact
Low. The change only impacts the case of typing inside a local function.
Is this a regression from a previous update?
No.
Root cause analysis
The semantic model shows local functions as static (since that is how they are emitted).
How was the bug found?
Internally.