From ffc3abc2322d5d91c6ea47f445b80a0e1bbbd63e Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Wed, 18 Sep 2024 09:36:31 -0700 Subject: [PATCH 01/16] add better completion based on current scope Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 217 ++++++++++++------ 1 file changed, 148 insertions(+), 69 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index b526143459da..8e68effd6443 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -143,6 +143,11 @@ import argparse import configargparse +import sys + + +def log(*args, **kwargs): + print(*args, **kwargs, file=sys.stderr) class ChplcheckProxy: @@ -229,12 +234,12 @@ def decl_kind(decl: chapel.NamedDecl) -> Optional[SymbolKind]: return SymbolKind.Method else: return SymbolKind.Function - elif isinstance(decl, chapel.Variable): + elif isinstance(decl, chapel.VarLikeDecl): if decl.intent() == "type": return SymbolKind.TypeParameter elif decl.intent() == "param": return SymbolKind.Constant - elif decl.is_field(): + elif isinstance(decl, chapel.Variable) and decl.is_field(): return SymbolKind.Field elif decl.intent() == "": return SymbolKind.Constant @@ -264,28 +269,26 @@ def decl_kind_to_completion_kind(kind: SymbolKind) -> CompletionItemKind: def completion_item_for_decl( - decl: chapel.NamedDecl, override_name: Optional[str] = None + decl: chapel.NamedDecl, + override_name: Optional[str] = None, + override_sort: Optional[str] = None, ) -> Optional[CompletionItem]: kind = decl_kind(decl) if not kind: return None - # For now, we show completion for global symbols (not x.), - # so it seems like we ought to rule out methods. - if kind == SymbolKind.Method: - return None - # We don't want to show operators in completion lists, as they're # not really useful to the user in this context. if kind == SymbolKind.Operator: return None name_to_use = override_name if override_name else decl.name() + sort_text = override_sort if override_sort else name_to_use return CompletionItem( label=name_to_use, kind=decl_kind_to_completion_kind(kind), insert_text=name_to_use, - sort_text=name_to_use, + sort_text=sort_text, ) @@ -349,9 +352,14 @@ def encode_deltas( class PositionList(Generic[EltT]): get_range: Callable[[EltT], Range] elts: List[EltT] = field(default_factory=list) + sort_by_end: bool = False def sort(self): - self.elts.sort(key=lambda x: self.get_range(x).start) + sort_by = "end" if self.sort_by_end else "start" + self.elts.sort(key=lambda x: getattr(self.get_range(x), sort_by)) + + def reverse(self): + self.elts.reverse() def append(self, elt: EltT): self.elts.append(elt) @@ -377,11 +385,20 @@ def clear(self): self.elts.clear() def find(self, pos: Position) -> Optional[EltT]: - idx = bisect_right( - self.elts, pos, key=lambda x: self.get_range(x).start + bisect = bisect_left if self.sort_by_end else bisect_right + sort_by = "end" if self.sort_by_end else "start" + check_by = "start" if self.sort_by_end else "end" + idx = bisect( + self.elts, pos, key=lambda x: getattr(self.get_range(x), sort_by) ) idx -= 1 - if idx < 0 or pos > self.get_range(self.elts[idx]).end: + + def cmp(x, y): + return x < y if self.sort_by_end else x > y + + if idx < 0 or cmp( + pos, getattr(self.get_range(self.elts[idx]), check_by) + ): return None return self.elts[idx] @@ -427,6 +444,27 @@ class ResolvedPair: resolved_to: NodeAndRange +@dataclass +class ScopedNodeAndRange: + node: chapel.AstNode + scopes: List[chapel.Scope] = field(default_factory=list) + rng: Range = field(init=False) + + @staticmethod + def create(node: chapel.AstNode) -> Optional["ScopedNodeAndRange"]: + scopes = [] + scope = node.scope() + while scope: + scopes.append(scope) + scope = scope.parent_scope() + if len(scopes) == 0: + return None + return ScopedNodeAndRange(node, scopes) + + def __post_init__(self): + self.rng = location_to_range(self.node.location()) + + @dataclass class References: in_file: "FileInfo" @@ -579,6 +617,7 @@ class FileInfo: use_resolver: bool use_segments: PositionList[ResolvedPair] = field(init=False) def_segments: PositionList[NodeAndRange] = field(init=False) + scope_segments: PositionList[ScopedNodeAndRange] = field(init=False) instantiation_segments: PositionList[ Tuple[NodeAndRange, chapel.TypedSignature] ] = field(init=False) @@ -588,11 +627,12 @@ class FileInfo: Dict[chapel.TypedSignature, CallsInTypeContext], ] = field(init=False) siblings: chapel.SiblingMap = field(init=False) - visible_decls: List[Tuple[str, chapel.AstNode]] = field(init=False) + # visible_decls: List[Tuple[str, chapel.AstNode]] = field(init=False) def __post_init__(self): self.use_segments = PositionList(lambda x: x.ident.rng) self.def_segments = PositionList(lambda x: x.rng) + self.scope_segments = PositionList(lambda x: x.rng, sort_by_end=True) self.instantiation_segments = PositionList(lambda x: x[0].rng) self.uses_here = {} self.rebuild_index() @@ -662,62 +702,81 @@ def _enter_Function(self, node: chapel.Function): @enter def _enter_NamedDecl(self, node: chapel.NamedDecl): self.def_segments.append(NodeAndRange(node)) + # s = ScopedNodeAndRange.create(node) + # if s: + # self.scope_segments.append(s) + + def get_visible_nodes( + self, pos: Position + ) -> List[Tuple[str, chapel.AstNode, int]]: + segment = self.scope_segments.find(pos) + if not segment: + return [] - def _collect_possibly_visible_decls(self, asts: List[chapel.AstNode]): - self.visible_decls = [] - for ast in asts: - if isinstance(ast, chapel.Comment): - continue + def visible_nodes_for_scope( + name: str, nodes: List[chapel.AstNode], in_bundled_module: bool + ) -> Optional[Tuple[str, chapel.AstNode]]: + # Don't show internal symbols to the user, even if they + # are technically in scope. The exception is if we're currently + # editing a standard file. + skip_prefixes = ["chpl_", "chpldev_", "_"] + if any(name.startswith(prefix) for prefix in skip_prefixes): + if not in_bundled_module: + return None + + # Only show nodes without @chpldoc.nodoc. The exception + # about standard files applies here too. + documented_nodes = [] + for node in nodes: + # apply aforementioned exception + if in_bundled_module: + documented_nodes.append(node) + continue - scope = ast.scope() - if not scope: - continue + # avoid nodes with nodoc attribute. + ag = node.attribute_group() + show = False + if not ag or not ag.get_attribute_named("chpldoc.nodoc"): + show = True + elif name in _ALLOWED_NODOC_DECLS: + # If users declare variables like 'here' themselves, + # we will not show them if they're @chpldoc.nodoc, + # since they're not special. + decl_file = node.location().path() + is_standard_decl = self.context.context.is_bundled_path( + decl_file + ) + show = is_standard_decl - file = ast.location().path() - in_bundled_module = self.context.context.is_bundled_path(file) + if show: + documented_nodes.append(node) - for name, nodes in scope.visible_nodes(): - # Don't show internal symbols to the user, even if they - # are technically in scope. The exception is if we're currently - # editing a standard file. - skip_prefixes = ["chpl_", "chpldev_", "_"] - if any(name.startswith(prefix) for prefix in skip_prefixes): - if not in_bundled_module: - continue + if len(documented_nodes) == 0: + return None - # Only show nodes without @chpldoc.nodoc. The exception - # about standard files applies here too. - documented_nodes = [] - for node in nodes: - # apply aforementioned exception - if in_bundled_module: - documented_nodes.append(node) - continue + # Just take the first value to avoid showing N entries for + # overloaded functions. + return name, documented_nodes[0] - # avoid nodes with nodoc attribute. - ag = node.attribute_group() - show = False - if not ag or not ag.get_attribute_named("chpldoc.nodoc"): - show = True - elif name in _ALLOWED_NODOC_DECLS: - # If users declare variables like 'here' themselves, - # we will not show them if they're @chpldoc.nodoc, - # since they're not special. - decl_file = node.location().path() - is_standard_decl = self.context.context.is_bundled_path( - decl_file - ) - show = is_standard_decl - - if show: - documented_nodes.append(node) - - if len(documented_nodes) == 0: - continue + visible_nodes = [] + file = segment.node.location().path() + in_bundled_module = self.context.context.is_bundled_path(file) + for depth, scope in enumerate(segment.scopes): + for name, nodes in scope.visible_nodes(): + visible_node = visible_nodes_for_scope( + name, nodes, in_bundled_module + ) + if visible_node: + d = depth + visible_path = visible_node[1].location().path() + # if from a bundled path, increase the depth by 1 + d += int(self.context.context.is_bundled_path(visible_path)) + # if from another file, increase the depth by 1 + d += int(visible_path != file) + + visible_nodes.append((visible_node[0], visible_node[1], d)) - # Just take the first value to avoid showing N entries for - # overloaded functions. - self.visible_decls.append((name, documented_nodes[0])) + return visible_nodes def _search_instantiations( self, @@ -769,12 +828,18 @@ def rebuild_index(self): refs.clear() self.use_segments.clear() self.def_segments.clear() + self.scope_segments.clear() + import time + start_time = time.time() self.visit(asts) + end_time = time.time() + log(f"Rebuilt index for {self.uri} in {end_time - start_time} seconds") self.use_segments.sort() self.def_segments.sort() + self.scope_segments.sort() self.siblings = chapel.SiblingMap(asts) - self._collect_possibly_visible_decls(asts) + # self._collect_possibly_visible_decls(asts) if self.use_resolver: # TODO: suppress resolution errors due to false-positives @@ -1729,12 +1794,26 @@ async def complete(ls: ChapelLanguageServer, params: CompletionParams): fi, _ = ls.get_file_info(text_doc.uri) + names = set() items = [] - for name, decl in fi.visible_decls: - if isinstance(decl, chapel.NamedDecl): - items.append(completion_item_for_decl(decl, override_name=name)) - - items = [item for item in items if item] + import time + start_time = time.time() + for name, node, depth in fi.get_visible_nodes(params.position): + if not isinstance(node, chapel.NamedDecl): + continue + # if name is already suggested, skip it + if name in names: + continue + # use the depth to sort the suggestions, lower depths first + sort_name = f"{depth:03d}{name}" + item = completion_item_for_decl( + node, override_name=name, override_sort=sort_name + ) + if item: + items.append(item) + names.add(name) + end_time = time.time() + log(f"Completion took {end_time - start_time} seconds") return CompletionList(is_incomplete=False, items=items) From 50394d7d57e804fc314d69a9a4c88f4b705032ca Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Wed, 18 Sep 2024 16:06:22 -0700 Subject: [PATCH 02/16] uncomment scope resolution Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 8e68effd6443..9eebcb370801 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -702,9 +702,9 @@ def _enter_Function(self, node: chapel.Function): @enter def _enter_NamedDecl(self, node: chapel.NamedDecl): self.def_segments.append(NodeAndRange(node)) - # s = ScopedNodeAndRange.create(node) - # if s: - # self.scope_segments.append(s) + s = ScopedNodeAndRange.create(node) + if s: + self.scope_segments.append(s) def get_visible_nodes( self, pos: Position From 04ac096ab4cfeaa15428a611cde70fe08633e2ec Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 23 Sep 2024 13:57:37 -0700 Subject: [PATCH 03/16] add creates_scope query Signed-off-by: Jade Abraham --- tools/chapel-py/src/method-tables/uast-methods.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/chapel-py/src/method-tables/uast-methods.h b/tools/chapel-py/src/method-tables/uast-methods.h index 2d6b1aa6b004..2cb39e794396 100644 --- a/tools/chapel-py/src/method-tables/uast-methods.h +++ b/tools/chapel-py/src/method-tables/uast-methods.h @@ -66,6 +66,8 @@ CLASS_BEGIN(AstNode) std::optional, return ScopeObject::tryCreate(contextObject, resolution::scopeForId(context, node->id()))) + PLAIN_GETTER(AstNode, creates_scope, "Returns true if this AST node creates a scope", + bool, return chpl::resolution::createsScope(node->tag())) PLAIN_GETTER(AstNode, type, "Get the type of this AST node, as a 3-tuple of (kind, type, param).", std::optional, From 4285ccbf80df981f7b73ef6e55108ebd488d08ef Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 23 Sep 2024 13:57:50 -0700 Subject: [PATCH 04/16] improve scope finding Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 9eebcb370801..c93d840ec164 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -355,8 +355,7 @@ class PositionList(Generic[EltT]): sort_by_end: bool = False def sort(self): - sort_by = "end" if self.sort_by_end else "start" - self.elts.sort(key=lambda x: getattr(self.get_range(x), sort_by)) + self.elts.sort(key=lambda x: self.get_range(x).start) def reverse(self): self.elts.reverse() @@ -385,20 +384,11 @@ def clear(self): self.elts.clear() def find(self, pos: Position) -> Optional[EltT]: - bisect = bisect_left if self.sort_by_end else bisect_right - sort_by = "end" if self.sort_by_end else "start" - check_by = "start" if self.sort_by_end else "end" - idx = bisect( - self.elts, pos, key=lambda x: getattr(self.get_range(x), sort_by) + idx = bisect_right( + self.elts, pos, key=lambda x: self.get_range(x).start ) idx -= 1 - - def cmp(x, y): - return x < y if self.sort_by_end else x > y - - if idx < 0 or cmp( - pos, getattr(self.get_range(self.elts[idx]), check_by) - ): + if idx < 0 or pos > self.get_range(self.elts[idx]).end: return None return self.elts[idx] @@ -677,6 +667,18 @@ def _note_reference(self, node: Union[chapel.Dot, chapel.Identifier]): ResolvedPair(NodeAndRange(node), NodeAndRange(to)) ) + def _note_scope(self, node: chapel.AstNode): + if not node.creates_scope(): + return + s = ScopedNodeAndRange.create(node) + if not s: + return + self.scope_segments.append(s) + + @enter + def _enter_AstNode(self, node: chapel.AstNode): + self._note_scope(node) + @enter def _enter_Identifier(self, node: chapel.Identifier): self._note_reference(node) @@ -691,6 +693,7 @@ def _enter_Module(self, node: chapel.Module): _ = node.scope_resolve() self.def_segments.append(NodeAndRange(node)) + self._note_scope(node) @enter def _enter_Function(self, node: chapel.Function): @@ -698,18 +701,17 @@ def _enter_Function(self, node: chapel.Function): _ = node.scope_resolve() self.def_segments.append(NodeAndRange(node)) + self._note_scope(node) @enter def _enter_NamedDecl(self, node: chapel.NamedDecl): self.def_segments.append(NodeAndRange(node)) - s = ScopedNodeAndRange.create(node) - if s: - self.scope_segments.append(s) + self._note_scope(node) def get_visible_nodes( self, pos: Position ) -> List[Tuple[str, chapel.AstNode, int]]: - segment = self.scope_segments.find(pos) + segment = self.scope_at_position(pos) if not segment: return [] @@ -836,7 +838,6 @@ def rebuild_index(self): log(f"Rebuilt index for {self.uri} in {end_time - start_time} seconds") self.use_segments.sort() self.def_segments.sort() - self.scope_segments.sort() self.siblings = chapel.SiblingMap(asts) # self._collect_possibly_visible_decls(asts) @@ -928,6 +929,15 @@ def get_use_or_def_segment_at_position( return None + def scope_at_position(self, position: Position) -> Optional[ScopedNodeAndRange]: + """ + Given a position, return the scope that contains it. + """ + for s in self.scope_segments.elts: + if s.rng.start <= position <= s.rng.end: + return s + + def file_lines(self) -> List[str]: file_text = self.context.context.get_file_text( self.uri[len("file://") :] From 613acdbec54fb33bb981cc96f6a22ab451f512b7 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 15:20:55 -0700 Subject: [PATCH 05/16] fix completion items for root level requests Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index c93d840ec164..0b5f0464a26a 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -711,10 +711,6 @@ def _enter_NamedDecl(self, node: chapel.NamedDecl): def get_visible_nodes( self, pos: Position ) -> List[Tuple[str, chapel.AstNode, int]]: - segment = self.scope_at_position(pos) - if not segment: - return [] - def visible_nodes_for_scope( name: str, nodes: List[chapel.AstNode], in_bundled_module: bool ) -> Optional[Tuple[str, chapel.AstNode]]: @@ -760,23 +756,40 @@ def visible_nodes_for_scope( # overloaded functions. return name, documented_nodes[0] + def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): + visible_nodes = [] + file = node.location().path() + in_bundled_module = self.context.context.is_bundled_path(file) + for depth, scope in enumerate(scopes): + for name, nodes in scope.visible_nodes(): + visible_node = visible_nodes_for_scope( + name, nodes, in_bundled_module + ) + if visible_node: + d = depth + visible_path = visible_node[1].location().path() + # if from a bundled path, increase the depth by 1 + d += int(self.context.context.is_bundled_path(visible_path)) + # if from another file, increase the depth by 1 + d += int(visible_path != file) + + visible_nodes.append((visible_node[0], visible_node[1], d)) + return visible_nodes + visible_nodes = [] - file = segment.node.location().path() - in_bundled_module = self.context.context.is_bundled_path(file) - for depth, scope in enumerate(segment.scopes): - for name, nodes in scope.visible_nodes(): - visible_node = visible_nodes_for_scope( - name, nodes, in_bundled_module - ) - if visible_node: - d = depth - visible_path = visible_node[1].location().path() - # if from a bundled path, increase the depth by 1 - d += int(self.context.context.is_bundled_path(visible_path)) - # if from another file, increase the depth by 1 - d += int(visible_path != file) + segment = self.scope_at_position(pos) - visible_nodes.append((visible_node[0], visible_node[1], d)) + # node_and_scopes: List[Tuple[chapel.AstNode, List[chapel.Scope]]] = [] + if segment: + visible_nodes.extend(visible_nodes_for_scopes(segment.node, segment.scopes)) + else: + # no segment found, use the top level nodes + for a in self.get_asts(): + if isinstance(a, chapel.Comment): + continue + s = a.scope() + if s: + visible_nodes.extend(visible_nodes_for_scopes(a, [s])) return visible_nodes @@ -1809,6 +1822,7 @@ async def complete(ls: ChapelLanguageServer, params: CompletionParams): import time start_time = time.time() for name, node, depth in fi.get_visible_nodes(params.position): + # log(node) if not isinstance(node, chapel.NamedDecl): continue # if name is already suggested, skip it From b95fc87b78773846b087f0fa7d9cf0817b90e331 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 15:40:16 -0700 Subject: [PATCH 06/16] cleanup logging Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 0b5f0464a26a..b6a018d6fc4a 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -844,11 +844,7 @@ def rebuild_index(self): self.use_segments.clear() self.def_segments.clear() self.scope_segments.clear() - import time - start_time = time.time() self.visit(asts) - end_time = time.time() - log(f"Rebuilt index for {self.uri} in {end_time - start_time} seconds") self.use_segments.sort() self.def_segments.sort() @@ -1819,10 +1815,7 @@ async def complete(ls: ChapelLanguageServer, params: CompletionParams): names = set() items = [] - import time - start_time = time.time() for name, node, depth in fi.get_visible_nodes(params.position): - # log(node) if not isinstance(node, chapel.NamedDecl): continue # if name is already suggested, skip it @@ -1836,8 +1829,6 @@ async def complete(ls: ChapelLanguageServer, params: CompletionParams): if item: items.append(item) names.add(name) - end_time = time.time() - log(f"Completion took {end_time - start_time} seconds") return CompletionList(is_incomplete=False, items=items) From c8cd9bd810a2ba83e84313e2ca3b6edaa72f0cae Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 16:59:35 -0700 Subject: [PATCH 07/16] add todo Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index b6a018d6fc4a..7a9b791e8dc2 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -1807,6 +1807,10 @@ async def hover(ls: ChapelLanguageServer, params: HoverParams): content = MarkupContent(MarkupKind.Markdown, text) return Hover(content, range=segment.get_location().range) + # TODO: can we make use of 'trigger_character' to provide completions? + # since we can't parse 'foo.', can we use the presence of a trigger '.' to + # read a identifier from the file buffer, lookup the scope for that name, + # and provide completions based on that scope? @server.feature(TEXT_DOCUMENT_COMPLETION, CompletionOptions()) async def complete(ls: ChapelLanguageServer, params: CompletionParams): text_doc = ls.workspace.get_text_document(params.text_document.uri) From fd15a7e368fd0b4b5e723e36d25aea8b7a68ee74 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 17:00:01 -0700 Subject: [PATCH 08/16] fix scope_at_position Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 7a9b791e8dc2..fcb0118ec8a7 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -942,9 +942,14 @@ def scope_at_position(self, position: Position) -> Optional[ScopedNodeAndRange]: """ Given a position, return the scope that contains it. """ + found = None for s in self.scope_segments.elts: if s.rng.start <= position <= s.rng.end: - return s + found = s + if s.rng.start > position: + break + return found + def file_lines(self) -> List[str]: From e5305fcb99731a5798416caa121e54057cbebb87 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 17:00:10 -0700 Subject: [PATCH 09/16] improve depth checking Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index fcb0118ec8a7..520bad00a342 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -761,6 +761,10 @@ def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): file = node.location().path() in_bundled_module = self.context.context.is_bundled_path(file) for depth, scope in enumerate(scopes): + files_named_in_use_or_import = set() + for m in scope.modules_named_in_use_or_import(): + files_named_in_use_or_import.add(m.location().path()) + for name, nodes in scope.visible_nodes(): visible_node = visible_nodes_for_scope( name, nodes, in_bundled_module @@ -768,10 +772,15 @@ def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): if visible_node: d = depth visible_path = visible_node[1].location().path() - # if from a bundled path, increase the depth by 1 - d += int(self.context.context.is_bundled_path(visible_path)) - # if from another file, increase the depth by 1 - d += int(visible_path != file) + + if visible_path != file: + # if from a different file, increase the depth by 1 + d += 1 + # if from a bundled path and not explicitly imported, increase the depth by 1 + d += int( + self.context.context.is_bundled_path(visible_path) + and visible_path not in files_named_in_use_or_import + ) visible_nodes.append((visible_node[0], visible_node[1], d)) return visible_nodes From a84c4a02bde05493cae60b495672e0ac4bd2cf2b Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Tue, 24 Sep 2024 17:00:24 -0700 Subject: [PATCH 10/16] format Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 520bad00a342..3199d82588e2 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -756,7 +756,9 @@ def visible_nodes_for_scope( # overloaded functions. return name, documented_nodes[0] - def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): + def visible_nodes_for_scopes( + node: chapel.AstNode, scopes: List[chapel.Scope] + ): visible_nodes = [] file = node.location().path() in_bundled_module = self.context.context.is_bundled_path(file) @@ -778,11 +780,16 @@ def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): d += 1 # if from a bundled path and not explicitly imported, increase the depth by 1 d += int( - self.context.context.is_bundled_path(visible_path) - and visible_path not in files_named_in_use_or_import + self.context.context.is_bundled_path( + visible_path + ) + and visible_path + not in files_named_in_use_or_import ) - visible_nodes.append((visible_node[0], visible_node[1], d)) + visible_nodes.append( + (visible_node[0], visible_node[1], d) + ) return visible_nodes visible_nodes = [] @@ -790,7 +797,9 @@ def visible_nodes_for_scopes(node: chapel.AstNode, scopes: List[chapel.Scope]): # node_and_scopes: List[Tuple[chapel.AstNode, List[chapel.Scope]]] = [] if segment: - visible_nodes.extend(visible_nodes_for_scopes(segment.node, segment.scopes)) + visible_nodes.extend( + visible_nodes_for_scopes(segment.node, segment.scopes) + ) else: # no segment found, use the top level nodes for a in self.get_asts(): @@ -947,7 +956,9 @@ def get_use_or_def_segment_at_position( return None - def scope_at_position(self, position: Position) -> Optional[ScopedNodeAndRange]: + def scope_at_position( + self, position: Position + ) -> Optional[ScopedNodeAndRange]: """ Given a position, return the scope that contains it. """ @@ -959,8 +970,6 @@ def scope_at_position(self, position: Position) -> Optional[ScopedNodeAndRange]: break return found - - def file_lines(self) -> List[str]: file_text = self.context.context.get_file_text( self.uri[len("file://") :] From 32dc95375a6217ee4bc0bd240aa3c7b5bda417e8 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Wed, 25 Sep 2024 08:43:32 -0700 Subject: [PATCH 11/16] improve depth checking Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 3199d82588e2..ccc7b1890c55 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -774,16 +774,17 @@ def visible_nodes_for_scopes( if visible_node: d = depth visible_path = visible_node[1].location().path() - if visible_path != file: # if from a different file, increase the depth by 1 d += 1 - # if from a bundled path and not explicitly imported, increase the depth by 1 + # if from a bundled path increase the depth by 1 d += int( self.context.context.is_bundled_path( visible_path - ) - and visible_path + )) + # if not explicitly used, increase the depth by 1 + d += int( + visible_path not in files_named_in_use_or_import ) From a2c2eab3d08777b4b88de0d444093b55108c274a Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Wed, 25 Sep 2024 08:43:48 -0700 Subject: [PATCH 12/16] add tests Signed-off-by: Jade Abraham --- tools/chpl-language-server/test/completion.py | 196 ++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 tools/chpl-language-server/test/completion.py diff --git a/tools/chpl-language-server/test/completion.py b/tools/chpl-language-server/test/completion.py new file mode 100644 index 000000000000..c795a2821730 --- /dev/null +++ b/tools/chpl-language-server/test/completion.py @@ -0,0 +1,196 @@ +""" +Test that completion works properly +""" + +import sys + +from lsprotocol.types import ClientCapabilities +from lsprotocol.types import Position, TextDocumentIdentifier +from lsprotocol.types import CompletionItem, CompletionList, CompletionParams +from lsprotocol.types import InitializeParams +import pytest +import pytest_lsp +from pytest_lsp import ClientServerConfig, LanguageClient + +from util.utils import * +from util.config import CLS_PATH + + +@pytest_lsp.fixture( + config=ClientServerConfig( + server_command=[sys.executable, CLS_PATH()], + client_factory=get_base_client, + ) +) +async def client(lsp_client: LanguageClient): + # Setup + params = InitializeParams(capabilities=ClientCapabilities()) + await lsp_client.initialize_session(params) + + yield + + # Teardown + await lsp_client.shutdown_session() + + +async def check_completion_items( + client: LanguageClient, + doc: TextDocumentIdentifier, + pos: Position, + expected: typing.List[str], +) -> typing.List[CompletionItem]: + """ + Check that the names returned by the completion items match what is expected. + + Expected is a list strings of length N that should match the first N completion items. + """ + items = await client.text_document_completion_async( + params=CompletionParams(text_document=doc, position=pos) + ) + assert items is not None + items = items.items if isinstance(items, CompletionList) else items + assert len(items) >= len(expected) + + sorted_items = sorted(items, key=lambda x: (x.sort_text or x.label).lower()) + print(" ") + for expect, item in zip(expected, sorted_items): + print(f"Expected: {expect}, Actual: {item.label}") + assert item.label == expect + return sorted_items + + +@pytest.mark.asyncio +async def test_empty(client: LanguageClient): + """ + Test that an empty file returns something + """ + test = ";" + async with source_file(client, test) as doc: + items = await check_completion_items(client, doc, pos((0, 0)), []) + assert len(items) > 0 + + +@pytest.mark.asyncio +async def test_basic_completion(client: LanguageClient): + """ + Test basic features + """ + file = """ + var myGlobal: int; + record R { + var abc: int; + proc foo() { } + proc bar(myFormal) { + var localVar = 2; + forall myIndex in 1..10 + with (var myTaskPrivate = abc, var myGlobal = 1) { + + } + begin { + + } + } + } + var a = new R(); + """ + + async with source_file(client, file) as doc: + global_scope = (pos((0, 0)), ["a", "myGlobal", "R"]) + r_scope = (pos((2, 0)), ["abc", "bar", "foo"] + global_scope[1]) + bar_scope = (pos((5, 0)), ["localVar", "myFormal", "this"] + r_scope[1]) + + # forall_scope changes where myGlobal is + forall_parent_scope_elms = bar_scope[1].copy() + forall_parent_scope_elms.remove("myGlobal") + forall_scope = ( + pos((8, 0)), + ["myGlobal", "myIndex", "myTaskPrivate"] + forall_parent_scope_elms, + ) + + begin_scope = (pos((11, 0)), bar_scope[1]) + + expected = [global_scope, r_scope, bar_scope, forall_scope, begin_scope] + for p, exp in expected: + await check_completion_items(client, doc, p, exp) + + +@pytest.mark.asyncio +async def test_std_lib(client: LanguageClient): + """ + Test that modules can be imported and used + """ + A = """ + use IO; + use B; + var mySymbol = 1; + """ + B = """ + proc foo() { } + """ + + async with source_files(client, A=A, B=B) as docs: + a_scope = (pos((0, 0)), ["mySymbol", "foo"]) + b_scope = (pos((1, 0)), ["foo"]) + + a_completion = await check_completion_items( + client, docs("A"), a_scope[0], a_scope[1] + ) + # a should have IO and writef somewhere in the list + assert any( + [x.label == "IO" or x.label == "writef" for x in a_completion] + ) + + await check_completion_items(client, docs("B"), b_scope[0], b_scope[1]) + + await save_file(client, docs("A"), docs("B")) + assert len(client.diagnostics[docs("A").uri]) == 0 + assert len(client.diagnostics[docs("B").uri]) == 0 + + +@pytest.mark.asyncio +async def test_use_in_module(client: LanguageClient): + """ + Test that modules can be imported and used inside of a module + """ + + file = """ + module file { + var mySymbol = 1; + module M { + use Random; + } + } + """ + + async with source_file(client, file) as doc: + file_scope = (pos((0, 0)), ["M", "mySymbol"]) + m_scope = (pos((3, 0)), ["M", "mySymbol"]) + + await check_completion_items(client, doc, file_scope[0], file_scope[1]) + m_scope_items = await check_completion_items( + client, doc, m_scope[0], m_scope[1] + ) + assert any([x.label == "Random" for x in m_scope_items]) + + +@pytest.mark.asyncio +async def test_use_in_scope(client: LanguageClient): + """ + Test that modules can be imported and used inside of an arbitrary scope + """ + + file = """ + proc bar() { + use Sort; + } + ; + """ + + async with source_file(client, file) as doc: + # bar and Sort are at the same "depth", so we can't rely on ordering + items = await check_completion_items(client, doc, pos((0, 0)), []) + assert any([x.label == "bar" or x.label == "sort" for x in items]) + + # outside of the scope, we should only see bar + items = await check_completion_items(client, doc, pos((3, 0)), ["bar"]) + assert all([x.label != "sort" for x in items]) From 54ec77de7eefb7d752e57c7a6686351168870a2f Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Wed, 25 Sep 2024 09:00:09 -0700 Subject: [PATCH 13/16] fix formatting Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index ccc7b1890c55..4039aebe71a7 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -781,11 +781,11 @@ def visible_nodes_for_scopes( d += int( self.context.context.is_bundled_path( visible_path - )) + ) + ) # if not explicitly used, increase the depth by 1 d += int( - visible_path - not in files_named_in_use_or_import + visible_path not in files_named_in_use_or_import ) visible_nodes.append( From 9a6d991b819fe1ba370a94fdc00c359ca3b78ba9 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 30 Sep 2024 16:37:00 -0700 Subject: [PATCH 14/16] cleanup implementation Signed-off-by: Jade Abraham --- .../src/chpl-language-server.py | 102 +++++++++++------- 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 4039aebe71a7..22cb2418072d 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -144,6 +144,7 @@ import argparse import configargparse import sys +import functools def log(*args, **kwargs): @@ -352,14 +353,10 @@ def encode_deltas( class PositionList(Generic[EltT]): get_range: Callable[[EltT], Range] elts: List[EltT] = field(default_factory=list) - sort_by_end: bool = False def sort(self): self.elts.sort(key=lambda x: self.get_range(x).start) - def reverse(self): - self.elts.reverse() - def append(self, elt: EltT): self.elts.append(elt) @@ -438,7 +435,6 @@ class ResolvedPair: class ScopedNodeAndRange: node: chapel.AstNode scopes: List[chapel.Scope] = field(default_factory=list) - rng: Range = field(init=False) @staticmethod def create(node: chapel.AstNode) -> Optional["ScopedNodeAndRange"]: @@ -451,8 +447,10 @@ def create(node: chapel.AstNode) -> Optional["ScopedNodeAndRange"]: return None return ScopedNodeAndRange(node, scopes) - def __post_init__(self): - self.rng = location_to_range(self.node.location()) + @property + @functools.cache + def rng(self): + return location_to_range(self.node.location()) @dataclass @@ -617,12 +615,11 @@ class FileInfo: Dict[chapel.TypedSignature, CallsInTypeContext], ] = field(init=False) siblings: chapel.SiblingMap = field(init=False) - # visible_decls: List[Tuple[str, chapel.AstNode]] = field(init=False) def __post_init__(self): self.use_segments = PositionList(lambda x: x.ident.rng) self.def_segments = PositionList(lambda x: x.rng) - self.scope_segments = PositionList(lambda x: x.rng, sort_by_end=True) + self.scope_segments = PositionList(lambda x: x.rng) self.instantiation_segments = PositionList(lambda x: x[0].rng) self.uses_here = {} self.rebuild_index() @@ -711,9 +708,19 @@ def _enter_NamedDecl(self, node: chapel.NamedDecl): def get_visible_nodes( self, pos: Position ) -> List[Tuple[str, chapel.AstNode, int]]: + """ + Returns the visible nodes at a given position. + """ def visible_nodes_for_scope( name: str, nodes: List[chapel.AstNode], in_bundled_module: bool ) -> Optional[Tuple[str, chapel.AstNode]]: + """ + Narrow the list of visible nodes to those that are actually visible + + The heuristic here is to avoid showing internal symbols to the user, + i.e. those that start with 'chpl_' or '_'. We also avoid showing nodes + with the @chpldoc.nodoc attribute. + """ # Don't show internal symbols to the user, even if they # are technically in scope. The exception is if we're currently # editing a standard file. @@ -756,51 +763,67 @@ def visible_nodes_for_scope( # overloaded functions. return name, documented_nodes[0] + @functools.cache + def files_named_in_use_or_import(scope: chapel.Scope) -> Set[str]: + files = set() + for m in scope.modules_named_in_use_or_import(): + files.add(m.location().path()) + return files + + def apply_depth_heuristic( + scope: chapel.Scope, + name: str, + node: chapel.AstNode, + original_depth: int, + cur_file: str, + ) -> Tuple[str, chapel.AstNode, int]: + """ + Heuristic to provide results in a more useful order, since + most clients will sort alphabetically. We can provide a + depth that is used to sort the results, so that the most + relevant results are shown first. + """ + depth = original_depth + vis_path = node.location().path() + if vis_path != cur_file: + # if from a different file, increase the depth by 1 + depth += 1 + # if from a bundled path increase the depth by 1 + depth += int(self.context.context.is_bundled_path(vis_path)) + # if not explicitly used, increase the depth by 1 + files_named_in_use = files_named_in_use_or_import(scope) + depth += int(vis_path not in files_named_in_use) + return (name, node, depth) + def visible_nodes_for_scopes( node: chapel.AstNode, scopes: List[chapel.Scope] ): visible_nodes = [] - file = node.location().path() - in_bundled_module = self.context.context.is_bundled_path(file) + cur_file = node.location().path() + in_bundled_module = self.context.context.is_bundled_path(cur_file) + # for each scope of the node for depth, scope in enumerate(scopes): - files_named_in_use_or_import = set() - for m in scope.modules_named_in_use_or_import(): - files_named_in_use_or_import.add(m.location().path()) - + # for all of the visible nodes in the scope for name, nodes in scope.visible_nodes(): + # narrow the list of visible nodes to those that are + # actually visible to the user (i.e. not nodoc/internal) visible_node = visible_nodes_for_scope( name, nodes, in_bundled_module ) - if visible_node: - d = depth - visible_path = visible_node[1].location().path() - if visible_path != file: - # if from a different file, increase the depth by 1 - d += 1 - # if from a bundled path increase the depth by 1 - d += int( - self.context.context.is_bundled_path( - visible_path - ) - ) - # if not explicitly used, increase the depth by 1 - d += int( - visible_path not in files_named_in_use_or_import - ) - - visible_nodes.append( - (visible_node[0], visible_node[1], d) - ) + if visible_node is None: + continue + vn = apply_depth_heuristic( + scope, *visible_node, depth, cur_file + ) + visible_nodes.append(vn) return visible_nodes visible_nodes = [] segment = self.scope_at_position(pos) - # node_and_scopes: List[Tuple[chapel.AstNode, List[chapel.Scope]]] = [] if segment: - visible_nodes.extend( - visible_nodes_for_scopes(segment.node, segment.scopes) - ) + vns = visible_nodes_for_scopes(segment.node, segment.scopes) + visible_nodes.extend(vns) else: # no segment found, use the top level nodes for a in self.get_asts(): @@ -868,7 +891,6 @@ def rebuild_index(self): self.def_segments.sort() self.siblings = chapel.SiblingMap(asts) - # self._collect_possibly_visible_decls(asts) if self.use_resolver: # TODO: suppress resolution errors due to false-positives From b19953b64bbce6ee810a50ae8facffc496671980 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 30 Sep 2024 16:40:14 -0700 Subject: [PATCH 15/16] fix formatting Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 22cb2418072d..012967f1a5b8 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -711,6 +711,7 @@ def get_visible_nodes( """ Returns the visible nodes at a given position. """ + def visible_nodes_for_scope( name: str, nodes: List[chapel.AstNode], in_bundled_module: bool ) -> Optional[Tuple[str, chapel.AstNode]]: From 960f23246666eb4451fc95942d960f7e6a10cb3a Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 14 Oct 2024 13:50:44 -0600 Subject: [PATCH 16/16] remove functools.cache Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 012967f1a5b8..8b1041f65368 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -448,7 +448,6 @@ def create(node: chapel.AstNode) -> Optional["ScopedNodeAndRange"]: return ScopedNodeAndRange(node, scopes) @property - @functools.cache def rng(self): return location_to_range(self.node.location())