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

Warn against unused imports #43

Closed
balta2ar opened this issue Nov 11, 2017 · 19 comments
Closed

Warn against unused imports #43

balta2ar opened this issue Nov 11, 2017 · 19 comments
Labels
area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. feature-request Request for new features or functionality

Comments

@balta2ar
Copy link

balta2ar commented Nov 11, 2017

Is it possible to gray out unused imports in the same way that PyCharm does?

@gandhis1
Copy link

This kind of crosses into linter territory, I would argue. Also, doing Sort Imports removes unused imports I believe.

@DonJayamanne
Copy link

@balta2ar I agree with @gandhis1. Primarily because one of the other popular languages (Nodejs) doesn't do this on VS Code, instead it relies on the linter warnings to inform the user of this problem.
In summary, we won't be implementing this for now in order to maintain consistency within VS Code.

@DonJayamanne DonJayamanne added the feature-request Request for new features or functionality label Nov 14, 2017
@brettcannon brettcannon added awaiting 1-decision area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. labels Nov 14, 2017
@jeffwidman
Copy link

jeffwidman commented Dec 7, 2017

Shouldn't this be closed then?

Also, I agree with the decision--this is linter territory, no sense adding/maintaining that logic here as well.

@brettcannon
Copy link
Member

@jeffwidman yep, it should be closed 😄

@balta2ar
Copy link
Author

Related issue in TypeScript repo: microsoft/TypeScript#8165

@balta2ar
Copy link
Author

balta2ar commented May 12, 2018

A very similar feature request has been added to May 2018 sprint: microsoft/vscode#15710. I hope you guys reconsider this after the API is mature in VSCode.

It is true that there is a small overlap with the linter's functionality, but red squiggle under a variable/import can mean a bunch of different things, and you don't know what it is until you hover with a mouse. In contrast, grayed out variable/import is definite, leaves no room for guesses, does not require reading the message, conveys the information immediately.

I suggest that you try it for yourself in PyCharm, for example, live with it for a while. Nobody likes bloated software, but this IS very useful in practice.

EDIT: another related request: microsoft/vscode#20219

@brettcannon brettcannon changed the title Gray out unused imports Warn against unused imports May 14, 2018
@brettcannon brettcannon reopened this May 14, 2018
@brettcannon
Copy link
Member

@MikhailArkhipov would it be possible to warn against unused imports in the analysis engine?

@MikhailArkhipov
Copy link

Yes, as well as about unresolved ones. Needs some work since this info is not currently in the portable LS. I will open PTVS issues on this.

@giyyapan
Copy link

@MikhailArkhipov Hi, about the analysis engine you're talking about, would it be a possible to remove all unused imports in the engine?
I didn't find any issue discussing "removing unused imports automatically", if it's not included in the engine, maybe I should open one.

@brettcannon
Copy link
Member

@giyyapan detection first, then we can talk about lightbulb quick fixes. 😉

@MikhailArkhipov
Copy link

There are 4 parts to this
a. Squiggle unresolved imports (PR microsoft/PTVS#4367)
b. Offer code action to install it
c. Detect unused imports in language server
d. Offer code action to remove/sort imports (also fix microsoft/PTVS#4255)

@MikhailArkhipov MikhailArkhipov modified the milestones: June 2018, July 2018 Jun 14, 2018
@brettcannon
Copy link
Member

I think in that list the priority should be a, c, d, b (as getting b to work well due to name differences between package names and the project name on PyPI is hit-or-miss).

@MikhailArkhipov MikhailArkhipov removed their assignment Jun 25, 2018
@MikhailArkhipov
Copy link

@AlexanderSher actually works on this (see #2049)

@simkimsia
Copy link

simkimsia commented Nov 18, 2018

Sorry as a user of vscode-python is this available yet? if so, how do I turn it on? if there's a workaround using linter, i will be just as happy to adopt. Now using pylint

@brettcannon
Copy link
Member

@simkimsia please follow the issues on the python-language-server repo if you would like to know when support lands in the language server.

As for linters, I believe both Pylint and flake8 can warn against unused imports if you turn on their respective warning.

@amittleider
Copy link

It would be even better to have the "Sort and remove unused imports" like PyCharm.

@Astrantia
Copy link

@brettcannon looks like this is done in LS?

@brettcannon
Copy link
Member

@Astrantia nope, this isn't done yet.

@luabud
Copy link
Member

luabud commented Sep 11, 2019

Closing because it's being tracked here: microsoft/python-language-server#18

@luabud luabud closed this as completed Sep 11, 2019
@ghost ghost removed the needs upstream fix label Sep 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests