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

Sort imports is severely bugged with Pylance language server #23

Closed
ErwanDL opened this issue Jul 1, 2020 · 20 comments
Closed

Sort imports is severely bugged with Pylance language server #23

ErwanDL opened this issue Jul 1, 2020 · 20 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@ErwanDL
Copy link

ErwanDL commented Jul 1, 2020

Environment data

  • VS Code version: 1.46.1
  • Extension version (available under the Extensions sidebar): 2020.6.91350
  • OS and version: macOS 10.14.6
  • Python version (& distribution if applicable, e.g. Anaconda): 3.7
  • Relevant/affected Python-related VS Code extensions and their versions: Pylance 2020.6.1
  • Value of the python.languageServer setting: "Pylance"

Expected behaviour

"Sort imports on save" functionality works correctly.

Actual behaviour

When using the new Pylance language server (which is, apart from this bug, absolutely incredible, thanks a lot !), the "sort imports" functionality is bugged, doesn't respect the sortImports settings of VSCode, and produces invalid Python code. I have looked at other similar issues (ex : microsoft/vscode#83586), but an important difference is that the issue I describe here is present even when there is no formatter (other than the import sorter) activated for Python code.

Steps to reproduce:

Open a new folder, create a Python file, write the following code in it :

import os
import logging

print(os.path.pathsep)
logging.info("hello world")

Activate the "source.organizeImports" functionality on save, without any additional args, deactivate any other formatter, linter, etc. Activate the Pylance language server.

Save the current file. The imports should be transformed to

import logging
import os

but what actually happens is

import logging
import logging

2020-07-01 12 05 08

If you disable Pylance and use Jedi instead, the imports are sorted correctly.

The bug can become much more severe with more complex imports if you save several times in a row, generating completely incorrect code with no obvious pattern :
2020-07-01 12 12 07

Logs

Output for Python in the Output panel (ViewOutput, change the drop-down the upper-right of the Output panel to Python)

> python3.7 ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> python3.6 ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> python2 ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> python3 ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import sys;print(sys.executable)"
> ~/Library/Caches/pypoetry/virtualenvs/covia-py3.6/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import jupyter"
> ~/Library/Caches/pypoetry/virtualenvs/covia-py3.6/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py -c "import notebook"
> ~/Library/Caches/pypoetry/virtualenvs/covia-py3.6/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py jupyter kernelspec --version
Starting Pylance language server.
Python interpreter path: ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python
> conda --version
> ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
cwd: ~/Documents/dev/other_test
> ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
cwd: ~/Documents/dev/other_test


Output from Console under the Developer Tools panel (toggle Developer Tools on under Help; turn on source maps to make any tracebacks be useful by running Enable source map support for extension debugging)

[Extension Host] Info Python Extension: 2020-07-01 12:09:21: > ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
console.ts:137 [Extension Host] Info Python Extension: 2020-07-01 12:09:21: cwd: ~/Documents/dev/other_test
console.ts:137 [Extension Host] Info Python Extension: 2020-07-01 12:09:21: > ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
console.ts:137 [Extension Host] Info Python Extension: 2020-07-01 12:09:21: cwd: ~/Documents/dev/other_test

The logs seem to indicate that sortImports.py is actually called twice at a time for some reason, maybe the two executions conflict with one another ?

@ErwanDL ErwanDL changed the title Sort imports is severly bugged with Pylance language server Sort imports is severely bugged with Pylance language server Jul 1, 2020
@jakebailey jakebailey transferred this issue from microsoft/vscode-python Jul 1, 2020
@jakebailey
Copy link
Member

Transferring this over; do you have isort enabled in the extension, or is Pylance the sole organizer?

