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

Organize imports and formatters are conflicting due to a race condition #83586

Closed
luabud opened this issue Oct 29, 2019 · 15 comments
Closed

Organize imports and formatters are conflicting due to a race condition #83586

luabud opened this issue Oct 29, 2019 · 15 comments
Assignees
Labels
editor-code-actions Editor inplace actions (Ctrl + .) formatting Source formatter issues *not-reproducible Issue cannot be reproduced by VS Code Team member as described
Milestone

Comments

@luabud
Copy link
Member

luabud commented Oct 29, 2019

microsoft/vscode-python#6933

This is not an extension issue though.

  • VSCode Version: 1.39.1
  • OS Version: Windows 10

Steps to Reproduce:

  1. Have Black installed
  2. Set config options as such:
{
  "editor.formatOnSave": true,
  "editor.codeActionsOnSave": {
    "source.organizeImports": true
  }
}
  1. Create a python file with the following contents:
from something import (
    aaaaa,
    bbbbb,
    ccccc,
    ddddd,
    eeeee,
    fffff,
    ggggg,
    hhhhh,
    iiiii,
    jjjjj,
    kkkkk,
)
  1. Save the file multiple times and watch it dance

Does this issue occur when all extensions are disabled?: No

The problem is that there's a race condition since there are two formatters being used when saving changes to a file. The results vary based on which formatter completes first. I.e. if black formats first, then you see one result, then isort formats the imports and changes the imports and you see another set of changes.
As you keep hitting save both trip on one another.

The Python extension wants to organize formatters and isort to be ran consistently, but we don't have control over that as VS Code is doing the excutions.

@baolsen
Copy link

baolsen commented Nov 8, 2019

I know this issue is closed but it is the closest I found to an issue I have with isort.
Did you have any setup.cfg files to configure isort?

I was struggling to get it to work with the setup.cfg files in my project directory.
isort -sp .\setup.cfg
Produces a different sorting than if I use the editor's on-save formatting.

Looking at your description of this issue it is possible that it is actually using setup.cfg but the race condition is messing up my imports.

In your journey with VSCode + isort did you find that it checked for and respected any setup.cfg file?

If not (or you are unsure) then at least I can open a new issue to be investigated.

@luabud
Copy link
Member Author

luabud commented Nov 8, 2019

@baolsen sorry I don't think I understand what you mean, this issue is open, not closed.

@pydolan
Copy link

pydolan commented Nov 9, 2019

@baolsen – I don't think the race issue is related to VS Code not reading isort settings from setup.cfg. At least for me, I can consistently get import sorting to work as expected if I specify the settings file in my VS Code settings:

  "python.sortImports.args": [
    "--settings-path=/path/to/my/setup.cfg"
  ],

Why the above doesn't happen automatically is unclear to me, but that is for a different Issue.

@reywood
Copy link

reywood commented Nov 22, 2019

@pydolan's suggestion did seem to make the behavior consistently correct for me. I recommend using the ${workspaceFolder} var like so instead of hard coding a path.

"python.sortImports.args": [
    "--settings-path=${workspaceFolder}/setup.cfg"
]

As an aside, I also had to add "editor.codeActionsOnSaveTimeout": 2000 to my config in order to extend the timeout beyond the default 750ms. Otherwise, my file was showing as unsaved after isort had sorted the imports. Seems like isort is taking more than 750ms to complete. See issue #72496 for a better description of the problem. This problem may only occur on larger projects.

@Spenhouet
Copy link

I really hope that this gets addressed soon. It keeps us from being able to sort imports on save.

This race condition leads to deleted or duplicated code. That feels quite significant to me for this issue to be fixed.

Code actions should run before the code formatter.
They should run strictly sequentially.

@Spenhouet
Copy link

@mjbvz The comments made by @baolsen @pydolan and @reywood seem to address an issue unrelated to this issue. For clarity: Can these comments be removed from this issue?

@mjbvz mjbvz added formatting Source formatter issues editor-code-actions Editor inplace actions (Ctrl + .) labels Mar 9, 2020
@mjbvz mjbvz added this to the March 2020 milestone Mar 9, 2020
@mjbvz
Copy link
Collaborator

mjbvz commented Mar 25, 2020

Here's what I see debugging this setup for JS/TS:

  1. Organize imports is called. It returns a code action with a command:
  2. That command is then invoked and edits the document (making sure to await the result in the command) :
  • Then the formatter is called. At this point the formatter has the updated document that organize imports just changed

From this, it seems to me that VS Code calls things in the correct order and does correctly await the result of organize imports

@luabud Can you please share a link to the python extension's implementation of organize imports?

@luabud
Copy link
Member Author

luabud commented Apr 1, 2020

I actually can't repro the race condition anymore, it's consistently organizing the imports first and then triggering the formatter. And it seems that if we wanted to change the order, we could do it in the extension side, so I'm closing this issue.

FWIW the code for sorting imports is here: https://github.com/microsoft/vscode-python/blob/master/src/client/providers/importSortProvider.ts

@luabud luabud closed this as completed Apr 1, 2020
@Spenhouet
Copy link

Spenhouet commented Apr 1, 2020

I can very much still reproduce this. I just used the settings.json from above:

{
  "editor.formatOnSave": true,
  "editor.codeActionsOnSave": {
    "source.organizeImports": true
  }
}

and opened a random python file. Directly on the first one it dances and deletes lines of code.

import datetime
import subprocess
import sys
from operator import attrgetter
from pathlib import Path
from typing import Any, List, Optional, Tuple

I extracted these lines of code into an empty python file ans pressed STRG + S multiple times (nothing else):

race_condition

@luabud
Copy link
Member Author

luabud commented Apr 2, 2020

@Spenhouet Oh that's weird! I can't repro this behaviour with the sample code you proved. I tried it with black, autopep8 and yapf.

I opened an issue on the Python extension repo so we can investigate it (microsoft/vscode-python#10923), as it may not be due to a race condition. It'd help immensely if you could fill up our bug report template there providing your environment information, the formatter you're using as well as the contents of the Python channel in the Output:

image

@Spenhouet
Copy link

@luabud Sorry, I was mixing up issues. I was actually referring to this: #90220

@mjbvz mjbvz added the *not-reproducible Issue cannot be reproduced by VS Code Team member as described label Apr 2, 2020
@copdips
Copy link

copdips commented Apr 25, 2020

same issue here, the worst thing is that it even deletes some imports

@luabud
Copy link
Member Author

luabud commented Apr 27, 2020

@copdips do you see the import deletion behaviour consistently? I wasn't able to reproduce it.

@copdips
Copy link

copdips commented Apr 27, 2020

@luabud

Yes, as per the GIF in #83586 (comment), it deletes imports.
But I managed to find a workaroud from: #90221 (comment) by using source.organizeImports.python instead of source.organizeImports

@Spenhouet
Copy link

@luabud I can reproduce this again with the following import:

from transform_data import (preprocess_continous_image, transform_image_parallelized,
                            weighted_majority_resampling)

format_issues

Maybe you could try if you can also reproduce it with that import.
Formatter is yapf.

@github-actions github-actions bot locked and limited conversation to collaborators May 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-code-actions Editor inplace actions (Ctrl + .) formatting Source formatter issues *not-reproducible Issue cannot be reproduced by VS Code Team member as described
Projects
None yet
Development

No branches or pull requests

7 participants