Address PR review: reuse parsed AST, hoist neutralize, filter fix

- document_symbols: reuse the AST `Parser.parse_immediate/2` already produced on
  `Parser.Context.ast` instead of reparsing the raw source with toxic2. That
  reparse was an extra parse for .ex/.exs and nonsensical for .eex/.html-eex
  (which are EEx-derived Code ASTs, not toxic2 trees).
- selection_ranges: neutralize the toxic2 error nodes once in `selection_ranges/3`
  and pass the neutralized AST to `delimiter_pair_ranges/4` and
  `ast_node_ranges/4`, instead of re-walking/neutralizing the whole tree once per
  cursor position.
- completion_engine: compute the loaded/application module-name list once per
  struct-module filter pass rather than per candidate (`has_struct_submodule?`
  was O(N^2)). Mirrors the elixir_sense copy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232

Author: lukaszsamson

apps/elixir_ls_utils/lib/completion_engine.ex

@@ -722,18 +722,37 @@ defmodule ElixirLS.Utils.CompletionEngine do
   end
 
   defp struct_module_filter(true, %State.Env{} = _env, %Metadata{} = metadata) do
+    # Build the loaded/application module-name list ONCE for the whole filter pass
+    # rather than recomputing `:code.all_loaded()` (and the application module scan)
+    # for every candidate module - that turned the filter into O(N^2) work.
+    all_module_names = all_module_names()
+
     fn module ->
       Struct.is_struct(module, metadata.structs) or
-        has_struct_submodule?(module, metadata.structs)
+        has_struct_submodule?(module, metadata.structs, all_module_names)
     end
   end
 
   defp struct_module_filter(false, %State.Env{} = _env, %Metadata{} = _metadata) do
     fn _ -> true end
   end
 
+  # The loaded (and, in interactive mode, application) module names as strings.
+  # Computed once per struct-module filter pass, not per candidate module.
+  defp all_module_names do
+    modules = Enum.map(:code.all_loaded(), &Atom.to_string(elem(&1, 0)))
+
+    case :code.get_mode() do
+      :interactive ->
+        modules ++ Enum.map(Applications.get_modules_from_applications(), &Atom.to_string/1)
+
+      _ ->
+        modules
+    end
+  end
+
   # Check if a module has any direct submodules that are structs
-  defp has_struct_submodule?(module, structs) do
+  defp has_struct_submodule?(module, structs, all_module_names) do
     module_str = Atom.to_string(module)
 
     # Check metadata structs (from current buffer)
@@ -747,30 +766,14 @@ defmodule ElixirLS.Utils.CompletionEngine do
     if metadata_result do
       true
     else
-      # Get all modules and check if any direct submodule is a struct
+      # Check if any direct submodule (from the precomputed list) is a struct
       module_str_with_dot = module_str <> "."
 
-      # Get all loaded modules
-      modules = Enum.map(:code.all_loaded(), &Atom.to_string(elem(&1, 0)))
-
-      # Add modules from applications if in interactive mode
-      modules =
-        case :code.get_mode() do
-          :interactive ->
-            modules ++
-              Enum.map(Applications.get_modules_from_applications(), &Atom.to_string/1)
-
-          _ ->
-            modules
-        end
-
-      # Find submodules
       submodules =
-        for mod <- modules,
+        for mod <- all_module_names,
             String.starts_with?(mod, module_str_with_dot),
             do: String.to_atom(mod)
 
-      # Check if any submodule is a struct
       Enum.any?(submodules, fn mod ->
         Code.ensure_loaded?(mod) and function_exported?(mod, :__struct__, 1)
       end)

apps/language_server/lib/language_server/providers/document_symbols.ex

@@ -24,20 +24,17 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
     :deprecated
   ]
 
