From 19f8736b67b42f8895fb1b7adab59d4abbf26ef3 Mon Sep 17 00:00:00 2001 From: zipg Date: Tue, 25 Aug 2026 22:34:57 +0800 Subject: [PATCH] fix(editor): improve MySQL routine syntax diagnostics --- .../src/components/editor/QueryEditor.vue | 11 + .../lib/sql/mysqlRoutineSyntaxDiagnostics.ts | 204 ++++++++++++++++++ .../mysqlRoutineSyntaxDiagnostics.test.ts | 152 +++++++++++++ 3 files changed, 367 insertions(+) create mode 100644 apps/desktop/src/lib/sql/mysqlRoutineSyntaxDiagnostics.ts create mode 100644 packages/app-tests/mysqlRoutineSyntaxDiagnostics.test.ts diff --git a/apps/desktop/src/components/editor/QueryEditor.vue b/apps/desktop/src/components/editor/QueryEditor.vue index 2ba7bf5e36..15eb7937e7 100644 --- a/apps/desktop/src/components/editor/QueryEditor.vue +++ b/apps/desktop/src/components/editor/QueryEditor.vue @@ -93,6 +93,7 @@ import { import { buildHoverTableSql, ddlForHoverPreview, hoverTableMatchesScope, normalizeAlignedSqlWhitespace, quoteIdentifier, quoteQualifiedName, reformatHoverDdl, scopeHoverTables, type HoverTableScope } from "@/lib/editor/hoverTableSql"; import { constrainSqlHoverLayout } from "@/lib/editor/sqlHoverLayout"; import { lineColumnToOffset, sqlErrorDecorationRange as resolveSqlErrorDecorationRange } from "@/lib/sql/sqlDiagnostics"; +import { analyzeMysqlRoutineSyntax, supportsMysqlRoutineSyntaxDiagnostics } from "@/lib/sql/mysqlRoutineSyntaxDiagnostics"; import { DBX_TABLE_REFERENCE_MIME, DBX_TABLE_REFERENCE_DROP_EVENT, @@ -3081,7 +3082,17 @@ async function refreshSemanticDiagnostics(options: { preserveOutsideRanges?: boo } const nextDiagnostics: SqlSemanticDiagnostic[] = []; + const mysqlRoutineAnalysis = props.databaseType === "mysql" && supportsMysqlRoutineSyntaxDiagnostics(sqlDriverProfile.value) ? analyzeMysqlRoutineSyntax(sql) : null; + if (mysqlRoutineAnalysis) { + nextDiagnostics.push( + ...mysqlRoutineAnalysis.diagnostics.filter((diagnostic) => { + const diagnosticRange = sqlTextSpanToRange(sql, diagnostic.span); + return !!diagnosticRange && diagnosticRanges.some((range) => rangesOverlap(diagnosticRange, range)); + }), + ); + } for (const range of diagnosticRanges) { + if (mysqlRoutineAnalysis?.routineRanges.some((routineRange) => rangesOverlap(routineRange, range))) continue; try { const analysis = await api.analyzeSqlReferences( range.sql, diff --git a/apps/desktop/src/lib/sql/mysqlRoutineSyntaxDiagnostics.ts b/apps/desktop/src/lib/sql/mysqlRoutineSyntaxDiagnostics.ts new file mode 100644 index 0000000000..13d756261e --- /dev/null +++ b/apps/desktop/src/lib/sql/mysqlRoutineSyntaxDiagnostics.ts @@ -0,0 +1,204 @@ +import { executableStatementRanges } from "@/lib/sql/sqlStatementRanges"; +import type { SqlTextSpan } from "@/types/database"; + +export interface MysqlRoutineSyntaxDiagnostic { + span: SqlTextSpan; + message: string; + severity: "error"; +} + +export interface MysqlRoutineSyntaxAnalysis { + diagnostics: MysqlRoutineSyntaxDiagnostic[]; + hasRoutine: boolean; + routineRanges: Array<{ from: number; to: number }>; +} + +interface RoutineToken { + kind: "word" | "leftParen" | "rightParen" | "comma" | "semicolon" | "other"; + value: string; + from: number; + to: number; +} + +interface RoutineDeclaration { + kind: "PROCEDURE" | "FUNCTION"; + createTokenIndex: number; + tokenIndex: number; +} + +const NON_ROUTINE_CREATE_TYPES = new Set(["DATABASE", "INDEX", "LOGFILE", "ROLE", "SCHEMA", "SERVER", "SPATIAL", "TABLE", "TEMPORARY", "UNIQUE", "USER", "VIEW"]); + +export function supportsMysqlRoutineSyntaxDiagnostics(driverProfile?: string): boolean { + const profile = driverProfile?.trim().toLowerCase(); + return !profile || profile === "mysql" || profile === "custom_mysql"; +} + +export function analyzeMysqlRoutineSyntax(sql: string): MysqlRoutineSyntaxAnalysis { + const diagnostics: MysqlRoutineSyntaxDiagnostic[] = []; + const routineRanges: Array<{ from: number; to: number }> = []; + + for (const statement of executableStatementRanges(sql, "mysql")) { + const tokens = tokenizeMysqlRoutineSql(statement.sql, statement.from); + const declarations = findRoutineDeclarations(tokens); + + declarations.forEach((declaration, declarationIndex) => { + const nextDeclarationTokenIndex = declarations[declarationIndex + 1]?.createTokenIndex ?? tokens.length; + const parameterStart = findTokenKind(tokens, declaration.tokenIndex + 1, nextDeclarationTokenIndex, "leftParen"); + if (parameterStart === -1) return; + const parameterEnd = findMatchingRightParen(tokens, parameterStart, nextDeclarationTokenIndex); + if (parameterEnd === -1) return; + routineRanges.push({ from: tokens[declaration.createTokenIndex].from, to: statement.to }); + + const previousParameterToken = tokens[parameterEnd - 1]; + if (previousParameterToken?.kind === "comma") { + diagnostics.push(diagnosticAtOffset(sql, previousParameterToken.from, previousParameterToken.to, "Trailing comma is not allowed in a MySQL routine parameter list")); + } + + for (let index = parameterEnd + 1; index < nextDeclarationTokenIndex; index += 1) { + const token = tokens[index]; + if (token.kind !== "word" || token.value !== "RETURN") continue; + if (declaration.kind === "PROCEDURE") { + diagnostics.push(diagnosticAtOffset(sql, token.from, token.to, "RETURN is not valid in a MySQL procedure; use LEAVE with a block label")); + continue; + } + const nextToken = tokens[index + 1]; + if (!nextToken || nextToken.kind === "semicolon") { + diagnostics.push(diagnosticAtOffset(sql, token.from, token.to, "RETURN in a MySQL function requires an expression")); + } + } + }); + } + + return { diagnostics, hasRoutine: routineRanges.length > 0, routineRanges }; +} + +function findRoutineDeclarations(tokens: readonly RoutineToken[]): RoutineDeclaration[] { + const declarations: RoutineDeclaration[] = []; + for (let index = 0; index < tokens.length; index += 1) { + if (tokens[index]?.kind !== "word" || tokens[index]?.value !== "CREATE") continue; + for (let lookahead = index + 1; lookahead < Math.min(tokens.length, index + 24); lookahead += 1) { + const token = tokens[lookahead]; + if (token.kind === "semicolon" || (token.kind === "word" && token.value === "CREATE")) break; + if (token.kind !== "word") continue; + if (token.value === "PROCEDURE" || token.value === "FUNCTION") { + declarations.push({ kind: token.value, createTokenIndex: index, tokenIndex: lookahead }); + break; + } + if (NON_ROUTINE_CREATE_TYPES.has(token.value)) break; + } + } + return declarations; +} + +function findTokenKind(tokens: readonly RoutineToken[], from: number, to: number, kind: RoutineToken["kind"]): number { + for (let index = from; index < to; index += 1) { + if (tokens[index]?.kind === kind) return index; + } + return -1; +} + +function findMatchingRightParen(tokens: readonly RoutineToken[], start: number, to: number): number { + let depth = 0; + for (let index = start; index < to; index += 1) { + if (tokens[index]?.kind === "leftParen") depth += 1; + if (tokens[index]?.kind !== "rightParen") continue; + depth -= 1; + if (depth === 0) return index; + } + return -1; +} + +function diagnosticAtOffset(sql: string, from: number, to: number, message: string): MysqlRoutineSyntaxDiagnostic { + const start = offsetToPosition(sql, from); + const end = offsetToPosition(sql, Math.max(from + 1, to)); + return { + span: { + start_line: start.line, + start_column: start.column, + end_line: end.line, + end_column: Math.max(end.column - 1, start.column), + }, + message, + severity: "error", + }; +} + +function offsetToPosition(sql: string, offset: number): { line: number; column: number } { + const boundedOffset = Math.max(0, Math.min(offset, sql.length)); + let line = 1; + let lineStart = 0; + for (let index = 0; index < boundedOffset; index += 1) { + if (sql[index] !== "\n") continue; + line += 1; + lineStart = index + 1; + } + return { line, column: boundedOffset - lineStart + 1 }; +} + +function tokenizeMysqlRoutineSql(sql: string, baseOffset = 0): RoutineToken[] { + const tokens: RoutineToken[] = []; + let index = 0; + + while (index < sql.length) { + const char = sql[index]; + const next = sql[index + 1] ?? ""; + if (char === "-" && next === "-" && /\s/.test(sql[index + 2] ?? "")) { + index = skipLine(sql, index + 2); + continue; + } + if (char === "#") { + index = skipLine(sql, index + 1); + continue; + } + if (char === "/" && next === "*") { + const end = sql.indexOf("*/", index + 2); + index = end === -1 ? sql.length : end + 2; + continue; + } + if (char === "'" || char === '"' || char === "`") { + const end = skipQuoted(sql, index, char); + tokens.push({ kind: "other", value: sql.slice(index, end), from: baseOffset + index, to: baseOffset + end }); + index = end; + continue; + } + + const kind = char === "(" ? "leftParen" : char === ")" ? "rightParen" : char === "," ? "comma" : char === ";" ? "semicolon" : null; + if (kind) { + tokens.push({ kind, value: char, from: baseOffset + index, to: baseOffset + index + 1 }); + index += 1; + continue; + } + + const word = /^[A-Za-z_][A-Za-z0-9_]*/.exec(sql.slice(index))?.[0]; + if (word) { + tokens.push({ kind: "word", value: word.toUpperCase(), from: baseOffset + index, to: baseOffset + index + word.length }); + index += word.length; + continue; + } + if (!/\s/.test(char)) tokens.push({ kind: "other", value: char, from: baseOffset + index, to: baseOffset + index + 1 }); + index += 1; + } + return tokens; +} + +function skipLine(sql: string, from: number): number { + const newline = sql.indexOf("\n", from); + return newline === -1 ? sql.length : newline + 1; +} + +function skipQuoted(sql: string, start: number, quote: string): number { + let index = start + 1; + while (index < sql.length) { + if (sql[index] === "\\" && index + 1 < sql.length) { + index += 2; + continue; + } + if (sql[index] === quote && sql[index + 1] === quote) { + index += 2; + continue; + } + if (sql[index] === quote) return index + 1; + index += 1; + } + return sql.length; +} diff --git a/packages/app-tests/mysqlRoutineSyntaxDiagnostics.test.ts b/packages/app-tests/mysqlRoutineSyntaxDiagnostics.test.ts new file mode 100644 index 0000000000..004db27785 --- /dev/null +++ b/packages/app-tests/mysqlRoutineSyntaxDiagnostics.test.ts @@ -0,0 +1,152 @@ +import { strict as assert } from "node:assert"; +import { test } from "vitest"; +import { analyzeMysqlRoutineSyntax, supportsMysqlRoutineSyntaxDiagnostics } from "../../apps/desktop/src/lib/sql/mysqlRoutineSyntaxDiagnostics.ts"; + +const reportedSql = `drop procedure if exists proc_check_gys; +CREATE PROCEDURE proc_check_gys( + in in_gys_name varchar(100), -- 供应商账号 + in in_yys varchar(100), -- 运营商账号 +) +run: +BEGIN + set @yys=''; + set @uid=0; + select uid,yys INTO @uid, @yys from uchome_space AS us where username=in_gys_name limit 1; + if @uid=0 THEN + select 1 as code,'供应商不存在' as msg; + return; + END if; + if @yys<> in_gys_name THEN + select 2 as code,'运营商不正确' as msg; + return; + end if; + select 0 as code,'' as msg; +end`; + +test("reports deterministic MySQL procedure syntax errors", () => { + const analysis = analyzeMysqlRoutineSyntax(reportedSql); + + assert.equal(analysis.hasRoutine, true); + assert.equal(analysis.routineRanges.length, 1); + assert.equal(reportedSql.slice(analysis.routineRanges[0].from, analysis.routineRanges[0].to).startsWith("CREATE PROCEDURE"), true); + assert.deepEqual( + analysis.diagnostics.map((diagnostic) => diagnostic.message), + ["Trailing comma is not allowed in a MySQL routine parameter list", "RETURN is not valid in a MySQL procedure; use LEAVE with a block label", "RETURN is not valid in a MySQL procedure; use LEAVE with a block label"], + ); + assert.deepEqual( + analysis.diagnostics.map((diagnostic) => [diagnostic.span.start_line, diagnostic.span.start_column]), + [ + [4, 26], + [13, 6], + [17, 6], + ], + ); +}); + +test("accepts a labeled MySQL procedure that exits with LEAVE", () => { + const analysis = analyzeMysqlRoutineSyntax(`CREATE DEFINER=CURRENT_USER PROCEDURE proc_check_gys( + IN in_gys_name VARCHAR(100), + IN amount DECIMAL(10, 2), + IN mode_name ENUM('direct', 'proxy') +) +run: BEGIN + IF in_gys_name = '' THEN + LEAVE run; + END IF; + SELECT 'RETURN;,)' AS message; + -- RETURN in a comment must be ignored +END`); + + assert.equal(analysis.hasRoutine, true); + assert.deepEqual(analysis.diagnostics, []); +}); + +test("checks bare RETURN only for MySQL functions", () => { + const validNumber = analyzeMysqlRoutineSyntax("CREATE FUNCTION answer() RETURNS INT BEGIN RETURN -1; END"); + const validString = analyzeMysqlRoutineSyntax("CREATE FUNCTION answer() RETURNS VARCHAR(10) BEGIN RETURN 'ok'; END"); + const invalid = analyzeMysqlRoutineSyntax("CREATE FUNCTION answer() RETURNS INT BEGIN RETURN; END"); + const invalidWithComment = analyzeMysqlRoutineSyntax("CREATE FUNCTION answer() RETURNS INT BEGIN RETURN /* missing value */; END"); + + assert.deepEqual(validNumber.diagnostics, []); + assert.deepEqual(validString.diagnostics, []); + assert.deepEqual( + invalid.diagnostics.map((diagnostic) => diagnostic.message), + ["RETURN in a MySQL function requires an expression"], + ); + assert.deepEqual( + invalidWithComment.diagnostics.map((diagnostic) => diagnostic.message), + ["RETURN in a MySQL function requires an expression"], + ); +}); + +test("ignores ordinary SQL and quoted routine keywords", () => { + assert.deepEqual(analyzeMysqlRoutineSyntax("SELECT 'CREATE PROCEDURE p() RETURN;' AS body"), { diagnostics: [], hasRoutine: false, routineRanges: [] }); +}); + +test("keeps multiple routine ranges separate from ordinary SQL", () => { + const sql = `DELIMITER $$ +CREATE PROCEDURE first_proc() BEGIN RETURN; END$$ +CREATE FUNCTION second_func() RETURNS INT BEGIN RETURN 1; END$$ +DELIMITER ; +SELECT missing_column FROM users;`; + const analysis = analyzeMysqlRoutineSyntax(sql); + + assert.equal(analysis.routineRanges.length, 2); + assert.equal( + analysis.routineRanges.some((range) => sql.slice(range.from, range.to).includes("SELECT missing_column")), + false, + ); + assert.deepEqual( + analysis.diagnostics.map((diagnostic) => diagnostic.message), + ["RETURN is not valid in a MySQL procedure; use LEAVE with a block label"], + ); +}); + +test("keeps CASE expression endings inside the routine statement", () => { + const sql = `CREATE PROCEDURE update_status() +BEGIN + SET @status_code = CASE WHEN 1 = 1 THEN 1 ELSE 0 END; + RETURN; +END; +SELECT missing_column FROM users;`; + const analysis = analyzeMysqlRoutineSyntax(sql); + + assert.deepEqual( + analysis.diagnostics.map((diagnostic) => diagnostic.message), + ["RETURN is not valid in a MySQL procedure; use LEAVE with a block label"], + ); + assert.equal(sql.slice(analysis.routineRanges[0].from, analysis.routineRanges[0].to).includes("RETURN;"), true); + assert.equal(sql.slice(analysis.routineRanges[0].from, analysis.routineRanges[0].to).includes("SELECT missing_column"), false); +}); + +test("uses custom delimiters to bound single-statement functions", () => { + const sql = `DELIMITER $$ +CREATE FUNCTION answer() RETURNS INT +RETURN$$ +DELIMITER ; +SELECT missing_column FROM users;`; + const analysis = analyzeMysqlRoutineSyntax(sql); + + assert.deepEqual( + analysis.diagnostics.map((diagnostic) => diagnostic.message), + ["RETURN in a MySQL function requires an expression"], + ); + assert.equal(sql.slice(analysis.routineRanges[0].from, analysis.routineRanges[0].to), "CREATE FUNCTION answer() RETURNS INT\nRETURN"); +}); + +test("limits routine diagnostics to native and custom MySQL profiles", () => { + assert.equal(supportsMysqlRoutineSyntaxDiagnostics(), true); + assert.equal(supportsMysqlRoutineSyntaxDiagnostics("mysql"), true); + assert.equal(supportsMysqlRoutineSyntaxDiagnostics("custom_mysql"), true); + assert.equal(supportsMysqlRoutineSyntaxDiagnostics("mariadb"), false); + assert.equal(supportsMysqlRoutineSyntaxDiagnostics("tidb"), false); + assert.equal(supportsMysqlRoutineSyntaxDiagnostics("oceanbase"), false); +}); + +test("leaves MySQL triggers and events to the existing diagnostics path", () => { + const trigger = analyzeMysqlRoutineSyntax("CREATE TRIGGER before_insert BEFORE INSERT ON users FOR EACH ROW SET NEW.created_at = NOW()"); + const event = analyzeMysqlRoutineSyntax("CREATE EVENT purge_logs ON SCHEDULE EVERY 1 DAY DO DELETE FROM logs WHERE created_at < NOW() - INTERVAL 30 DAY"); + + assert.deepEqual(trigger, { diagnostics: [], hasRoutine: false, routineRanges: [] }); + assert.deepEqual(event, { diagnostics: [], hasRoutine: false, routineRanges: [] }); +});