diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index a5dc18c36..33985c710 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -6,6 +6,89 @@ from pathlib import Path from graphify.extractors.base import _file_stem, _make_id +# Recovers CREATE FUNCTION/PROCEDURE statements the grammar could not parse +# structurally. Used by BOTH recovery sites — the walk-time ERROR-node scan and +# the whole-file has_error fallback (#2180). They MUST share one pattern: when +# they disagreed, the same statement produced two nodes with different names +# (the ERROR scan captured `dbo.[usp_Mixed]`, the fallback stopped at `dbo`, +# and _add_node's id-dedupe never fired because the ids differed). +# +# Each name part is a bare identifier, a double-quoted (delimited) one, or a +# T-SQL bracket-delimited one, so CREATE OR REPLACE FUNCTION "public"."fn"(...) +# and CREATE PROCEDURE [dbo].[usp_Load] ... are both recovered. A bare [\w$.]+ +# stops dead at the leading delimiter, which silently dropped every quoted +# PL/pgSQL routine (#2180) and every bracket-named T-SQL procedure. T-SQL's +# AS BEGIN...END body idiom always lands in recovery — the grammar has no +# create_procedure parse for it — and T-SQL spells re-creation CREATE OR ALTER +# (it has no OR REPLACE), so accept that form too, mirroring fb_proc_or_trigger. +# Inside a bracket-delimited part, a literal ] is escaped by doubling +# ([a]]b] names the identifier a]b), so consume ]] before treating a +# lone ] as the closing delimiter — stopping at the first ] truncated +# the name and minted a phantom that could collide with a real [a]. +# PROC is T-SQL's official shorthand for PROCEDURE and equally common in the +# wild; the optional (?:EDURE)? still requires trailing whitespace, so a word +# that merely starts with PROC cannot match. +_ROUTINE_RECOVERY_RX = re.compile( + r"CREATE\s+(?:OR\s+(?:REPLACE|ALTER)\s+)?(?:FUNCTION|PROC(?:EDURE)?)\s+" + r"(?:IF\s+NOT\s+EXISTS\s+)?" + r"((?:\"(?:[^\"\n]|\"\")+\"|\[(?:[^\]\n]|\]\])+\]|[\w$]+)" + r"(?:\s*\.\s*(?:\"(?:[^\"\n]|\"\")+\"|\[(?:[^\]\n]|\]\])+\]|[\w$]+))*)", + re.IGNORECASE, +) + +# Matches literals or comments for _mask_sql_comments. Literals are matched +# first so a comment opener INSIDE a literal ('-- not a comment', [a--b]) is +# not treated as a comment — but they are not all treated alike: +# +# - single-quoted strings are BLANKED like comments: routine names never live +# in single quotes, and dynamic SQL (EXEC(N'CREATE PROC [dbo].[Fake] ...')) +# would otherwise fabricate a routine node whenever an unrelated parse +# error arms the whole-file scan; +# - double-quoted and bracket-delimited identifiers are PRESERVED verbatim — +# they are exactly the delimited names the recovery regex must see ("" and +# ]] escapes consumed, mirroring _ROUTINE_RECOVERY_RX). +# +# Literal patterns are deliberately single-line: this mask only runs on files +# that already failed to parse, where an unclosed quote is likely, and a +# multi-line literal match would let one unclosed delimiter swallow real DDL +# below it — a masked-away routine (false negative) is recoverable by fixing +# the file, a swallowed one is silent. A comment opener inside a multi-line +# string therefore still masks to end-of-line, which can only hide a routine, +# never fabricate one. DOTALL so a block comment may span lines; an UNCLOSED +# block comment runs to end-of-file (matching SQL semantics) — requiring the +# closing */ left everything after an unterminated /* unmasked, and an +# unterminated comment is exactly the kind of error that arms recovery. +_SQL_COMMENT_OR_LITERAL_RX = re.compile( + r"('(?:[^'\n]|'')*')" # group 1: single-quoted string — blanked + r"|\"(?:[^\"\n]|\"\")*\"" # double-quoted identifier — preserved + r"|\[(?:[^\]\n]|\]\])*\]" # bracket-delimited identifier — preserved + r"|(--[^\n]*|/\*.*?(?:\*/|\Z))", # group 2: comment — blanked + re.DOTALL, +) + + +def _mask_sql_comments(text: str) -> str: + """Blank comment and string-literal spans, preserving every offset. + + Non-newline characters inside a blanked span become spaces and newlines + are kept, so positions and line numbers computed against the masked text + are valid against the original. Double-quoted and bracket-delimited + identifiers are preserved verbatim (they carry recoverable routine + names); single-quoted strings are blanked (they can carry dynamic SQL + that must not be recovered), and a `--` or `/*` inside any literal does + not start a comment. Used by both routine-recovery scans so commented-out + or string-embedded CREATE PROCEDURE/FUNCTION DDL in a file that has an + unrelated parse error cannot fabricate a routine node. + """ + return _SQL_COMMENT_OR_LITERAL_RX.sub( + lambda m: ( + "".join("\n" if c == "\n" else " " for c in m.group(0)) + if m.group(1) or m.group(2) + else m.group(0) + ), + text, + ) + def _norm_ident(name: str) -> str: """Normalize a SQL identifier for name-based reference resolution. @@ -254,18 +337,14 @@ def walk(node) -> None: # do not scan the body for FROM/JOIN references: PL/pgSQL loop # variables and locals would produce junk reads_from targets. # - # Each name part is either a bare identifier or a double-quoted - # (delimited) one, so schema-qualified generated DDL such as - # CREATE OR REPLACE FUNCTION "public"."fn"(...) is recovered too. - # A bare [\w$.]+ stops dead at the leading quote, which silently - # dropped every quoted PL/pgSQL routine (#2180). - text = _read(node) - for m in re.finditer( - r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" - r"(?:IF\s+NOT\s+EXISTS\s+)?" - r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", - text, re.IGNORECASE, - ): + # Name and keyword shapes accepted here are defined once in + # _ROUTINE_RECOVERY_RX, shared with the whole-file fallback below — + # see the comment on the constant. Comments are masked the same + # way too: an ERROR blob can swallow commented-out DDL along with + # the statements around it, and unmasked text would fabricate a + # routine node from it exactly as the whole-file scan once did. + text = _mask_sql_comments(_read(node)) + for m in _ROUTINE_RECOVERY_RX.finditer(text): name = m.group(1) m_line = line + text[: m.start()].count("\n") nid = _make_id(stem, name) @@ -429,8 +508,9 @@ def _collect_defined_names(node) -> None: # (keyword_create/keyword_function/object_reference/... ) and the ERROR # node holds only the offending body line, e.g. `PERFORM x();` or # `x := 1;` -- so no CREATE text is inside any ERROR node at all - # 3. the name is a quoted identifier ("public"."fn"), which a bare - # [\w$.]+ pattern cannot match + # 3. the name is a delimited identifier — quoted ("public"."fn") or + # T-SQL-bracketed ([dbo].[usp_Load]) — which a bare [\w$.]+ pattern + # cannot match # Shapes 2 and 3 silently dropped the routine: no node, no warning, exit 0. # Scanning the raw source catches all three, and _add_node dedupes by id so # routines already recovered from the tree are not emitted twice. @@ -441,12 +521,10 @@ def _collect_defined_names(node) -> None: # observed drop shape leaves an ERROR node in the tree, so has_error loses # nothing while protecting clean corpora (#2180 follow-up). if root.has_error: - for m in re.finditer( - r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" - r"(?:IF\s+NOT\s+EXISTS\s+)?" - r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", - src_text, re.IGNORECASE, - ): + # Comments are masked (offset-preserving) so commented-out DDL cannot + # fabricate a routine when an unrelated error arms this scan; DDL + # inside string bodies remains covered only by the has_error gate. + for m in _ROUTINE_RECOVERY_RX.finditer(_mask_sql_comments(src_text)): fn_name = m.group(1) fn_line = src_text[: m.start()].count("\n") + 1 _add_node(_make_id(stem, fn_name), f"{fn_name}()", fn_line) diff --git a/tests/test_multilang.py b/tests/test_multilang.py index cb390eebc..6ece59ed8 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -487,6 +487,220 @@ def test_sql_no_dangling_edges(): for e in r["edges"]: assert e["source"] in node_ids, f"dangling source: {e['source']}" +def test_sql_tsql_bracketed_procedure_is_recovered(tmp_path): + """T-SQL CREATE PROCEDURE [Schema].[Name] ... AS BEGIN...END. + + The grammar has no create_procedure parse for T-SQL's AS BEGIN...END body + idiom, so the statement lands in ERROR recovery — where a name pattern + without a bracket-delimited alternative recovered nothing. In a T-SQL + codebase that brackets every identifier (a common house standard), that + made every stored procedure invisible. + """ + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "proc.sql" + p.write_text( + "CREATE PROCEDURE [dbo].[usp_LoadDebtors]\n" + " @BatchId INT\n" + "AS\n" + "BEGIN\n" + " SET NOCOUNT ON;\n" + " INSERT INTO dbo.Debtors (Id) SELECT Id FROM staging.Debtors;\n" + "END;\n" + ) + r = extract_sql(p) + routine = [n["label"] for n in r["nodes"] if n["label"] != "proc.sql"] + assert routine == ["[dbo].[usp_LoadDebtors]()"], routine + + +def test_sql_tsql_create_or_alter_procedure_is_recovered(tmp_path): + """T-SQL spells idempotent re-creation CREATE OR ALTER (no OR REPLACE).""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "proc.sql" + p.write_text( + "CREATE OR ALTER PROCEDURE [Utils].[ValidateSourceView]\n" + "AS\nBEGIN\n SELECT 1;\nEND;\n" + "CREATE OR ALTER PROCEDURE usp_Bare AS\nBEGIN\n SELECT 1;\nEND;\n" + ) + r = extract_sql(p) + labels = sorted(n["label"] for n in r["nodes"] if n["label"] != "proc.sql") + assert labels == ["[Utils].[ValidateSourceView]()", "usp_Bare()"], labels + + +def test_sql_escaped_closing_bracket_in_routine_name_is_consumed(tmp_path): + """T-SQL escapes a literal ] inside a bracketed identifier by doubling it: + [a]]b] names the identifier a]b. A pattern that stops at the first ] + truncated the name to [dbo].[a] — a phantom that could collide with a + genuinely-named [dbo].[a].""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "proc.sql" + p.write_text("CREATE PROCEDURE [dbo].[a]]b]\nAS\nBEGIN\n SELECT 1;\nEND;\n") + r = extract_sql(p) + routine = [n["label"] for n in r["nodes"] if n["label"] != "proc.sql"] + assert routine == ["[dbo].[a]]b]()"], routine + + +def test_sql_recovery_sites_agree_on_the_captured_name(tmp_path): + """A mixed-delimiter name (dbo.[usp_Mixed]) must yield exactly ONE node. + + The walk-time ERROR scan and the whole-file has_error fallback recover the + same statement; when their patterns drifted, they captured different names + (`dbo.[usp_Mixed]` vs `dbo`), minted different ids, and _add_node's + id-dedupe never fired — one procedure became two nodes, one of them a + phantom named after the schema. Sharing _ROUTINE_RECOVERY_RX makes the + drift impossible; this pins the observable behaviour. + """ + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "proc.sql" + p.write_text("CREATE PROCEDURE dbo.[usp_Mixed] AS\nBEGIN\n SELECT 1;\nEND;\n") + r = extract_sql(p) + routine = [n["label"] for n in r["nodes"] if n["label"] != "proc.sql"] + assert routine == ["dbo.[usp_Mixed]()"], routine + + +def test_sql_tsql_proc_shorthand_is_recovered(tmp_path): + """T-SQL's official PROC shorthand, including CREATE OR ALTER PROC with a + qualified bracket-delimited name, must recover like the long form.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "proc.sql" + p.write_text( + "CREATE OR ALTER PROC [dbo].[usp_Short]\n" + "AS\nBEGIN\n SELECT 1;\nEND;\n" + "CREATE PROC usp_BareShort AS\nBEGIN\n SELECT 1;\nEND;\n" + ) + r = extract_sql(p) + labels = sorted(n["label"] for n in r["nodes"] if n["label"] != "proc.sql") + assert labels == ["[dbo].[usp_Short]()", "usp_BareShort()"], labels + + +def test_sql_commented_ddl_is_not_fabricated_by_error_recovery(tmp_path): + """An unrelated syntax error arms the whole-file recovery scan; DDL that + exists only inside comments must not fabricate routine nodes from it.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "broken.sql" + p.write_text( + "-- CREATE PROCEDURE [dbo].[usp_LineComment] AS BEGIN SELECT 1; END;\n" + "/*\nCREATE OR ALTER PROC dbo.usp_BlockComment AS\nBEGIN SELECT 1; END;\n*/\n" + "CREATE PROCEDURE [dbo].[usp_Real]\nAS\nBEGIN\n SELECT 1;\nEND;\n" + "THIS IS NOT SQL AT ALL %%%;\n" + # Sandwiched between broken segments so one ERROR blob's byte span + # covers the comment: the walk-time ERROR scan sees it too (the + # leading comments above land in proper comment nodes instead, and a + # trailing comment falls outside the ERROR span): + "%%% BROKEN JUNK\n" + "-- CREATE PROC [dbo].[usp_ErrComment] AS BEGIN SELECT 1; END;\n" + "%%% MORE BROKEN JUNK\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"] if n["label"] != "broken.sql"] + assert "[dbo].[usp_Real]()" in labels, labels + assert not any("Comment" in l for l in labels), ( + f"commented-out DDL fabricated a node: {labels}" + ) + + +def test_sql_comment_openers_inside_string_literals_do_not_hide_ddl(tmp_path): + """A `--` or `/*` inside a string literal is data, not a comment opener. + + Both recovery scans mask comments before matching, and a mask that reads + `'-- note'` as a line comment blanks to end-of-line — hiding a routine + declared after the literal — while a `/*` inside a string blanks + everything through the next real `*/`, swallowing whole statements + between. Both must recover normally. This is behavior coverage: the + grammar's own fb_proc_or_trigger recovery can rescue these shapes even + under a literal-blind mask, so the mutation-killing pin for the mask + itself is test_mask_sql_comments_preserves_literals_and_blanks_comments. + """ + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "strings.sql" + # The lines carrying the literals start with junk so they cannot parse as + # proper statements: literal and routine land in the SAME ERROR blob, and + # only the mask decides whether the routine survives. (A literal in a + # statement that parses never reaches the masked scans at all.) + p.write_text( + "%%% INSERT INTO log VALUES ('-- note'); " + "CREATE PROCEDURE [dbo].[usp_AfterLineString] AS BEGIN SELECT 1; END;\n" + "%%% SELECT 'open /* here' AS x; " + "CREATE PROCEDURE [dbo].[usp_AfterBlockString] AS BEGIN SELECT 1; END;\n" + "/* a real comment, giving the false opener a closer to reach */\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"] if n["label"] != "strings.sql"] + assert "[dbo].[usp_AfterLineString]()" in labels, labels + assert "[dbo].[usp_AfterBlockString]()" in labels, labels + + +def test_mask_sql_comments_literal_and_comment_handling(): + """Unit pin for _mask_sql_comments: comment openers inside literals are + data, not comment starts; single-quoted strings are blanked (dynamic SQL + must not be recoverable); double-quoted and bracket identifiers are + preserved verbatim (they carry recoverable names); comments blank to + spaces with newlines and offsets preserved, an unclosed block comment + running to end-of-file.""" + from graphify.extractors.sql import _mask_sql_comments as mask + + # a comment opener inside a literal never blanks past the literal + assert mask("select '-- x' from t") == "select from t" + assert mask("select 'a /* b' as x, 1") == "select as x, 1" + # '' escape is consumed as one literal (no blanking past the string) + src = "select 'it''s -- ok', 1" + expected = "select " + " " * len("'it''s -- ok'") + ", 1" + assert mask(src) == expected and len(mask(src)) == len(src) + # identifier literals are preserved verbatim, escapes included + assert mask('select "a--b" from t') == 'select "a--b" from t' + assert mask('CREATE FUNCTION "public"."a""b"()') == 'CREATE FUNCTION "public"."a""b"()' + assert mask("select [a--b] from t") == "select [a--b] from t" + # dynamic SQL contents cannot survive the mask + dyn = "EXEC(N'CREATE PROC [dbo].[Fake] AS BEGIN SELECT 1; END');" + assert "CREATE" not in mask(dyn) and len(mask(dyn)) == len(dyn) + # comments blank to spaces, newlines kept, offsets stable + assert mask("a -- b") == "a " + assert mask("x /* y */ z") == "x z" + masked = mask("a /* m\nl */ b") + assert masked == "a \n b" and len(masked) == len("a /* m\nl */ b") + # an unclosed block comment masks to end-of-file, newlines kept + unclosed = "/* CREATE PROC [dbo].[Ghost] AS\nBEGIN SELECT 1; END" + assert "CREATE" not in mask(unclosed) and len(mask(unclosed)) == len(unclosed) + + +def test_sql_dynamic_sql_and_unclosed_comment_do_not_fabricate_routines(tmp_path): + """DDL text reachable only through a single-quoted string (dynamic SQL) or + an unterminated block comment must not mint routine nodes when an + unrelated parse error arms the recovery scans.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "dynamic.sql" + p.write_text( + "THIS IS NOT SQL AT ALL %%%;\n" + "EXEC(N'CREATE OR ALTER PROC [dbo].[usp_Dynamic] AS BEGIN SELECT 1; END');\n" + "CREATE PROCEDURE [dbo].[usp_Real]\nAS\nBEGIN\n SELECT 1;\nEND;\n" + "/* an unterminated comment swallows the rest of the file\n" + "CREATE PROC [dbo].[usp_Unterminated] AS BEGIN SELECT 1; END;\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"] if n["label"] != "dynamic.sql"] + assert "[dbo].[usp_Real]()" in labels, labels + assert not any("Dynamic" in label or "Unterminated" in label for label in labels), ( + f"string-embedded or comment-swallowed DDL fabricated a node: {labels}" + ) + + +def test_sql_escaped_double_quote_in_routine_name_is_consumed(tmp_path): + """ANSI SQL escapes a literal double quote inside a delimited identifier + by doubling it: "a""b" names the identifier a"b. A pattern that stops at + the first closing quote truncated the recovered name to "dbo"."a" — the + "" twin of the bracket ]] escape handled above. + + The AS BEGIN body idiom keeps the whole statement in ERROR recovery: for a + statement the grammar CAN parse, tree-sitter-sql itself truncates the + object_reference at the "" escape (emitting `"b"` as a stray ERROR child), + which is an upstream grammar defect this extractor cannot repair.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "fn.sql" + p.write_text('CREATE PROCEDURE "dbo"."a""b"\nAS\nBEGIN\n SELECT 1;\nEND;\n') + r = extract_sql(p) + routine = [n["label"] for n in r["nodes"] if n["label"] != "fn.sql"] + assert routine == ['"dbo"."a""b"()'], routine + + def test_sql_cte_is_not_read_as_a_table(): """#2577: a name bound by WITH ... AS (...) is scoped to its statement, not a table.