search

microsoft / pylance-release

Documentation and issues for Pylance
Creative Commons Attribution 4.0 International
1.72k stars 766 forks source link

Function argument hints and docs only shown with an additional comma #1128

Closed NiklasRosenstein closed 3 years ago

NiklasRosenstein commented 3 years ago

Environment data

Expected behaviour

Inside a function call, after a comma, the language server shows hints about the function arguments and it's docstrings.

Actual behaviour

Pylance only shows that information after an additional comma.

Screen Shot 2021-04-05 at 12 46 51 Screen Shot 2021-04-05 at 12 47 05

Logs

Background analysis message: getDiagnosticsForRange
Background analysis message: getDiagnosticsForRange
Background analysis message: getSemanticTokens delta
[BG(1)] getSemanticTokens delta previousResultId:1617655032602 at /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py ...
[BG(1)]   parsing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (3ms)
[BG(1)]   binding: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (0ms)
[BG(1)] getSemanticTokens delta previousResultId:1617655032602 at /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (28ms)
Background analysis message: analyze
[BG(1)] analyzing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py ...
[BG(1)]   checking: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (3ms)
[BG(1)] analyzing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (3ms)
Background analysis message: resumeAnalysis
Background analysis message: getDiagnosticsForRange
Background analysis message: getDiagnosticsForRange
Background analysis message: setFileOpened
Background analysis message: markFilesDirty
[FG] parsing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (3ms)
[FG] binding: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (1ms)
Background analysis message: getDiagnosticsForRange
Background analysis message: getDiagnosticsForRange
Background analysis message: getSemanticTokens delta
[BG(1)] getSemanticTokens delta previousResultId:1617655035941 at /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py ...
[BG(1)]   parsing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (2ms)
[BG(1)]   binding: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (0ms)
[BG(1)] getSemanticTokens delta previousResultId:1617655035941 at /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (26ms)
Background analysis message: analyze
[BG(1)] analyzing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py ...
[BG(1)]   checking: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (1ms)
[BG(1)] analyzing: /Users/nrosenstein/projects/craftr/craftr-dsl/src/craftr/dsl/__init__.py (1ms)
Background analysis message: resumeAnalysis
Background analysis message: getDiagnosticsForRange
Background analysis message: getDiagnosticsForRange
jakebailey commented 3 years ago

Is there a reason why your call and dict literal are unclosed? VS Code defaults to ensuring matching parens/braces/brackets.

Our parser tries to recover as best as it can, but without the ending paren and curly brace, we might be getting things wrong. Can you add in the ) and } and retest?

NiklasRosenstein commented 3 years ago

Hey @jakebailey!

I have that feature disabled in VSCode because I prefer to type the closing brackets and quotation myself.

Sure no problem. Unfortunately it looks like that doesn't fix it.

image image

Seems to me like something must be wrong here to begin with, given that the double comma is actually invalid syntax and the single comma is valid (with the closing parentheses).

Lmk if you need anything else. Cheers!

jakebailey commented 3 years ago

Thanks. Is the code for Closure available somewhere?

NiklasRosenstein commented 3 years ago

Actually what is curious is that with the closing control characters, it emphasises the right argument (here: owner), and without them it emphasises the argument after (i.e. delegate). Also some other things seems to be happening here when moving the cursor.

So in all of the below cases, the two commas are necessary for the hint to appear at all. But only in the case where the closing control characters are present, and the cursor is located after the second comma, it emphasises the correct third argument.

image

image

image

Yes, you can find the code to that class here:

https://github.com/craftr-build/craftr/blob/5.x/craftr-core/src/craftr/core/closure/__init__.py

erictraut commented 3 years ago

It appears that the signature help is dismissed and doesn't reappear when a nested call expression is completed.

Here's a simplified repro:

def func1(a: int, b: int) -> None: ...
def func2() -> int: ...

func1(func2(),
erictraut commented 3 years ago

I found the bug. We use a heuristic in the signature provider to deal with "broken" parse trees, such as when the expression is missing a closing parenthesis. This heuristic was failing in this particular case. I've added some additional logic to prevent this failure.

@jakebailey, here's a PR for you to review when you have time: https://github.com/microsoft/pyright/pull/1732

jakebailey commented 3 years ago

This issue has been fixed in version 2021.4.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202140-7-april-2021

NiklasRosenstein commented 3 years ago

I can confirm it works. Thank you! 😃