-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
fix(cpp): keep symbols for nested C++ types and C++/CLI sources (#2876) #2886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8edbf38
2807680
bc519f6
b869c80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1958,13 +1958,111 @@ def _augment_cpp_string_tests(path: Path, result: dict) -> dict: | |
| return result | ||
|
|
||
|
|
||
| # ── C++/CLI normalization (#2876) ──────────────────────────────────────────── | ||
| # tree-sitter-cpp implements none of `ref class`, `Type^`, `Type%`, `gcnew` or | ||
| # `[assembly:…]`. The ERROR lands on the type header, which dissolves the whole | ||
| # class body — a 141-method .NET interop wrapper yielded 12 junk symbols. These | ||
| # rewrites map each spelling onto the nearest standard C++ one. | ||
|
|
||
| # Only files carrying one of these engage the rewrite, so plain C/C++/CUDA is | ||
| # parsed byte-for-byte as before. `^`/`%` alone are not markers: they are the | ||
| # ordinary XOR and modulo operators. | ||
| _CPP_CLI_MARKER_RE = re.compile( | ||
| rb"\b(?:ref|value)\s+(?:class|struct)\b" | ||
| rb"|\binterface\s+class\b" | ||
| rb"|\bgcnew\b" | ||
| rb"|\[\s*(?:assembly|module)\s*:" | ||
| ) | ||
|
|
||
| # `public ref class Foo` / `value struct Bar` / `interface class Baz` → the | ||
| # access specifier goes too: it is not legal at namespace scope, and leaving it | ||
| # behind is what made recovery invent a stray `public` node. | ||
| _CPP_CLI_CLASS_RE = re.compile( | ||
| rb"(?:\b(?:public|private|protected)\s+)?\b(?:ref|value)\s+(?=(?:class|struct)\b)" | ||
| rb"|(?:\b(?:public|private|protected)\s+)?\binterface\s+(?=class\b)" | ||
| ) | ||
| # Handle (`String^ s`) and tracking-reference (`int% n`) suffixes. | ||
| # | ||
| # `^` and `%` are also XOR and modulo, and `String^ s` is lexically identical to | ||
| # `a^ b` — attachment to the preceding token does not separate them, because | ||
| # `a% b` and `hash^ mask` are attached too. Rewriting on attachment alone | ||
| # corrupted those into `a b` / `hash mask`. So the rewrite is restricted to | ||
| # the two positions where an operator reading is impossible or implausible. | ||
| # | ||
| # 1. Followed by a token that cannot begin an operand: `f(String^, int)`, | ||
| # `List<String^>`, `(String^)x`, `Object^;`. `a^,` is not valid C++, so | ||
| # there is no arithmetic to lose here. `*` and `&` are deliberately NOT in | ||
| # the set — `a^*p` and `a^&b` are valid XOR expressions, and `String^*` is | ||
| # rare enough not to be worth trading for them. | ||
| _CPP_CLI_SUFFIX_UNAMBIGUOUS_RE = re.compile(rb"(?<=[A-Za-z0-9_>])[\^%](?=\s*[,)\]>;])") | ||
| # 2. A type-shaped left side followed by a declarator: `System::String^ s`, | ||
| # `List<int>^ items`, `DataTable^ t`, `int% n`. Qualified names, a closing | ||
| # generic bracket, .NET's PascalCase convention and the primitive value | ||
| # types are all type positions; requiring one keeps lowercase operands like | ||
| # `count% 2` and `hash^ mask` as arithmetic. A capitalized name must also | ||
| # carry a lowercase letter, so SCREAMING_CASE constants stay arithmetic | ||
| # too (`MASK^ value`); a lone capital is exempt for generic parameters | ||
| # (`T^ x`). The type is captured and re-emitted so the substitution stays | ||
| # byte-length preserving. | ||
| _CPP_CLI_SUFFIX_DECL_RE = re.compile( | ||
| rb"(\b[A-Za-z_][A-Za-z0-9_]*(?:::[A-Za-z_][A-Za-z0-9_]*)+" # System::String | ||
| rb"|\b[A-Z][A-Za-z0-9_]*[a-z][A-Za-z0-9_]*" # String, DataTable | ||
| rb"|\b[A-Z](?=[\^%])" # T | ||
| rb"|\b(?:bool|char|wchar_t|short|int|long|float|double|unsigned|signed)" | ||
| rb"|>)" # List<int>^ | ||
| rb"[\^%](?=\s+[A-Za-z_])" | ||
| ) | ||
| # `[assembly:AssemblyVersion("1.0")]` and friends. | ||
| _CPP_CLI_ATTR_RE = re.compile(rb"\[\s*(?:assembly|module)\s*:[^\[\]]*\]", re.S) | ||
|
|
||
|
|
||
| def _blank_keeping_newlines(m: "re.Match[bytes]") -> bytes: | ||
| """Replace a match with spaces, but keep its line breaks. | ||
|
|
||
| Byte length alone is not enough. Both the class-header and the attribute | ||
| pattern can span lines — ``[assembly:AssemblyVersion(\\n "1.0"\\n)]`` is | ||
| ordinary formatting — and blanking a newline merges two source lines, which | ||
| shifts the reported line number of every symbol below it. Preserving CR and | ||
| LF in place keeps line and column stable as well as offset. | ||
| """ | ||
| return re.sub(rb"[^\r\n]", b" ", m.group(0)) | ||
|
|
||
|
|
||
| def _normalize_cpp_cli(source: bytes) -> bytes | None: | ||
| """Rewrite C++/CLI spellings to standard C++ ones, or None if not C++/CLI. | ||
|
|
||
| The rewrite is **byte-length preserving** — dropped tokens are overwritten | ||
| with spaces, never deleted, and ``gcnew`` becomes ``new`` plus padding — and | ||
| line breaks inside a removed token are kept, so every offset, line and | ||
| column still points at the same place in the file on disk and reported | ||
| source locations stay accurate (#2876). | ||
| """ | ||
| if not _CPP_CLI_MARKER_RE.search(source): | ||
| return None | ||
| out = _CPP_CLI_CLASS_RE.sub(_blank_keeping_newlines, source) | ||
| out = re.sub(rb"\bgcnew\b", b"new ", out) | ||
| out = _CPP_CLI_SUFFIX_UNAMBIGUOUS_RE.sub(b" ", out) | ||
| out = _CPP_CLI_SUFFIX_DECL_RE.sub(rb"\1 ", out) | ||
| return _CPP_CLI_ATTR_RE.sub(_blank_keeping_newlines, out) | ||
|
|
||
|
|
||
| def extract_cpp(path: Path) -> dict: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
25 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
26 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
27 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. |
||
| """Extract functions, classes, and includes from a .cpp/.cc/.cxx/.hpp file. | ||
|
|
||
| C++/CLI sources are normalized to standard C++ first (#2876); see | ||
| :func:`_normalize_cpp_cli`. | ||
|
|
||
| Recovers doctest/Catch2 ``TEST_CASE("name")`` test cases that tree-sitter-cpp | ||
| drops as ERROR nodes (issue #2594), mirroring the Spock fallback for Groovy. | ||
| """ | ||
| result = _extract_generic(path, _CPP_CONFIG) | ||
| try: | ||
| source = path.read_bytes() | ||
| except OSError: | ||
| # Let _extract_generic report the read failure in its usual shape. | ||
| return _augment_cpp_string_tests(path, _extract_generic(path, _CPP_CONFIG)) | ||
| result = _extract_generic( | ||
| path, _CPP_CONFIG, source_override=_normalize_cpp_cli(source) or source | ||
| ) | ||
| return _augment_cpp_string_tests(path, result) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,251 @@ | ||
| """C++ nested types and C++/CLI keep their symbols (#2876).""" | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from graphify.extract import _normalize_cpp_cli, extract_cpp | ||
|
|
||
| pytest.importorskip("tree_sitter_cpp") | ||
|
|
||
|
|
||
| def _labels(path: Path) -> list[str]: | ||
| return [n["label"] for n in extract_cpp(path)["nodes"]] | ||
|
|
||
|
|
||
| def test_nested_cpp_class_is_extracted(tmp_path): | ||
| # A nested type is a field_declaration whose `type` field IS the | ||
| # class_specifier; the member-variable branch used to consume it and return | ||
| # before the walk could descend, dropping Inner with no parse error. | ||
| p = tmp_path / "nested.h" | ||
| p.write_text( | ||
| "namespace N {\n" | ||
| " class Outer\n" | ||
| " {\n" | ||
| " public:\n" | ||
| " class Inner\n" | ||
| " {\n" | ||
| " public:\n" | ||
| " static void Method() { }\n" | ||
| " };\n" | ||
| " };\n" | ||
| "}\n" | ||
| ) | ||
| result = extract_cpp(p) | ||
| assert result.get("parse_errors") is None | ||
| assert [n["label"] for n in result["nodes"]] == [ | ||
| "nested.h", "Outer", "Inner", ".Method()", | ||
| ] | ||
| # Inner is contained by Outer, not by the file (#2040). | ||
| outer = next(n["id"] for n in result["nodes"] if n["label"] == "Outer") | ||
| inner = next(n["id"] for n in result["nodes"] if n["label"] == "Inner") | ||
| assert any( | ||
| e["source"] == outer and e["target"] == inner and e["relation"] == "contains" | ||
| for e in result["edges"] | ||
| ) | ||
|
|
||
|
|
||
| def test_nested_type_declared_with_an_instance(tmp_path): | ||
| """`class Inner { } inst;` declares both a type and a member.""" | ||
| p = tmp_path / "both.h" | ||
| p.write_text( | ||
| "class Outer\n" | ||
| "{\n" | ||
| "public:\n" | ||
| " class Inner { int x; } inst;\n" | ||
| "};\n" | ||
| ) | ||
| labels = _labels(p) | ||
| assert "Inner" in labels | ||
| assert "inst" in labels | ||
|
|
||
|
|
||
| def test_cpp_cli_class_body_survives(tmp_path): | ||
| p = tmp_path / "cli.h" | ||
| p.write_text( | ||
| "namespace N {\n" | ||
| " public ref class Wrapper\n" | ||
| " {\n" | ||
| " public:\n" | ||
| " static void Init() { }\n" | ||
| ' static System::String^ Name() { return gcnew System::String(""); }\n' | ||
| " };\n" | ||
| "}\n" | ||
| ) | ||
| result = extract_cpp(p) | ||
| assert result.get("parse_errors") is None | ||
| labels = [n["label"] for n in result["nodes"]] | ||
| assert "Wrapper" in labels | ||
| assert ".Init()" in labels | ||
| assert ".Name()" in labels | ||
| # recovery no longer invents a `Wrapper()` free function or a `public` node | ||
| assert "Wrapper()" not in labels | ||
| assert "public" not in labels | ||
|
|
||
|
|
||
| def test_cli_normalization_preserves_byte_offsets(tmp_path): | ||
| src = ( | ||
| '[assembly:AssemblyVersion("1.0")];\n' | ||
| "public ref struct S { void F(System::Object^ o, int% n) { gcnew S(); } };\n" | ||
| ).encode() | ||
| out = _normalize_cpp_cli(src) | ||
| assert out is not None | ||
| assert len(out) == len(src) | ||
| # every line still starts at the same offset | ||
| assert [i for i, b in enumerate(src) if b == 0x0A] == [ | ||
| i for i, b in enumerate(out) if b == 0x0A | ||
| ] | ||
| assert b"ref struct" not in out | ||
| assert b"gcnew" not in out | ||
| assert b"assembly" not in out | ||
|
|
||
|
|
||
| def test_plain_cpp_is_not_rewritten(tmp_path): | ||
| src = b"int f(int a, int b) { return (a ^ b) % 7; }\n" | ||
| assert _normalize_cpp_cli(src) is None | ||
|
|
||
|
|
||
| def test_operators_survive_in_a_cli_file(tmp_path): | ||
| """The `^`/`%` rewrite only touches the suffix spelling, not the operators.""" | ||
| src = b"ref class C { int f(int a, int b) { return (a ^ b) % 7; } };\n" | ||
| out = _normalize_cpp_cli(src) | ||
| assert out is not None | ||
| assert b"(a ^ b) % 7" in out | ||
|
|
||
|
|
||
| def test_multiline_cli_attribute_keeps_line_numbers(tmp_path): | ||
| """A removed token that spans lines must keep its line breaks. | ||
|
|
||
| `[assembly:AssemblyVersion(\n "1.0"\n)]` is ordinary formatting. Blanking | ||
| its newlines preserved byte length but merged source lines, so every symbol | ||
| below it reported a line number that was too low. | ||
| """ | ||
| p = tmp_path / "cli.h" | ||
| p.write_text( | ||
| "[assembly:AssemblyVersion(\n" | ||
| ' "1.0.0.0"\n' | ||
| ")];\n" | ||
| "namespace N {\n" | ||
| " public ref class Wrapper\n" | ||
| " {\n" | ||
| " public:\n" | ||
| " static void Init() { }\n" | ||
| " };\n" | ||
| "}\n" | ||
| ) | ||
| nodes = {n["label"]: n["source_location"] for n in extract_cpp(p)["nodes"]} | ||
| assert nodes["Wrapper"] == "L5" | ||
| assert nodes[".Init()"] == "L8" | ||
|
|
||
|
|
||
| def test_cli_normalization_preserves_line_breaks(tmp_path): | ||
| src = ( | ||
| "[assembly:AssemblyVersion(\n" | ||
| ' "1.0.0.0"\n' | ||
| ")];\n" | ||
| "namespace N {\n" | ||
| " public\n" # access specifier split from the keyword | ||
| " ref class W { };\n" | ||
| "}\n" | ||
| ).encode() | ||
| out = _normalize_cpp_cli(src) | ||
| assert out is not None | ||
| assert len(out) == len(src) | ||
| assert [i for i, b in enumerate(src) if b == 0x0A] == [ | ||
| i for i, b in enumerate(out) if b == 0x0A | ||
| ] | ||
| assert b"ref class" not in out | ||
| assert b"assembly" not in out | ||
|
|
||
|
|
||
| def test_attached_arithmetic_is_not_mistaken_for_a_handle(tmp_path): | ||
| """`a% b` and `hash^ mask` are modulo and XOR, not CLI type suffixes. | ||
|
|
||
| Attachment to the preceding token does not separate the two readings — | ||
| `String^ s` and `a^ b` are lexically identical — so rewriting on | ||
| attachment alone turned arithmetic into `a b` and broke the statement. | ||
| """ | ||
| p = tmp_path / "ops.h" | ||
| p.write_text( | ||
| "namespace N {\n" | ||
| " public ref class Hasher\n" | ||
| " {\n" | ||
| " public:\n" | ||
| ' static System::String^ Name() { return gcnew System::String(""); }\n' | ||
| " static int Mix(int a, int b) { return a% b; }\n" | ||
| " static int Fold(int hash, int mask) { return hash^ mask; }\n" | ||
| " static void Track(System::Object^ o, int% n) { }\n" | ||
| " };\n" | ||
| "}\n" | ||
| ) | ||
| result = extract_cpp(p) | ||
| assert result.get("parse_errors") is None | ||
| labels = [n["label"] for n in result["nodes"]] | ||
| for expected in ("Hasher", ".Name()", ".Mix()", ".Fold()", ".Track()"): | ||
| assert expected in labels | ||
|
|
||
| # The operator characters themselves survive in the parsed source. | ||
| out = _normalize_cpp_cli(p.read_bytes()) | ||
| assert b"return a% b;" in out | ||
| assert b"return hash^ mask;" in out | ||
|
|
||
|
|
||
| def test_screaming_case_constants_stay_arithmetic(tmp_path): | ||
| """`MASK^ value` is XOR against a constant, not a `MASK^` handle. | ||
|
|
||
| SCREAMING_CASE is the macro/constant convention and never a .NET type | ||
| name, so a capitalized left side must also carry a lowercase letter. A | ||
| lone capital stays a type position for generic parameters (`T^ x`). | ||
| """ | ||
| p = tmp_path / "consts.h" | ||
| p.write_text( | ||
| '''namespace N { | ||
| public ref class Hasher | ||
| { | ||
| public: | ||
| static int Fold(int value) { return MASK^ value; } | ||
| static int Trim(int value) { return LIMIT% value; } | ||
| static System::Object^ Box(T^ item) { return gcnew System::Object(); } | ||
| }; | ||
| } | ||
| ''' | ||
| ) | ||
| out = _normalize_cpp_cli(p.read_bytes()) | ||
| assert b"return MASK^ value;" in out | ||
| assert b"return LIMIT% value;" in out | ||
| assert b"System::Object Box(T item)" in out | ||
|
|
||
| result = extract_cpp(p) | ||
| assert result.get("parse_errors") is None | ||
| assert ".Fold()" in _labels(p) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("expr", [ | ||
| b"int m = a% b;", | ||
| b"int x = hash^ mask;", | ||
| b"int c = count% 2;", | ||
| b"int d = (a ^ b) % 7;", | ||
| b"int e = a^*p;", | ||
| b"int f = a^&b;", | ||
| b"x %= y;", | ||
| ]) | ||
| def test_arithmetic_forms_survive(expr): | ||
| src = b"ref class C { void f() { " + expr + b" } };" | ||
| out = _normalize_cpp_cli(src) | ||
| assert out is not None | ||
| assert expr in out, f"{expr!r} was rewritten" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("decl,rewritten", [ | ||
| (b"System::String^ s;", b"System::String s;"), | ||
| (b"List<int>^ items;", b"List<int> items;"), | ||
| (b"DataTable^ t;", b"DataTable t;"), | ||
| (b"int% n;", b"int n;"), | ||
| (b"void F(String^, int);", b"void F(String , int);"), | ||
| (b"array<String^>^ a;", b"array<String > a;"), | ||
| ]) | ||
| def test_cli_type_suffixes_are_still_rewritten(decl, rewritten): | ||
| src = b"ref class C { " + decl + b" };" | ||
| out = _normalize_cpp_cli(src) | ||
| assert out is not None | ||
| assert len(out) == len(src) | ||
| assert rewritten in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract_cpp()24 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.