Skip to content
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

Fix activeParameter of signature help when there is no closing paren #2204

Conversation

ben-krieger
Copy link
Contributor

@ben-krieger ben-krieger commented Feb 27, 2025

In 0.13.0, I believe that activeParameter of signature help is following an incorrect pattern of 0, 0, 1, 2... when the closing right paren is missing. This PR adds tests which cover that case and others and provides a fix.

@ben-krieger
Copy link
Contributor Author

Tests are failing as expected:

$ ~/.zig/zig-linux-x86_64-0.14.0-dev.3388+e0a955afb/zig build test -Dtest-filter="no right paren"
test
└─ run test 0/1 passed, 1 failed
error: 'lsp_features.signature_help.test.no right paren' failed: expected 1, found 0
/home/ben/.zig/zig-linux-x86_64-0.14.0-dev.3388+e0a955afb/lib/std/testing.zig:103:17: 0x11a21ad in expectEqualInner__anon_32742 (test)
                return error.TestExpectedEqual;
                ^
/home/ben/.zig/zig-linux-x86_64-0.14.0-dev.3388+e0a955afb/lib/std/testing.zig:169:21: 0x11a2288 in expectEqualInner__anon_32736 (test)
                    try expectEqual(expected_payload, actual_payload);
                    ^
/home/ben/src/zls/tests/lsp_features/signature_help.zig:382:5: 0x11a38e6 in testSignatureHelp (test)
    try std.testing.expectEqual(expected_active_parameter, response.activeParameter);
    ^
/home/ben/src/zls/tests/lsp_features/signature_help.zig:68:5: 0x11a432c in test.no right paren (test)
    try testSignatureHelp(
    ^
error: while executing test 'lsp_features.signature_help.test.no right paren', the following test command failed:
/home/ben/src/zls/.zig-cache/o/bbd37958551a3731541edbf7f8d1f980/test --seed=0xf61ad0c1 --cache-dir=/home/ben/src/zls/.zig-cache --listen=-
Build Summary: 12/14 steps succeeded; 1 failed; 0/1 tests passed; 1 failed
test transitive failure
└─ run test 0/1 passed, 1 failed

So the first failed test is this:

fn foo(a: u32, b: u32) void {
    foo(0,<cursor>
}

where the expected activeParameter is 1, but instead 0 is being returned.

@ben-krieger
Copy link
Contributor Author

The problem seems to be that within getSignatureInfo the lastToken will be .comma whether you're left, on, or right of it. Since for left and on we wish to discard it, we do. But for right, we should not. Unfortunately, without a closing right paren (or any other token), there's no way to tell that we've "passed" the comma.

@ben-krieger
Copy link
Contributor Author

Pushed a fix for the broken tests. I'd love to see this in 0.14.0, so please let me know how I can help.

@ben-krieger ben-krieger force-pushed the signature-help-tests-multiline-no-rparen branch from b5dda70 to aab9d31 Compare February 27, 2025 20:34
@ben-krieger ben-krieger changed the title Add signature help tests for multiline and missing right paren Fix activeParameter of signature help when there is no closing paren Feb 27, 2025
@Techatrix Techatrix force-pushed the signature-help-tests-multiline-no-rparen branch from aab9d31 to 9d8ef0f Compare March 2, 2025 16:33
Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed test coverage. The fix looks good to me.

@Techatrix Techatrix merged commit 70b7e16 into zigtools:master Mar 2, 2025
6 checks passed
@ben-krieger ben-krieger deleted the signature-help-tests-multiline-no-rparen branch March 3, 2025 02:10
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