From e363239d1813b4ea0607ec89e9519ecd48a249af Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 20:15:53 +0800 Subject: [PATCH 1/8] Hierarchical documentSymbol support. --- pyls/plugins/symbols.py | 64 +++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index ced97218..8d83da9a 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -1,5 +1,6 @@ # Copyright 2017 Palantir Technologies, Inc. import logging +from jedi.api.classes import Definition from pyls import hookimpl from pyls.lsp import SymbolKind @@ -8,27 +9,40 @@ @hookimpl def pyls_document_symbols(config, document): - all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) - definitions = document.jedi_names(all_scopes=all_scopes) - return [{ - 'name': d.name, - 'containerName': _container(d), - 'location': { - 'uri': document.uri, + # all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) + definitions = document.jedi_names(all_scopes=False) + def transform(d: Definition): + includeD = _include_def(d) + if includeD is None: + return None + children = [dt for dt in (transform(d1) for d1 in d.defined_names()) if dt] if includeD else None + detailName = d.full_name + if detailName and detailName.startswith("__main__."): + detailName = detailName[9:] + return { + 'name': d.name, + 'detail': detailName, 'range': _range(d), - }, - 'kind': _kind(d), - } for d in definitions if _include_def(d)] - + 'selectionRange': _name_range(d), + 'kind': _kind(d), + 'children': children + } + return [dt for dt in (transform(d) for d in definitions) if dt] def _include_def(definition): - return ( - # Don't tend to include parameters as symbols - definition.type != 'param' and - # Unused vars should also be skipped - definition.name != '_' and - _kind(definition) is not None - ) + # True: include def and sub-defs + # False: include def but do not include sub-defs + # None: Do not include def or sub-defs + if (# Unused vars should also be skipped + definition.name != '_' and + not definition._name.is_import() and + definition.is_definition() and + not definition.in_builtin_module() and + _kind(definition) is not None + ): + # for `statement`, we do not enumerate its child nodes. It tends to cause Error. + return definition.type not in ("statement",) + return None def _container(definition): @@ -56,6 +70,17 @@ def _range(definition): } +def _name_range(definition): + # Gets the range of symbol name (e.g. function name of a function) + definition = definition._name.tree_name + (start_line, start_column) = definition.start_pos + (end_line, end_column) = definition.end_pos + return { + 'start': {'line': start_line - 1, 'character': start_column}, + 'end': {'line': end_line - 1, 'character': end_column} + } + + _SYMBOL_KIND_MAP = { 'none': SymbolKind.Variable, 'type': SymbolKind.Class, @@ -85,7 +110,8 @@ def _range(definition): 'constant': SymbolKind.Constant, 'variable': SymbolKind.Variable, 'value': SymbolKind.Variable, - 'param': SymbolKind.Variable, + # Don't tend to include parameters as symbols + # 'param': SymbolKind.Variable, 'statement': SymbolKind.Variable, 'boolean': SymbolKind.Boolean, 'int': SymbolKind.Number, From 024c03659523423a62d5f991c288781b8e2a6936 Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 20:34:58 +0800 Subject: [PATCH 2/8] Remove type annotation because it will break Python 2 build. --- pyls/plugins/symbols.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index 8d83da9a..790c38c0 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -1,6 +1,5 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -from jedi.api.classes import Definition from pyls import hookimpl from pyls.lsp import SymbolKind @@ -11,11 +10,11 @@ def pyls_document_symbols(config, document): # all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) definitions = document.jedi_names(all_scopes=False) - def transform(d: Definition): - includeD = _include_def(d) - if includeD is None: + def transform(d): + include_d = _include_def(d) + if include_d is None: return None - children = [dt for dt in (transform(d1) for d1 in d.defined_names()) if dt] if includeD else None + children = [dt for dt in (transform(d1) for d1 in d.defined_names()) if dt] if include_d else None detailName = d.full_name if detailName and detailName.startswith("__main__."): detailName = detailName[9:] From 8ceb315ea4403760bf42859fc06b78b6df730091 Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 20:53:48 +0800 Subject: [PATCH 3/8] Keep legacy documentSymbol implementation working. ... depends on whether LSP client supports hierarchicalDocumentSymbol. --- pyls/plugins/symbols.py | 27 +++++++++++++++++++++++---- test/plugins/test_symbols.py | 2 +- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index 790c38c0..b49b48e7 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -8,10 +8,13 @@ @hookimpl def pyls_document_symbols(config, document): - # all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) + if not config.capabilities.get("documentSymbol", {}).get("hierarchicalDocumentSymbolSupport", False): + return pyls_document_symbols_legacy(config, document) + # returns DocumentSymbol[] + hide_imports = config.plugin_settings('jedi_symbols').get('hide_imports', False) definitions = document.jedi_names(all_scopes=False) def transform(d): - include_d = _include_def(d) + include_d = _include_def(d, hide_imports) if include_d is None: return None children = [dt for dt in (transform(d1) for d1 in d.defined_names()) if dt] if include_d else None @@ -28,17 +31,33 @@ def transform(d): } return [dt for dt in (transform(d) for d in definitions) if dt] -def _include_def(definition): +def pyls_document_symbols_legacy(config, document): + # returns SymbolInformation[] + all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) + hide_imports = config.plugin_settings('jedi_symbols').get('hide_imports', False) + definitions = document.jedi_names(all_scopes=all_scopes) + return [{ + 'name': d.name, + 'containerName': _container(d), + 'location': { + 'uri': document.uri, + 'range': _range(d), + }, + 'kind': _kind(d), + } for d in definitions if _include_def(d, hide_imports) is not None] + +def _include_def(definition, hide_imports=True): # True: include def and sub-defs # False: include def but do not include sub-defs # None: Do not include def or sub-defs if (# Unused vars should also be skipped definition.name != '_' and - not definition._name.is_import() and definition.is_definition() and not definition.in_builtin_module() and _kind(definition) is not None ): + if definition._name.is_import(): + return None if hide_imports else False # for `statement`, we do not enumerate its child nodes. It tends to cause Error. return definition.type not in ("statement",) return None diff --git a/test/plugins/test_symbols.py b/test/plugins/test_symbols.py index b4bb8b0d..5a6ebb55 100644 --- a/test/plugins/test_symbols.py +++ b/test/plugins/test_symbols.py @@ -23,7 +23,7 @@ def main(x): def test_symbols(config): doc = Document(DOC_URI, DOC) - config.update({'plugins': {'jedi_symbols': {'all_scopes': False}}}) + config.update({'plugins': {'jedi_symbols': {'all_scopes': False, 'hide_imports': False}}}) symbols = pyls_document_symbols(config, doc) # All four symbols (import sys, a, B, main, y) From ff9966fdc6674bd30e23a2b3e21477ded68bd1c7 Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 21:22:32 +0800 Subject: [PATCH 4/8] Fix linting errors. --- pyls/plugins/symbols.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index b49b48e7..470ca1fb 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -13,6 +13,7 @@ def pyls_document_symbols(config, document): # returns DocumentSymbol[] hide_imports = config.plugin_settings('jedi_symbols').get('hide_imports', False) definitions = document.jedi_names(all_scopes=False) + def transform(d): include_d = _include_def(d, hide_imports) if include_d is None: @@ -31,6 +32,7 @@ def transform(d): } return [dt for dt in (transform(d) for d in definitions) if dt] + def pyls_document_symbols_legacy(config, document): # returns SymbolInformation[] all_scopes = config.plugin_settings('jedi_symbols').get('all_scopes', True) @@ -46,11 +48,13 @@ def pyls_document_symbols_legacy(config, document): 'kind': _kind(d), } for d in definitions if _include_def(d, hide_imports) is not None] + def _include_def(definition, hide_imports=True): + # returns # True: include def and sub-defs # False: include def but do not include sub-defs # None: Do not include def or sub-defs - if (# Unused vars should also be skipped + if ( # Unused vars should also be skipped definition.name != '_' and definition.is_definition() and not definition.in_builtin_module() and From 13f441695508fba11600f8fb3bb489f5c255e677 Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 21:43:34 +0800 Subject: [PATCH 5/8] Make it possible to force using DocumentSymbol[] in plugin settings. --- pyls/plugins/symbols.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index 470ca1fb..237e998f 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -8,7 +8,11 @@ @hookimpl def pyls_document_symbols(config, document): - if not config.capabilities.get("documentSymbol", {}).get("hierarchicalDocumentSymbolSupport", False): + useHierarchicalSymbols = config.plugin_settings('jedi_symbols').get('hierarchical_symbols', None) + if useHierarchicalSymbols is None: + useHierarchicalSymbols = config.capabilities.get("documentSymbol", {}).get( + "hierarchicalDocumentSymbolSupport", False) + if not useHierarchicalSymbols: return pyls_document_symbols_legacy(config, document) # returns DocumentSymbol[] hide_imports = config.plugin_settings('jedi_symbols').get('hide_imports', False) From 688dee8de26026863ec4b875dd9b772bd1c7164b Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 21:45:00 +0800 Subject: [PATCH 6/8] Add test_symbols_hierarchical. --- test/plugins/test_symbols.py | 46 +++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/test/plugins/test_symbols.py b/test/plugins/test_symbols.py index 5a6ebb55..a048a76a 100644 --- a/test/plugins/test_symbols.py +++ b/test/plugins/test_symbols.py @@ -23,10 +23,16 @@ def main(x): def test_symbols(config): doc = Document(DOC_URI, DOC) + config.update({'plugins': {'jedi_symbols': {'all_scopes': False, 'hide_imports': True}}}) + symbols = pyls_document_symbols(config, doc) + + # Only local symbols (a, B, main, y) + assert len(symbols) == 4 + config.update({'plugins': {'jedi_symbols': {'all_scopes': False, 'hide_imports': False}}}) symbols = pyls_document_symbols(config, doc) - # All four symbols (import sys, a, B, main, y) + # All five symbols (import sys, a, B, main, y) assert len(symbols) == 5 def sym(name): @@ -47,6 +53,14 @@ def sym(name): def test_symbols_all_scopes(config): doc = Document(DOC_URI, DOC) + + config.update({'plugins': {'jedi_symbols': {'all_scopes': True, 'hide_imports': True}}}) + symbols = pyls_document_symbols(config, doc) + + # Only local symbols (a, B, __init__, x, y, main, y) + assert len(symbols) == 7 + + config.update({'plugins': {'jedi_symbols': {'all_scopes': True, 'hide_imports': False}}}) symbols = pyls_document_symbols(config, doc) # All eight symbols (import sys, a, B, __init__, x, y, main, y) @@ -63,3 +77,33 @@ def sym(name): # Not going to get too in-depth here else we're just testing Jedi assert sym('a')['location']['range']['start'] == {'line': 2, 'character': 0} + +def test_symbols_hierarchical(config): + doc = Document(DOC_URI, DOC) + + config.update({'plugins': {'jedi_symbols': {'hierarchical_symbols': True, 'hide_imports': True}}}) + symbols = pyls_document_symbols(config, doc) + + # Only local symbols (a, B, main, y) + assert len(symbols) == 4 + + config.update({'plugins': {'jedi_symbols': {'hierarchical_symbols': True, 'hide_imports': False}}}) + symbols = pyls_document_symbols(config, doc) + + # All five symbols (import sys, a, B, main, y) + assert len(symbols) == 5 + + def sym(name): + return [s for s in symbols if s['name'] == name][0] + def child_sym(sym, name): + if not sym['children']: + return None + return [s for s in sym['children'] if s['name'] == name][0] + + # Check we have some sane mappings to VSCode constants + assert sym('a')['kind'] == SymbolKind.Variable + assert sym('B')['kind'] == SymbolKind.Class + assert len(sym('B')['children']) == 1 + assert child_sym(sym('B'), '__init__')['kind'] == SymbolKind.Function + assert child_sym(sym('B'), '__init__')['detail'] == 'B.__init__' + assert sym('main')['kind'] == SymbolKind.Function \ No newline at end of file From bc895dda134589f53284befeb5f9c1186b7f8b2c Mon Sep 17 00:00:00 2001 From: forest93 Date: Sun, 7 Apr 2019 21:48:14 +0800 Subject: [PATCH 7/8] Fix linting errors. --- test/plugins/test_symbols.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/plugins/test_symbols.py b/test/plugins/test_symbols.py index a048a76a..be294c2c 100644 --- a/test/plugins/test_symbols.py +++ b/test/plugins/test_symbols.py @@ -78,6 +78,7 @@ def sym(name): # Not going to get too in-depth here else we're just testing Jedi assert sym('a')['location']['range']['start'] == {'line': 2, 'character': 0} + def test_symbols_hierarchical(config): doc = Document(DOC_URI, DOC) @@ -95,6 +96,7 @@ def test_symbols_hierarchical(config): def sym(name): return [s for s in symbols if s['name'] == name][0] + def child_sym(sym, name): if not sym['children']: return None @@ -106,4 +108,4 @@ def child_sym(sym, name): assert len(sym('B')['children']) == 1 assert child_sym(sym('B'), '__init__')['kind'] == SymbolKind.Function assert child_sym(sym('B'), '__init__')['detail'] == 'B.__init__' - assert sym('main')['kind'] == SymbolKind.Function \ No newline at end of file + assert sym('main')['kind'] == SymbolKind.Function From c20ca995b16fd86a60f06b4f0518335d36c06884 Mon Sep 17 00:00:00 2001 From: forest93 Date: Tue, 9 Apr 2019 23:44:50 +0800 Subject: [PATCH 8/8] Use correct client capability node. --- pyls/plugins/symbols.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pyls/plugins/symbols.py b/pyls/plugins/symbols.py index 237e998f..4c3b3b37 100644 --- a/pyls/plugins/symbols.py +++ b/pyls/plugins/symbols.py @@ -10,8 +10,10 @@ def pyls_document_symbols(config, document): useHierarchicalSymbols = config.plugin_settings('jedi_symbols').get('hierarchical_symbols', None) if useHierarchicalSymbols is None: - useHierarchicalSymbols = config.capabilities.get("documentSymbol", {}).get( - "hierarchicalDocumentSymbolSupport", False) + useHierarchicalSymbols = (config.capabilities + .get("textDocument", {}) + .get("documentSymbol", {}) + .get("hierarchicalDocumentSymbolSupport", False)) if not useHierarchicalSymbols: return pyls_document_symbols_legacy(config, document) # returns DocumentSymbol[]