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

organizeImports duplicates and removes code (destroys syntax) #9889

Closed
Spenhouet opened this issue Feb 4, 2020 · 5 comments
Closed

organizeImports duplicates and removes code (destroys syntax) #9889

Spenhouet opened this issue Feb 4, 2020 · 5 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug info-needed Issue requires more information from poster

Comments

@Spenhouet
Copy link

Spenhouet commented Feb 4, 2020

Environment data

  • VS Code version: 1.41.1
  • Extension version (available under the Extensions sidebar): 2020.1.58038
  • OS and version: Windows 10 (build 1909)
  • Python version: 3.7.4 64bit (direct install)
  • Type of virtual environment used: N/A
  • Jedi or Language Server?: false (jedi is disabled)

Expected behaviour

Already sorted imports should not change.

I would expect organizeImports to organize imports and not to format my code (less alone destroying it).

Actual behaviour

Import is reformatted (and destroyed).

  • Unstable / inconsistent formatting - Formatting changes with every save
  • Removes or duplicates code - Destroys code base and syntax

The following import:

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey, CaseInsensitiveSet,
                                UpperLowerBound)

will either be formatted to this (duplicated and destroyed code):

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey,
                                CaseInsensitiveSet, UpperLowerBound)
    UpperLowerBound)

or that (unnecessary change / reformatting):

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey,
                                CaseInsensitiveSet, UpperLowerBound)  

If it messes up the imports and you keep on saving your file, the rest of the code will be destroyed. I got random deletions and duplication of other lines in the code.

Steps to reproduce:

  1. Use the following setting:
{
    "editor.formatOnSave": false,
    "python.jediEnabled": false,
    "[python]": {
        "editor.codeActionsOnSave": {
            "source.organizeImports": true,
        }
    }
}
  1. Open a python file
  2. Enter the following import
from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey, CaseInsensitiveSet,
                                UpperLowerBound)
  1. Press save

Logs

Output for Python:

> ~\AppData\Local\Programs\Python\Python37\python.exe c:\Users\spenh\.vscode\extensions\ms-python.python-2020.1.58038\pythonFiles\sortImports.py ~\AppData\Local\Temp\tmp-12028QiovWWcV0f0E.py --diff
> ~\AppData\Local\Programs\Python\Python37\python.exe c:\Users\spenh\.vscode\extensions\ms-python.python-2020.1.58038\pythonFiles\sortImports.py ~\AppData\Local\Temp\tmp-12028QiovWWcV0f0E.py --diff

Note here: why is sort run twice? Maybe this issue is some form of race condition where sort is started twice and overwrites change (still: it should not change anything for the given code).

Output from Console :

[Extension Host] Info Python Extension: 2020-02-04 09:59:00: Cached data exists getEnvironmentVariables, c:\Users\spenh\Documents\myproject\myprojectname\.vscode\test.py

2 [Extension Host] Info Python Extension: 2020-02-04 09:59:00: > ~\AppData\Local\Programs\Python\Python37\python.exe c:\Users\spenh\.vscode\extensions\ms-python.python-2020.1.58038\pythonFiles\sortImports.py ~\AppData\Local\Temp\tmp-12028pKLviFI8s93m.py --diff

[Extension Host] Notification handler 'textDocument/publishDiagnostics' failed with message: Cannot read property 'connected' of undefined
t.log @ console.ts:137
$logExtensionHostMessage @ mainThreadConsole.ts:39
_doInvokeHandler @ rpcProtocol.ts:398
_invokeHandler @ rpcProtocol.ts:383
_receiveRequest @ rpcProtocol.ts:299
_receiveOneMessage @ rpcProtocol.ts:226
(anonymous) @ rpcProtocol.ts:101
fire @ event.ts:581
fire @ ipc.net.ts:453
_receiveMessage @ ipc.net.ts:733
(anonymous) @ ipc.net.ts:592
fire @ event.ts:581
acceptChunk @ ipc.net.ts:239
(anonymous) @ ipc.net.ts:200
t @ ipc.net.ts:28
emit @ events.js:200
addChunk @ _stream_readable.js:294
readableAddChunk @ _stream_readable.js:275
Readable.push @ _stream_readable.js:210
onStreamRead @ internal/stream_base_commons.js:166
@Spenhouet Spenhouet added triage-needed Needs assignment to the proper sub-team bug Issue identified by VS Code Team member as probable bug labels Feb 4, 2020
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Feb 4, 2020
@kimadeline
Copy link

kimadeline commented Feb 4, 2020

Hi @Spenhouet 👋 Thank you for reaching out.

I will address the points you raised separately:

  • Code being duplicated and deleted: Sadly I couldn't reproduce this behaviour which the code snippet you provided.
    • Could you link to a sample repo or paste some code with which you can reproduce this behaviour consistently?
    • The import sorting logic in the extension (whether called by "source.organizeImports": true or Command Palette > Sort Imports) calls isort, what is the result of running python -m isort yourfile.py --diff directly? If the lines are being duplicated or removed there then it's an issue with isort, which you should file on their repo;
  • Code being reformatted: I could replicate it with the import example you mentioned, however it's part of isort's behaviour, not the extension (you can see it when running python -m isort yourfile.py --diff in your terminal). If you think this could be enhanced you are welcome to open an issue on their repo;
  • Why is sort run twice: it isn't run twice, but rather a bug when printing the command in the output panel: Python commands are printed twice in the output channel #7160, feel free to upvote it to help bump up its priority.

