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

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

Closed
NiklasRosenstein opened this issue Apr 5, 2021 · 8 comments
Closed
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@NiklasRosenstein
Copy link

Environment data

  • Language Server version: 2021.3.4
  • OS and version: osx 15.15.7
  • Python version (& distribution if applicable, e.g. Anaconda): Miniconda Python 3.9.1

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
@github-actions github-actions bot added the triage label Apr 5, 2021
@jakebailey
Copy link
Member

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?

@jakebailey jakebailey added the waiting for user response Requires more information from user label Apr 5, 2021
@github-actions github-actions bot removed the triage label Apr 5, 2021
@NiklasRosenstein
Copy link
Author

NiklasRosenstein commented Apr 5, 2021

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
Copy link
Member

Thanks. Is the code for Closure available somewhere?

@NiklasRosenstein
Copy link
Author

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

@jakebailey jakebailey added needs investigation Could be an issue - needs investigation and removed waiting for user response Requires more information from user labels Apr 5, 2021
@erictraut
Copy link
Contributor

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
Copy link
Contributor

erictraut commented Apr 5, 2021

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: microsoft/pyright#1732

@erictraut erictraut added bug Something isn't working and removed needs investigation Could be an issue - needs investigation labels Apr 5, 2021
@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Apr 5, 2021
@jakebailey
Copy link
Member

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
Copy link
Author

I can confirm it works. Thank you! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

3 participants