From 6086a5728fb309bf2b87fa513f8994e2209a8a5b Mon Sep 17 00:00:00 2001 From: anusbutt Date: Tue, 21 Jul 2026 19:55:37 +0500 Subject: [PATCH] fix(parser): resolve custom-language names via configurable name_field (#691) The generic walker only found a definition's name from an identifier-like child or a field literally named "name", so BibTeX/LaTeX/Markdown nodes whose name lives in a differently named or nested field were extracted unnamed and silently dropped. Add an optional `name_field` (string or ordered list) to languages.toml. For custom languages only, resolve each candidate field-first across all candidates, then by descendant node type, then descend to a clean text leaf. Built-in language extraction is unchanged. Co-Authored-By: Claude Opus 4.8 --- code_review_graph/custom_languages.py | 31 ++++++ code_review_graph/parser.py | 95 ++++++++++++++++ docs/CUSTOM_LANGUAGES.md | 53 +++++++++ tests/test_custom_languages.py | 153 ++++++++++++++++++++++++++ 4 files changed, 332 insertions(+) diff --git a/code_review_graph/custom_languages.py b/code_review_graph/custom_languages.py index 0f63a545..93e13440 100644 --- a/code_review_graph/custom_languages.py +++ b/code_review_graph/custom_languages.py @@ -67,6 +67,10 @@ "call_node_types", ) +#: Hard cap on ``name_field`` probe candidates per language. Bounds the +#: per-node name resolution work (fail-safe ingestion invariant). +MAX_NAME_FIELD_CANDIDATES = 8 + @dataclass(frozen=True) class CustomLanguage: @@ -80,6 +84,7 @@ class CustomLanguage: import_node_types: tuple[str, ...] = () call_node_types: tuple[str, ...] = () comment: str = "" + name_field: tuple[str, ...] = () @dataclass(frozen=True) @@ -294,6 +299,31 @@ def _validate_entry( ) return None + # ``name_field``: optional ordered list of name-resolution candidates. + # Accepts a bare string (normalised to a 1-tuple) or a list of non-empty + # strings. Each candidate is later probed first as a tree-sitter field + # name and then as a descendant node type (see CodeParser._get_name). + raw_name_field = table.get("name_field", []) + if isinstance(raw_name_field, str): + raw_name_field = [raw_name_field] + if not isinstance(raw_name_field, list) or any( + not isinstance(item, str) or not item.strip() for item in raw_name_field + ): + logger.warning( + "%s: custom language %r: name_field must be a string or a list of " + "non-empty strings — skipping", + config_path, name, + ) + return None + if len(raw_name_field) > MAX_NAME_FIELD_CANDIDATES: + logger.warning( + "%s: custom language %r: name_field has more than %d candidates — " + "skipping", + config_path, name, MAX_NAME_FIELD_CANDIDATES, + ) + return None + name_field = tuple(item.strip() for item in raw_name_field) + comment = table.get("comment", "") if not isinstance(comment, str): comment = "" @@ -319,4 +349,5 @@ def _validate_entry( import_node_types=node_types["import_node_types"], call_node_types=node_types["call_node_types"], comment=comment, + name_field=name_field, ) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 8c8b4268..129bf43a 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -12817,6 +12817,98 @@ def _qualify(self, name: str, file_path: str, enclosing_class: Optional[str]) -> return f"{file_path}::{enclosing_class}.{name}" return f"{file_path}::{name}" + # Leaf node types that typically carry a definition's name across grammars. + # Used when descending from a configured ``name_field`` target to a clean + # text leaf (config-driven custom languages only). + _CUSTOM_NAME_LEAF_TYPES = ( + "word", "identifier", "name", "simple_identifier", + "command_name", "key_brace", "type_identifier", + "property_identifier", "constant", + ) + #: Depth guard for custom name-field descent / descendant search. + _MAX_CUSTOM_NAME_DEPTH = 4 + #: Cap on a resolved custom name before it is rejected as junk. + _MAX_CUSTOM_NAME_LEN = 256 + + def _custom_name_leaf(self, node, depth: int): + """Return the first text-bearing leaf of a known name type within + ``depth`` levels of ``node`` (preorder), or None.""" + if node.type in self._CUSTOM_NAME_LEAF_TYPES and node.child_count == 0: + return node + if depth <= 0: + return None + for child in node.children: + found = self._custom_name_leaf(child, depth - 1) + if found is not None: + return found + return None + + def _find_custom_descendant(self, node, type_name: str, depth: int): + """Return the first descendant of ``node`` whose type == ``type_name`` + within ``depth`` levels (preorder), or None.""" + if depth <= 0: + return None + for child in node.children: + if child.type == type_name: + return child + found = self._find_custom_descendant(child, type_name, depth - 1) + if found is not None: + return found + return None + + def _clean_custom_name(self, text: str) -> Optional[str]: + """Strip wrapping braces/quotes/whitespace; reject empty, multi-line, + or oversized text so a bad target never becomes a garbage name.""" + cleaned = text.strip().strip("{}\"'").strip() + if not cleaned or "\n" in cleaned or len(cleaned) > self._MAX_CUSTOM_NAME_LEN: + return None + return cleaned + + def _custom_leaf_name(self, node) -> Optional[str]: + """Resolve a configured ``name_field`` target node to a clean name. + + Prefers a text-bearing leaf of a known name type; falls back to the + target's own text (covers fieldless wrappers like Markdown ``inline``). + """ + leaf = self._custom_name_leaf(node, self._MAX_CUSTOM_NAME_DEPTH) + target = leaf if leaf is not None else node + return self._clean_custom_name( + target.text.decode("utf-8", errors="replace") + ) + + def _resolve_custom_name(self, node, language: str) -> Optional[str]: + """Resolve a definition name for a config-driven custom language using + the language's ordered ``name_field`` candidates. + + Two passes so grammar *fields* are always preferred over a broader + *type* search: this avoids matching an unrelated same-typed node in a + different field (e.g. LaTeX ``\\newcommand`` whose ``implementation`` + body contains a ``text`` node — the ``declaration`` field must win). + Returns None when no candidate resolves (caller then applies the legacy + ``name`` field fallback). + """ + custom = self._custom_languages.get(language) + candidates = custom.name_field if custom is not None else () + if not candidates: + return None + # Pass 1: field lookups (authoritative). + for cand in candidates: + target = node.child_by_field_name(cand) + if target is not None: + resolved = self._custom_leaf_name(target) + if resolved: + return resolved + # Pass 2: typed-descendant search (for fieldless shapes). + for cand in candidates: + target = self._find_custom_descendant( + node, cand, self._MAX_CUSTOM_NAME_DEPTH + ) + if target is not None: + resolved = self._custom_leaf_name(target) + if resolved: + return resolved + return None + def _get_name(self, node, language: str, kind: str) -> Optional[str]: """Extract the name from a class/function definition node.""" # Dart: function_signature has a return-type node before the identifier; @@ -13083,6 +13175,9 @@ def _leaf_name(qi): # grammar-specific (Erlang ``atom``, Haskell ``variable``) and thus # not in the generic child-type list above. if language in self._custom_languages: + resolved = self._resolve_custom_name(node, language) + if resolved: + return resolved name_child = node.child_by_field_name("name") if name_child is not None: return name_child.text.decode("utf-8", errors="replace") diff --git a/docs/CUSTOM_LANGUAGES.md b/docs/CUSTOM_LANGUAGES.md index a331edd3..e2256ce6 100644 --- a/docs/CUSTOM_LANGUAGES.md +++ b/docs/CUSTOM_LANGUAGES.md @@ -47,6 +47,7 @@ Each custom language is one `[languages.]` table. | `class_node_types` | list of strings | no* | Node types that define classes/records/types. Matching nodes become `Class` nodes. | | `import_node_types` | list of strings | no* | Node types for import/include statements. Each yields an `IMPORTS_FROM` edge. | | `call_node_types` | list of strings | no* | Node types for call expressions. Each yields a `CALLS` edge from the enclosing function. | +| `name_field` | string or list of strings | no | Ordered candidates for locating a definition's name when it is not a `name` field or a plain `identifier` child (see below). | | `comment` | string | no | Free-form note for humans; ignored by the parser. | \* At least one of the four node-type lists must be non-empty, otherwise the @@ -66,6 +67,58 @@ The loader never crashes a build. Anything invalid is skipped with a - Two custom languages cannot claim the same extension (first one wins). - At most **20** custom languages are loaded per repo. - Malformed TOML disables custom languages for that build (with a warning). +- `name_field` must be a string or a list of non-empty strings (max 8 + candidates); anything else skips the entry with a warning. + +### Naming definitions with `name_field` + +By default the parser finds a definition's name from an `identifier`-like child +or a field literally called `name`. Many grammars keep the name elsewhere — in +a differently named field, or nested a level or two below it. When that happens +the definition is extracted **unnamed and silently dropped**. `name_field` tells +the parser where to look. + +Each candidate is tried in order and resolved in two passes: + +1. **Field first** — a child accessed by that tree-sitter field name. Fields are + tried across *all* candidates before any type search, so a precise field + always wins over a broader match. +2. **Typed descendant** — if no candidate matched a field, the first descendant + whose *node type* equals a candidate (bounded depth). This covers names that + sit under a fieldless wrapper. + +The resolved node is then descended to its first text-bearing leaf and cleaned +(surrounding `{}`/quotes/whitespace stripped; multi-line or oversized text is +rejected). Because resolution is anchored on your configured candidates, it +never grabs an unrelated inner identifier. + +```toml +[languages.bibtex] +extensions = [".bib"] +grammar = "bibtex" +class_node_types = ["entry"] +name_field = ["key"] # @article{smith2020,...} -> "smith2020" + +[languages.latex] +extensions = [".tex"] +grammar = "latex" +class_node_types = ["section", "chapter", "subsection"] +function_node_types = ["new_command_definition"] +name_field = ["name", "text", "declaration"] +# \section{Introduction} -> "Introduction" (via `text`) +# \newcommand{\foo}{bar} -> "\foo" (via `declaration`) + +[languages.markdown] +extensions = [".md"] +grammar = "markdown" +class_node_types = ["section"] +name_field = ["inline"] # "# My Heading" -> "My Heading" (typed descendant) +``` + +Use a **list** when a grammar's node types keep their names in different places +(LaTeX `section` uses `text`, `\newcommand` uses `declaration`): the first +candidate that resolves wins. Omitting `name_field` preserves the previous +behavior exactly. ## Finding the right node type names diff --git a/tests/test_custom_languages.py b/tests/test_custom_languages.py index ea34fe6a..ee8ede6a 100644 --- a/tests/test_custom_languages.py +++ b/tests/test_custom_languages.py @@ -67,6 +67,34 @@ def load(repo_root: Path): ) +# --- name_field fixtures (issue #691): BibTeX / LaTeX / Markdown ------------- + +NAME_FIELD_TOML = """\ +[languages.bibtex] +extensions = [".bib"] +grammar = "bibtex" +class_node_types = ["entry"] +name_field = ["key"] + +[languages.latex] +extensions = [".tex"] +grammar = "latex" +class_node_types = ["section", "chapter", "subsection"] +function_node_types = ["new_command_definition"] +name_field = ["name", "text", "declaration"] + +[languages.markdown] +extensions = [".md"] +grammar = "markdown" +class_node_types = ["section"] +name_field = ["inline"] +""" + +BIBTEX_SOURCE = "@article{smith2020,\n title = {Hello},\n author = {Smith}\n}\n" +LATEX_SOURCE = "\\section{Introduction}\n\\newcommand{\\foo}{bar}\n" +MARKDOWN_SOURCE = "# My Heading\n\nSome text.\n" + + @pytest.fixture(autouse=True) def _clear_loader_cache(): custom_languages.clear_cache() @@ -304,3 +332,128 @@ def test_full_build_includes_custom_language(self, tmp_path): assert row[0] == "erlang" finally: store.close() + + +class TestNameFieldLoader: + """Loader validation for the optional name_field key (issue #691).""" + + def test_name_field_string_normalised_to_tuple(self, tmp_path): + write_config(tmp_path, ( + "[languages.bibtex]\n" + 'extensions = [".bib"]\n' + 'grammar = "bibtex"\n' + 'class_node_types = ["entry"]\n' + 'name_field = "key"\n' + )) + result = load(tmp_path) + assert result["bibtex"].name_field == ("key",) + + def test_name_field_list_preserved_in_order(self, tmp_path): + write_config(tmp_path, NAME_FIELD_TOML) + result = load(tmp_path) + assert result["latex"].name_field == ("name", "text", "declaration") + + def test_name_field_omitted_defaults_empty(self, tmp_path): + write_config(tmp_path, ERLANG_TOML) + assert load(tmp_path)["erlang"].name_field == () + + def test_invalid_name_field_type_skips_entry(self, tmp_path, caplog): + write_config(tmp_path, ( + "[languages.bibtex]\n" + 'extensions = [".bib"]\n' + 'grammar = "bibtex"\n' + 'class_node_types = ["entry"]\n' + "name_field = 5\n" + )) + with caplog.at_level(logging.WARNING): + result = load(tmp_path) + assert "bibtex" not in result + assert "name_field" in caplog.text + + def test_empty_name_field_element_skips_entry(self, tmp_path, caplog): + write_config(tmp_path, ( + "[languages.bibtex]\n" + 'extensions = [".bib"]\n' + 'grammar = "bibtex"\n' + 'class_node_types = ["entry"]\n' + 'name_field = ["key", ""]\n' + )) + with caplog.at_level(logging.WARNING): + result = load(tmp_path) + assert "bibtex" not in result + + def test_one_bad_name_field_does_not_break_others(self, tmp_path, caplog): + write_config(tmp_path, ( + "[languages.bibtex]\n" + 'extensions = [".bib"]\n' + 'grammar = "bibtex"\n' + 'class_node_types = ["entry"]\n' + "name_field = 5\n" + "\n" + "[languages.markdown]\n" + 'extensions = [".md"]\n' + 'grammar = "markdown"\n' + 'class_node_types = ["section"]\n' + 'name_field = ["inline"]\n' + )) + with caplog.at_level(logging.WARNING): + result = load(tmp_path) + assert "bibtex" not in result + assert "markdown" in result + + +class TestNameFieldResolution: + """End-to-end name resolution for the four shapes in issue #691.""" + + def _parse(self, tmp_path: Path, filename: str, source: str): + write_config(tmp_path, NAME_FIELD_TOML) + src = tmp_path / filename + src.write_text(source, encoding="utf-8") + parser = CodeParser(tmp_path) + return parser.parse_file(src) + + def _named(self, nodes, kind): + return {n.name for n in nodes if n.kind == kind} + + def test_bibtex_entry_named_by_key(self, tmp_path): + nodes, _ = self._parse(tmp_path, "refs.bib", BIBTEX_SOURCE) + assert "smith2020" in self._named(nodes, "Class") + + def test_bibtex_does_not_use_inner_field_labels(self, tmp_path): + # Anti-trap: title/author are identifiers deeper in the entry subtree. + nodes, _ = self._parse(tmp_path, "refs.bib", BIBTEX_SOURCE) + names = self._named(nodes, "Class") + assert "title" not in names + assert "author" not in names + + def test_latex_section_named_without_braces(self, tmp_path): + nodes, _ = self._parse(tmp_path, "paper.tex", LATEX_SOURCE) + classes = self._named(nodes, "Class") + assert "Introduction" in classes + assert "{Introduction}" not in classes + + def test_latex_newcommand_resolved_via_declaration(self, tmp_path): + # Ordered list must pick `declaration` (field), not the `text` node + # inside the implementation body ({bar}). + nodes, _ = self._parse(tmp_path, "paper.tex", LATEX_SOURCE) + funcs = self._named(nodes, "Function") + assert "\\foo" in funcs + assert "bar" not in funcs + + def test_markdown_section_named_via_typed_descendant(self, tmp_path): + nodes, _ = self._parse(tmp_path, "README.md", MARKDOWN_SOURCE) + assert "My Heading" in self._named(nodes, "Class") + + def test_without_name_field_definitions_are_dropped(self, tmp_path): + # Same BibTeX source but no name_field → entry has no resolvable name + # and is dropped, proving the feature is the reason it now appears. + write_config(tmp_path, ( + "[languages.bibtex]\n" + 'extensions = [".bib"]\n' + 'grammar = "bibtex"\n' + 'class_node_types = ["entry"]\n' + )) + src = tmp_path / "refs.bib" + src.write_text(BIBTEX_SOURCE, encoding="utf-8") + nodes, _ = CodeParser(tmp_path).parse_file(src) + assert self._named(nodes, "Class") == set()