We may want to disable our organize import functionality (at least the one that's used on save and such) given we require the core extension.

@jakebailey
Copy link
Member

The logs seem to indicate that sortImports.py is actually called twice at a time for some reason, maybe the two executions conflict with one another ?

This could be something along the lines of:

  • Extension sorts the imports
  • Pylance sorts the imports
  • Extensions sees the file change and sorts again

Or similar race conditions.

@jakebailey jakebailey added the bug Something isn't working label Jul 1, 2020
@gramster
Copy link
Member

gramster commented Jul 2, 2020

Is this a dup of microsoft/vscode-python#10579? In which case it may not be a pylance issue.

@jakebailey
Copy link
Member

Could be, yeah. My thought is that Pylance also potentially doing import sorting itself could be triggering the race much more often (rather than trying to make it race by hand).

@ErwanDL
Copy link
Author

ErwanDL commented Jul 2, 2020

@jakebailey Yes I have isort activated in the Python extension (it's the default and only import sorter used by the extension I believe). Does Pylance do its own import sorting as well ? Because in the logs I pasted, it's only isort being called twice :

This is the command being called twice at a time in the logs :

> ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
cwd: ~/Documents/dev/other_test
> ~/.pyenv/versions/3.7.2/Python.framework/Versions/3.7/bin/python ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/pyvsc-run-isolated.py ~/.vscode/extensions/ms-python.python-2020.6.91350/pythonFiles/sortImports.py - --diff
cwd: ~/Documents/dev/other_test

And this is the sortImports.py file that is called, which in turns calls isort :

# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

import io
import os
import os.path
import sys

isort_path = os.path.join(os.path.dirname(__file__), "lib", "python")
sys.path.insert(0, isort_path)

# Work around stdin buffering issues on windows (https://bugs.python.org/issue40540)
# caused in part by isort seeking within the stdin stream by replacing the
# stream with something which is definitely seekable.
try:
    # python 3
    stdin = sys.stdin.buffer
except AttributeError:
    # python 2
    stdin = sys.stdin

sys.stdin = io.BytesIO(stdin.read())
# End workaround

import isort.main

isort.main.main()

@jakebailey
Copy link
Member

Yes, Pylance has its own.

@ErwanDL
Copy link
Author

ErwanDL commented Jul 2, 2020

I have not seen any setting or bullet point in Pylance's features list about the import sorter ; maybe it could be helpful clarifying that it exists, and also adding an option to enable/disable it and use the Python extension's one instead ? 😃 That would be awesome !

@jakebailey
Copy link
Member

jakebailey commented Jul 2, 2020

I think we may just disable the one in Pylance for now, and allow the core extension to continue to offer this using isort. (The built-in one is good too, but we need to work on the interplay.)

@ErwanDL
Copy link
Author

ErwanDL commented Jul 2, 2020

That would be great ! Do you think this will be a hotfix or should we users wait the next planned release ?

@jakebailey
Copy link
Member

We're still working on what "next release" or "hotfix" will mean, but I expect that this will be disabled in the next version (whichever that is) and we can see if that fixes the race.

@gramster
Copy link
Member

gramster commented Jul 2, 2020 via email

@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Jul 2, 2020
@jakebailey
Copy link
Member

The race in the core extension will likely be fixed in microsoft/vscode-python#12728, but we didn't intend to enable our own import sorting at release, so it'll be disabled in the next release until we can better work out the dynamic between the different sorting providers.

@karrtikr
Copy link

karrtikr commented Jul 3, 2020

Although this is similar to microsoft/vscode-python#10579, this isn't a dup.

The cause here was that both Pylance and the extension initialized the isort process to get the diff. The result (which is the diff/workspace edit) from both the processes is then applied almost simultaneously, leading to this configuration.

So the correct solution would be to ensure we're using either the extension, or Pylance for import sorting, but not both.

@Minituff
Copy link

Minituff commented Jul 9, 2020

Is there a way to disable Pylance import sorting? I cannot find the option in vscode as I could with Pyright.

@jakebailey
Copy link
Member

In our next release, it will be disabled.

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.7.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#202070-9-july-2020

@ThiefMaster
Copy link

May I ask what's the reason for reinventing the wheel with import sorting instead of using isort under the hood?

This used to be a huge mess when I was still using PyCharm since it was pretty much impossible to get the same logic in the editor and in isort. Also, in CI and pre-commit hooks I obviously use isort (because neither have access to pylance), so unless it's 100% compatible and can reuse the isort config file, it means I now have to put the config on how to sort imports in two places...

@jakebailey
Copy link
Member

jakebailey commented Jul 20, 2020

Note the history of this code; Pylance is mostly pyright, which is its own extension that has existed for a year+. It worked without the Python extension, and we inherited many of its behaviors which are implemented via the language server protocol, "organize imports" included.

We are well aware of isort, but we mistakenly left our import sorting enabled at release. We now have it disabled so that the Python extension can continue to run isort as it did beforehand without our involvement, similarly to what we do for code formatting.

The actual import sorting code still needs to be around; we need to be able to calculate a semantically correct text change to add imports for auto-import fixes and completions, but you should definitely be able to use isort as before without Pylance getting in your way. If not, please file another issue as that's not the currently intended behavior.

@jakebailey
Copy link
Member

jakebailey commented Jul 20, 2020

As for why not use isort internally, using external tooling like that (including formatters) is "simple" to an extent (just exec!), but the main issue is not about using them but about the workflow on getting users to actually install the tooling in the correct environment.

The Python extension and PTVS have invested great amounts of time in getting good support for installing packages into the slew of different types of Python environments, the UI experience to manage them, as well as the special casing needed for package managers like conda which do things differently (including the actual running of modules).

The language server cannot (at the moment anyway) offer prompts to ask for permission to install things, and the extension and PTVS will still need to retain their package installing code for other reasons, so it turns into a code duplication problem. Our current setup doesn't change this, and continues to require the extension / PTVS to manage and run these external tools.

@reservoirinvest
Copy link

How can vscode be set up to sort module level import only at top of file with pylance (ver 2020.7.3) ?

The setting:

{
    "python.languageServer": "Pylance",
    "python.formatting.provider": "autopep8",
    "python.formatting.autopep8Args": [
        "--ignore",
        "E402"
    ],
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
        "source.organizeImports": false
    },
   ...
}

...does not work

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

7 participants