Thank you!

@kimadeline kimadeline added the info-needed Issue requires more information from poster label Feb 4, 2020
@Spenhouet
Copy link
Author

Spenhouet commented Feb 5, 2020

Hi @kimadeline,

thank you for your time.

I did look further into this and want to reiterate and reformulate this issue.

First off: My goal is to enforce a consistent formatting and sorted imports without the need of manual action and also without GIT changes with every save.
Currently I find my self unable to setup our project like that.
Therefore I want to address this issue, try to find the source of that and to inform the right entity.

This being said, let us reiterate my problems.

Further using the import example from above:

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey, CaseInsensitiveSet,
                                UpperLowerBound)

I noticed that I have two context menu entries: Organize Imports, Sort Imports
So I run them separately on the above import.

Organize Imports:

from utils.python_utils import (
    CaseInsensitiveDict, CaseInsensitiveKey, CaseInsensitiveSet,
    UpperLowerBound)

Sort Imports:

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey,
                                CaseInsensitiveSet, UpperLowerBound)

Both commands also work both ways and are always consistent without constantly changing (if run multiple times).

I noticed that if I press save, both commands seem to run at the same time. This is what results in the destroyed code:

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey,
                                CaseInsensitiveSet, UpperLowerBound)
    UpperLowerBound)

Always reproduceable.

I did find this:
image

  1. Organize Imports - is a VS code feature
  2. Python Refactor: Sort Imports - isort functionality via extension "Python Refactor"
  3. Pyright: Organize Imports - other import formatting via extension "Pyright"

If I choose Organize Imports 1. I get the following context menu:
image

This leads me to conclude that both 2. and 3. are activated by the following setting:

    "[python]": {
        "editor.codeActionsOnSave": {
            "source.organizeImports": true,
        }
    },

This is where the destructive behavior is coming from.

Result 1:
Using only one code action (deactivating pyright) resolves the destructive behavior.
There should be a option to activate source.organizeImports extension wise.

For example:

"editor.codeActionsOnSave": {
    "source.organizeImports.python_refactor": true,
    "source.organizeImports.pyright": false
}

Should I create a new issue for that?

But even with pyright deactivated I still have other issues, therefore let us reformulate the the issue:

Goal: On save: Code automatically formatted & imports sorted
What we need: formatting provider (who currently does not sort imports) and therefore a tool to sort imports

Since the tool to sort imports is only supposed to do exactly that (yes, it will have to reformat to achieve that), we can expect the following workflow / execution:

  1. Sort imports
  2. Formatting
    This needs to run sequentially since we want the imports sorted and then formatted by our formatting rules.

From here on with the following settings (pyright disabled):

{
    "editor.formatOnSave": true,
    "python.jediEnabled": false,
    "python.formatting.provider": "yapf",
    "python.formatting.yapfArgs": [
        "--style",
        "{based_on_style: google, column_limit: 100}"
    ]
}

With every press on save it will no reformat the import, switching between these two:

(1)

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey, CaseInsensitiveSet,
                                UpperLowerBound)

and
(2)

from utils.python_utils import (CaseInsensitiveDict, CaseInsensitiveKey,
                                CaseInsensitiveSet, UpperLowerBound)

Again, as with the first issue, let's look at what happens if we only run one of the following actions:

  1. Sort imports
  2. Formatting

Running only 1. will yield (2).
Running only 2. will yield (1)

If run independently, both run consistently and do not change on multiple successive runs.

This lead me to conclude that 1. and 2. are run either parallel or in random order but not consistently successive.

Result 2:
VS code should consistently (same order) run code actions and formatting in succession.
The formatting provider should only be run after the code actions have finished.
These should not overlap.

Should I create a new issue for that?

@kimadeline
Copy link

Hi @Spenhouet ,

Thank you for reformulating your issue.

First off, if you encounter the line addition/deletion issue when manually organizing imports with Pyright, it's an issue on their side, so please report it on their repo.

This being said, let us address the other points you raised.

VS code should consistently (same order) run code actions and formatting in succession.

The race condition between editor.formatOnSave and editor.codeActionsOnSave is a known issue that has already been reported upstream: microsoft/vscode#83586. You are welcome to upvote it or leave a comment there to help prioritizing it.

There should be a option to activate source.organizeImports extension wise.

We (the Python extension) only use the hook provided by VS Code to register our import sorting command. It would be up to VS Code to offer the possibility to select which extension to use to organize imports, similar to the editor.defaultFormatter dropdown setting. It is a feature request that you should file against VS Code directly.

@ghost ghost removed the triage label Feb 6, 2020
@Spenhouet
Copy link
Author

First off, if you encounter the line addition/deletion issue when manually organizing imports with Pyright, it's an issue on their side, so please report it on their repo.

That is not the issue. Pyright on its own does work fine (as does Python Refactor).
But if multiple code actions exist, they too have a race condition between each other.

I will create issues on the VS code repository.

@Spenhouet
Copy link
Author

@lock lock bot locked as resolved and limited conversation to collaborators Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants