From e1d7e79cfe107699add20151833431733e8992d1 Mon Sep 17 00:00:00 2001 From: Eduardo Garcia-Prieto Date: Sun, 23 Aug 2026 00:40:07 +1000 Subject: [PATCH 1/4] fix(sql): recover T-SQL bracket-named and CREATE OR ALTER routines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T-SQL's AS BEGIN...END body idiom never parses structurally (the grammar has no create_procedure form for it), so recovery from ERROR nodes is the only path such routines have into the graph — and the recovery pattern matched only bare or double-quoted names and only OR REPLACE. In a T-SQL codebase that brackets every identifier ([dbo].[usp_X], a common house standard) and uses CREATE OR ALTER, every stored procedure silently vanished: 0/26 recovered in the reporting corpus. - accept bracket-delimited name parts and OR ALTER, mirroring what fb_proc_or_trigger already does for Firebird - hoist the pattern into a shared _ROUTINE_RECOVERY_RX used by both recovery sites (walk-time ERROR scan and whole-file has_error fallback): when the two drifted, a mixed-delimiter name (dbo.[usp_Mixed]) was captured differently by each, minting a second phantom node named after the schema that id-dedupe could not catch Recovered routines stay name-only nodes (no body reads_from edges), matching the existing PL/pgSQL recovery. Each new guard was mutation-tested: dropping the bracket alternative, dropping OR ALTER, and re-introducing the pattern drift each fail exactly their test. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 ++ graphify/extractors/sql.py | 74 +++++++++++++++++++++++-------- tests/test_multilang.py | 91 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6930b4e76..d7877ea3ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) +## 0.9.49 (unreleased) + +- Fix: T-SQL stored procedures and functions are no longer silently dropped from the graph — the ERROR-node recovery now accepts bracket-delimited names (`CREATE PROCEDURE [dbo].[usp_Load]`) and T-SQL's `CREATE OR ALTER`, both of which previously recovered nothing (the `AS BEGIN...END` body idiom never parses structurally, so recovery was these objects' only path into the graph). The two recovery sites now share one pattern, fixing a related defect where a mixed-delimiter name (`dbo.[usp_Mixed]`) produced a second phantom node named after the schema. The `PROC` shorthand is accepted alongside `PROCEDURE`, and the whole-file recovery scan now masks SQL comments so commented-out DDL in a file with an unrelated parse error cannot fabricate routine nodes. Recovered routines are name-only nodes (no body `reads_from` edges), matching the existing PL/pgSQL recovery. + ## 0.9.48 (2026-08-20) - Fix: a control character in a node label or id no longer aborts the whole export; the GraphML and Obsidian exporters scrub only the characters those formats forbid (tab, newline, and non-ASCII letters are preserved), and `graph.json` and its byte-identity round-trip are untouched (#2897, thanks @abhay-codes07). diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index a5dc18c368..d0c93a08e0 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -6,6 +6,50 @@ 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. +# 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 SQL line comments (-- ...) and block comments (/* ... */) for +# _mask_sql_comments. DOTALL so a block comment may span lines. +_SQL_COMMENT_RX = re.compile(r"--[^\n]*|/\*.*?\*/", re.DOTALL) + + +def _mask_sql_comments(text: str) -> str: + """Blank out comment spans, preserving every character offset. + + Non-newline characters inside a comment become spaces and newlines are + kept, so positions and line numbers computed against the masked text are + valid against the original. Used by the whole-file routine recovery so + commented-out CREATE PROCEDURE/FUNCTION DDL in a file that has an + unrelated parse error cannot fabricate a routine node. + """ + return _SQL_COMMENT_RX.sub( + lambda m: "".join("\n" if c == "\n" else " " for c in m.group(0)), text + ) + def _norm_ident(name: str) -> str: """Normalize a SQL identifier for name-based reference resolution. @@ -254,18 +298,11 @@ 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). + # 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. 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, - ): + 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 +466,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 +479,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 cb390eebcc..aff020d752 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -487,6 +487,97 @@ 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_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" + ) + 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_cte_is_not_read_as_a_table(): """#2577: a name bound by WITH ... AS (...) is scoped to its statement, not a table. From a888007d84f457ff6e9e9c3e249fc4b6b60fb746 Mon Sep 17 00:00:00 2001 From: Eduardo Garcia-Prieto Date: Sun, 23 Aug 2026 05:20:13 +1000 Subject: [PATCH 2/4] fix(sql): consume T-SQL ]] escapes inside bracket-delimited routine names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [a]]b] names the identifier a]b; stopping at the first ] truncated the recovered routine to [dbo].[a] — a phantom that could collide with a genuinely named [dbo].[a]. The bracketed-part alternative now consumes ]] before treating a lone ] as the closing delimiter, in both name-part positions of the shared _ROUTINE_RECOVERY_RX. Mutation-tested: reverting the escape handling fails the new regression test. Addresses the CodeRabbit P2 review finding on PR #2. Co-Authored-By: Claude Fable 5 --- graphify/extractors/sql.py | 8 ++++++-- tests/test_multilang.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index d0c93a08e0..44344d1245 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -21,14 +21,18 @@ # 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$]+))*)", + r"((?:\"[^\"\n]+\"|\[(?:[^\]\n]|\]\])+\]|[\w$]+)" + r"(?:\s*\.\s*(?:\"[^\"\n]+\"|\[(?:[^\]\n]|\]\])+\]|[\w$]+))*)", re.IGNORECASE, ) diff --git a/tests/test_multilang.py b/tests/test_multilang.py index aff020d752..0666f64764 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -526,6 +526,19 @@ def test_sql_tsql_create_or_alter_procedure_is_recovered(tmp_path): 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].""" + import tree_sitter_sql # noqa: F401 — required by the recovery path under test + 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. From dc5f2beae3ad5bce53588b720caf87df6799f68d Mon Sep 17 00:00:00 2001 From: Eduardo Garcia-Prieto Date: Sun, 23 Aug 2026 13:18:36 +1000 Subject: [PATCH 3/4] chore: leave the changelog entry to the release process Upstream batches changelog entries in maintainer release commits; the entry's content moves to the PR description. --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7877ea3ba..b6930b4e76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,6 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) -## 0.9.49 (unreleased) - -- Fix: T-SQL stored procedures and functions are no longer silently dropped from the graph — the ERROR-node recovery now accepts bracket-delimited names (`CREATE PROCEDURE [dbo].[usp_Load]`) and T-SQL's `CREATE OR ALTER`, both of which previously recovered nothing (the `AS BEGIN...END` body idiom never parses structurally, so recovery was these objects' only path into the graph). The two recovery sites now share one pattern, fixing a related defect where a mixed-delimiter name (`dbo.[usp_Mixed]`) produced a second phantom node named after the schema. The `PROC` shorthand is accepted alongside `PROCEDURE`, and the whole-file recovery scan now masks SQL comments so commented-out DDL in a file with an unrelated parse error cannot fabricate routine nodes. Recovered routines are name-only nodes (no body `reads_from` edges), matching the existing PL/pgSQL recovery. - ## 0.9.48 (2026-08-20) - Fix: a control character in a node label or id no longer aborts the whole export; the GraphML and Obsidian exporters scrub only the characters those formats forbid (tab, newline, and non-ASCII letters are preserved), and `graph.json` and its byte-identity round-trip are untouched (#2897, thanks @abhay-codes07). From b6d4dfc4166d04a0644ab24e60a1093bb2618a1d Mon Sep 17 00:00:00 2001 From: Eduardo Garcia-Prieto Date: Sun, 23 Aug 2026 13:55:06 +1000 Subject: [PATCH 4/4] fix(sql): treat comment openers inside string literals as data, and mask the ERROR-node scan The recovery mask read '-- note' inside a string literal as a line comment (blanking to end-of-line) and a /* inside a string as a block comment opener (blanking through the next real */), so real DDL sharing the span could be hidden. The mask now preserves single-quoted strings (with '' escapes), double-quoted identifiers, and bracket-delimited identifiers (with ]] escapes) before blanking comments. Literal patterns are deliberately single-line: the mask only runs on files that already failed to parse, where an unclosed quote is likely, and a multi-line match would let one unclosed delimiter swallow real DDL below it. The walk-time ERROR-node scan now masks too: an ERROR blob whose byte span covers commented-out DDL fabricated a routine node from it exactly as the whole-file scan once did (reproduced: a -- CREATE PROC line sandwiched between broken segments). Both guards are mutation-tested: a literal-blind mask fails the new unit pin; an unmasked ERROR scan fails the extended fabrication test. --- graphify/extractors/sql.py | 44 +++++++++++++++++++++++------- tests/test_multilang.py | 55 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index 44344d1245..632e7da326 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -36,9 +36,23 @@ re.IGNORECASE, ) -# Matches SQL line comments (-- ...) and block comments (/* ... */) for -# _mask_sql_comments. DOTALL so a block comment may span lines. -_SQL_COMMENT_RX = re.compile(r"--[^\n]*|/\*.*?\*/", re.DOTALL) +# Matches string/identifier literals (preserved) or comments (blanked) 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. 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. +_SQL_COMMENT_OR_LITERAL_RX = re.compile( + r"'(?:[^'\n]|'')*'" # single-quoted string, '' escape (one line) + r"|\"[^\"\n]*\"" # double-quoted identifier (one line) + r"|\[(?:[^\]\n]|\]\])*\]" # bracket-delimited identifier, ]] escape + r"|(--[^\n]*|/\*.*?\*/)", # group 1: the comment span to blank + re.DOTALL, +) def _mask_sql_comments(text: str) -> str: @@ -46,12 +60,19 @@ def _mask_sql_comments(text: str) -> str: Non-newline characters inside a comment become spaces and newlines are kept, so positions and line numbers computed against the masked text are - valid against the original. Used by the whole-file routine recovery so - commented-out CREATE PROCEDURE/FUNCTION DDL in a file that has an - unrelated parse error cannot fabricate a routine node. + valid against the original. String and identifier literals are preserved + verbatim, so a `--` or `/*` inside one does not start a comment. Used by + the whole-file routine recovery so commented-out CREATE PROCEDURE/FUNCTION + DDL in a file that has an unrelated parse error cannot fabricate a routine + node. """ - return _SQL_COMMENT_RX.sub( - lambda m: "".join("\n" if c == "\n" else " " for c in m.group(0)), text + 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) + else m.group(0) + ), + text, ) @@ -304,8 +325,11 @@ def walk(node) -> None: # # 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. - text = _read(node) + # 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") diff --git a/tests/test_multilang.py b/tests/test_multilang.py index 0666f64764..8208c5f70a 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -582,6 +582,13 @@ def test_sql_commented_ddl_is_not_fabricated_by_error_recovery(tmp_path): "/*\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"] @@ -591,6 +598,54 @@ def test_sql_commented_ddl_is_not_fabricated_by_error_recovery(tmp_path): ) +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_preserves_literals_and_blanks_comments(): + """Unit pin for _mask_sql_comments: comment openers inside string or + identifier literals are data; real comments blank to spaces with newlines + and offsets preserved.""" + from graphify.extractors.sql import _mask_sql_comments as mask + + assert mask("select '-- x' from t") == "select '-- x' from t" + assert mask("select 'it''s -- ok'") == "select 'it''s -- ok'" + assert mask('select "a--b" from t') == 'select "a--b" from t' + assert mask("select [a--b] from t") == "select [a--b] from t" + assert mask("select 'a /* b' as x") == "select 'a /* b' as x" + 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") + + 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.