fix(highlight): PR #118 round 1 — drop locals-predicate captures; stale comments

[P2] The shared JavaScript highlights query guards its builtin captures
(console, require, …) with `#is-not? local`, a PROPERTY predicate
(`Query::property_predicates`) that needs a scope map from the grammar's
LOCALS_QUERY — which pmacs does not run. `compute_highlight_spans` took
every capture, so a locally-shadowed `console`/`require` still surfaced
as `@variable.builtin`/`@function.builtin`; a theme distinguishing
`.builtin` would mis-style the shadowed local.

Full locals processing is substrate work; conservatively fail-closed
instead: drop captures whose pattern carries an `#is?`/`#is-not? local`
property predicate (the identifier falls back to its non-builtin
capture). The text predicates (`#eq?`/`#match?`/`#any-of?`, already
applied by the capture iterator) and `#set!` settings are untouched.
This is a general engine fix — it corrects the same latent mis-styling
for any grammar using the locals predicate, not just JS/TS.

- javascript_shadowed_builtin_is_not_mislabeled: a local `const console`
  produces no `*.builtin` capture (directly observed to fail — two
  `variable.builtin` captures — before the fix).

[P3] Comments this PR invalidated: `lsp.lua` no longer claims Python has
no grammar; `syntax.lua`'s `_has_language` gate comment uses a
still-grammarless example (an init.lua `shebangs.ruby`) instead of
python/javascript; and the rewritten Ruby shebang test's doc no longer
describes it as a Python test.

Gates: fmt; clippy -D warnings; test --lib; --features crdt;
m4_acceptance --skip basedpyright; GPU; full workspace sweep;
git diff --check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJ9FQ832QwftJXCD9LeFan
This commit is contained in:
Levi Neuwirth 2026-07-14 17:45:09 +01:00
parent bcec61e020
commit 665fd82860
4 changed files with 72 additions and 11 deletions

View File

@ -201,11 +201,13 @@ pmacs.lsp.config.zig = pmacs.lsp.config.zig or {
}
-- LSP-side extension → language map, deliberately independent of the
-- tree-sitter detection in `pmacs.parse` (which is grammar-gated:
-- Python has an LSP server but no bundled grammar). Consulted only
-- when `pmacs.parse.language_for_path` finds nothing, so grammar-
-- backed languages keep their existing detection. Extensible from
-- init.lua: `pmacs.lsp.filetypes.foo = "bar"`.
-- tree-sitter detection in `pmacs.parse`. Consulted only when
-- `pmacs.parse.language_for_path` finds nothing (an extension with a
-- server but no bundled grammar), so grammar-backed languages keep their
-- existing detection. Every language with an LSP config now also ships a
-- grammar, so this is mainly the LSP-only fallback that keeps a language
-- id stable if a grammar is ever dropped, plus the seam for user-added
-- mappings. Extensible from init.lua: `pmacs.lsp.filetypes.foo = "bar"`.
pmacs.lsp.filetypes = pmacs.lsp.filetypes or {}
pmacs.lsp.filetypes.py = pmacs.lsp.filetypes.py or "python"
pmacs.lsp.filetypes.pyi = pmacs.lsp.filetypes.pyi or "python"

View File

@ -220,9 +220,11 @@ local function attach_for_active_buffer()
-- and silently swap the grammar — and diverge from the LSP side, which
-- keeps its existing attachment across the switch. A first-seen buffer
-- (no view yet) resolves normally. Gate dispatch on `_has_language`: the
-- resolution chain also yields languages with no grammar (python,
-- javascript), and dispatching one would raise "unknown language"
-- (caught, but noise) and never gives a wrong-grammar tree.
-- resolution chain can still yield a language with no grammar — a
-- shebang or filetype mapping to a server-only or unsupported language
-- (e.g. an init.lua `pmacs.parse.shebangs.ruby = "ruby"`) — and
-- dispatching one would raise "unknown language" (caught, but noise) and
-- never gives a wrong-grammar tree.
local lang = pmacs.parse._has_view(buf) and parse_lang_by_buffer[key]
or resolve_active_language(buf)
if not lang or not pmacs.parse._has_language(lang) then return end

