Skip to content

feat: run the new grammar engine behind an experimental flag (#467 step 5)#472

Open
SJrX wants to merge 1 commit into
issue-345-4from
issue-345-5
Open

feat: run the new grammar engine behind an experimental flag (#467 step 5)#472
SJrX wants to merge 1 commit into
issue-345-4from
issue-345-5

Conversation

@SJrX

@SJrX SJrX commented Jun 21, 2026

Copy link
Copy Markdown
Owner

What

Wires the new list-of-successes engine into the actual InvalidValue inspection — but behind an opt-in flag, so it can be exercised in the real IDE while the original SyntacticMatch/SemanticMatch path stays the default for everyone else.

Stacked on #471 (issue-345-4). This is the first PR where parse() does something user-visible.

Why a flag

Until now the new engine lived entirely beside the old one with unit tests. A flag (mirroring the existing Podman experimental toggle) turns the scary all-or-nothing migration into "flip it, try it, iterate," and lets the real inspection tests run against the new engine end-to-end — a much stronger correctness signal than parallel unit tests.

Changes

  • ExperimentalSettings — project-level PersistentStateComponent with useGrammarParseEngine, its own storage (kept out of PodmanQuadletSettings). A checkbox is added to the shared "systemd Unit Files" settings page.
  • GrammarOptionValue — when the flag is on, generateProblemDescriptors runs validate() and maps ParseOutcome onto the same problem descriptors as the old path:
    • Valid → nothing,
    • SemanticError → highlight the bad token + literal-replacement quickfixes,
    • SyntaxError → highlight from the furthest offset.
      Flag off → today's code, untouched.
  • EDT safety, kept IntelliJ-free in the enginevalidate() gains a pure step cap (fails open to Valid rather than flag what it couldn't fully explore) and an onStep callback; GrammarOptionValue passes ProgressManager::checkCanceled, so cancellation works without any IntelliJ type leaking into the grammar package.
  • TestGrammarParseEngineInspectionTest drives the RestrictAddressFamilies= inspection end-to-end with the flag on (valid → 0 highlights, invalid → 3), plus a flag-off sanity check. It resets the flag in tearDown because the light-test project is shared across classes.

Notes

  • As discussed, the whole inspection suite is not dual-run under both engines — the engine itself is covered by ParseTest; this just proves the wiring + ParseOutcome→descriptor mapping.
  • Known minor difference to reconcile before the flag could ever become default: the new engine's bad-token localization can differ slightly from the old engine for some grammars (surfaced as a quickfix range diff, e.g. ::tcp vs :tcp, in a CGroupSocketBind test when the flag leaked — now fixed via tearDown). Validity detection matches; only the highlighted range/quickfix offset can differ.

Try it

./gradlew runIde, enable Settings → Tools → systemd Unit Files → "Use the new grammar engine for value validation (experimental)", and edit a .service file.

Refs #467 #345

🤖 Generated with Claude Code

…ep 5)

Wires the list-of-successes engine into the actual InvalidValue inspection, gated
behind an opt-in setting, so it can be exercised in the real IDE while the original
SyntacticMatch/SemanticMatch path stays the default.

- ExperimentalSettings: a project-level PersistentStateComponent with
  useGrammarParseEngine (separate storage from PodmanQuadletSettings); a checkbox is
  added to the shared "systemd Unit Files" settings page.
- GrammarOptionValue.generateProblemDescriptors: when the flag is on, validate() via
  parse() and map ParseOutcome onto the same problem descriptors (SemanticError ->
  highlight the bad token + literal-replacement quickfixes; SyntaxError -> highlight
  from the furthest offset; Valid -> nothing). Flag off -> unchanged.
- Safety for running on the highlighting thread, kept IntelliJ-free in the engine:
  validate() gains a pure step cap (fail-open to Valid) and an onStep callback;
  GrammarOptionValue passes ProgressManager::checkCanceled.
- Test: GrammarParseEngineInspectionTest drives the AF inspection end-to-end with the
  flag on (valid -> 0, invalid -> 3) and resets the flag in tearDown so the shared
  light-test project doesn't leak the opt-in into other tests.

Per discussion, the whole inspection suite is NOT dual-run; the engine is covered by
ParseTest and this spot-checks the wiring + mapping. Known minor difference: the new
engine's bad-token localization can differ slightly from the old engine for some
grammars (e.g. a quickfix range), to be reconciled before the flag becomes default.

Refs #467 #345

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Test Results

1 121 tests  +3   1 121 ✅ +3   56s ⏱️ +7s
  296 suites +1       0 💤 ±0 
  296 files   +1       0 ❌ ±0 

Results for commit 26877c2. ± Comparison against base commit 44b6e41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant