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

"Remove unused imports" removes all imports #1547

Closed
ttSpace opened this issue Jul 2, 2021 · 14 comments
Closed

"Remove unused imports" removes all imports #1547

ttSpace opened this issue Jul 2, 2021 · 14 comments
Assignees
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@ttSpace
Copy link

ttSpace commented Jul 2, 2021

Describe the bug

image

Steps to Reproduce

  1. Type a few imports, like import array; import glob; import math
  2. Type print(math.cos(10))
  3. In the line of import you can see a pattern, then select the "Remove unused imports"
    image

Actual behavior

It removes all imports.
image

@rchiodo
Copy link
Contributor

rchiodo commented Jul 2, 2021

This sounds like it might be a pylance bug?

@rchiodo rchiodo transferred this issue from microsoft/PTVS Jul 12, 2021
@rchiodo
Copy link
Contributor

rchiodo commented Jul 12, 2021

Yeah repros in VS code.

@yvvt0379
Copy link

@ttSpace please try writing imports in seperate lines, like this:

import array
import glob
import math
math.cos(0)

and try using remove unused imports and see whether all imports are removed.

@ttSpace
Copy link
Author

ttSpace commented Jul 13, 2021

@CodeCrazy-ywt You can see it, like this:

remove_unuesd_import

@yvvt0379
Copy link

As you can see, it doesn't remove all imports. Is that okay?

@jakebailey
Copy link
Member

The code action doesn't remove all imports, only the one the quick fix is attached to (per diagnostic), so that's working as expected.

It deleting the full line sounds like a bug, but I don't know if that's because our change is wrong, or if the editor is applying all of the fixes with the same name on the same line at once.

@yvvt0379
Copy link

Actually we seldom write many imports in a single line, so there's no need to worry about that.

@heejaechang heejaechang self-assigned this Aug 10, 2021
@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 17, 2021
@heejaechang
Copy link
Contributor

it should be fixed in 2021.8.2 released today (2021.8.19)

@heejaechang heejaechang removed the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 19, 2021
@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 23, 2021
@linette-zyy
Copy link

image
This issue still occurs on this build.

@jakebailey
Copy link
Member

Those versions don't align with any version we have for Pylance. Can you check the logs for the Pylance version you're using?

@linette-zyy
Copy link

I see below info in Pylance output window.

Info: Info: Pylance language server 2021.4.1 (pyright 6410a8a2) startingServer root directory: c:\program files\microsoft visual studio\2022\preview\common7\ide\extensions\microsoft\python\core\pylance\dist

Error: Error reading settings: TypeError: Cannot read property '0' of null
Warning: Info: Info: Info: Assuming Python platform WindowsstubPath typings is not a valid directory.Searching for source filesNo source files found.



Info: Info: Warning: Info: Info: Info: Info: Info: Info: Info: Info: No configuration file found.Setting pythonPath for service "PythonApplication10": "C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python39_64\python.exe"stubPath C:\Users\vyizh12\source\repos\PythonApplication10\PythonApplication10\typings is not a valid directory.Assuming Python version 3.9Assuming Python platform WindowsSearching for source filesFound 1 source fileBackground analysis(1) root directory: c:\program files\microsoft visual studio\2022\preview\common7\ide\extensions\microsoft\python\core\pylance\distBackground analysis(1) startedBackground analysis(2) root directory: c:\program files\microsoft visual studio\2022\preview\common7\ide\extensions\microsoft\python\core\pylance\distBackground analysis(2) started

@jakebailey
Copy link
Member

VS is still on a 4 month old build of Pylance (note the version difference). When it's updated, it will get the fix.

@ttSpace
Copy link
Author

ttSpace commented Sep 3, 2021

I use it in today's build, is that supposed to happen like the following? I have to click a few times to remove unused import.
Build of Visual Studio: 17.0.0 Preview 4.0 [31702.283.main]
Build of Python Package: 17.0.21243.2

remove_unuesd_import

Info: Info: Pylance language server 2021.8.3 (pyright fa4194fd) startingServer root directory: c:\program files\microsoft visual studio\2022\preview\common7\ide\extensions\microsoft\python\core\pylance\dist

Info: Info: Info: Warning: Info: No pyproject.toml file found.Setting pythonPath for service "PythonApplication15": "C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python39_64\python.exe"stubPath C:\Users\vting\source\repos\PythonApplication15\PythonApplication15\typings is not a valid directory.Info: Assuming Python version 3.9


Assuming Python platform Windows

Info: Searching for source filesInfo: Found 1 source file
Info: Info: Background analysis(1) root directory: c:\program files\microsoft visual studio\2022\preview\common7\ide\extensions\microsoft\python\core\pylance\distBackground analysis(1) started

No configuration file found.

@jakebailey
Copy link
Member

Quick fixes are offered per-diagnostic. In VS Code, you can hover over each diagnostic and get a lightbulb per, and that's how the LSP works.

I'm not sure how VS does this, but it looks like it offers a lightbulb per line.

We're doing what we expect (one quick fix per diagnostic), but it seems like this UI only offers up one of them per line, or something. #1722 covers a quick fix that would remove all, but overall I think that this would be something to report on PTVS for investigation, as Pylance isn't behaving incorrectly according to the protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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