Integrate the toxic2 tolerant parser across providers#1267
Merged
Conversation
… toxic2 ranges - Point elixir_sense to a local path dep (carries the toxic2-backed parser) - Reimplement SelectionRanges AST node ranges using toxic2's range: metadata and comments from Toxic2.string_to_quoted_with_comments, dropping the bespoke AstUtils.node_range computation - Delete now-unused AstUtils module and its tests - Adjust document_symbols test for toxic2-recovered AST Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Derive structural and comment folds from the error-tolerant toxic2 parser (node source ranges + Toxic2.string_to_quoted_with_comments) instead of the Elixir-tokenizer-backed token-pair / special-token passes. The line-based indentation pass is kept (it supplies assignment/clause folds that have no single closing token) and AST folds override it at shared start lines, as the token-pair pass used to. Comments inside strings/heredocs are no longer mistaken for fold-able comment blocks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parse with toxic2 (range: true, no literal_encoder) and read each node's range: meta for symbol range and selection range, replacing the token-metadata end-position heuristics (kept only as a fallback for range-less nodes). Also: preserve nil args (bare identifiers like var / __MODULE__) in neutralize_errors across document_symbols, selection_ranges and folding_range - the previous `not is_list(args)` clause turned them into zero-arity calls; and ignore error-recovery placeholders when computing function/type arity so an incomplete `def foo(` reports foo/0 rather than an inflated arity. Adjust the records test for the toxic2 call-node range (starts at `Record`). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every node extract_* takes a location from carries a toxic2 range:, so the old
token-metadata heuristic was unreachable - and it harbored a latent bug
(elixir_position_to_lsp({nil, nil}) returns end-of-file because nil sorts above
integers). Replace location_to_range with a range:-only version that degrades a
range-less node to a zero-width range at its line/column, and drop the now-dead
symbol argument.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The LS Parser now produces a range-bearing toxic2 AST for Context.ast (and
the metadata built from it) on every .ex/.exs parse, replacing the
Code.string_to_quoted! AST and the ElixirSense fault-tolerant fallback.
This is the foundation that lets range-aware providers read node ranges
straight off Context.ast.
- parse_file/3 stays the sole diagnostics source (Code.with_diagnostics)
and the EEx/HEEx parser; it now returns a tagged {:ok, ast, diagnostics}
/ {:error, diagnostics} so a falsey-but-valid AST (literal nil/false) is
no longer mistaken for a parse failure.
- do_parse/2 decides the flag from the Code success tag: clean -> :exact,
else toxic recovered something usable -> :fixed, else :not_parsable.
- parse_elixir_toxic/3 builds the AST/metadata via
ElixirSense.Core.Parser.parse_to_neutralized_ast(range: true,
keep_range: true), keeping the catch/telemetry safety net.
- fault_tolerant_parse/2 removed (toxic always recovers; cursor env is
derived separately in Metadata.get_cursor_env).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r helper DocumentSymbols, FoldingRange and SelectionRanges each carried a byte-identical private neutralize_errors/1. Replace all three with the shared ElixirSense.Core.Parser.neutralize_errors/3 (keep_range: true so the range: meta survives). document_symbols/folding_range pass their parse diagnostics (also gets the call-arg sentinel cleaning); selection_ranges' self-neutralizing ast_node_ranges/4 passes [] (range-only, and __error__ nodes carry no range, so the diagnostics-driven cleaning is a no-op there). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
definition/references/implementation/declaration/call_hierarchy/hover locators and the llm_environment command now classify the symbol under the cursor via ElixirSense.Core.SurroundContext.Toxic.surround_context/2 (stage 0 delegates to Code.Fragment). Completion (cursor_context/container_cursor_to_quoted) untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the Elixir-tokenizer passes (token_pair_ranges via FoldingRange.Token/
TokenPair, special_token_group_ranges via FoldingRange.SpecialToken, and the
stop-token machinery) with a toxic2-AST pass, delimiter_pair_ranges/4. It derives
outer/inner ranges for ()/[]/{}/%{}/<<>>, calls, bracket access x[y], and
do/else/rescue/after/catch/end blocks from the toxic2 closing:/do:/end:/section-key
range: metadata, plus a stab pattern .. -> range. String/heredoc/sigil ranges now
come from ast_node_ranges (the toxic nodes carry range:), so the special-token pass
is gone. Both selection and folding providers no longer use :elixir_tokenizer for
their output (FoldingRange was already migrated).
Adversarial review found a crash: a cursor exactly on a block section keyword
(else/rescue/...) made two sibling, non-nested ranges and tripped the
"increasingly narrowing" merge invariant. Fixed by selecting the cursor's section
with half-open containment (end exclusive). A fuzz over real files dropped
selection-range crashes from 238 (old tokenizer code) to 6 pre-existing
"no intersection" cases in the shared merge. Bracket-access ranges (lost in the
first cut) restored via the from_brackets meta. Regression tests added.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ules These FoldingRange submodules were lib-dead after both providers moved off the Elixir tokenizer (FoldingRange.provide and SelectionRanges no longer use them). Remove the modules and their tests. convert_text_to_input and @type input drop the :tokens field (now lines-only); Indentation and CommentBlock provide_ranges only ever read :lines, so their doctests and the folding_range_test passes keep working. The only remaining ElixirSense.Core.Normalized.Tokenizer user is now elixir_sense's Source.which_func. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
elixir-ls already extracts join bindings (backported in 906c8c8) but had no unit test for it - only commented-out integration TODOs. Port elixir_sense's focused, self-contained QueryTest (mock Post/Comment schemas) to lock in the behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cent bracket crash) container_ranges decided cursor containment with the inclusive in?/2 on end-exclusive ranges, so adjacent bracket accesses (foo[bar][baz]) both claimed the shared boundary column and emitted two non-nested sibling ranges, raising "ranges_1 is not increasingly narrowing" in the merge. Use the half-open check (end exclusive) like do_block_ranges already does. Found by gpt-5.5 adversarial review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Toxic selection_ranges: the symbol-under-cursor pass called Code.Fragment.surround_context directly. Route it through ElixirSense.Core.SurroundContext.Toxic instead - the same toxic2-backed entry point the navigation providers already use. Navigable shapes now get their span from the AST range: metadata; only purely lexical units (a bare do/end, exotic operators) reach Toxic's internal Code.Fragment fallback. AST ranges alone don't cover these symbol-level spans (e.g. the do/end keyword units, the dot-path callee), so the pass is kept rather than removed. document_symbols: drop the stale 'extract module name location from Code.Fragment.surround_context?' TODO - module name locations come from the toxic2 AST range metadata now. This removes the last direct Code.Fragment.surround_context call in the providers; the only remaining uses are Toxic's own internal fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pass the AST already parsed at the top of selection_ranges/3 into SurroundContext.Toxic.surround_context/3 instead of having it re-parse the source on every cursor position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move elixir_sense from a local path dep back to the git dep model and pin it to the pushed toxic2-parser SHA (b928399b) via dep_versions.exs + mix.lock. This transitively pulls toxic2 (lukaszsamson/toxic2). elixir_sense/toxic2 require Elixir ~> 1.19, so drop the 1.16/1.17/1.18 jobs from the CI matrix (smoke tests + test matrix, Linux + Windows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ixes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8454b33de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| otp: 27.x | ||
| - elixir: 1.18.x | ||
| otp: 27.x | ||
| # toxic2/elixir_sense require Elixir ~> 1.19; 1.16/1.17/1.18 dropped. |
There was a problem hiding this comment.
Here the workflow drops 1.16–1.18 because the new toxic2/elixir_sense stack requires Elixir ~> 1.19, but the public/version gates are unchanged: mix.exs:12 and apps/language_server/mix.exs:18 still advertise >= 1.16.0, ElixirLS.Utils.MinimumVersion.check_elixir_version/0 still accepts 1.16+, and README.md:114-116 still lists 1.16–1.18 as supported. Users on those versions will be told the release is compatible and only hit dependency/runtime failures after install, so update the mix requirements/support check/docs together with dropping the CI jobs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR integrates the toxic2 error-tolerant parser (via the pinned elixir_sense SHA) across ElixirLS language-server providers, migrating selection ranges, folding ranges, document symbols, and navigation-related “symbol under cursor” classification to use toxic2’s ranged AST and recovered parsing.
Changes:
- Reimplements selection ranges, folding ranges, and document symbols to derive ranges from toxic2 AST
range:/closing:/do:/end:metadata (removing the old tokenizer/token-pair infrastructure). - Routes multiple navigation providers (definition/references/implementation/declaration/hover/call hierarchy) through
ElixirSense.Core.SurroundContext.Toxic. - Pins
elixir_senseto a toxic2-compatible commit, adds toxic2 to the lockfile, and updates CI to drop Elixir < 1.19 jobs.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mix.lock | Pins elixir_sense commit and adds git dependency for toxic2. |
| dep_versions.exs | Updates the pinned elixir_sense SHA. |
| apps/language_server/test/providers/selection_ranges_test.exs | Adds regression coverage for bracket-access boundary crashes and block-section boundaries. |
| apps/language_server/test/providers/plugins/ecto/query_test.exs | Adds coverage for join-association binding extraction. |
| apps/language_server/test/providers/folding_range/token_pairs_test.exs | Removes doctest tied to deleted token-pair folding implementation. |
| apps/language_server/test/providers/folding_range/special_token_test.exs | Removes doctest tied to deleted special-token folding implementation. |
| apps/language_server/test/providers/folding_range_test.exs | Removes token-pair/special-token folding tests now replaced by toxic2-based folding. |
| apps/language_server/test/providers/document_symbols_test.exs | Updates expectations for ranges and recovered symbols from error-tolerant parsing. |
| apps/language_server/test/ast_utils_test.exs | Removes tests for deleted AstUtils implementation. |
| apps/language_server/lib/language_server/providers/selection_ranges.ex | Replaces tokenizer-driven selection ranges with toxic2 AST/comment/range-driven logic and toxic surround-context classifier. |
| apps/language_server/lib/language_server/providers/references/locator.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/implementation/locator.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/hover/docs.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/folding_range/token.ex | Deletes tokenizer normalization module (no longer needed). |
| apps/language_server/lib/language_server/providers/folding_range/token_pairs.ex | Deletes token-pair folding implementation (replaced by toxic2 AST ranges). |
| apps/language_server/lib/language_server/providers/folding_range/special_token.ex | Deletes special-token folding implementation (replaced by toxic2 AST ranges). |
| apps/language_server/lib/language_server/providers/folding_range.ex | Reimplements folding ranges using toxic2 AST ranges + toxic2 comment stream + indentation pass. |
| apps/language_server/lib/language_server/providers/execute_command/llm_environment.ex | Switches cursor context extraction to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/document_symbols.ex | Reimplements ranges/selection ranges using toxic2 range: metadata and adds arity handling for recovered AST. |
| apps/language_server/lib/language_server/providers/definition/locator.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/declaration/locator.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/providers/call_hierarchy/locator.ex | Switches symbol-under-cursor detection to SurroundContext.Toxic. |
| apps/language_server/lib/language_server/parser.ex | Uses toxic2 ranged AST for .ex/.exs Context.ast while keeping Code diagnostics as the diagnostics source; updates parse result tagging. |
| apps/language_server/lib/language_server/ast_utils.ex | Deletes AstUtils implementation (now replaced by toxic2 range metadata usage). |
| .github/workflows/ci.yml | Drops Elixir 1.16/1.17/1.18 from CI matrices due to toxic2/elixir_sense requirements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
27
to
45
| 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). | ||
| symbols = | ||
| ast | ||
| |> ElixirSense.Core.Parser.neutralize_errors(diagnostics, true) | ||
| |> extract_modules() | ||
| |> Enum.reject(&is_nil/1) | ||
|
|
||
| {:ok, build_symbols(symbols, uri, source_file.text, hierarchical)} | ||
| end |
Comment on lines
+153
to
+155
| def delimiter_pair_ranges({:ok, ast}, lines, line, character) do | ||
| ast = ElixirSense.Core.Parser.neutralize_errors(ast, [], true) | ||
|
|
- 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
Reverses the version-floor drop that came from toxic2's `~> 1.19` pin. toxic2 and elixir_sense now build on 1.16-1.20 again, so the language server does too. - Restore the 1.16/1.17/1.18 jobs in the smoke-test and compatibility-test CI matrices. - Bump elixir_sense to 99570b9b (its 1.16-compatible build, which pins the 1.16-compatible toxic2) in dep_versions.exs + mix.lock. The public version gates (mix.exs `>= 1.16.0`, MinimumVersion, README support matrix) are unchanged - they already advertised 1.16+, which is now accurate again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232
…end tests The `describe "token pairs"` and `describe "special tokens"` blocks were removed in 41fdf45 along with the tokenizer-based FoldingRange.Token/TokenPair/ SpecialToken modules they called directly (`TokenPair.provide_ranges/1` etc.) - folding moved onto the toxic2 AST region pass. Those per-shape assertions were lost, leaving only the coarse "end to end" cases. Re-add coverage for the same structural shapes, but driven through the public `FoldingRange.provide/1` against the AST-based implementation: nested do/end (defmodule/def/if-else), binary `<<>>`, list `[]` / tuple `{}`, map `%{}`, string and charlist heredocs, sigil heredocs, and try/rescue/after blocks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232
…ixir_sense Mirror the elixir_sense fix in the ElixirLS.Utils.CompletionEngine copy: the `:bitstring_modifier` branch now parses the modifiers already typed after `::` into a state and keeps only the modifiers that may still legally follow (type / sign / endianness / utf / size / unit rules), instead of only dropping names already present. Fixes invalid suggestions like `signed`/`little` after `binary-`, `size`/`unit` after `utf8-`, `signed`/`binary` after `float-`. Assert the invalid options are absent (not just valid ones present) in both elixir_ls_utils (complete_test) and language_server (suggestions_test) suites. Bump elixir_sense to 89eda3fc (same fix in the shared engine). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrates the toxic2 error-tolerant parser (via the
matching elixir_sense branch) across the ElixirLS providers, replacing the tokenizer-driven and
Code.Fragment-driven implementations with passes derived from toxic2's ranged AST.Companion PR: elixir-lsp/elixir_sense#336 (this branch pins that elixir_sense SHA).
What changed
Providers reimplemented on toxic2 ranges
toxic2 parse; the old
FoldingRange.Token/TokenPair/SpecialTokentokenizer is deleted.Half-open containment fixes an adjacent-bracket (
foo[bar][baz]) crash.heuristics removed).
routed through
ElixirSense.Core.SurroundContext.Toxicfor symbol-under-cursor classification.selection_rangesreuses the AST it already parsed instead of re-parsing per cursor position.Context.ast/metadata for.ex/.exsbuilt from the toxic2 ranged AST;neutralize_errorsdeduped onto the shared
ElixirSense.Core.Parserhelper.No more direct
Code.Fragment.surround_contextin the providers — the only remaining use isthe internal fallback inside the toxic2 classifier.
Build / CI
toxic2-parser SHA (
dep_versions.exs+mix.lock); this transitively pulls toxic2.~> 1.19, so drop the 1.16/1.17/1.18 jobs from the CImatrix (smoke tests + test matrix, Linux + Windows).
Testing
apps/language_serverprovider suite green (1170 passing) against the git-pinned deps.mix compile --warnings-as-errors,mix format --check-formatted, andmix dialyzerall clean.🤖 Generated with Claude Code