Fix T-SQL bracket-quoted identifiers producing mangled labels

graphify/extractors/sql.py

@@ -27,6 +27,112 @@ def _norm_ident(name: str) -> str:
     return ".".join(parts)
 
 
+def _debracket_tsql(source: bytes) -> tuple[bytes, bool]:
+    """Rewrite T-SQL `[bracket]`-quoted identifiers to `` `backtick` ``-quoted ones.
+
+    tree-sitter-sql has no grammar token for T-SQL bracket quoting: each `[`
+    and `]` lands as its own one-byte ERROR node, one character short of the
+    real pair. That shifts every subsequent token in the statement by one
+    byte, which corrupts the object_reference text — `[dbo].[Customer]` reads
+    back as `dbo].[Customer` — so the node label keeps a stray bracket
+    fragment on each side of the dot instead of the real name (#2712).
+
+    Backtick quoting parses as a clean atomic `identifier` token (the
+    grammar's MySQL-dialect support) and is not otherwise valid T-SQL syntax,
+    so substituting one for the other before parsing sidesteps the grammar
+    gap rather than trying to patch the corrupted text after the fact. `]]`
+    is T-SQL's escape for a literal `]` inside a bracketed name; it is
+    unescaped here and re-escaped as a doubled backtick if needed.
+
+    Only scans outside `'...'` string literals, `--` line comments, and
+    `/* */` block comments, so a literal `[` in either is left untouched. A
+    `[...]` span is left alone (not substituted) when it is unterminated,
+    empty, spans a newline, or its content is purely numeric — those are
+    Postgres/MySQL array-type syntax (`int[]`, `numeric(10)[3]`), never a
+    valid T-SQL identifier, and must not be corrupted.
+
+    Returns ``(source, False)`` unchanged if nothing qualified.
+    """
+    out = bytearray()
+    i, n = 0, len(source)
+    changed = False
+    while i < n:
+        c = source[i]
+        if c == ord("-") and i + 1 < n and source[i + 1] == ord("-"):
+            j = source.find(b"\n", i)
+            j = n if j == -1 else j
+            out += source[i:j]
+            i = j
+            continue
+        if c == ord("/") and i + 1 < n and source[i + 1] == ord("*"):
+            j = source.find(b"*/", i + 2)
+            j = n if j == -1 else j + 2
+            out += source[i:j]
+            i = j
+            continue
+        if c == ord("'"):
+            j = i + 1
+            while j < n:
+                if source[j] == ord("'"):
+                    if j + 1 < n and source[j + 1] == ord("'"):
+                        j += 2
+                        continue
+                    j += 1
+                    break
+                j += 1
+            out += source[i:j]
+            i = j
+            continue
+        if c == ord("["):
+            j = i + 1
+            content = bytearray()
+            terminated = False
+            while j < n:
+                if source[j] == ord("]"):
+                    if j + 1 < n and source[j + 1] == ord("]"):
+                        content.append(ord("]"))
+                        j += 2
+                        continue
+                    j += 1
+                    terminated = True
+                    break
+                if source[j] == ord("\n"):
+                    break
+                content.append(source[j])
+                j += 1
+            if not terminated or not content or content.strip().isdigit():
+                out.append(c)
+                i += 1
+                continue
+            changed = True
+            out.append(0x60)
+            out += bytes(content).replace(b"`", b"``")
+            out.append(0x60)
+            i = j
+            continue
+        out.append(c)
+        i += 1
+    return bytes(out), changed
+
+
+def _strip_backtick_parts(name: str) -> str:
+    """Undo `_debracket_tsql`'s synthetic backtick-quoting for a display label.
+
+    Splits on `.` and strips a matching pair of backticks (unescaping a
+    doubled backtick back to one) from each part. Only ever called on source
+    that `_debracket_tsql` has already confirmed contains no genuine
+    backtick, so every backtick encountered here is one it introduced;
+    double-quoted (ANSI/Postgres) identifiers are untouched either way.
+    """
+    parts = []
+    for part in name.split("."):
+        p = part.strip()
+        if len(p) >= 2 and p[0] == "`" and p[-1] == "`":
+            p = p[1:-1].replace("``", "`")
+        parts.append(p)
+    return ".".join(parts)
+
+
 def extract_sql(path: Path, content: str | bytes | None = None) -> dict:
     """Extract tables, views, functions, and relationships from .sql files via tree-sitter."""
     try:
@@ -53,6 +159,16 @@ def extract_sql(path: Path, content: str | bytes | None = None) -> dict:
             else content if content is not None
             else path.read_bytes()
         )
+        # A backtick already in the source means it's either genuinely
+        # backtick-quoted (MySQL dialect) or mixes dialects in a way this
+        # module cannot safely disambiguate; skip debracketing rather than
+        # risk misreading a real backtick as one we introduced (#2712).
+        debracketed = False
+        if b"`" not in source:
+            _new_source, _changed = _debracket_tsql(source)
+            if _changed:
+                source = _new_source
+                debracketed = True
         tree = parser.parse(source)
         root = tree.root_node
     except Exception as e:
@@ -71,10 +187,18 @@ def extract_sql(path: Path, content: str | bytes | None = None) -> dict:
     def _read(n) -> str:
         return source[n.start_byte:n.end_byte].decode("utf-8", errors="replace")
 
+    def _clean_name(name: str) -> str:
+        """Strip synthetic backtick-quoting from a name, when this file was debracketed."""
+        return _strip_backtick_parts(name) if debracketed else name
+
+    def _ident(n) -> str:
+        """Read an identifier/object_reference node as a clean display name."""
+        return _clean_name(_read(n))
+
     def _obj_name(n) -> str | None:
         for c in n.children:
             if c.type == "object_reference":
-                return _read(c)
+                return _ident(c)
         return None
 
     def _add_node(nid: str, label: str, line: int) -> None:
@@ -138,7 +262,7 @@ def walk(node) -> None:
                                     if cc.type == "keyword_references":
                                         found_ref = True
                                     elif found_ref and cc.type == "object_reference":
-                                        ref_name = _read(cc)
+                                        ref_name = _ident(cc)
                                         break
                                 if ref_name:
                                     ref_nid = table_nids.get(_norm_ident(ref_name)) or _ref_stub(ref_name)
@@ -155,7 +279,7 @@ def walk(node) -> None:
                                         if cc.type == "keyword_references":
                                             found_ref = True
                                         elif found_ref and cc.type == "object_reference":
-                                            ref_name = _read(cc)
+                                            ref_name = _ident(cc)
                                             break
                                     if ref_name:
                                         ref_nid = table_nids.get(_norm_ident(ref_name)) or _ref_stub(ref_name)
@@ -216,7 +340,7 @@ def walk(node) -> None:
                                 if ccc.type == "keyword_references":
                                     found_ref = True
                                 elif found_ref and ccc.type == "object_reference":
