Replace parser/surround-context with the toxic2 tolerant parser#336
Merged
Conversation
- ElixirSense.Core.Parser parses everything via Toxic2 (no Code.string_to_quoted fast path); error recovery and cursor location now come from toxic2 - Range-based cursor marking (ElixirSense.Core.Parser.Cursor) using toxic2 source ranges, replacing the __cursor__() text-injection heuristics - get_cursor_env derives the cursor env from a separate marked parse - Drop the bespoke fix_parse_error loop and container_cursor fallbacks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exposes ElixirSense.Core.Parser.parse_to_neutralized_ast/2 - the toxic2 best-effort AST plus diagnostic stream, with __error__ placeholder nodes neutralized. keep_range: true preserves toxic2 range: metadata for range-aware consumers (the LS parser and document-symbol / folding / selection-range providers); the default strips it for the classic line:/column: shape used by metadata building. neutralize_errors/3 now threads keep_range through every clause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduces ElixirSense.Core.SurroundContext.Toxic.surround_context/2, the seam for the toxic2 range-based symbol-under-cursor classifier. It returns the same shape as Code.Fragment.surround_context/2 so to_binding/get_cursor_env/locators are unchanged. Stage 0 delegates entirely to Code.Fragment; classification is added incrementally per symbol-kind with a fallback. Navigation locators (definition/references/ implementation/hover) now call the seam. Completion stays on Code.Fragment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements the toxic2 range-based symbol-under-cursor classifier (item 2), replacing the Code.Fragment.surround_context delegation from stage 0. It walks to the deepest ranged node containing the cursor and classifies it into the same context shapes Code.Fragment emits, so SurroundContext.to_binding and the locators are unchanged. Handled via toxic: aliases, remote dot calls + all inside_dot forms, bare vars and __MODULE__, module attributes (incl. @type/@spec/@doc with args), alias/@attr structs, sigils, capture args (&1), symbolic operators, local arity (foo/1), parenthesized local calls. Falls back to Code.Fragment (so the function is total and never worse than before) for: bare :atom literals and keyword keys (no range meta), interpolated-atom / bracket-access / string-interpolation synthetic dots (from_brackets/delimiter/from_interpolation), word operators, exotic alias heads (__MODULE__.Foo / @attr.Foo), var-type structs (statement- position dependent), multialias Foo.{...}, invalid @@ names, and the & capture prefix. Containment is end-exclusive to match Code.Fragment's :none boundary. Validated by a differential test over ~90k cursor positions in real source files (matches Code.Fragment except ~8 benign positions on rare/invalid syntax) plus a unit test of the navigable shapes and the regressions found in adversarial review (__MODULE__.Foo, @attr.Foo, @@, &+/2 all now match Code.Fragment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…alized.Tokenizer
Ports the AST-based bitstring-modifier detection from elixir-ls completion-engine
rework (98e983d) into Source.bitstring_options/1: parse the prefix with
Code.Fragment.container_cursor_to_quoted and match the cursor's Macro.path against
{:"::", _, [_,_]} inside {:<<>>, _, [_|_]} (stripping - operator wrappers), instead
of scanning Elixir tokens. Same return contract (the modifiers string after the
last ::, or nil) so the bitstring completion reducer is unchanged.
Removes the last ElixirSense.Core.Normalized.Tokenizer user; deletes the module,
its test, and its now-stale .dialyzer_ignore entries. Edge cases verified against
the old behavior (adversarial review): <<x::size( -> nil and a newline before the
modifier are both functionally equivalent/better.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings the completion engine to parity with elixir-ls's port-complete-merges rework (wholesale onto Elixir 1.20), namespace-translated (ElixirLS.Utils / ElixirLS.LanguageServer.* -> ElixirSense.*): - completion_engine.ex: rewritten - cursor parsing delegated to Code.Fragment, @bitstring_modifiers embedded, struct/map-field + bitstring-modifier detection done via container_context (AST), upstream correctness fixes through 1.20. - reducers: bitstring.ex and struct.ex deleted (folded into CompleteEngine add_bitstring_options/add_struct_fields + add_keywords); complete_engine.ex and record.ex updated; suggestion.ex @reducers list updated. - plugins/ecto/query.ex: only the clauses_suggestions hunk applied (the rest of the file had diverged - extract_bindings is an elixir_sense improvement - so a surgical apply rather than a file overwrite). - tests: complete_test.exs and suggestions_test.exs ported as test/elixir_sense/providers/completion/{completion_engine,suggestions}_test.exs (292 tests). A few env-specific assertions adapted to elixir_sense's test env (app name :elixir_sense, no stream_data dep, struct-completion namespaces). - test/support/module_with_typespecs.ex synced (the ported tests need the Local/Behaviour/Impl/MacroBehaviour fixtures elixir-ls had added). - .dialyzer_ignore.exs: ignore the upstream-faithful unreachable :error clauses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… review) - Multi-line remote call: derive the dot end from the callee meta (line+column), not the dot token, so `A.\n bar` with the cursor on the dot gets the correct cross-line end instead of an impossible same-line one (which broke get_call_arity -> wrong overload). - Spaced `@ attr`: require the attribute name to be contiguous with `@` (one column after); otherwise it is the unary `@` operator on a local var, which Code.Fragment reports as :local_or_var (was wrongly classified as a module attribute). - Remote arity slash `A.bar/1`: defer any `<ref>/<int>` slash to Code.Fragment (arity vs division is lexical), so the slash no longer resolves to Kernel.//2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_encoder
Parse with a literal_encoder that wraps literals in {:__block__, meta, [literal]} so
bare :atom literals carry a range and can be classified from the (authoritative on
valid code) toxic2 parse instead of the lexical Code.Fragment fallback. Shrinks the
fallback surface: bare :atom, x = :ok, and the :erlang.foo dot-left operand (which
previously had no range and fell back) now classify natively.
- New clause for {:__block__, meta, [atom]}: only identifier atoms with a colon-led
range (width == len+1) -> :unquoted_atom; keyword keys (format: :keyword),
nil/true/false, and operator/special atoms (:%{}, :+, :.) still defer to
Code.Fragment (the :key/:keyword and :none/:operator distinctions are lexical).
- Unwrap the now-wrapped atom in inside_dot and the wrapped integer in the arity
checks (foo/1, A.bar/1). Differential vs Code.Fragment over real files is unchanged
(same handful of exotic residuals); nav + completion suites green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…literal_encoder
Push more literal classification onto the parse tree. The literal_encoder
already gives bare :atoms a range; extend the wrapped-literal classify clause
to also handle:
- keyword keys (format: :keyword) -> {:key, charlist}, with :none when the
cursor is on the trailing colon (key span excludes the colon)
- bare nil/true/false (no leading colon) -> {:keyword, charlist}
This shrinks the Code.Fragment lexical fallback surface further; the
differential against Code.Fragment stays at the same 7 exotic residuals.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ode.Fragment A step range `a..b//c` lowers to a single ternary `:..//` node, but Code.Fragment classifies the `..` and `//` lexically as two separate operators. The cursor on the `..` was reporting the whole `..//` atom name; report `..` over the two `..` columns instead and let the `//` columns fall back to Code.Fragment (the `..//` node range does not reach them). This was the most common differential residual (every step range in real code). After this, a 6-file real-source scan drops to 2 residuals, both cases where toxic is *more* precise than the lexical oracle (`&(-&1)` -> `:capture_arg`/`:operator` where Code.Fragment returns `:none`); those are documented as intentional divergences and kept because they drive better navigation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
/simplify cleanup:
- Add surround_context/3 taking an already-parsed AST, so callers that parse
the source themselves (selection_ranges) reuse that AST instead of triggering
a fresh toxic2 parse per cursor position. Both arities share a private
classify_at/3; the parse opts move to a private parse/1.
- classify({:__block__, ...}): compute Atom.to_string once (reused for length
and charlist) and reuse the keyword-key span binding instead of rebuilding
the {{sl,sc},{el,ec-1}} tuple.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Switch the toxic2 parser dependency from a local path dep to the published git repo (lukaszsamson/toxic2) so a clean checkout / CI can resolve it. toxic2 requires Elixir ~> 1.19, which transitively raises this branch's minimum: bump mix.exs to ~> 1.19 and drop the 1.16/1.17/1.18 jobs from the CI matrix (Linux + Windows) accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 98e983d port carried warnings that the static-analysis gate (compile
--warnings-as-errors + credo --strict) on Elixir 1.19/1.20 rejects:
- prefix unused clause args with _ (metadata/env/module/cursor_position/name/head)
- drop the duplicate :type key in the erlang module result map
- value_from_alias/2 always returns {:alias, _}, so remove the two dead
:error branches the 1.20 type checker proves unreachable (matches elixir-ls)
- wrap the single-line MIT license header (was one 434-char line)
- mark the deliberate single-condition cond with a credo disable
No behavior change; full suite (1668) + dialyzer still green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR switches ElixirSense’s parsing/error-recovery and “symbol under cursor” classification to use the toxic2 error-tolerant parser (range-based), and ports the elixir-ls completion engine to remove the Normalized.Tokenizer shim. It also updates project/CI requirements to align with toxic2’s Elixir compatibility.
Changes:
- Replace custom parsing +
Code.Fragment-based navigation context with toxic2 parsing and a newElixirSense.Core.SurroundContext.Toxicrange-based classifier. - Port/reshape the completion subsystem (incl. structs fields, bitstring modifiers, and keywords) and remove the old struct/bitstring reducers and tokenizer.
- Add/adjust extensive tests/fixtures and bump Elixir requirement/CI matrix; pin toxic2 as a git dependency.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/support/module_with_typespecs.ex | Extends fixture coverage for specs/defaults/behaviours/macros. |
| test/elixir_sense/providers/completion/completion_engine_test.exs | Adds a large completion regression/port validation test suite. |
| test/elixir_sense/core/surround_context/toxic_test.exs | Adds tests ensuring Toxic surround_context matches Code.Fragment for navigable shapes. |
| test/elixir_sense/core/normalized/tokenizer_test.exs | Removes tokenizer tests (tokenizer dropped). |
| test/elixir_sense/core/metadata_builder/error_recovery_test.exs | Aligns error recovery tests with toxic2 tolerant parsing + cursor marking. |
| mix.lock | Pins toxic2 git dependency by SHA. |
| mix.exs | Adds toxic2 dep and bumps required Elixir to ~> 1.19. |
| lib/elixir_sense/providers/references/locator.ex | Routes navigation context through SurroundContext.Toxic. |
| lib/elixir_sense/providers/plugins/ecto/query.ex | Adapts to new get_module_funs/4 shape and hint-filtering. |
| lib/elixir_sense/providers/implementation/locator.ex | Routes navigation context through SurroundContext.Toxic. |
| lib/elixir_sense/providers/hover/docs.ex | Routes navigation context through SurroundContext.Toxic. |
| lib/elixir_sense/providers/definition/locator.ex | Routes navigation context through SurroundContext.Toxic. |
| lib/elixir_sense/providers/completion/suggestion.ex | Updates reducer pipeline and suggestion union types for new engine outputs. |
| lib/elixir_sense/providers/completion/reducers/struct.ex | Removes legacy struct-field reducer (handled by completion engine now). |
| lib/elixir_sense/providers/completion/reducers/record.ex | Adjusts reducers ordering/coverage and parses nominal types. |
| lib/elixir_sense/providers/completion/reducers/complete_engine.ex | Adds reducers for struct fields / bitstring options / keywords sourced from engine. |
| lib/elixir_sense/providers/completion/reducers/bitstring.ex | Removes legacy bitstring reducer (handled by completion engine now). |
| lib/elixir_sense/providers/completion/completion_engine.ex | Ports elixir-ls completion engine logic and expands container-context handling. |
| lib/elixir_sense/core/surround_context/toxic.ex | Adds toxic2 range-based “symbol under cursor” classifier with Code.Fragment fallback. |
| lib/elixir_sense/core/source.ex | Reworks bitstring modifier detection away from tokenizer. |
| lib/elixir_sense/core/parser/cursor.ex | Adds toxic2-range-based cursor marker placement in AST. |
| lib/elixir_sense/core/parser.ex | Replaces parser/error recovery pipeline with toxic2 tolerant parsing + neutralization. |
| lib/elixir_sense/core/normalized/tokenizer.ex | Removes private :elixir_tokenizer wrapper module. |
| lib/elixir_sense/core/metadata.ex | Switches cursor env derivation to parser cursor marking instead of string injection. |
| lib/elixir_sense/core/compiler.ex | Handles cursor marker placement in defmodule tolerant-parse scenarios. |
| lib/elixir_sense.ex | Updates doctest output to reflect new parsed AST shape. |
| .github/workflows/ci.yml | Drops older Elixir jobs; tests only 1.19/1.20 due to toxic2 requirement. |
| .dialyzer_ignore.exs | Updates ignores for removed tokenizer/parser paths and new completion engine patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+259
to
+273
| defp range_contains?(meta, cursor) do | ||
| case Keyword.get(meta, :range) do | ||
| {start_position, end_position} -> | ||
| if before_or_at?(end_position, start_position) do | ||
| # toxic2 gives incomplete nodes an inverted/zero-width range (the end | ||
| # token is missing) - treat them as extending to the cursor | ||
| before_or_at?(start_position, cursor) | ||
| else | ||
| before_or_at?(start_position, cursor) and before_or_at?(cursor, end_position) | ||
| end | ||
|
|
||
| _ -> | ||
| false | ||
| end | ||
| end |
Comment on lines
+759
to
+767
| # 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) |
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: a2b0e592b0
ℹ️ 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".
| nil, | ||
| parser_options | ||
| ) | ||
| case tolerant_parse(source) do |
There was a problem hiding this comment.
When callers use the public ElixirSense.string_to_quoted/5 API with parser_options, those options are still accepted and forwarded as :parser_options, but this path now calls tolerant_parse(source) with no options. That silently ignores options such as :existing_atoms_only or :literal_encoder, changing the returned AST and potentially creating atoms even when the caller requested existing atoms only. Please thread the parser options into the toxic2 parse or remove the public parameter.
Useful? React with 👍 / 👎.
Comment on lines
+810
to
+811
| @bitstring_modifiers | ||
| |> Enum.filter(&(Matcher.match?(&1.name, hint) and &1.name not in existing)) |
There was a problem hiding this comment.
When completing after a modifier that restricts what can follow, such as <<x::binary- or <<x::utf8-, this filter only excludes names already typed, so it will suggest invalid modifiers such as signed, little, or size that Elixir rejects. The previous reducer used ElixirSense.Core.Bitstring.available_options/1 to apply the type/sign/endianness/UTF rules; use the parsed modifier state here instead of the flat list before formatting.
Useful? React with 👍 / 👎.
Comment on lines
+97
to
+99
| def add_struct_fields(_hint, _env, _file_metadata, _context, acc) do | ||
| add_suggestions(:struct_field, acc) | ||
| end |
There was a problem hiding this comment.
When a reducer chain explicitly enables :structs_fields without :variable_fields (for example, Record.add_fields/5 now narrows follow-up reducers to :structs_fields in the maybe-record-update path), populate/6 groups completions by their type, and struct fields emitted by the engine have type: :field with subtype: :struct_field. As a result, add_suggestions(:struct_field, acc) always returns an empty list and drops those completions; filter the :field group by subtype here instead.
Useful? React with 👍 / 👎.
…code - parser: thread caller `parser_options` (`:existing_atoms_only`, `:literal_encoder`) from the public `string_to_quoted/5` API through `string_to_ast` into the toxic2 parse instead of silently dropping them. - complete_engine: `add_struct_fields` / `add_fields` now filter the `:field` suggestion group by `:subtype` (struct fields are emitted as `type: :field, subtype: :struct_field`, so looking up a `:struct_field` type found nothing and dropped them when a chain enabled `:structs_fields` without `:variable_fields`). Matches the elixir-ls copy, which already had this. - 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)). - cursor: document why range containment is intentionally end-inclusive (a completion/nav cursor sits at the exclusive end of the token being typed). - Remove the now-dead `ElixirSense.Core.Bitstring` module + test: the bitstring modifier completion uses the inlined `@bitstring_modifiers` list (elixir-ls #1264), so nothing referenced it. 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 now compiles on 1.16-1.20 (lukaszsamson/toxic2 elixir-1.16-support branch, which fixes a compile-time regex `#Reference` escape that broke 1.16/1.17), so elixir_sense can support the same range again. - Lower `elixir:` back to `~> 1.16`. - Bump the toxic2 ref to c47c911 (the 1.16-compatible build) and mix.lock. - Restore the 1.16/1.17/1.18 jobs in the Linux + Windows test matrices. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232
Restoring the 1.16-1.18 CI matrix surfaced tests that had never run against the
toxic2 parser on < 1.18 (CI had been 1.19+):
- surround_context/2: `Code.Fragment` only classifies `&1` capture args and
keyword keys as `:capture_arg` / `{:key, _}` from Elixir 1.18 on. Toxic parses
them structurally on every version, so assert Toxic's own (correct) result
always and only compare against the version-dependent oracle on >= 1.18.
- error_recovery: the incomplete fn/catch/guard-clause cursor-env tests used a
trailing-fragment completion (the production path - the editor always has text
after the cursor) on >= 1.18 but a raw no-suffix parse on < 1.18 that was never
validated and yields a nil cursor env on 1.16/1.17. Collapse both to the
suffix-based production path, which works uniformly across versions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XRd15BX6wBpZ8ThaNSo232
The `:bitstring_modifier` completion branch only split the text after `::` and dropped already-typed names, so it offered combinations Elixir rejects - e.g. `signed`/`little`/`native`/`utf8` after `binary-`, `size`/`unit`/`signed` after `utf8-`, `signed`/`binary` after `float-`. Restore the type/sign/endianness/utf/size/unit validity rules by parsing the modifiers already typed after `::` into a state and keeping only the modifiers that may still legally follow. The logic is inlined as private helpers in the completion engine (self-contained; the old `ElixirSense.Core.Bitstring` module stays removed). The flat `@bitstring_modifiers` list is still the display source (names + size/unit arity), now filtered by the parsed state. Tests assert the invalid options are absent (not just that valid ones are present) for `binary-`, `utf8-` and `float-`. 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
Replaces ElixirSense's custom parsing / error-recovery and
Code.Fragment-based"symbol under cursor" classification with the toxic2
error-tolerant Elixir parser, and ports the elixir-ls completion subsystem wholesale so the
Normalized.Tokenizershim can be dropped.What changed
Parsing / error recovery
(
Code.string_to_quoted→Toxic2.parse_to_ast).parse_to_neutralized_ast/2with akeep_rangeoption;Context.ast/metadata now comefrom toxic2 ranged nodes.
Symbol-under-cursor classification (
ElixirSense.Core.SurroundContext.Toxic)range:metadata, returningthe same shape as
Code.Fragment.surround_context/2. Navigation locators (definition /references / implementation / hover) route through it; it falls back to
Code.Fragmentonly forshapes the parse tree can't disambiguate.
literal_encoder, so bare:atoms, keyword keys, andnil/true/falseare classified from the parse tree rather than lexically.Code.Fragment(operator-name arity captures,
&(-&1), step-range vars).surround_context/3accepts an already-parsed AST so callers don't re-parse per position.Completion
98e983d) wholesale, allowingNormalized.Tokenizerto be dropped (bitstring context now detected via the AST).Build / CI
toxic2git repo instead of a local path dep.~> 1.19; bump the requirement and trim the CI matrix to 1.19/1.20accordingly.
Testing
mix test: 1668 passing),mix dialyzerclean.SurroundContext.Toxicclassifier is validated by a differential againstCode.Fragment.surround_contextover real source files; the only divergences are thedocumented cases where the parse-based result is more precise.
🤖 Generated with Claude Code