Fix autocomplete for variables whose names match grammar keywords #15

Closed
josh-williams wants to merge 2 commits from fix-autocomplete-on-keywords into main
josh-williams commented 2026-02-20 18:26:43 +01:00 (Migrated from github.com)

Summary

When a user-provided variable has the same name as a Liquid keyword (e.g. case, assign, render, for), typing case. inside {{ }} produced no property completions. The variable name itself would autocomplete, but member access failed entirely.

Root cause: The kw<word> template uses @specialize to replace identifier tokens with keyword tokens. In the liquidDirective section, this creates specializations for ~25 keywords (case, if, for, echo, assign, etc.) on the identifier token. Lezer's @specialize should fall back to the original identifier when the keyword isn't valid in the current parse context, but it wasn't doing so — the parser would close the Interpolation with an error node and the content fell through to the HTML parser as Text.

Fix: Introduce a kwx<word> template using @extend (instead of @specialize). @extend makes both the keyword token and the original identifier token available simultaneously, letting the GLR parser choose the correct interpretation per context. The liquidDirective section and RenderParameter now use kwx<>, while expression-context keywords (empty, forloop, tablerowloop, continue, contains) retain @specialize so they're still preferred over VariableName.

Test plan

  • Added 4 parse tests: keyword-named variables in interpolation ({{ case.id }}), tag context ({% if case.id %}), liquid tag ({% liquid\necho case.id %}), and multiple keywords as variables.
  • Verified all 18 existing tests still pass (no regressions for case/for/if directives, operators, render parameters, liquid tags, etc.).
  • Manually verified autocomplete returns property suggestions for keyword-named variables via a completion source test.
## Summary When a user-provided variable has the same name as a Liquid keyword (e.g. `case`, `assign`, `render`, `for`), typing `case.` inside `{{ }}` produced no property completions. The variable name itself would autocomplete, but member access failed entirely. **Root cause:** The `kw<word>` template uses `@specialize` to replace identifier tokens with keyword tokens. In the `liquidDirective` section, this creates specializations for ~25 keywords (`case`, `if`, `for`, `echo`, `assign`, etc.) on the identifier token. Lezer's `@specialize` should fall back to the original identifier when the keyword isn't valid in the current parse context, but it wasn't doing so — the parser would close the `Interpolation` with an error node and the content fell through to the HTML parser as `Text`. **Fix:** Introduce a `kwx<word>` template using `@extend` (instead of `@specialize`). `@extend` makes both the keyword token and the original identifier token available simultaneously, letting the GLR parser choose the correct interpretation per context. The `liquidDirective` section and `RenderParameter` now use `kwx<>`, while expression-context keywords (`empty`, `forloop`, `tablerowloop`, `continue`, `contains`) retain `@specialize` so they're still preferred over `VariableName`. ## Test plan - Added 4 parse tests: keyword-named variables in interpolation (`{{ case.id }}`), tag context (`{% if case.id %}`), liquid tag (`{% liquid\necho case.id %}`), and multiple keywords as variables. - Verified all 18 existing tests still pass (no regressions for `case`/`for`/`if` directives, operators, render parameters, liquid tags, etc.). - Manually verified autocomplete returns property suggestions for keyword-named variables via a completion source test.
marijnh commented 2026-02-21 14:36:09 +01:00 (Migrated from github.com)

Wow, that's an interesting feature. Are you sure this doesn't also apply to the remaining tag keywords? {{ continue.length }}, for example, seems to be supported.

Wow, that's an interesting feature. Are you sure this doesn't also apply to the remaining tag keywords? `{{ continue.length }}`, for example, seems to be supported.
josh-williams commented 2026-02-22 16:08:35 +01:00 (Migrated from github.com)

Good catch. I tested against the LiquidJS playground and confirmed that continue, forloop, tablerowloop, and in all work as variable names in Liquid. Only empty and contains are truly reserved in expression context.

I've updated the PR to switch those four to @extend as well. The only keywords still using @specialize (via kw<>) are empty and contains, which LiquidJS rejects as variable names.

Good catch. I tested against the [LiquidJS playground](https://liquidjs.com/playground.html) and confirmed that `continue`, `forloop`, `tablerowloop`, and `in` all work as variable names in Liquid. Only `empty` and `contains` are truly reserved in expression context. I've updated the PR to switch those four to @extend as well. The only keywords still using `@specialize` (via `kw<>`) are `empty` and `contains`, which LiquidJS rejects as variable names.
marijnh commented 2026-02-23 08:24:17 +01:00 (Migrated from github.com)

Thanks. I've merged this as 4ff79e865a

Thanks. I've merged this as 4ff79e865a979a7b23f9201cb736df21b0dc0174

Pull request closed

Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
codemirror/lang-liquid!15
No description provided.