Code action to add/delete imports by youben11 · Pull Request #685 · palantir/python-language-server · GitHub

This is based on the previous work of @gatesn

Actions to import suggested modules based on the index of symbols of all available python modules (might be slow to build but cached for future use)
Linting hint about unresolved symbols only as we aren't sure if those symbols aren't just variables.

Last features

Actions to remove unreferenced imports
Linting warning about unreferenced imports, variables and functions

related #86

@gatesn is there any way to make the call to pyls_code_actions non blocking?

…ver into importmagic

@youben11 I've checked out your version and tried it with a Webdriver/Selenium sample. Works fantastic to e.g autoimport from selenium import webdriver. What doesn't work though is importing local references from another file. Is that the supposed way how importmagic works?

Hi @zewa666, thank you for testing this functionality and providing feedback.

Yeah the index of symbols is built from the python modules found under sys.path, but I will certainly add the local workspace so you can have suggestions to import from project files.

Let's make this return a future, that way we can skip results instead of blocking on indexing.

Should this not be a warning / error? Or does it overlap with e.g. pyflakes results

This is a hint because we actually don't know if this is a module to be imported or just a non defined variable, so better not to trigger a false positive. The unreferenced symbols are flagged with Warning because we know in advance if the symbol is either an import or a varialbe/function and the error should be consistent.

I wonder if it's easier to just call pyls_lint(document), then try to find the context diagnostic in the results. That way we don't have to parse the messages, just have to do an equality check.

e.g. action_diags = [diag for diag in pyls_lint(document) if diag in context.get('diagnostics', [])]

True that the diagnostics list can grow bigger than the local diagnostics and we will just pass most of them. Also we don't need to do the equality check if we call pyls_lint() as all the diagnostics will be processed.

I think newer versions of clients support putting the edit in the code action:

/**
 * A code action represents a change that can be performed in code, e.g. to fix a problem or
 * to refactor code.
 *
 * A CodeAction must set either `edit` and/or a `command`. If both are supplied, the `edit` is applied first, then the `command` is executed.
 */
export interface CodeAction {

	/**
	 * A short, human-readable, title for this code action.
	 */
	title: string;

	/**
	 * The kind of the code action.
	 *
	 * Used to filter code actions.
	 */
	kind?: CodeActionKind;

	/**
	 * The diagnostics that this code action resolves.
	 */
	diagnostics?: Diagnostic[];

	/**
	 * The workspace edit this code action performs.
	 */
	edit?: WorkspaceEdit;

	/**
	 * A command this code action executes. If a code action
	 * provides an edit and a command, first the edit is
	 * executed and then the command.
	 */
	command?: Command;
}

But I'm not sure which client capability we should check. What you've got now is probably safer

The importmagic doesn't return the location of unreferenced/unresolved symbols so we needed to look them up ourselves, the search logic was a simple naive .find() call which might return false positives (e.g 'os' is found in 'pos'). Next I will be looking on how to simply tokenize the source code and search among the tokens so we make sure the matching is correct.

raw string matching lead to false positives (e.g 'os' if found under 'pos' but it's not the actual symbol that we were searching).

@youben11 perhaps you could use Jedi's names() call instead of walking the AST yourself. It may help, haven't verified: https://jedi.readthedocs.io/en/latest/docs/api.html#jedi.names

@gatesn the disadvantage with jedi.names is that it does only return definitions, symbols such as function calls aren't returned. The implementation using tokenize is pretty straightforward, however, I have to deal with the two different version of python here as the function takes different parameters for py2 and py3.

Update

Actually there is a backward compatible function to use that works for both version.

Any motion on this PR? Would love to have this functionality in the language server!

Hi there! Thanks for working on the import function 👍

After reading the comments it is not clear to me what is left to do. Since I really want this feature I would be happy to contribute - if I would know hat to do :D

This project is not maintained anymore. However, the community created a fork of it at

https://github.com/python-lsp/python-lsp-server/

So please submit this PR there if you want to see it finally merged.

If someone is looking for the same feature in the latest pylsp: python-lsp/python-lsp-server#199

youben11 added 8 commits October 28, 2019 22:16

Auto import actions using importmagic

e7e7dd2

import re

f9ef0a0

typo

4c89036

test against a specific action

4383f1e

disable actions test

7ed8a3b

missing white-space

4ae20c8

separate action generation logic: fix linting problem

8242066

fix: pass document instead of source

5175028

youben11 force-pushed the importmagic branch from aa4cd2e to 5175028 Compare October 29, 2019 19:24

filter results based on minScore parameter

f54910d

youben11 force-pushed the importmagic branch from c73dedf to 2668de3 Compare October 29, 2019 20:01

update test

2668de3

youben11 mentioned this pull request Oct 29, 2019

Add code actions to add/remove/sort imports #86

Open

gatesn suggested changes Nov 4, 2019

View reviewed changes

Comment thread pyls/plugins/importmagic_lint.py Outdated

Comment thread pyls/plugins/importmagic_lint.py Outdated

Comment thread pyls/plugins/importmagic_lint.py Outdated

Comment thread pyls/plugins/importmagic_lint.py Outdated

gatesn and others added 4 commits November 4, 2019 13:01

Merge branch 'develop' into importmagic

ecabe85

private function

936f121

change log level

b0f04a3

Merge branch 'importmagic' of github.com:youben11/python-language-ser…

2261042

…ver into importmagic

youben11 changed the title Code action to add import Code action to add/delete imports Nov 4, 2019

warn about unreferenced import/variable/function

d57d904

youben11 force-pushed the importmagic branch from 335b4c4 to e18d9b4 Compare November 9, 2019 20:14

youben11 requested a review from gatesn November 9, 2019 20:15

youben11 force-pushed the importmagic branch from e18d9b4 to c7e5b36 Compare November 9, 2019 20:18

remove unreferenced imports

c7e5b36

gatesn suggested changes Nov 12, 2019

View reviewed changes

config option to enable importmagic

061f0cc

youben11 force-pushed the importmagic branch from ba5c166 to e9996f7 Compare November 12, 2019 13:21

youben11 force-pushed the importmagic branch from 95bffde to f99d33e Compare November 12, 2019 19:40

non blocking index build

f99d33e

youben11 force-pushed the importmagic branch 3 times, most recently from 4c501a4 to 6812993 Compare November 14, 2019 08:43

store cache as dict

6812993

youben11 force-pushed the importmagic branch from 7be97a3 to d178670 Compare November 14, 2019 09:07

youben11 added 2 commits November 14, 2019 10:08

test remove action

d178670

better linting test

ae63149

youben11 added 3 commits November 15, 2019 10:58

search symbols in source tokens instead of raw string:

a712a73

raw string matching lead to false positives (e.g 'os' if found under 'pos' but it's not the actual symbol that we were searching).

check for None value

80def3f

fix range

0a9d163

youben11 force-pushed the importmagic branch from beec6a1 to 0a9d163 Compare November 15, 2019 10:19

youben11 force-pushed the importmagic branch from 226dc59 to e7ee0ca Compare November 15, 2019 14:16

youben11 added 4 commits November 15, 2019 15:17

use backward compatible tokenizer func

e7ee0ca

fix indexing and pattern matching

809a4af

typo

1c509f9

new style class

b82ff8f

youben11 force-pushed the importmagic branch from 494e129 to 107c431 Compare November 15, 2019 15:17

use list comprehension for better perf

107c431

youben11 requested a review from gatesn November 15, 2019 15:33

gmoutso mentioned this pull request Feb 3, 2020

Use import magic for auto-imports #157

Closed

krassowski mentioned this pull request Apr 7, 2021

Feature request: Auto-suggest to add missing import pappasam/jedi-language-server#126

Closed

+                                  'end': {'line': line_no, 'character': pos + len(unres)}
+                              },
+                              'message': "Unresolved import '%s'" % unres,
+                              'severity': lsp.DiagnosticSeverity.Hint,

+                  index = _get_index(sys.path)
+                  actions = []
+                  diagnostics = context.get('diagnostics', [])
+                  for diagnostic in diagnostics:

+                          'newText': args['newText']
+                      }]
+                  }]}
+                  workspace.apply_edit(edit)

		_index_cache = {}


		def _get_index(sys_path):

Code action to add/delete imports by youben11 · Pull Request #685 · palantir/python-language-server · GitHub

Uh oh!

Code action to add/delete imports - #685

Code action to add/delete imports#685youben11 wants to merge 33 commits intopalantir:developpalantir/python-language-server:developfrom youben11:importmagicyouben11/python-language-server:importmagicCopy head branch name to clipboard

Conversation

youben11 commented Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Last features

Uh oh!

youben11 commented Oct 29, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zewa666 commented Nov 5, 2019

Uh oh!

youben11 commented Nov 9, 2019

Uh oh!

Uh oh!

gatesn Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

gatesn Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

youben11 Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gatesn Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

youben11 Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

gatesn Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

youben11 commented Nov 14, 2019

Uh oh!

gatesn commented Nov 15, 2019

Uh oh!

youben11 commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update

Uh oh!

gjabell commented Jul 7, 2020

Uh oh!

weeman1337 commented May 2, 2021

Uh oh!

ccordoba12 commented May 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mte90 commented Mar 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Code action to add/delete imports#685
youben11 wants to merge 33 commits into
palantir:developfrom
youben11:importmagic

youben11 commented Oct 28, 2019 •
edited
Loading

youben11 commented Nov 15, 2019 •
edited
Loading

ccordoba12 commented May 2, 2021 •
edited
Loading