From e7e7dd25ceb3ef15baf61202fc000f2c9b093321 Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 28 Oct 2019 22:16:03 +0100 Subject: [PATCH 01/31] Auto import actions using importmagic --- pyls/_utils.py | 4 + pyls/plugins/importmagic_lint.py | 180 ++++++++++++++++++++++++++ setup.py | 3 + test/plugins/test_importmagic_lint.py | 68 ++++++++++ test/test_utils.py | 6 + 5 files changed, 261 insertions(+) create mode 100644 pyls/plugins/importmagic_lint.py create mode 100644 test/plugins/test_importmagic_lint.py diff --git a/pyls/_utils.py b/pyls/_utils.py index 48216b8f..df7dc969 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -192,3 +192,7 @@ def is_process_alive(pid): return e.errno == errno.EPERM else: return True + + +def camel_to_underscore(camelcase): + return re.sub('([A-Z]+)', r'_\1',camelcase).lower() diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py new file mode 100644 index 00000000..6c01b777 --- /dev/null +++ b/pyls/plugins/importmagic_lint.py @@ -0,0 +1,180 @@ +# Copyright 2017 Palantir Technologies, Inc. +import logging +import re +import sys +import importmagic +from pyls import hookimpl, lsp, _utils + + +log = logging.getLogger(__name__) + +SOURCE = 'importmagic' +ADD_IMPORT_COMMAND = 'importmagic.addimport' +MAX_COMMANDS = 4 +UNRES_RE = re.compile(r"Unresolved import '(?P[\w.]+)'") + +_index_cache = {} + + +def _get_index(sys_path): + """Build index of symbols from python modules. + Cache the index so we don't build it multiple times unnecessarily. + """ + key = tuple(sys_path) + if key not in _index_cache: + log.debug("Started building importmagic index") + index = importmagic.SymbolIndex() + # The build tend to be noisy + logging.getLogger('importmagic.index').setLevel(logging.ERROR) + index.build_index(paths=sys_path) + _index_cache[key] = index + logging.getLogger('importmagic.index').setLevel(logging.DEBUG) + log.debug("Finished building importmagic index") + return _index_cache[key] + + +@hookimpl +def pyls_commands(): + return [ADD_IMPORT_COMMAND] + + +@hookimpl +def pyls_lint(document): + """Build a diagnostics of unresolved symbols. Every entry follows this format: + { + 'source': 'importmagic', + 'range': { + 'start': { + 'line': start_line, + 'character': start_column, + }, + 'end': { + 'line': end_line, + 'character': end_column, + }, + }, + 'message': 'Unresolved import ', + 'severity': lsp.DiagnosticSeverity.Hint, + } + + Args: + document: The document to be linted. + Returns: + A list of dictionaries. + """ + scope = importmagic.Scope.from_source(document.source) + unresolved, _unreferenced = scope.find_unresolved_and_unreferenced_symbols() + + diagnostics = [] + + # Annoyingly, we only get the text of an unresolved import, so we'll look for it ourselves + for unres in unresolved: + if unres not in document.source: + continue + + for line_no, line in enumerate(document.lines): + pos = line.find(unres) + if pos < 0: + continue + + diagnostics.append({ + 'source': SOURCE, + 'range': { + 'start': {'line': line_no, 'character': pos}, + 'end': {'line': line_no, 'character': pos + len(unres)} + }, + 'message': "Unresolved import '%s'" % unres, + 'severity': lsp.DiagnosticSeverity.Hint, + }) + + return diagnostics + + +@hookimpl +def pyls_code_actions(config, document, context): + """Build a list of actions to be suggested to the user. Each action follow this format: + { + 'title': 'importmagic', + 'command': command ('importmagic.add_import'), + 'arguments': + { + 'uri': document.uri, + 'version': document.version, + 'startLine': start_line, + 'endLine': end_line, + 'newText': text, + } + } + """ + # Update the style configuration + conf = config.plugin_settings('importmagic_lint') + log.debug("Got importmagic settings: %s", conf) + importmagic.Imports.set_style(**{_utils.camel_to_underscore(k): v for k, v in conf.items()}) + + actions = [] + diagnostics = context.get('diagnostics', []) + for diagnostic in diagnostics: + if diagnostic.get('source') != SOURCE: + continue + m = UNRES_RE.match(diagnostic['message']) + if not m: + continue + + unres = m.group('unresolved') + # Might be slow but is cached once built + index = _get_index(sys.path) + + for score, module, variable in sorted(index.symbol_scores(unres)[:MAX_COMMANDS], reverse=True): + if score < 1: + # These tend to be terrible + continue + + # Generate the patch we would need to apply + imports = importmagic.Imports(index, document.source) + if variable: + imports.add_import_from(module, variable) + else: + imports.add_import(module) + start_line, end_line, text = imports.get_update() + + actions.append({ + 'title': _command_title(variable, module), + 'command': ADD_IMPORT_COMMAND, + 'arguments': [{ + 'uri': document.uri, + 'version': document.version, + 'startLine': start_line, + 'endLine': end_line, + 'newText': text + }] + }) + return actions + + +@hookimpl +def pyls_execute_command(workspace, command, arguments): + if command != ADD_IMPORT_COMMAND: + return + + args = arguments[0] + + edit = {'documentChanges': [{ + 'textDocument': { + 'uri': args['uri'], + 'version': args['version'] + }, + 'edits': [{ + 'range': { + 'start': {'line': args['startLine'], 'character': 0}, + 'end': {'line': args['endLine'], 'character': 0}, + }, + 'newText': args['newText'] + }] + }]} + workspace.apply_edit(edit) + + +def _command_title(variable, module): + if not variable: + return 'Import "%s"' % module + return 'Import "%s" from "%s"' % (variable, module) diff --git a/setup.py b/setup.py index 395449c0..2b9c7d46 100755 --- a/setup.py +++ b/setup.py @@ -48,6 +48,7 @@ 'all': [ 'autopep8', 'flake8', + 'importmagic', 'mccabe', 'pycodestyle', 'pydocstyle>=2.0.0', @@ -58,6 +59,7 @@ ], 'autopep8': ['autopep8'], 'flake8': ['flake8'], + 'importmagic': ['importmagic'], 'mccabe': ['mccabe'], 'pycodestyle': ['pycodestyle'], 'pydocstyle': ['pydocstyle>=2.0.0'], @@ -78,6 +80,7 @@ 'pyls': [ 'autopep8 = pyls.plugins.autopep8_format', 'flake8 = pyls.plugins.flake8_lint', + 'importmagic = pyls.plugins.importmagic_lint', 'jedi_completion = pyls.plugins.jedi_completion', 'jedi_definition = pyls.plugins.definition', 'jedi_hover = pyls.plugins.hover', diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py new file mode 100644 index 00000000..b2093ce7 --- /dev/null +++ b/test/plugins/test_importmagic_lint.py @@ -0,0 +1,68 @@ +# Copyright 2019 Palantir Technologies, Inc. +import tempfile +import os +from pyls import lsp, uris +from pyls.plugins import importmagic_lint +from pyls.workspace import Document + +DOC_URI = uris.from_fs_path(__file__) +DOC = """ +files = listdir() +print(files) +""" + + +def temp_document(doc_text): + temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False) + name = temp_file.name + temp_file.write(doc_text) + temp_file.close() + doc = Document(uris.from_fs_path(name)) + + return name, doc + + +def test_importmagic_lint(): + try: + name, doc = temp_document(DOC) + diags = importmagic_lint.pyls_lint(doc) + unres_symbol = [d for d in diags if d['source'] == 'importmagic'][0] + + assert unres_symbol['message'] == "Unresolved import 'listdir'" + assert unres_symbol['range']['start'] == {'line': 1, 'character': 8} + assert unres_symbol['range']['end'] == {'line': 1, 'character': 15} + assert unres_symbol['severity'] == lsp.DiagnosticSeverity.Hint + + finally: + os.remove(name) + + +def test_importmagic_actions(config): + context = { + 'diagnostict': [ + { + 'range': + { + 'start': {'line': 1, 'character': 8}, + 'end': {'line': 1, 'character': 15} + }, + 'message': "Unresolved import 'listdir'", + 'severity': 4, + 'source': 'importmagic' + } + ] + } + + try: + name, doc = temp_document(DOC) + action = importmagic_lint.pyls_code_actions(config, doc, context)[0] + arguments = action['arguments'] + + assert action['title'] == 'Import "listdir" from "os"' + assert action['command'] == 'importmagic.addimport' + assert arguments['startLine'] == 1 + assert arguments['endLine'] == 1 + assert arguments['newText'] == 'from os import listdir\n\n\n' + + finally: + os.remove(name) \ No newline at end of file diff --git a/test/test_utils.py b/test/test_utils.py index 65152d94..bf48920d 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -87,3 +87,9 @@ def test_clip_column(): assert _utils.clip_column(2, ['123\n', '123'], 0) == 2 assert _utils.clip_column(3, ['123\n', '123'], 0) == 3 assert _utils.clip_column(4, ['123\n', '123'], 1) == 3 + + +def test_camel_to_underscore(): + assert _utils.camel_to_underscore('camelCase') == 'camel_case' + assert _utils.camel_to_underscore('hangClosing') == 'hang_closing' + assert _utils.camel_to_underscore('ignore') == 'ignore' From f9ef0a07cf6565d1661dc3a0d5756d224a806de0 Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 28 Oct 2019 22:31:25 +0100 Subject: [PATCH 02/31] import re --- pyls/_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyls/_utils.py b/pyls/_utils.py index df7dc969..e880f52c 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -5,6 +5,7 @@ import os import sys import threading +import re PY2 = sys.version_info.major == 2 From 4c890367660d7af598396c07ff0a8fca38c1e8ce Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 28 Oct 2019 22:38:16 +0100 Subject: [PATCH 03/31] typo --- test/plugins/test_importmagic_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index b2093ce7..4aa2771a 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -39,7 +39,7 @@ def test_importmagic_lint(): def test_importmagic_actions(config): context = { - 'diagnostict': [ + 'diagnostics': [ { 'range': { From 4383f1ee0a00c1d6aa58e6af5a2dba89367d866f Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 28 Oct 2019 22:45:42 +0100 Subject: [PATCH 04/31] test against a specific action --- test/plugins/test_importmagic_lint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index 4aa2771a..b07b2165 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -55,10 +55,10 @@ def test_importmagic_actions(config): try: name, doc = temp_document(DOC) - action = importmagic_lint.pyls_code_actions(config, doc, context)[0] + actions = importmagic_lint.pyls_code_actions(config, doc, context) + action = [a for a in actions if a['title'] == 'Import "listdir" from "os"'][0] arguments = action['arguments'] - assert action['title'] == 'Import "listdir" from "os"' assert action['command'] == 'importmagic.addimport' assert arguments['startLine'] == 1 assert arguments['endLine'] == 1 From 7ed8a3b3660c80304a6747cb9381f0e3fc35c7aa Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 19:30:14 +0100 Subject: [PATCH 05/31] disable actions test --- test/plugins/test_importmagic_lint.py | 52 +++++++++++++-------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index b07b2165..d82c7686 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -37,32 +37,32 @@ def test_importmagic_lint(): os.remove(name) -def test_importmagic_actions(config): - context = { - 'diagnostics': [ - { - 'range': - { - 'start': {'line': 1, 'character': 8}, - 'end': {'line': 1, 'character': 15} - }, - 'message': "Unresolved import 'listdir'", - 'severity': 4, - 'source': 'importmagic' - } - ] - } +# def test_importmagic_actions(config): +# context = { +# 'diagnostics': [ +# { +# 'range': +# { +# 'start': {'line': 1, 'character': 8}, +# 'end': {'line': 1, 'character': 15} +# }, +# 'message': "Unresolved import 'listdir'", +# 'severity': 4, +# 'source': 'importmagic' +# } +# ] +# } - try: - name, doc = temp_document(DOC) - actions = importmagic_lint.pyls_code_actions(config, doc, context) - action = [a for a in actions if a['title'] == 'Import "listdir" from "os"'][0] - arguments = action['arguments'] +# try: +# name, doc = temp_document(DOC) +# actions = importmagic_lint.pyls_code_actions(config, doc, context) +# action = [a for a in actions if a['title'] == 'Import "listdir" from "os"'][0] +# arguments = action['arguments'] - assert action['command'] == 'importmagic.addimport' - assert arguments['startLine'] == 1 - assert arguments['endLine'] == 1 - assert arguments['newText'] == 'from os import listdir\n\n\n' +# assert action['command'] == 'importmagic.addimport' +# assert arguments['startLine'] == 1 +# assert arguments['endLine'] == 1 +# assert arguments['newText'] == 'from os import listdir\n\n\n' - finally: - os.remove(name) \ No newline at end of file +# finally: +# os.remove(name) From 4ae20c853bfff9f908d068e029431dddb8cf0cca Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 19:36:10 +0100 Subject: [PATCH 06/31] missing white-space --- pyls/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index e880f52c..e5ecef9c 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -196,4 +196,4 @@ def is_process_alive(pid): def camel_to_underscore(camelcase): - return re.sub('([A-Z]+)', r'_\1',camelcase).lower() + return re.sub('([A-Z]+)', r'_\1', camelcase).lower() From 8242066ce75820f509c33dac2e6e3e5ecd7a2583 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 19:54:12 +0100 Subject: [PATCH 07/31] separate action generation logic: fix linting problem --- pyls/plugins/importmagic_lint.py | 44 ++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 6c01b777..15035f3a 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -129,28 +129,34 @@ def pyls_code_actions(config, document, context): # These tend to be terrible continue - # Generate the patch we would need to apply - imports = importmagic.Imports(index, document.source) - if variable: - imports.add_import_from(module, variable) - else: - imports.add_import(module) - start_line, end_line, text = imports.get_update() - - actions.append({ - 'title': _command_title(variable, module), - 'command': ADD_IMPORT_COMMAND, - 'arguments': [{ - 'uri': document.uri, - 'version': document.version, - 'startLine': start_line, - 'endLine': end_line, - 'newText': text - }] - }) + actions.append(generate_add_action(document.source, index, module, variable)) + return actions +def generate_add_action(document, index, module, variable): + # Generate the patch we would need to apply + imports = importmagic.Imports(index, document.source) + if variable: + imports.add_import_from(module, variable) + else: + imports.add_import(module) + start_line, end_line, text = imports.get_update() + + action = { + 'title': _command_title(variable, module), + 'command': ADD_IMPORT_COMMAND, + 'arguments': [{ + 'uri': document.uri, + 'version': document.version, + 'startLine': start_line, + 'endLine': end_line, + 'newText': text + }] + } + return action + + @hookimpl def pyls_execute_command(workspace, command, arguments): if command != ADD_IMPORT_COMMAND: From 5175028cdaea074eb59bbc57a781e5dc1ec79973 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 20:13:48 +0100 Subject: [PATCH 08/31] fix: pass document instead of source --- pyls/plugins/importmagic_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 15035f3a..b8cc7f00 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -129,7 +129,7 @@ def pyls_code_actions(config, document, context): # These tend to be terrible continue - actions.append(generate_add_action(document.source, index, module, variable)) + actions.append(generate_add_action(document, index, module, variable)) return actions From f54910dede0c55ecec22eeac3706a45cc74fa756 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 20:43:37 +0100 Subject: [PATCH 09/31] filter results based on minScore parameter --- pyls/plugins/importmagic_lint.py | 5 +++-- vscode-client/package.json | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index b8cc7f00..aa7fb636 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -108,6 +108,7 @@ def pyls_code_actions(config, document, context): """ # Update the style configuration conf = config.plugin_settings('importmagic_lint') + min_score = conf.get('minScore', 1) log.debug("Got importmagic settings: %s", conf) importmagic.Imports.set_style(**{_utils.camel_to_underscore(k): v for k, v in conf.items()}) @@ -125,8 +126,8 @@ def pyls_code_actions(config, document, context): index = _get_index(sys.path) for score, module, variable in sorted(index.symbol_scores(unres)[:MAX_COMMANDS], reverse=True): - if score < 1: - # These tend to be terrible + if score < min_score: + # Skip low score results continue actions.append(generate_add_action(document, index, module, variable)) diff --git a/vscode-client/package.json b/vscode-client/package.json index a5798ec2..d02b8185 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -35,6 +35,11 @@ }, "uniqueItems": true }, + "pyls.plugins.importmagic_lint.minScore": { + "type": "number", + "default": 1, + "description": "The minimum score used to filter module import suggestions." + }, "pyls.plugins.jedi_completion.enabled": { "type": "boolean", "default": true, From 2668de3a7f21d692a52335de474ca2c0fc69860e Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 29 Oct 2019 20:54:56 +0100 Subject: [PATCH 10/31] update test --- test/plugins/test_importmagic_lint.py | 62 +++++++++++++-------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index d82c7686..47009ebc 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -7,8 +7,8 @@ DOC_URI = uris.from_fs_path(__file__) DOC = """ -files = listdir() -print(files) +time.sleep(10) +print("test") """ @@ -28,41 +28,41 @@ def test_importmagic_lint(): diags = importmagic_lint.pyls_lint(doc) unres_symbol = [d for d in diags if d['source'] == 'importmagic'][0] - assert unres_symbol['message'] == "Unresolved import 'listdir'" - assert unres_symbol['range']['start'] == {'line': 1, 'character': 8} - assert unres_symbol['range']['end'] == {'line': 1, 'character': 15} + assert unres_symbol['message'] == "Unresolved import 'time.sleep'" + assert unres_symbol['range']['start'] == {'line': 1, 'character': 0} + assert unres_symbol['range']['end'] == {'line': 1, 'character': 10} assert unres_symbol['severity'] == lsp.DiagnosticSeverity.Hint finally: os.remove(name) -# def test_importmagic_actions(config): -# context = { -# 'diagnostics': [ -# { -# 'range': -# { -# 'start': {'line': 1, 'character': 8}, -# 'end': {'line': 1, 'character': 15} -# }, -# 'message': "Unresolved import 'listdir'", -# 'severity': 4, -# 'source': 'importmagic' -# } -# ] -# } +def test_importmagic_actions(config): + context = { + 'diagnostics': [ + { + 'range': + { + 'start': {'line': 1, 'character': 0}, + 'end': {'line': 1, 'character': 10} + }, + 'message': "Unresolved import 'time.sleep'", + 'severity': lsp.DiagnosticSeverity.Hint, + 'source': importmagic_lint.SOURCE + } + ] + } -# try: -# name, doc = temp_document(DOC) -# actions = importmagic_lint.pyls_code_actions(config, doc, context) -# action = [a for a in actions if a['title'] == 'Import "listdir" from "os"'][0] -# arguments = action['arguments'] + try: + name, doc = temp_document(DOC) + actions = importmagic_lint.pyls_code_actions(config, doc, context) + action = [a for a in actions if a['title'] == 'Import "time"'][0] + arguments = action['arguments'][0] -# assert action['command'] == 'importmagic.addimport' -# assert arguments['startLine'] == 1 -# assert arguments['endLine'] == 1 -# assert arguments['newText'] == 'from os import listdir\n\n\n' + assert action['command'] == importmagic_lint.ADD_IMPORT_COMMAND + assert arguments['startLine'] == 1 + assert arguments['endLine'] == 1 + assert arguments['newText'] == 'import time\n\n\n' -# finally: -# os.remove(name) + finally: + os.remove(name) From 936f12180c9b7dd7313af2283104f3fe9c12dade Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 4 Nov 2019 19:42:35 +0100 Subject: [PATCH 11/31] private function --- pyls/plugins/importmagic_lint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index aa7fb636..9e2cf258 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -130,12 +130,12 @@ def pyls_code_actions(config, document, context): # Skip low score results continue - actions.append(generate_add_action(document, index, module, variable)) + actions.append(_generate_add_action(document, index, module, variable)) return actions -def generate_add_action(document, index, module, variable): +def _generate_add_action(document, index, module, variable): # Generate the patch we would need to apply imports = importmagic.Imports(index, document.source) if variable: From b0f04a343933b860c43a613ed186d8a934c02444 Mon Sep 17 00:00:00 2001 From: youben11 Date: Mon, 4 Nov 2019 19:48:57 +0100 Subject: [PATCH 12/31] change log level --- pyls/plugins/importmagic_lint.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 9e2cf258..0d899c36 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -22,14 +22,12 @@ def _get_index(sys_path): """ key = tuple(sys_path) if key not in _index_cache: - log.debug("Started building importmagic index") + log.info("Started building importmagic index") index = importmagic.SymbolIndex() # The build tend to be noisy - logging.getLogger('importmagic.index').setLevel(logging.ERROR) index.build_index(paths=sys_path) _index_cache[key] = index - logging.getLogger('importmagic.index').setLevel(logging.DEBUG) - log.debug("Finished building importmagic index") + log.info("Finished building importmagic index") return _index_cache[key] From d57d904d20d4c73bcf6f3f7043a4839c0e83ebcc Mon Sep 17 00:00:00 2001 From: youben11 Date: Wed, 6 Nov 2019 17:19:31 +0100 Subject: [PATCH 13/31] warn about unreferenced import/variable/function --- pyls/plugins/importmagic_lint.py | 36 +++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 0d899c36..71d976f3 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -38,7 +38,8 @@ def pyls_commands(): @hookimpl def pyls_lint(document): - """Build a diagnostics of unresolved symbols. Every entry follows this format: + """Build a diagnostics of unresolved and unreferenced symbols. + Every entry follows this format: { 'source': 'importmagic', 'range': { @@ -51,8 +52,8 @@ def pyls_lint(document): 'character': end_column, }, }, - 'message': 'Unresolved import ', - 'severity': lsp.DiagnosticSeverity.Hint, + 'message': message_to_be_displayed, + 'severity': sevirity_level, } Args: @@ -61,12 +62,13 @@ def pyls_lint(document): A list of dictionaries. """ scope = importmagic.Scope.from_source(document.source) - unresolved, _unreferenced = scope.find_unresolved_and_unreferenced_symbols() + unresolved, unreferenced = scope.find_unresolved_and_unreferenced_symbols() diagnostics = [] # Annoyingly, we only get the text of an unresolved import, so we'll look for it ourselves for unres in unresolved: + # TODO (youben): delete this test as it double execution time (next for loop will do the same) if unres not in document.source: continue @@ -85,6 +87,30 @@ def pyls_lint(document): 'severity': lsp.DiagnosticSeverity.Hint, }) + for unref in unreferenced: + for line_no, line in enumerate(document.lines): + pos = line.find(unref) + if pos < 0: + continue + + # Find out if the unref is a module or a variable/func + imports = importmagic.Imports(importmagic.SymbolIndex(), document.source) + modules = [m.name for m in list(imports._imports)] + if unref in modules: + message = "Unreferenced import '%s'" % unref + else: + message = "Unreferenced variable/function '%s'" % unref + + diagnostics.append({ + 'source': SOURCE, + 'range': { + 'start': {'line': line_no, 'character': pos}, + 'end': {'line': line_no, 'character': pos + len(unref)} + }, + 'message': message, + 'severity': lsp.DiagnosticSeverity.Warning, + }) + return diagnostics @@ -121,7 +147,7 @@ def pyls_code_actions(config, document, context): unres = m.group('unresolved') # Might be slow but is cached once built - index = _get_index(sys.path) + index = _get_index(sys.path) # TODO (youben): add project path for indexing for score, module, variable in sorted(index.symbol_scores(unres)[:MAX_COMMANDS], reverse=True): if score < min_score: From c7e5b3611433e6fbe262315bebea194a9d097a60 Mon Sep 17 00:00:00 2001 From: youben11 Date: Sat, 9 Nov 2019 20:14:52 +0100 Subject: [PATCH 14/31] remove unreferenced imports --- pyls/plugins/importmagic_lint.py | 103 +++++++++++++++++++++++-------- 1 file changed, 78 insertions(+), 25 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 71d976f3..5ae0a62c 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -10,8 +10,10 @@ SOURCE = 'importmagic' ADD_IMPORT_COMMAND = 'importmagic.addimport' +REMOVE_IMPORT_COMMAND = 'importmagic.removeimport' MAX_COMMANDS = 4 UNRES_RE = re.compile(r"Unresolved import '(?P[\w.]+)'") +UNREF_RE = re.compile(r"Unreferenced import '(?P[\w.]+)'") _index_cache = {} @@ -31,9 +33,22 @@ def _get_index(sys_path): return _index_cache[key] +def _get_imports_list(source, index=None): + """Get modules, functions and variables that are imported. + """ + if index is None: + index = importmagic.SymbolIndex() + imports = importmagic.Imports(index, source) + imported = [i.name for i in list(imports._imports)] + # Go over from imports + for from_import in list(imports._imports_from.values()): + imported.extend([i.name for i in list(from_import)]) + return imported + + @hookimpl def pyls_commands(): - return [ADD_IMPORT_COMMAND] + return [ADD_IMPORT_COMMAND, REMOVE_IMPORT_COMMAND] @hookimpl @@ -68,10 +83,6 @@ def pyls_lint(document): # Annoyingly, we only get the text of an unresolved import, so we'll look for it ourselves for unres in unresolved: - # TODO (youben): delete this test as it double execution time (next for loop will do the same) - if unres not in document.source: - continue - for line_no, line in enumerate(document.lines): pos = line.find(unres) if pos < 0: @@ -93,10 +104,9 @@ def pyls_lint(document): if pos < 0: continue - # Find out if the unref is a module or a variable/func - imports = importmagic.Imports(importmagic.SymbolIndex(), document.source) - modules = [m.name for m in list(imports._imports)] - if unref in modules: + # Find out if the unref is an import or a variable/func + imports = _get_imports_list(document.source) + if unref in imports: message = "Unreferenced import '%s'" % unref else: message = "Unreferenced variable/function '%s'" % unref @@ -119,7 +129,7 @@ def pyls_code_actions(config, document, context): """Build a list of actions to be suggested to the user. Each action follow this format: { 'title': 'importmagic', - 'command': command ('importmagic.add_import'), + 'command': command, 'arguments': { 'uri': document.uri, @@ -136,31 +146,49 @@ def pyls_code_actions(config, document, context): log.debug("Got importmagic settings: %s", conf) importmagic.Imports.set_style(**{_utils.camel_to_underscore(k): v for k, v in conf.items()}) + # Might be slow but is cached once built + # TODO (youben): add project path for indexing + index = _get_index(sys.path) actions = [] diagnostics = context.get('diagnostics', []) for diagnostic in diagnostics: if diagnostic.get('source') != SOURCE: continue - m = UNRES_RE.match(diagnostic['message']) - if not m: - continue + message = diagnostic.get('message', '') + if message.startswith('Unreferenced'): + m = UNREF_RE.match(message) + if not m: + continue + unref = m.group('unreferenced') + actions.append(_generate_remove_action(document, index, unref)) + elif message.startswith('Unresolved'): + m = UNRES_RE.match(message) + if not m: + continue + unres = m.group('unresolved') + actions.extend(_get_actions_for_unres(document, index, min_score, unres)) - unres = m.group('unresolved') - # Might be slow but is cached once built - index = _get_index(sys.path) # TODO (youben): add project path for indexing + return actions - for score, module, variable in sorted(index.symbol_scores(unres)[:MAX_COMMANDS], reverse=True): - if score < min_score: - # Skip low score results - continue - actions.append(_generate_add_action(document, index, module, variable)) +def _get_actions_for_unres(document, index, min_score, unres): + """Get the list of possible actions to be applied to solve an unresolved symbol. + Get a maximun of MAX_COMMANDS actions with the highest score, also filter low score actions + using the min_score value. + """ + actions = [] + for score, module, variable in sorted(index.symbol_scores(unres)[:MAX_COMMANDS], reverse=True): + if score < min_score: + # Skip low score results + continue + actions.append(_generate_add_action(document, index, module, variable)) return actions def _generate_add_action(document, index, module, variable): - # Generate the patch we would need to apply + """Generate the patch we would need to apply to import a module. + """ imports = importmagic.Imports(index, document.source) if variable: imports.add_import_from(module, variable) @@ -169,7 +197,7 @@ def _generate_add_action(document, index, module, variable): start_line, end_line, text = imports.get_update() action = { - 'title': _command_title(variable, module), + 'title': _add_command_title(variable, module), 'command': ADD_IMPORT_COMMAND, 'arguments': [{ 'uri': document.uri, @@ -182,9 +210,30 @@ def _generate_add_action(document, index, module, variable): return action +def _generate_remove_action(document, index, unref): + """Generate the patch we would need to apply to remove an import. + """ + imports = importmagic.Imports(index, document.source) + imports.remove(unref) + start_line, end_line, text = imports.get_update() + + action = { + 'title': _remove_command_title(unref), + 'command': REMOVE_IMPORT_COMMAND, + 'arguments': [{ + 'uri': document.uri, + 'version': document.version, + 'startLine': start_line, + 'endLine': end_line, + 'newText': text + }] + } + return action + + @hookimpl def pyls_execute_command(workspace, command, arguments): - if command != ADD_IMPORT_COMMAND: + if command not in [ADD_IMPORT_COMMAND, REMOVE_IMPORT_COMMAND]: return args = arguments[0] @@ -205,7 +254,11 @@ def pyls_execute_command(workspace, command, arguments): workspace.apply_edit(edit) -def _command_title(variable, module): +def _add_command_title(variable, module): if not variable: return 'Import "%s"' % module return 'Import "%s" from "%s"' % (variable, module) + + +def _remove_command_title(import_name): + return 'Remove import of "%s"' % import_name From 061f0ccd11eeb5676352612a2d8dd4eb476ffb75 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 11:35:54 +0100 Subject: [PATCH 15/31] config option to enable importmagic --- vscode-client/package.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vscode-client/package.json b/vscode-client/package.json index d02b8185..f7443179 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -35,6 +35,11 @@ }, "uniqueItems": true }, + "pyls.plugins.importmagic_lint.enabled": { + "type": "boolean", + "default": true, + "description": "Enable or disable the plugin." + }, "pyls.plugins.importmagic_lint.minScore": { "type": "number", "default": 1, From e9996f7ce940d73b54c7331953155ef6940f36a1 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 14:03:44 +0100 Subject: [PATCH 16/31] fix camel_to_underscore --- pyls/_utils.py | 3 ++- test/test_utils.py | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index fa7d6cd5..9794fbe5 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -202,4 +202,5 @@ def is_process_alive(pid): def camel_to_underscore(camelcase): - return re.sub('([A-Z]+)', r'_\1', camelcase).lower() + s1 = re.sub('([^_])([A-Z][a-z]+)', r'\1_\2', camelcase) + return re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower() diff --git a/test/test_utils.py b/test/test_utils.py index bf48920d..f8ece3a3 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -93,3 +93,9 @@ def test_camel_to_underscore(): assert _utils.camel_to_underscore('camelCase') == 'camel_case' assert _utils.camel_to_underscore('hangClosing') == 'hang_closing' assert _utils.camel_to_underscore('ignore') == 'ignore' + assert _utils.camel_to_underscore('CamelCase') == 'camel_case' + assert _utils.camel_to_underscore('SomeLongCamelCase') == 'some_long_camel_case' + assert _utils.camel_to_underscore('already_using_underscore') == 'already_using_underscore' + assert _utils.camel_to_underscore('Using_only_someUnderscore') == 'using_only_some_underscore' + assert _utils.camel_to_underscore('Using_Only_Some_underscore') == 'using_only_some_underscore' + assert _utils.camel_to_underscore('ALL_UPPER_CASE') == 'all_upper_case' From 48d1f01fc60d0edd5240ed67b2ca8f236cb8f7c9 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 14:24:43 +0100 Subject: [PATCH 17/31] clarify remove command action title --- pyls/plugins/importmagic_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 5ae0a62c..85f572ed 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -261,4 +261,4 @@ def _add_command_title(variable, module): def _remove_command_title(import_name): - return 'Remove import of "%s"' % import_name + return 'Remove unused import of "%s"' % import_name From 2d12e54531f2cd8c27d34891d2fbd4b08fdca2be Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 14:40:24 +0100 Subject: [PATCH 18/31] clean message checking --- pyls/plugins/importmagic_lint.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 85f572ed..11d63880 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -154,18 +154,16 @@ def pyls_code_actions(config, document, context): for diagnostic in diagnostics: if diagnostic.get('source') != SOURCE: continue + message = diagnostic.get('message', '') - if message.startswith('Unreferenced'): - m = UNREF_RE.match(message) - if not m: - continue - unref = m.group('unreferenced') + unref_match = UNREF_RE.match(message) + unres_match = UNRES_RE.match(message) + + if unref_match: + unref = unref_match.group('unreferenced') actions.append(_generate_remove_action(document, index, unref)) - elif message.startswith('Unresolved'): - m = UNRES_RE.match(message) - if not m: - continue - unres = m.group('unresolved') + elif unres_match: + unres = unres_match.group('unresolved') actions.extend(_get_actions_for_unres(document, index, min_score, unres)) return actions From 71b83854cf80787dbc209fd286ea352c053a9056 Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 17:08:39 +0100 Subject: [PATCH 19/31] get diagnostics from pyls_lint --- pyls/plugins/importmagic_lint.py | 8 +++----- test/plugins/test_importmagic_lint.py | 17 +---------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 11d63880..3330e8d2 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -125,7 +125,7 @@ def pyls_lint(document): @hookimpl -def pyls_code_actions(config, document, context): +def pyls_code_actions(config, document): """Build a list of actions to be suggested to the user. Each action follow this format: { 'title': 'importmagic', @@ -150,11 +150,9 @@ def pyls_code_actions(config, document, context): # TODO (youben): add project path for indexing index = _get_index(sys.path) actions = [] - diagnostics = context.get('diagnostics', []) - for diagnostic in diagnostics: - if diagnostic.get('source') != SOURCE: - continue + diagnostics = pyls_lint(document) + for diagnostic in diagnostics: message = diagnostic.get('message', '') unref_match = UNREF_RE.match(message) unres_match = UNRES_RE.match(message) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index 47009ebc..9a2a8033 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -38,24 +38,9 @@ def test_importmagic_lint(): def test_importmagic_actions(config): - context = { - 'diagnostics': [ - { - 'range': - { - 'start': {'line': 1, 'character': 0}, - 'end': {'line': 1, 'character': 10} - }, - 'message': "Unresolved import 'time.sleep'", - 'severity': lsp.DiagnosticSeverity.Hint, - 'source': importmagic_lint.SOURCE - } - ] - } - try: name, doc = temp_document(DOC) - actions = importmagic_lint.pyls_code_actions(config, doc, context) + actions = importmagic_lint.pyls_code_actions(config, doc) action = [a for a in actions if a['title'] == 'Import "time"'][0] arguments = action['arguments'][0] From f99d33ed30e9bfe0339824a9a6a47b0ce791185d Mon Sep 17 00:00:00 2001 From: youben11 Date: Tue, 12 Nov 2019 18:05:33 +0100 Subject: [PATCH 20/31] non blocking index build --- pyls/plugins/importmagic_lint.py | 52 +++++++++++++++++++-------- test/plugins/test_importmagic_lint.py | 7 ++++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 3330e8d2..82596339 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -2,6 +2,7 @@ import logging import re import sys +from concurrent.futures import ThreadPoolExecutor import importmagic from pyls import hookimpl, lsp, _utils @@ -15,22 +16,37 @@ UNRES_RE = re.compile(r"Unresolved import '(?P[\w.]+)'") UNREF_RE = re.compile(r"Unreferenced import '(?P[\w.]+)'") -_index_cache = {} +_index_cache = None -def _get_index(sys_path): +def _build_index(paths): """Build index of symbols from python modules. - Cache the index so we don't build it multiple times unnecessarily. """ - key = tuple(sys_path) - if key not in _index_cache: - log.info("Started building importmagic index") - index = importmagic.SymbolIndex() - # The build tend to be noisy - index.build_index(paths=sys_path) - _index_cache[key] = index - log.info("Finished building importmagic index") - return _index_cache[key] + log.info("Started building importmagic index") + index = importmagic.SymbolIndex() + index.build_index(paths=paths) + log.info("Finished building importmagic index") + return index + + +def _cache_index_callback(future): + global _index_cache + # Cache the index + _index_cache = future.result() + + +def _get_index(): + """Get the cached index if built and index project files on each call. + Return an empty index if not built yet. + """ + # Index haven't been built yet + if _index_cache is None: + return importmagic.SymbolIndex() + + # Index project files + # TODO(youben) index project files + #index.build_index(paths=[]) + return _index_cache def _get_imports_list(source, index=None): @@ -46,6 +62,13 @@ def _get_imports_list(source, index=None): return imported +@hookimpl +def pyls_initialize(): + pool = ThreadPoolExecutor() + builder = pool.submit(_build_index, (sys.path)) + builder.add_done_callback(_cache_index_callback) + + @hookimpl def pyls_commands(): return [ADD_IMPORT_COMMAND, REMOVE_IMPORT_COMMAND] @@ -146,9 +169,8 @@ def pyls_code_actions(config, document): log.debug("Got importmagic settings: %s", conf) importmagic.Imports.set_style(**{_utils.camel_to_underscore(k): v for k, v in conf.items()}) - # Might be slow but is cached once built - # TODO (youben): add project path for indexing - index = _get_index(sys.path) + # Get empty index while it's building so we don't block here + index = _get_index() actions = [] diagnostics = pyls_lint(document) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index 9a2a8033..f444bbf3 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -1,6 +1,7 @@ # Copyright 2019 Palantir Technologies, Inc. import tempfile import os +from time import sleep from pyls import lsp, uris from pyls.plugins import importmagic_lint from pyls.workspace import Document @@ -39,7 +40,11 @@ def test_importmagic_lint(): def test_importmagic_actions(config): try: + importmagic_lint.pyls_initialize() name, doc = temp_document(DOC) + while importmagic_lint._index_cache is None: + # wait for the index to be ready + sleep(1) actions = importmagic_lint.pyls_code_actions(config, doc) action = [a for a in actions if a['title'] == 'Import "time"'][0] arguments = action['arguments'][0] @@ -51,3 +56,5 @@ def test_importmagic_actions(config): finally: os.remove(name) + +# TODO(youben) write test for remove action From 6812993520124c75ac0b477885341b4cdde4f337 Mon Sep 17 00:00:00 2001 From: youben11 Date: Wed, 13 Nov 2019 23:01:22 +0100 Subject: [PATCH 21/31] store cache as dict --- pyls/plugins/importmagic_lint.py | 12 ++++++------ test/plugins/test_importmagic_lint.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 82596339..15eb4f40 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -16,7 +16,7 @@ UNRES_RE = re.compile(r"Unresolved import '(?P[\w.]+)'") UNREF_RE = re.compile(r"Unreferenced import '(?P[\w.]+)'") -_index_cache = None +_index_cache = {} def _build_index(paths): @@ -30,9 +30,8 @@ def _build_index(paths): def _cache_index_callback(future): - global _index_cache # Cache the index - _index_cache = future.result() + _index_cache['default'] = future.result() def _get_index(): @@ -40,13 +39,14 @@ def _get_index(): Return an empty index if not built yet. """ # Index haven't been built yet - if _index_cache is None: + index = _index_cache.get('default') + if index is None: return importmagic.SymbolIndex() # Index project files # TODO(youben) index project files - #index.build_index(paths=[]) - return _index_cache + # index.build_index(paths=[]) + return _index_cache['default'] def _get_imports_list(source, index=None): diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index f444bbf3..99f3068c 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -42,7 +42,7 @@ def test_importmagic_actions(config): try: importmagic_lint.pyls_initialize() name, doc = temp_document(DOC) - while importmagic_lint._index_cache is None: + while importmagic_lint._index_cache.get('default') is None: # wait for the index to be ready sleep(1) actions = importmagic_lint.pyls_code_actions(config, doc) From d178670c4175727b2075989dcb818247b468535a Mon Sep 17 00:00:00 2001 From: youben11 Date: Thu, 14 Nov 2019 10:08:25 +0100 Subject: [PATCH 22/31] test remove action --- pyls/plugins/importmagic_lint.py | 1 + test/plugins/test_importmagic_lint.py | 30 +++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 15eb4f40..ebb729b1 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -64,6 +64,7 @@ def _get_imports_list(source, index=None): @hookimpl def pyls_initialize(): + _index_cache['default'] = None pool = ThreadPoolExecutor() builder = pool.submit(_build_index, (sys.path)) builder.add_done_callback(_cache_index_callback) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index 99f3068c..264401fd 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -7,10 +7,14 @@ from pyls.workspace import Document DOC_URI = uris.from_fs_path(__file__) -DOC = """ +DOC_ADD = """ time.sleep(10) print("test") """ +DOC_REMOVE = """ +import time +print("useless import") +""" def temp_document(doc_text): @@ -38,10 +42,10 @@ def test_importmagic_lint(): os.remove(name) -def test_importmagic_actions(config): +def test_importmagic_add_import_action(config): try: importmagic_lint.pyls_initialize() - name, doc = temp_document(DOC) + name, doc = temp_document(DOC_ADD) while importmagic_lint._index_cache.get('default') is None: # wait for the index to be ready sleep(1) @@ -57,4 +61,22 @@ def test_importmagic_actions(config): finally: os.remove(name) -# TODO(youben) write test for remove action + +def test_importmagic_remove_import_action(config): + try: + importmagic_lint.pyls_initialize() + name, doc = temp_document(DOC_REMOVE) + while importmagic_lint._index_cache.get('default') is None: + # wait for the index to be ready + sleep(1) + actions = importmagic_lint.pyls_code_actions(config, doc) + action = [a for a in actions if a['title'] == 'Remove unused import "time"'][0] + arguments = action['arguments'][0] + + assert action['command'] == importmagic_lint.REMOVE_IMPORT_COMMAND + assert arguments['startLine'] == 1 + assert arguments['endLine'] == 2 + assert arguments['newText'] == '' + + finally: + os.remove(name) From ae6314931673271d14ba3e9af3e9e2f9145ee604 Mon Sep 17 00:00:00 2001 From: youben11 Date: Thu, 14 Nov 2019 11:18:29 +0100 Subject: [PATCH 23/31] better linting test --- test/plugins/test_importmagic_lint.py | 46 +++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index 264401fd..af41d018 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -7,15 +7,45 @@ from pyls.workspace import Document DOC_URI = uris.from_fs_path(__file__) + +DOC_LINT = """ +import os +time.sleep(10) +t = 5 + +def useless_func(): + pass +""" + DOC_ADD = """ time.sleep(10) print("test") """ + DOC_REMOVE = """ import time print("useless import") """ +LINT_DIAGS = { + "Unresolved import 'time.sleep'": { + 'range': {'start': {'line': 2, 'character': 0}, 'end': {'line': 2, 'character': 10}}, + 'severity': lsp.DiagnosticSeverity.Hint, + }, + "Unreferenced variable/function 'useless_func'": { + 'range': {'start': {'line': 5, 'character': 4}, 'end': {'line': 5, 'character': 16}}, + 'severity': lsp.DiagnosticSeverity.Warning, + }, + "Unreferenced variable/function 't'": { + 'range': {'start': {'line': 3, 'character': 0}, 'end': {'line': 3, 'character': 1}}, + 'severity': lsp.DiagnosticSeverity.Warning, + }, + "Unreferenced import 'os'": { + 'range': {'start': {'line': 1, 'character': 7}, 'end': {'line': 1, 'character': 9}}, + 'severity': lsp.DiagnosticSeverity.Warning, + }, +} + def temp_document(doc_text): temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False) @@ -29,14 +59,16 @@ def temp_document(doc_text): def test_importmagic_lint(): try: - name, doc = temp_document(DOC) + name, doc = temp_document(DOC_LINT) diags = importmagic_lint.pyls_lint(doc) - unres_symbol = [d for d in diags if d['source'] == 'importmagic'][0] - - assert unres_symbol['message'] == "Unresolved import 'time.sleep'" - assert unres_symbol['range']['start'] == {'line': 1, 'character': 0} - assert unres_symbol['range']['end'] == {'line': 1, 'character': 10} - assert unres_symbol['severity'] == lsp.DiagnosticSeverity.Hint + importmagic_diags = [d for d in diags if d['source'] == 'importmagic'] + assert len(importmagic_diags) == len(LINT_DIAGS) + + for diag in importmagic_diags: + expected_diag = LINT_DIAGS.get(diag['message']) + assert expected_diag is not None, "Didn't expect diagnostic with message '{}'".format(diag['message']) + assert expected_diag['range'] == diag['range'] + assert expected_diag['severity'] == diag['severity'] finally: os.remove(name) From a712a732bdbca30a4c74c3cc99e00a5b41472821 Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 10:58:19 +0100 Subject: [PATCH 24/31] search symbols in source tokens instead of raw string: 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). --- pyls/plugins/importmagic_lint.py | 77 ++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index ebb729b1..56e63d28 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -2,6 +2,8 @@ import logging import re import sys +import tokenize +from io import BytesIO from concurrent.futures import ThreadPoolExecutor import importmagic from pyls import hookimpl, lsp, _utils @@ -62,6 +64,59 @@ def _get_imports_list(source, index=None): return imported +def _tokenize(source): + """Tokenize python source code. + """ + stream = BytesIO(source.encode()) + return list(tokenize.tokenize(stream.readline)) + + +def _search_symbol(source, symbol): + """Search symbol in python source code. + + Args: + source: str object of the source code + symbol: str object of the symbol to search + + Returns: + list of locations where the symbol was found. Each element have the following format + { + 'start': { + 'line': int, + 'character': int + }, + 'end': { + 'line': int, + 'character': int + } + } + """ + symbol_tokens = _tokenize(symbol) + source_tokens = _tokenize(source) + + get_str = lambda token: token[1] + symbol_tokens_str = list(map(get_str, symbol_tokens)) + source_tokens_str = list(map(get_str, source_tokens)) + + symbol_len = len(symbol_tokens) + locations = [] + for i in len(source_tokens): + if source_tokens_str[i:i+symbol_len] == symbol_tokens_str: + location_range = { + 'start': { + 'line': source_tokens[2][0] - 1, + 'character': source_tokens[2][1], + }, + 'end': { + 'line': source_tokens[3][0] - 1, + 'character': source_tokens[3][1], + } + } + locations.append(location_range) + + return locations + + @hookimpl def pyls_initialize(): _index_cache['default'] = None @@ -107,27 +162,16 @@ def pyls_lint(document): # Annoyingly, we only get the text of an unresolved import, so we'll look for it ourselves for unres in unresolved: - for line_no, line in enumerate(document.lines): - pos = line.find(unres) - if pos < 0: - continue - + for location_range in _search_symbol(document.source, unres): diagnostics.append({ 'source': SOURCE, - 'range': { - 'start': {'line': line_no, 'character': pos}, - 'end': {'line': line_no, 'character': pos + len(unres)} - }, + 'range': location_range, 'message': "Unresolved import '%s'" % unres, 'severity': lsp.DiagnosticSeverity.Hint, }) for unref in unreferenced: - for line_no, line in enumerate(document.lines): - pos = line.find(unref) - if pos < 0: - continue - + for location_range in _search_symbol(document.source, unref): # Find out if the unref is an import or a variable/func imports = _get_imports_list(document.source) if unref in imports: @@ -137,10 +181,7 @@ def pyls_lint(document): diagnostics.append({ 'source': SOURCE, - 'range': { - 'start': {'line': line_no, 'character': pos}, - 'end': {'line': line_no, 'character': pos + len(unref)} - }, + 'range': location_range, 'message': message, 'severity': lsp.DiagnosticSeverity.Warning, }) From 80def3ff222ac2092760ca7bf765e930bb937e55 Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 11:12:36 +0100 Subject: [PATCH 25/31] check for None value --- pyls/plugins/importmagic_lint.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 56e63d28..e64da234 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -68,7 +68,10 @@ def _tokenize(source): """Tokenize python source code. """ stream = BytesIO(source.encode()) - return list(tokenize.tokenize(stream.readline)) + tokens = tokenize.tokenize(stream.readline) + if tokens is None: + return [] + return list(tokens) def _search_symbol(source, symbol): From 0a9d163a6c7777309a077b0a8e7eabacbec4b86e Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 11:13:04 +0100 Subject: [PATCH 26/31] fix range --- pyls/plugins/importmagic_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index e64da234..a13d7724 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -103,7 +103,7 @@ def _search_symbol(source, symbol): symbol_len = len(symbol_tokens) locations = [] - for i in len(source_tokens): + for i in range(len(source_tokens) - symbol_len + 1): if source_tokens_str[i:i+symbol_len] == symbol_tokens_str: location_range = { 'start': { From e7ee0ca65f12f3b19710c3499ad579032bbaa19f Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 14:53:38 +0100 Subject: [PATCH 27/31] use backward compatible tokenizer func --- pyls/plugins/importmagic_lint.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index a13d7724..a02fe0ec 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -3,7 +3,6 @@ import re import sys import tokenize -from io import BytesIO from concurrent.futures import ThreadPoolExecutor import importmagic from pyls import hookimpl, lsp, _utils @@ -21,6 +20,19 @@ _index_cache = {} +class _SourceReader(): + # Used to tokenize python source code + def __init__(self, source): + self.lines = re.findall(r'[^\n]*\n', source) + # To pop lines later + self.lines.reverse() + + def readline(self): + if self.lines: + return self.lines.pop() + return '' + + def _build_index(paths): """Build index of symbols from python modules. """ @@ -66,12 +78,11 @@ def _get_imports_list(source, index=None): def _tokenize(source): """Tokenize python source code. + Returns only NAME tokens. """ - stream = BytesIO(source.encode()) - tokens = tokenize.tokenize(stream.readline) - if tokens is None: - return [] - return list(tokens) + readline = _SourceReader(source).readline + filter_name = lambda token: token[0] == tokenize.NAME + return filter(filter_name, tokenize.generate_tokens(readline)) def _search_symbol(source, symbol): @@ -94,8 +105,8 @@ def _search_symbol(source, symbol): } } """ - symbol_tokens = _tokenize(symbol) - source_tokens = _tokenize(source) + symbol_tokens = list(_tokenize(symbol)) + source_tokens = list(_tokenize(source)) get_str = lambda token: token[1] symbol_tokens_str = list(map(get_str, symbol_tokens)) From 809a4af47b483f87e30d4d522a5a1e245d84681f Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 15:42:37 +0100 Subject: [PATCH 28/31] fix indexing and pattern matching --- pyls/plugins/importmagic_lint.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index a02fe0ec..c12999a4 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -23,7 +23,7 @@ class _SourceReader(): # Used to tokenize python source code def __init__(self, source): - self.lines = re.findall(r'[^\n]*\n', source) + self.lines = re.findall(r'[^\n]*\n?', source) # To pop lines later self.lines.reverse() @@ -118,12 +118,12 @@ def _search_symbol(source, symbol): if source_tokens_str[i:i+symbol_len] == symbol_tokens_str: location_range = { 'start': { - 'line': source_tokens[2][0] - 1, - 'character': source_tokens[2][1], + 'line': source_tokens[i][2][0] - 1, + 'character': source_tokens[i][2][1], }, 'end': { - 'line': source_tokens[3][0] - 1, - 'character': source_tokens[3][1], + 'line': source_tokens[i + symbol_len - 1][3][0] - 1, + 'character': source_tokens[i + symbol_len - 1][3][1], } } locations.append(location_range) From 1c509f90d7db588038d606534ce62b804d4393bd Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 15:54:30 +0100 Subject: [PATCH 29/31] typo --- pyls/plugins/importmagic_lint.py | 1 + test/plugins/test_importmagic_lint.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index c12999a4..dafa68ff 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -186,6 +186,7 @@ def pyls_lint(document): for unref in unreferenced: for location_range in _search_symbol(document.source, unref): + # TODO(youben) use jedi.names to get the type of unref # Find out if the unref is an import or a variable/func imports = _get_imports_list(document.source) if unref in imports: diff --git a/test/plugins/test_importmagic_lint.py b/test/plugins/test_importmagic_lint.py index af41d018..ef056d94 100644 --- a/test/plugins/test_importmagic_lint.py +++ b/test/plugins/test_importmagic_lint.py @@ -102,7 +102,7 @@ def test_importmagic_remove_import_action(config): # wait for the index to be ready sleep(1) actions = importmagic_lint.pyls_code_actions(config, doc) - action = [a for a in actions if a['title'] == 'Remove unused import "time"'][0] + action = [a for a in actions if a['title'] == 'Remove unused import of "time"'][0] arguments = action['arguments'][0] assert action['command'] == importmagic_lint.REMOVE_IMPORT_COMMAND From b82ff8f660fdbbc8ed602d1177565ac8dbbce9e5 Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 16:02:04 +0100 Subject: [PATCH 30/31] new style class --- pyls/plugins/importmagic_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index dafa68ff..79cf68cf 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -20,7 +20,7 @@ _index_cache = {} -class _SourceReader(): +class _SourceReader(object): # Used to tokenize python source code def __init__(self, source): self.lines = re.findall(r'[^\n]*\n?', source) From 107c431635a2fa6e99384ea9cb776b54539c0043 Mon Sep 17 00:00:00 2001 From: youben11 Date: Fri, 15 Nov 2019 16:17:55 +0100 Subject: [PATCH 31/31] use list comprehension for better perf --- pyls/plugins/importmagic_lint.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pyls/plugins/importmagic_lint.py b/pyls/plugins/importmagic_lint.py index 79cf68cf..d8472ad1 100644 --- a/pyls/plugins/importmagic_lint.py +++ b/pyls/plugins/importmagic_lint.py @@ -81,8 +81,7 @@ def _tokenize(source): Returns only NAME tokens. """ readline = _SourceReader(source).readline - filter_name = lambda token: token[0] == tokenize.NAME - return filter(filter_name, tokenize.generate_tokens(readline)) + return [token for token in tokenize.generate_tokens(readline) if token[0] == tokenize.NAME] def _search_symbol(source, symbol): @@ -105,12 +104,11 @@ def _search_symbol(source, symbol): } } """ - symbol_tokens = list(_tokenize(symbol)) - source_tokens = list(_tokenize(source)) + symbol_tokens = _tokenize(symbol) + source_tokens = _tokenize(source) - get_str = lambda token: token[1] - symbol_tokens_str = list(map(get_str, symbol_tokens)) - source_tokens_str = list(map(get_str, source_tokens)) + symbol_tokens_str = [token[1] for token in symbol_tokens] + source_tokens_str = [token[1] for token in source_tokens] symbol_len = len(symbol_tokens) locations = []