Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ jobs:
working-directory: crates/codegraph-core
run: napi build --release

# Runs `cargo test`, which exercises the grammar-ABI regression test
# added in #1054. Without this step a future tree-sitter / grammar
# version drift would only surface as a runtime "files dropped"
# warning during benchmarks, not as a test failure on the PR.
- name: Run Rust tests
working-directory: crates/codegraph-core
run: cargo test --release

- name: Upload artifact
uses: actions/upload-artifact@v7
with:
Expand Down
32 changes: 28 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/codegraph-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ napi = { version = "3", features = ["serde-json"] }
napi-derive = "3"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
tree-sitter = "0.24"
tree-sitter = "0.25"
tree-sitter-javascript = "0.23"
tree-sitter-typescript = "0.23"
tree-sitter-python = "0.23"
Expand Down
86 changes: 86 additions & 0 deletions crates/codegraph-core/src/parser_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,90 @@ impl LanguageKind {
Self::OcamlInterface => tree_sitter_ocaml::LANGUAGE_OCAML_INTERFACE.into(),
}
}

/// Every variant in declaration order. Adding a new `LanguageKind` variant
/// requires adding it here too — the regression test in this file's
/// `tests` module iterates this list to confirm each grammar loads at
/// runtime, so missing entries silently lose ABI coverage for that
/// language. See #1054 (tree-sitter-hcl 1.1.0 shipped ABI 15 while the
/// runtime was pinned at ABI 14, and `set_language` rejected the grammar
/// at runtime instead of at compile time).
pub fn all() -> &'static [LanguageKind] {
use LanguageKind::*;
&[
JavaScript, TypeScript, Tsx, Python, Go, Rust, Java, CSharp, Ruby, Php, Hcl, C,
Cpp, Kotlin, Swift, Scala, Bash, Elixir, Lua, Dart, Zig, Haskell, Ocaml,
OcamlInterface,
]
}
}

#[cfg(test)]
mod tests {
use super::*;
use tree_sitter::Parser;

/// Catches tree-sitter ABI version mismatches between the runtime crate
/// and individual grammar crates. When a grammar ships parser code built
/// against a newer ABI than the runtime supports, `set_language` rejects
/// it with `LanguageError`, `parse_file` silently returns `None`, and
/// every file in that language is "dropped" — the user sees a warning
/// and the JS layer falls back to WASM. See #1054 (tree-sitter-hcl 1.1.0
/// vs tree-sitter 0.24).
#[test]
fn all_grammars_have_compatible_abi() {
let mut failures: Vec<String> = Vec::new();
for &kind in LanguageKind::all() {
let mut parser = Parser::new();
let language = kind.tree_sitter_language();
if let Err(e) = parser.set_language(&language) {
failures.push(format!(" {:?}: {:?}", kind, e));
}
}
assert!(
failures.is_empty(),
"Tree-sitter grammar ABI mismatch — bump `tree-sitter` in Cargo.toml \
or pin the failing grammar crate down (#1054):\n{}",
failures.join("\n")
);
}

/// Every variant declared in the enum must appear in `all()`. Without
/// this check, a new variant added to the enum would silently lose
/// ABI coverage from `all_grammars_have_compatible_abi`.
#[test]
fn all_kinds_listed_in_all() {
// Exhaustive match — fails to compile if a variant is added without
// updating the body. The match itself is a no-op; the compile-time
// exhaustiveness check is the test. If this match starts failing,
// also update `LanguageKind::all()`.
let kind = LanguageKind::JavaScript;
let _: () = match kind {
LanguageKind::JavaScript
| LanguageKind::TypeScript
| LanguageKind::Tsx
| LanguageKind::Python
| LanguageKind::Go
| LanguageKind::Rust
| LanguageKind::Java
| LanguageKind::CSharp
| LanguageKind::Ruby
| LanguageKind::Php
| LanguageKind::Hcl
| LanguageKind::C
| LanguageKind::Cpp
| LanguageKind::Kotlin
| LanguageKind::Swift
| LanguageKind::Scala
| LanguageKind::Bash
| LanguageKind::Elixir
| LanguageKind::Lua
| LanguageKind::Dart
| LanguageKind::Zig
| LanguageKind::Haskell
| LanguageKind::Ocaml
| LanguageKind::OcamlInterface => (),
};
assert_eq!(LanguageKind::all().len(), 24);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The assert_eq!(LanguageKind::all().len(), 24) check doesn't fully close the gap it aims to close. If a developer adds a new variant to the enum (forced by the compile-time match) but forgets to add it to all() and also forgets to bump this count, both the compile check and the runtime assertion pass silently — the new language gets zero ABI coverage. The assertion only catches someone who updates all() but forgets to bump the number, which is the less dangerous case. Naming the constant and improving the assertion message would make the three required update sites more explicit.

Suggested change
assert_eq!(LanguageKind::all().len(), 24);
}
}
// IMPORTANT: this constant must equal the number of arms in the match
// above AND the length of the slice returned by `LanguageKind::all()`.
// Because both checks require the same manual update, they reinforce
// each other: a developer who updates the match is reminded to also
// update `all()` and this count.
const EXPECTED_LEN: usize = 24;
assert_eq!(
LanguageKind::all().len(),
EXPECTED_LEN,
"A LanguageKind variant is in the exhaustive match but missing from \
`all()` (or vice-versa). Update `all()` and bump EXPECTED_LEN.",
);
}
}

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 122da28 — applied the suggested EXPECTED_LEN constant rename and the clearer assertion message verbatim. Both regression tests still pass locally.

Loading