View File

@ -886,6 +886,24 @@ pub fn compute_highlight_spans_in_range(
let root = bundle.tree.root_node();
let mut iter = cursor.captures(query, root, source);
while let Some((qmatch, capture_idx)) = iter.next() {
// Fail-closed on the locals property predicate. The capture
// iterator already applies text predicates (`#eq?`/`#match?`/
// `#any-of?`), but `#is? local` / `#is-not? local` are *property*
// predicates (`Query::property_predicates`) that need a scope map
// built from the grammar's LOCALS_QUERY, which pmacs does not run.
// Applying such a capture regardless mis-styles shadowed locals —
// e.g. a local `console`/`require` in JS/TS would still capture as
// `@variable.builtin`/`@function.builtin`. Until locals processing
// exists, drop captures whose pattern carries one; the identifier
// falls back to its non-builtin capture. `#set!` (property
// *settings*) is a different API and is not consulted here.
if query
.property_predicates(qmatch.pattern_index)
.iter()
.any(|(prop, _)| &*prop.key == "local")
{
continue;
}
let cap = qmatch.captures[*capture_idx];
spans.push(HighlightSpan {
start_byte: cap.node.start_byte() as u32,
@ -1331,6 +1349,43 @@ mod tests {
}
}
#[test]
fn javascript_shadowed_builtin_is_not_mislabeled() {
// `#is-not? local` (JS/TS use it for console/require/etc.) needs a
// scope map from the LOCALS_QUERY we don't run, so
// `compute_highlight_spans` drops captures guarded by it. Here
// `console` is a LOCAL declaration — it must not surface as a
// `*.builtin` capture (which is what a naive run of the shared JS
// query would produce).
let reg = SyntaxRegistry::new();
let language = reg.language("javascript").expect("javascript loads");
let query = reg
.highlights_query("javascript")
.expect("javascript highlights compile");
let mut buf = fresh_buffer("shadow.js");
buf.apply_edit(EditOp::Insert {
pos: 0,
bytes: b"const console = 5;\nconsole;\n",
})
.unwrap();
let view = ParseView::new(&buf, language, "javascript".to_owned());
let handle = view.handle();
let _vid = buf.attach_view(Box::new(view));
let bundle = parse_synchronously(&handle);
let spans = compute_highlight_spans(&query, &bundle);
assert!(!spans.is_empty(), "the JS query produced highlight spans");
let names = query.capture_names();
let builtin: Vec<&str> = spans
.iter()
.map(|s| names[s.capture_index as usize])
.filter(|n| n.contains("builtin"))
.collect();
assert!(
builtin.is_empty(),
"a locally-shadowed `console` must not get a *.builtin capture; got {builtin:?}"
);
}
#[test]
fn gap_grammar_extensions_resolve() {
let reg = SyntaxRegistry::new();

View File

@ -5994,11 +5994,13 @@ fn m4_shebang_does_not_override_extension() {
);
}
/// Finding-1 gate: an extensionless `#!/usr/bin/env python3` script
/// resolves to python for LSP, but python has no grammar — syntax must
/// Finding-1 gate: an extensionless `#!/usr/bin/env ruby` script resolves
/// to `ruby` via the shebang map, but ruby has no grammar — syntax must
/// skip it *silently*. Without the `_has_language` gate, `_dispatch`
/// raises "unknown language: python" (caught by the after-load pcall and
/// raises "unknown language: ruby" (caught by the after-load pcall and
/// reported through `pmacs.error`), which we assert does NOT happen.
/// (python/js/lua/bash all ship grammars now, so the gate needs a
/// genuinely grammarless example.)
#[test]
fn m4_shebang_extensionless_grammarless_language_is_silent() {
use pmacs::editor::EditorState;