-  def symbols(uri, %Parser.Context{source_file: source_file}, hierarchical) do
-    # Parse with the error-tolerant toxic2 parser so every node carries a `range:` (end-exclusive,
-    # 1-based) - that is the source of every symbol's range and selection range, replacing the old
-    # token-metadata end-position heuristics. No `literal_encoder` here: wrapping literals would
-    # change the atom/keyword shapes that the `extract_*` clauses pattern-match on (defstruct fields
-    # etc.). Structural nodes (def/defmodule/@/...) carry ranges regardless.
-    {ast, diagnostics} = Toxic2.parse_to_ast(source_file.text, token_metadata: true, range: true)
-
-    # `{:__error__, ...}` placeholder nodes (best-effort recovery on invalid code) carry map args
-    # that crash `Macro` traversal; neutralize them via the shared helper (keep_range: true so the
-    # `range:` meta survives - it is the source of every symbol's range).
+  def symbols(uri, %Parser.Context{source_file: source_file, ast: ast}, hierarchical) do
+    # Reuse the AST `Parser.parse_immediate/2` already produced on the context rather than
+    # reparsing the raw source here. For `.ex/.exs` that is the error-tolerant, already-neutralized
+    # toxic2 tree carrying `range:` meta (end-exclusive, 1-based) - the source of every symbol's
+    # range and selection range. For `.eex/.html-eex` it is the EEx-derived Code AST (reparsing the
+    # raw template text with toxic2 would be nonsensical); such nodes lack `range:` meta, so
+    # `location_to_range/2` degrades to a zero-width range at the node's `line`/`column` anchor.
+    # No `literal_encoder` is applied in either path, so the atom/keyword shapes the `extract_*`
+    # clauses pattern-match on (defstruct fields etc.) are preserved.
     symbols =
       ast
-      |> ElixirSense.Core.Parser.neutralize_errors(diagnostics, true)
       |> extract_modules()
       |> Enum.reject(&is_nil/1)
 

apps/language_server/lib/language_server/providers/selection_ranges.ex

@@ -44,7 +44,12 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
         end
       )
 
-    parse_result = {:ok, ast}
+    # Neutralize `{:__error__, ...}` placeholder nodes once here rather than once per cursor
+    # position - `delimiter_pair_ranges/4` and `ast_node_ranges/4` both walk this tree for every
+    # requested position, so neutralizing inside them repeated an O(AST) traversal per cursor.
+    # `surround_context_ranges/4` keeps the raw `ast` (it does its own handling).
+    neutralized_parse_result =
+      {:ok, ElixirSense.Core.Parser.neutralize_errors(ast, [], true)}
 
     comment_groups = group_comments(comments)
 
@@ -61,12 +66,12 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
       cell_pair_ranges = cell_pair_ranges(lines, cell_pairs, line, character)
 
       delimiter_pair_ranges =
-        delimiter_pair_ranges(parse_result, lines, line, character)
+        delimiter_pair_ranges(neutralized_parse_result, lines, line, character)
         |> deduplicate
 
       comment_block_ranges = comment_block_ranges(lines, comment_groups, line, character)
 
-      ast_node_ranges = ast_node_ranges(parse_result, line, character, options)
+      ast_node_ranges = ast_node_ranges(neutralized_parse_result, line, character, options)
 
       surround_context_ranges = surround_context_ranges(ast, text, line, character)
 
@@ -150,9 +155,8 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
   # tokenizer-driven token-pair pass (FoldingRange.Token/TokenPair). String/heredoc/sigil ranges,
   # which the old special-token pass produced, already come from `ast_node_ranges` (the toxic AST
   # nodes carry `range:`), so they are not reproduced here.
+  # `ast` is already neutralized by the caller (`selection_ranges/3`).
   def delimiter_pair_ranges({:ok, ast}, lines, line, character) do
-    ast = ElixirSense.Core.Parser.neutralize_errors(ast, [], true)
-
     {_ast, {acc, _stack}} =
       Macro.traverse(
         ast,
@@ -414,13 +418,10 @@ defmodule ElixirLS.LanguageServer.Providers.SelectionRanges do
 
   @empty_node {:__block__, [], []}
 
+  # `ast` is already neutralized by the caller (`selection_ranges/3`) - toxic2's best-effort
+  # `{:__error__, meta, %{...}}` nodes (whose map args would crash `Macro.traverse`) have been
+  # replaced there, once per parse rather than once per cursor position.
   def ast_node_ranges({:ok, ast}, line, character, _options) do
-    # toxic2 returns a best-effort AST for invalid code with `{:__error__, meta, %{...}}` nodes whose
-    # map args would crash `Macro.traverse`; neutralize them here so this function is safe for any
-    # toxic2 AST regardless of caller. No diagnostics in scope (this is range-only and `__error__`
-    # nodes carry no `range:`, so the diagnostics-driven call-arg cleaning is a no-op for ranges).
-    ast = ElixirSense.Core.Parser.neutralize_errors(ast, [], true)
-
     {_new_ast, {acc, [@empty_node]}} =
       Macro.traverse(
         ast,