Skip to content

Fix signature help crash when invoked through extension property#82537

Merged
jasonmalinowski merged 2 commits into
dotnet:mainfrom
deepakrathore33:fix/signature-help-extension-property
Feb 26, 2026
Merged

Fix signature help crash when invoked through extension property#82537
jasonmalinowski merged 2 commits into
dotnet:mainfrom
deepakrathore33:fix/signature-help-extension-property

Conversation

@deepakrathore33

Copy link
Copy Markdown
Contributor

var includeInstance = throughExpression.Kind() is not (SyntaxKind.IdentifierName or SyntaxKind.SimpleMemberAccessExpression or SyntaxKind.PredefinedType) ||
semanticModel.LookupSymbols(throughExpression.SpanStart, name: throughSymbol?.Name).Any(static s => s is not INamedTypeSymbol) ||
(throughSymbol is not INamespaceOrTypeSymbol && semanticModel.LookupSymbols(throughExpression.SpanStart, container: throughSymbol?.ContainingType).Any(static s => s is not INamedTypeSymbol));
throughSymbol is not INamespaceOrTypeSymbol;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we be reordering the conditions here? This last condition now is a lot cheaper than previous LookupSymbols? Did we also understand what this check was trying to do before? I admit it's not entirely clear to me -- if this is passing tests it feels like we're regressing "something" but maybe the pervious LookupSymbols is covering it in practice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The old LookupSymbols(container) was added for #4144 (calling a method through an inaccessible inherited field). But all it really did was ask "is this an instance, not a type?" Which throughSymbol is not INamespaceOrTypeSymbol already answers, just cheaper. Moving that check earlier covers both the old case and the new extension property case.
The extension bug happened because LookupSymbols(container) returns empty for extension container types by design in the binder.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for doing a bit of digging, and since we did add a test that makes me feel good about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants