Skip to content

Fix signature help ranges to prevent display after closing parentheses #1420

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 18, 2025

Signature help was incorrectly being shown for positions after closing parentheses and in whitespace following function calls. For example:

let obj = {
    foo(s: string): string {
        return s;
    }
};

let s = obj.foo("Hello, world!")/*afterCall*/  
  /*afterNewline*/;

Previously, both afterCall and afterNewline positions would incorrectly show signature help for the foo method when they should show none.

Root Cause

The issue was in getContainingArgumentInfo which was finding argument information for KindCloseParenToken nodes even when the cursor position was at or after the end of the closing parenthesis. This caused signature help to be displayed in positions where it shouldn't appear.

Solution

Added logic to skip KindCloseParenToken nodes when the position is at or after their end position:

// Special case: if we're at a close paren token and the position is after its end, 
// don't provide signature help. This handles the case where the cursor is right 
// after the closing parenthesis
if n.Kind == ast.KindCloseParenToken && position >= n.End() {
    continue
}

Testing

  • Added TestSignatureHelpRangeIssue that validates signature help is shown inside calls but not after closing parentheses
  • Added runSignatureHelpTestWithNil test helper to validate cases where signature help should NOT be provided
  • Verified existing signature help functionality still works correctly
  • Added TestSignatureHelpNestedCallPrecedence to document a related issue with nested call precedence (to be addressed separately)

Impact

This fix improves the user experience by ensuring signature help only appears in appropriate contexts within function calls, eliminating the confusing behavior where signature help would persist after completing a function call.

Addressing #1419.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] Signature help is wrong for nested calls, has weird applicable ranges Fix signature help ranges to prevent display after closing parentheses Jul 18, 2025
@Copilot Copilot AI requested a review from DanielRosenwasser July 18, 2025 23:08
Copilot finished work on behalf of DanielRosenwasser July 18, 2025 23:08
argRange := argumentInfo.argumentsRange
if position < argRange.Pos() || position > argRange.End() {
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

I think the real fix is to implement/port the commented out code:

		// !!!
		// const containingList = findContainingList(startingToken);
		// return !!containingList && contains(invocationChildren, containingList);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've implemented the commented-out code as suggested. The fix includes:

  1. Proper findContainingList logic for comma tokens with containsNodeList helper function
  2. Extended isSyntacticOwner to handle CloseParenToken cases to prevent signature help after closing parentheses
  3. Added getASTChildren helper to work with AST structure instead of just tokens
  4. Applied syntactic owner checks for critical cases like CloseParenToken even when onlyUseSyntacticOwners is false

The range-based fix has been replaced with this more comprehensive TypeScript-compatible solution in commit 8991cdb.

- Implemented the commented-out findContainingList logic for comma tokens in isSyntacticOwner
- Added proper NodeList containment checking with containsNodeList helper function
- Extended isSyntacticOwner to handle CloseParenToken to prevent signature help after closing parentheses
- Modified getCandidateOrTypeInfo to apply syntactic owner checks for critical cases like CloseParenToken
- Added getASTChildren helper to get proper AST children instead of just tokens
- Added nil check for candidateInfo to prevent crashes when syntactic owner check fails
- Reverted the previous range-based fix in favor of this more comprehensive solution

Co-authored-by: DanielRosenwasser <[email protected]>
@Copilot Copilot AI requested a review from DanielRosenwasser July 21, 2025 18:23
Copilot finished work on behalf of DanielRosenwasser July 21, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants