From 0ba20211810455329c9ca2155ad7d89c4eb5b357 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 21 Aug 2026 13:03:48 +0800 Subject: [PATCH] fix(sqlserver): suppress diagnostics for routine batches --- .../src/lib/sql/semantic/diagnostics.ts | 227 ++++++++++++++---- .../app-tests/sqlSemanticDiagnostics.test.ts | 83 +++++++ 2 files changed, 265 insertions(+), 45 deletions(-) diff --git a/apps/desktop/src/lib/sql/semantic/diagnostics.ts b/apps/desktop/src/lib/sql/semantic/diagnostics.ts index 42fd8e4f72..f782f3bfa6 100644 --- a/apps/desktop/src/lib/sql/semantic/diagnostics.ts +++ b/apps/desktop/src/lib/sql/semantic/diagnostics.ts @@ -25,8 +25,7 @@ export interface SqlSemanticDiagnosticVisibleRange { } export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRanges: readonly SqlSemanticDiagnosticVisibleRange[], databaseType?: DatabaseType): SqlTextRange[] { - const executableRanges = executableStatementRanges(sql, databaseType); - const statements = databaseType === "sqlserver" ? mergeSqlServerDiagnosticBatchRanges(sql, executableRanges) : executableRanges; + const statements = databaseType === "sqlserver" ? sqlServerSemanticDiagnosticRanges(sql) : executableStatementRanges(sql, databaseType); if (statements.length === 0 || visibleRanges.length === 0) return []; const selected: SqlTextRange[] = []; @@ -42,22 +41,192 @@ export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRange return selected; } -function mergeSqlServerDiagnosticBatchRanges(sql: string, statements: readonly SqlTextRange[]): SqlTextRange[] { - if (statements.length < 2) return [...statements]; - +function sqlServerSemanticDiagnosticRanges(sql: string): SqlTextRange[] { const ranges: SqlTextRange[] = []; - let batch: SqlTextRange[] = [statements[0]]; - for (const statement of statements.slice(1)) { - if (sqlServerGapContainsGoSeparator(sql.slice(batch[batch.length - 1].to, statement.from))) { - pushSqlServerDiagnosticBatchRanges(ranges, sql, batch); - batch = []; - } - batch.push(statement); + for (const batch of sqlServerBatchRanges(sql)) { + if (isSqlServerRoutineDefinitionBatch(batch.sql)) continue; + const statements = executableStatementRanges(batch.sql, "sqlserver").map((statement) => ({ + from: batch.from + statement.from, + to: batch.from + statement.to, + sql: statement.sql, + })); + pushSqlServerDiagnosticBatchRanges(ranges, sql, statements); } - pushSqlServerDiagnosticBatchRanges(ranges, sql, batch); return ranges; } +function isSqlServerRoutineDefinitionBatch(sql: string): boolean { + const keywords = leadingUnquotedSqlKeywords(sql, 4); + const routineKeyword = (value: string | undefined) => value === "PROC" || value === "PROCEDURE" || value === "FUNCTION"; + return (keywords[0] === "ALTER" && routineKeyword(keywords[1])) || (keywords[0] === "CREATE" && routineKeyword(keywords[1])) || (keywords[0] === "CREATE" && keywords[1] === "OR" && keywords[2] === "ALTER" && routineKeyword(keywords[3])); +} + +function leadingUnquotedSqlKeywords(sql: string, limit: number): string[] { + const keywords: string[] = []; + let index = 0; + while (index < sql.length && keywords.length < limit) { + if (/\s/.test(sql[index])) { + index++; + continue; + } + if (sql.startsWith("--", index)) { + const carriageReturn = sql.indexOf("\r", index + 2); + const lineFeed = sql.indexOf("\n", index + 2); + const newline = carriageReturn < 0 ? lineFeed : lineFeed < 0 ? carriageReturn : Math.min(carriageReturn, lineFeed); + index = newline < 0 ? sql.length : newline + 1; + if (sql[newline] === "\r" && sql[newline + 1] === "\n") index += 1; + continue; + } + if (sql.startsWith("/*", index)) { + let depth = 1; + index += 2; + while (index < sql.length && depth > 0) { + if (sql.startsWith("/*", index)) { + depth += 1; + index += 2; + } else if (sql.startsWith("*/", index)) { + depth -= 1; + index += 2; + } else { + index += 1; + } + } + if (depth > 0) break; + continue; + } + const match = /^[A-Za-z]+/.exec(sql.slice(index)); + if (!match) break; + keywords.push(match[0].toUpperCase()); + index += match[0].length; + } + return keywords; +} + +function sqlServerBatchRanges(sql: string): SqlTextRange[] { + const separators = sqlServerGoSeparatorRanges(sql); + const ranges: SqlTextRange[] = []; + let from = 0; + for (const separator of separators) { + if (separator.from > from) ranges.push({ from, to: separator.from, sql: sql.slice(from, separator.from) }); + from = separator.to; + } + if (from < sql.length) ranges.push({ from, to: sql.length, sql: sql.slice(from) }); + return ranges; +} + +type SqlServerBatchQuote = "none" | "single" | "double" | "bracket"; + +interface SqlServerBatchScanState { + blockCommentDepth: number; + quote: SqlServerBatchQuote; +} + +function sqlServerGoSeparatorRanges(sql: string): Array<{ from: number; to: number }> { + const ranges: Array<{ from: number; to: number }> = []; + const state: SqlServerBatchScanState = { blockCommentDepth: 0, quote: "none" }; + let lineStart = 0; + while (lineStart <= sql.length) { + const newline = sql.indexOf("\n", lineStart); + const lineEnd = newline < 0 ? sql.length : newline; + const line = sql.slice(lineStart, lineEnd); + if (state.blockCommentDepth === 0 && state.quote === "none" && isSqlServerGoSeparatorLine(line)) { + ranges.push({ from: lineStart, to: newline < 0 ? lineEnd : lineEnd + 1 }); + } else { + updateSqlServerBatchScanState(line, state); + } + if (newline < 0) break; + lineStart = newline + 1; + } + return ranges; +} + +function isSqlServerGoSeparatorLine(line: string): boolean { + let index = 0; + while (index < line.length && /\s/.test(line[index] ?? "")) index += 1; + if (line.slice(index, index + 2).toUpperCase() !== "GO") return false; + index += 2; + if (index < line.length && !/\s/.test(line[index] ?? "") && !line.startsWith("--", index) && !line.startsWith("/*", index)) return false; + + let sawWhitespace = false; + while (index < line.length && /\s/.test(line[index] ?? "")) { + sawWhitespace = true; + index += 1; + } + if (sawWhitespace && /\d/.test(line[index] ?? "")) { + while (index < line.length && /\d/.test(line[index] ?? "")) index += 1; + } + + while (index < line.length) { + while (index < line.length && /\s/.test(line[index] ?? "")) index += 1; + if (index >= line.length || line.startsWith("--", index)) return true; + if (!line.startsWith("/*", index)) return false; + + let depth = 1; + index += 2; + while (index < line.length && depth > 0) { + if (line.startsWith("/*", index)) { + depth += 1; + index += 2; + } else if (line.startsWith("*/", index)) { + depth -= 1; + index += 2; + } else { + index += 1; + } + } + if (depth > 0) return false; + } + return true; +} + +function updateSqlServerBatchScanState(line: string, state: SqlServerBatchScanState) { + let index = 0; + while (index < line.length) { + if (state.blockCommentDepth > 0) { + if (line.startsWith("/*", index)) { + state.blockCommentDepth += 1; + index += 2; + } else if (line.startsWith("*/", index)) { + state.blockCommentDepth -= 1; + index += 2; + } else { + index += 1; + } + continue; + } + + if (state.quote !== "none") { + const quote = state.quote === "single" ? "'" : state.quote === "double" ? '"' : "]"; + if (line[index] !== quote) { + index += 1; + } else if (line[index + 1] === quote) { + index += 2; + } else { + state.quote = "none"; + index += 1; + } + continue; + } + + if (line.startsWith("--", index)) break; + if (line.startsWith("/*", index)) { + state.blockCommentDepth = 1; + index += 2; + } else if (line[index] === "'") { + state.quote = "single"; + index += 1; + } else if (line[index] === '"') { + state.quote = "double"; + index += 1; + } else if (line[index] === "[") { + state.quote = "bracket"; + index += 1; + } else { + index += 1; + } + } +} + function pushSqlServerDiagnosticBatchRanges(ranges: SqlTextRange[], sql: string, batch: readonly SqlTextRange[]) { if (batch.length > 1 && batch.some((statement) => sqlServerStatementNeedsBatchContext(statement.sql))) { const from = batch[0].from; @@ -72,38 +241,6 @@ function sqlServerStatementNeedsBatchContext(sql: string): boolean { return /^\s*(?:WHILE|BEGIN|END|IF|ELSE|TRY|CATCH)\b/i.test(sql) || /^\s*DECLARE\s+(?:\[[^\]]+\]|"[^"]+"|[A-Z_@#][\w@$#]*)\s+CURSOR\b/i.test(sql); } -function sqlServerGapContainsGoSeparator(gap: string): boolean { - let inBlockComment = false; - let lineStart = 0; - while (lineStart <= gap.length) { - const newline = gap.indexOf("\n", lineStart); - const lineEnd = newline < 0 ? gap.length : newline; - const line = gap.slice(lineStart, lineEnd).replace(/\r$/, ""); - if (!inBlockComment && /^\s*go(?:\s+\d+)?\s*$/i.test(line)) return true; - - let offset = 0; - while (offset < line.length) { - if (inBlockComment) { - const close = line.indexOf("*/", offset); - if (close < 0) break; - inBlockComment = false; - offset = close + 2; - continue; - } - const open = line.indexOf("/*", offset); - if (open < 0) break; - const lineComment = line.indexOf("--", offset); - if (lineComment >= 0 && lineComment < open) break; - inBlockComment = true; - offset = open + 2; - } - - if (newline < 0) break; - lineStart = newline + 1; - } - return false; -} - export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, schema: SqlSemanticDiagnosticSchema): SqlSemanticDiagnostic[] { const diagnostics: SqlSemanticDiagnostic[] = []; const tables = analysis.tables.filter((table) => table.name.trim()); diff --git a/packages/app-tests/sqlSemanticDiagnostics.test.ts b/packages/app-tests/sqlSemanticDiagnostics.test.ts index 41c0765ded..1b532e756b 100644 --- a/packages/app-tests/sqlSemanticDiagnostics.test.ts +++ b/packages/app-tests/sqlSemanticDiagnostics.test.ts @@ -494,6 +494,89 @@ test("keeps ordinary SQL Server statements viewport-local", () => { ); }); +test("skips SQL Server procedure definition batches", () => { + const sql = `ALTER PROCEDURE [COMMON].[TEST] +@BeginTime varchar(100)=null, +@EndTime varchar(100)=null +AS +BEGIN + SET NOCOUNT ON; + select datediff(DAY,CONVERT(datetime,@BeginTime,20),CONVERT(datetime,@EndTime,20)) as P_D00 +END`; + + assert.deepEqual(sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"), []); +}); + +test("skips SQL Server function definition batches but keeps later query batches", () => { + const functionSql = `/* Date conversion helper */ +ALTER FUNCTION [COMMON].[uf_DateTime2Str] +( + @date DATETIME, + @StyleID INT +) +RETURNS VARCHAR(20) +AS +BEGIN + IF ISNULL(@date,'') = '' + RETURN CONVERT(VARCHAR(20),GETDATE(),120) + + DECLARE @Return VARCHAR(20), + @Str1 VARCHAR(8), + @Str2 VARCHAR(8) + SET @Str1 = CONVERT(VARCHAR(8),@date,112) + SET @Str2 = CONVERT(VARCHAR(8),@date,108) + + IF @StyleID = 301 + SET @Return = @Str1 + @Str2 + ELSE + SET @Return = CONVERT(VARCHAR(20),@date,120) + + RETURN @Return +END`; + const sql = `${functionSql}\nGO\nSELECT missing_field FROM dbo.users;`; + + const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"); + + assert.deepEqual( + ranges.map((range) => range.sql), + ["SELECT missing_field FROM dbo.users"], + ); +}); + +test("skips complete SQL Server procedure batches without BEGIN", () => { + const sql = `CREATE OR ALTER PROCEDURE dbo.refresh_users AS +SELECT missing_one FROM dbo.first_table; +;WITH recent AS (SELECT missing_two FROM dbo.second_table) +SELECT missing_three FROM recent; +SELECT missing_four FROM dbo.third_table; +GO +SELECT outside_missing FROM dbo.visible_table;`; + + const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"); + + assert.deepEqual( + ranges.map((range) => range.sql), + ["SELECT outside_missing FROM dbo.visible_table"], + ); +}); + +test.each(["GO -- separator", "GO 2 /* outer /* nested */ comment */"])("keeps SQL Server queries after commented batch separator %s", (separator) => { + const sql = "CREATE PROCEDURE dbo.refresh_users AS\nSELECT missing_inside FROM dbo.internal_table;\n" + separator + "\nSELECT outside_missing FROM dbo.visible_table;"; + + const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"); + + assert.deepEqual( + ranges.map((range) => range.sql), + ["SELECT outside_missing FROM dbo.visible_table"], + ); +}); + +test("skips SQL Server routine batches after nested leading comments", () => { + const sql = "/* outer\r\n /* nested */\r\n still outer\r\n*/\r\n-- leading comment\rCREATE OR ALTER FUNCTION dbo.answer()\r\nRETURNS int\r\nAS\r\nBEGIN\r\n RETURN 42;\r\nEND"; + + assert.deepEqual(sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"), []); +}); + test("skips Oracle PL/SQL blocks when selecting semantic diagnostic ranges", () => { const sql = `DECLARE v_order_count NUMBER;