-
Notifications
You must be signed in to change notification settings - Fork 725
Quick Info fixes #1971
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
Quick Info fixes #1971
Conversation
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.
Pull Request Overview
This PR improves the hover information display for overloaded functions and interface properties by showing context-specific signature information. When hovering over a function call or property, the language server now displays the resolved signature at the call site rather than the full overload set or interface type.
Key changes:
- Added logic to detect when a symbol is a property backed by a method declaration and treat it as a method
- Enhanced variable/property hover to show the resolved call signature when hovering on a call expression
- Fixed signature selection to skip JSDoc-only declarations and properly handle overloads
- Refined call/new expression detection to ensure the node is the expression being called
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ls/hover.go | Core logic changes for context-aware hover information with signature resolution and method detection |
| testdata/baselines/reference/fourslash/quickInfo/quickInfoDisplayPartsVar.baseline | Updated baseline showing resolved signatures instead of overload sets for variables |
| testdata/baselines/reference/fourslash/quickInfo/quickInfoDisplayPartsLet.baseline | Updated baseline showing resolved signatures instead of overload sets for let bindings |
| testdata/baselines/reference/fourslash/quickInfo/quickInfoDisplayPartsConst.baseline | Updated baseline showing resolved signatures instead of overload sets for const bindings |
| testdata/baselines/reference/fourslash/quickInfo/quickInfoDisplayPartsTypeParameterInInterface.baseline | Updated baseline showing resolved generic signatures for interface members |
| testdata/baselines/reference/fourslash/quickInfo/quickInfoDisplayPartsInterfaceMembers.baseline | Updated baseline showing resolved return types for interface instances |
| } | ||
| flags := symbol.Flags | ||
| if flags&ast.SymbolFlagsProperty != 0 && declaration != nil && ast.IsMethodDeclaration(declaration) { | ||
| flags = ast.SymbolFlagsMethod |
Copilot
AI
Oct 28, 2025
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 flags reassignment overwrites all flags with only SymbolFlagsMethod, losing other potentially important flag information. Consider using bitwise operations to add the Method flag while preserving other flags: flags = (flags &^ ast.SymbolFlagsProperty) | ast.SymbolFlagsMethod.
| flags = ast.SymbolFlagsMethod | |
| flags = (flags &^ ast.SymbolFlagsProperty) | ast.SymbolFlagsMethod |
| b.WriteString(": ") | ||
| b.WriteString(c.TypeToStringEx(c.GetTypeOfSymbolAtLocation(symbol, node), container, typeFormatFlags)) | ||
| if callNode := getCallOrNewExpression(node); callNode != nil { | ||
| b.WriteString(c.SignatureToStringEx(c.GetResolvedSignature(callNode), container, typeFormatFlags|checker.TypeFormatFlagsWriteCallStyleSignature|checker.TypeFormatFlagsWriteTypeArgumentsOfSignature|checker.TypeFormatFlagsWriteArrowStyleSignature)) |
Copilot
AI
Oct 28, 2025
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.
GetResolvedSignature may return nil if signature resolution fails. This would cause SignatureToStringEx to receive a nil signature, which could lead to a panic or incorrect output. Add a nil check: if sig := c.GetResolvedSignature(callNode); sig != nil { ... } else { /* fall back to type display */ }.
| b.WriteString(c.SignatureToStringEx(c.GetResolvedSignature(callNode), container, typeFormatFlags|checker.TypeFormatFlagsWriteCallStyleSignature|checker.TypeFormatFlagsWriteTypeArgumentsOfSignature|checker.TypeFormatFlagsWriteArrowStyleSignature)) | |
| if sig := c.GetResolvedSignature(callNode); sig != nil { | |
| b.WriteString(c.SignatureToStringEx(sig, container, typeFormatFlags|checker.TypeFormatFlagsWriteCallStyleSignature|checker.TypeFormatFlagsWriteTypeArgumentsOfSignature|checker.TypeFormatFlagsWriteArrowStyleSignature)) | |
| } else { | |
| b.WriteString(c.TypeToStringEx(c.GetTypeOfSymbolAtLocation(symbol, node), container, typeFormatFlags)) | |
| } |
|
This fixes a few hover tests; here's a patch you could apply: diff --git a/internal/fourslash/_scripts/failingTests.txt b/internal/fourslash/_scripts/failingTests.txt
index 2aba718e6..29254c11a 100644
--- a/internal/fourslash/_scripts/failingTests.txt
+++ b/internal/fourslash/_scripts/failingTests.txt
@@ -118,7 +118,6 @@ TestCompletionListInvalidMemberNames2
TestCompletionListInvalidMemberNames_escapeQuote
TestCompletionListInvalidMemberNames_startWithSpace
TestCompletionListInvalidMemberNames_withExistingIdentifier
-TestCompletionListObjectMembersInTypeLocationWithTypeof
TestCompletionListOfGenericSymbol
TestCompletionListOnAliases
TestCompletionListStringParenthesizedExpression
@@ -251,7 +250,6 @@ TestGetJavaScriptQuickInfo7
TestGetJavaScriptQuickInfo8
TestGetJavaScriptSyntacticDiagnostics24
TestGetOccurrencesIfElseBroken
-TestGetQuickInfoForIntersectionTypes
TestHoverOverComment
TestImportCompletionsPackageJsonExportsSpecifierEndsInTs
TestImportCompletionsPackageJsonExportsTrailingSlash1
@@ -425,7 +423,6 @@ TestQuickInfoJSDocFunctionThis
TestQuickInfoJSExport
TestQuickInfoJsDocGetterSetterNoCrash1
TestQuickInfoJsDocNonDiscriminatedUnionSharedProp
-TestQuickInfoJsPropertyAssignedAfterMethodDeclaration
TestQuickInfoJsdocTypedefMissingType
TestQuickInfoMappedSpreadTypes
TestQuickInfoMappedType
diff --git a/internal/fourslash/tests/gen/completionListObjectMembersInTypeLocationWithTypeof_test.go b/internal/fourslash/tests/gen/completionListObjectMembersInTypeLocationWithTypeof_test.go
index 18576a0bf..7c1baea14 100644
--- a/internal/fourslash/tests/gen/completionListObjectMembersInTypeLocationWithTypeof_test.go
+++ b/internal/fourslash/tests/gen/completionListObjectMembersInTypeLocationWithTypeof_test.go
@@ -11,7 +11,7 @@ import (
func TestCompletionListObjectMembersInTypeLocationWithTypeof(t *testing.T) {
t.Parallel()
- t.Skip()
+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `// @strict: true
const languageService = { getCompletions() {} }
diff --git a/internal/fourslash/tests/gen/getQuickInfoForIntersectionTypes_test.go b/internal/fourslash/tests/gen/getQuickInfoForIntersectionTypes_test.go
index d3d68ddc6..fb71b8925 100644
--- a/internal/fourslash/tests/gen/getQuickInfoForIntersectionTypes_test.go
+++ b/internal/fourslash/tests/gen/getQuickInfoForIntersectionTypes_test.go
@@ -9,7 +9,7 @@ import (
func TestGetQuickInfoForIntersectionTypes(t *testing.T) {
t.Parallel()
- t.Skip()
+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `function f(): string & {(): any} {
return <any>{};
diff --git a/internal/fourslash/tests/gen/quickInfoJsPropertyAssignedAfterMethodDeclaration_test.go b/internal/fourslash/tests/gen/quickInfoJsPropertyAssignedAfterMethodDeclaration_test.go
index fec1804e5..44bde5ac7 100644
--- a/internal/fourslash/tests/gen/quickInfoJsPropertyAssignedAfterMethodDeclaration_test.go
+++ b/internal/fourslash/tests/gen/quickInfoJsPropertyAssignedAfterMethodDeclaration_test.go
@@ -9,7 +9,7 @@ import (
func TestQuickInfoJsPropertyAssignedAfterMethodDeclaration(t *testing.T) {
t.Parallel()
- t.Skip()
+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `// @noLib: true
// @allowJs: true |
In this PR:
@typetags.(method), not(property).