-                                    ref_name = _read(ccc)
+                                    ref_name = _ident(ccc)
                                     break
                             if ref_name:
                                 ref_nid = (table_nids.get(_norm_ident(ref_name))
@@ -232,11 +356,11 @@ def walk(node) -> None:
                 if c.type == "keyword_trigger":
                     after_trigger = True
                 elif after_trigger and not trig_name and c.type == "object_reference":
-                    trig_name = _read(c)
+                    trig_name = _ident(c)
                 elif c.type == "keyword_for":
                     after_for = True
                 elif after_for and not tbl_name and c.type == "object_reference":
-                    tbl_name = _read(c)
+                    tbl_name = _ident(c)
             if trig_name:
                 trig_nid = _make_id(stem, trig_name)
                 _add_node(trig_nid, trig_name, line)
@@ -357,7 +481,7 @@ def _walk_from_refs(node, caller_nid: str, line: int,
                 if c.type == "relation":
                     for cc in c.children:
                         if cc.type == "object_reference":
-                            tbl = _read(cc)
+                            tbl = _ident(cc)
                             if _norm_ident(tbl) in cte_names:
                                 continue
                             tbl_nid = table_nids.get(_norm_ident(tbl)) or _ref_stub(tbl)

tests/test_multilang.py

@@ -672,6 +672,67 @@ def test_sql_schema_qualified_alter_fk():
         assert e["source"] in node_ids, f"dangling source: {e['source']}"
         assert e["target"] in node_ids, f"dangling target: {e['target']}"
 
+def test_sql_tsql_bracket_identifiers_produce_clean_labels(tmp_path):
+    """#2712: [dbo].[Alpha] must label as dbo.Alpha, not the delimiter-mangled
+    `dbo].[Alpha` tree-sitter-sql's grammar produces for bracket quoting (it has
+    no token for `[...]`, so each bracket lands as its own one-byte ERROR node,
+    one character short of the real pair)."""
+    pytest.importorskip("tree_sitter_sql")
+    p = tmp_path / "schema.sql"
+    p.write_text(
+        "CREATE TABLE [dbo].[Alpha] (\n"
+        "    [Id] INT NOT NULL PRIMARY KEY\n"
+        ");\n"
+        "GO\n"
+        "CREATE TABLE [dbo].[Beta] (\n"
+        "    [Id] INT NOT NULL PRIMARY KEY\n"
+        ");\n"
+        "GO\n"
+    )
+    r = extract_sql(p)
+    labels = [n["label"] for n in r["nodes"]]
+    assert "dbo.Alpha" in labels, f"got {labels}"
+    assert "dbo.Beta" in labels, f"got {labels}"
+    assert not any("]" in l or "[" in l for l in labels), (
+        f"a bracket fragment leaked into a label: {labels}"
+    )
+
+def test_sql_tsql_bracket_reference_resolves_by_clean_name(tmp_path):
+    """#2712: a bracket-quoted FOREIGN KEY ... REFERENCES [dbo].[Alpha] must
+    resolve onto the real Alpha table node, not dangle or mint a stub keyed by
+    the mangled `Alpha` text."""
+    pytest.importorskip("tree_sitter_sql")
+    p = tmp_path / "schema.sql"
+    p.write_text(
+        "CREATE TABLE [dbo].[Alpha] ([Id] INT NOT NULL PRIMARY KEY);\n"
+        "GO\n"
+        "CREATE TABLE [dbo].[Beta] (\n"
+        "    [Id] INT NOT NULL PRIMARY KEY,\n"
+        "    [AlphaId] INT NOT NULL,\n"
+        "    CONSTRAINT [FK_Beta_Alpha] FOREIGN KEY ([AlphaId])\n"
+        "        REFERENCES [dbo].[Alpha] ([Id])\n"
+        ");\n"
+        "GO\n"
+    )
+    r = extract_sql(p)
+    nid = {n["label"]: n["id"] for n in r["nodes"]}
+    assert "dbo.Alpha" in nid and "dbo.Beta" in nid
+    refs = {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == "references"}
+    assert (nid["dbo.Beta"], nid["dbo.Alpha"]) in refs, f"got {refs}"
+
+def test_sql_tsql_bracket_debracketing_does_not_corrupt_array_types(tmp_path):
+    """#2712 follow-up: the bracket->backtick rewrite must not misfire on
+    Postgres/MySQL array-type syntax (`text[]`, `numeric(10)[3]`), which uses
+    `[...]` for something other than a T-SQL quoted identifier."""
+    pytest.importorskip("tree_sitter_sql")
+    p = tmp_path / "schema.sql"
+    p.write_text("CREATE TABLE t (id INT, tags text[], scores numeric(10,2)[3]);\n")
+    r = extract_sql(p)
+    labels = [n["label"] for n in r["nodes"]]
+    assert any(l == "t" for l in labels), f"table extraction broke on array types: {labels}"
+    for l in labels:
+        assert "`" not in l, f"a synthetic backtick leaked into a label: {labels}"
+
 def test_sql_plpgsql_functions_survive_parse_errors():
     """PL/pgSQL bodies make tree-sitter-sql emit ERROR nodes; the functions
     must still be extracted (#1910), without cascading into later statements."""