fix(sqlserver): accept cursor declarations in SQL analysis

Closes #5860

apps/desktop/src/lib/sql/semantic/diagnostics.ts

@@ -25,7 +25,8 @@ export interface SqlSemanticDiagnosticVisibleRange {
 }
 
 export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRanges: readonly SqlSemanticDiagnosticVisibleRange[], databaseType?: DatabaseType): SqlTextRange[] {
-  const statements = executableStatementRanges(sql, databaseType);
+  const executableRanges = executableStatementRanges(sql, databaseType);
+  const statements = databaseType === "sqlserver" ? mergeSqlServerDiagnosticBatchRanges(sql, executableRanges) : executableRanges;
   if (statements.length === 0 || visibleRanges.length === 0) return [];
 
   const selected: SqlTextRange[] = [];
@@ -41,6 +42,68 @@ export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRange
   return selected;
 }
 
+function mergeSqlServerDiagnosticBatchRanges(sql: string, statements: readonly SqlTextRange[]): SqlTextRange[] {
+  if (statements.length < 2) return [...statements];
+
+  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);
+  }
+  pushSqlServerDiagnosticBatchRanges(ranges, sql, batch);
+  return ranges;
+}
+
+function pushSqlServerDiagnosticBatchRanges(ranges: SqlTextRange[], sql: string, batch: readonly SqlTextRange[]) {
+  if (batch.length > 1 && batch.some((statement) => sqlServerStatementNeedsBatchContext(statement.sql))) {
+    const from = batch[0].from;
+    const to = batch[batch.length - 1].to;
+    ranges.push({ from, to, sql: sql.slice(from, to) });
+  } else {
+    ranges.push(...batch);
+  }
+}
+
+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());

crates/dbx-core/src/sql_analysis.rs

@@ -127,7 +127,8 @@ fn parse_sqlserver(sql: &str) -> Result<Vec<Statement>, ParserError> {
         Err(error) => {
             let mut fallback_tokens = Tokenizer::new(&dialect, sql).tokenize_with_location()?;
             normalize_sqlserver_create_proc_tokens(&mut fallback_tokens);
-            let mut changed = normalize_sqlserver_alter_table_add_tokens(&mut fallback_tokens);
+            let mut changed = normalize_sqlserver_cursor_tokens(&mut fallback_tokens);
+            changed |= normalize_sqlserver_alter_table_add_tokens(&mut fallback_tokens);
             changed |= remove_sqlserver_query_hint_tokens(&mut fallback_tokens);
             if changed {
                 if let Ok(statements) =
@@ -141,6 +142,109 @@ fn parse_sqlserver(sql: &str) -> Result<Vec<Statement>, ParserError> {
     }
 }
 
+fn normalize_sqlserver_cursor_tokens(tokens: &mut Vec<TokenWithSpan>) -> bool {
+    let mut removals = Vec::new();
+    let mut index = 0usize;
+
+    while let Some(statement_start) = next_significant_token_index(tokens, index) {
+        if matches!(tokens[statement_start].token, Token::EOF) {
+            break;
+        }
+        if matches!(tokens[statement_start].token, Token::SemiColon) {
+            index = statement_start + 1;
+            continue;
+        }
+
+        let statement_end = sqlserver_statement_end_index(tokens, statement_start);
+        let significant_indexes = tokens
+            .iter()
+            .enumerate()
+            .take(statement_end)
+            .skip(statement_start)
+            .filter_map(|(index, token)| (!matches!(token.token, Token::Whitespace(_))).then_some(index))
+            .collect::<Vec<_>>();
+
+        if let Some(removal) = sqlserver_cursor_option_removal(tokens, &significant_indexes) {
+            removals.push(removal);
+        }
+
+        index = statement_end.saturating_add(1);
+    }
+
+    let changed = !removals.is_empty();
+    for removal in removals.into_iter().rev() {
+        tokens.drain(removal);
+    }
+    changed
+}
+
+fn sqlserver_cursor_option_removal(tokens: &[TokenWithSpan], indexes: &[usize]) -> Option<std::ops::Range<usize>> {
+    if indexes.len() < 4
+        || !unquoted_token_word_eq(&tokens[indexes[0]], "DECLARE")
+        || !is_sqlserver_named_cursor(&tokens[indexes[1]])
+        || !unquoted_token_word_eq(&tokens[indexes[2]], "CURSOR")
+    {
+        return None;
+    }
+
+    let mut position = 3usize;
+    if indexes.get(position).is_some_and(|index| {
+        unquoted_token_word_eq(&tokens[*index], "LOCAL") || unquoted_token_word_eq(&tokens[*index], "GLOBAL")
+    }) {
+        position += 1;
+    }
+    if indexes.get(position).is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "FAST_FORWARD")) {
+        position += 1;
+    }
+    if position == 3 || !indexes.get(position).is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "FOR")) {
+        return None;
+    }
+
+    let query_position = position + 1;
+    let query_start = *indexes.get(query_position)?;
+    if (!unquoted_token_word_eq(&tokens[query_start], "SELECT")
+        && !unquoted_token_word_eq(&tokens[query_start], "WITH"))
+        || sqlserver_cursor_query_has_forbidden_clause(tokens, &indexes[query_position..])
+    {
+        return None;
+    }
+
+    Some(indexes[3]..indexes[position])
+}
+
+fn is_sqlserver_named_cursor(token: &TokenWithSpan) -> bool {
+    let Token::Word(word) = &token.token else {
+        return false;
+    };
+    !word.value.starts_with('@') && (word.quote_style.is_some() || word.keyword == Keyword::NoKeyword)
+}
+
+fn sqlserver_cursor_query_has_forbidden_clause(tokens: &[TokenWithSpan], indexes: &[usize]) -> bool {
+    let mut depth = 0usize;
+    for (position, index) in indexes.iter().copied().enumerate() {
+        match tokens[index].token {
+            Token::LParen => depth += 1,
+            Token::RParen => depth = depth.saturating_sub(1),
+            _ if depth == 0
+                && (unquoted_token_word_eq(&tokens[index], "INTO")
+                    || unquoted_token_word_eq(&tokens[index], "COMPUTE")
+                    || unquoted_token_word_eq(&tokens[index], "FOR")
+                        && indexes
+                            .get(position + 1)
+                            .is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "BROWSE"))) =>
+            {
+                return true;
+            }
+            _ => {}
+        }
+    }
+    false
+}
+
+fn unquoted_token_word_eq(token: &TokenWithSpan, expected: &str) -> bool {
+    matches!(&token.token, Token::Word(word) if word.quote_style.is_none() && word.value.eq_ignore_ascii_case(expected))
+}
+
 fn normalize_sqlserver_alter_table_add_tokens(tokens: &mut Vec<TokenWithSpan>) -> bool {
     let mut insertions = Vec::new();
     let mut index = 0usize;
@@ -477,8 +581,16 @@ fn normalize_dialect(dialect: Option<&str>) -> String {
 
 impl Analyzer {
     fn visit_statement(&mut self, statement: &Statement) {
-        if let Statement::Query(query) = statement {
+        match statement {
-            self.visit_query_in_new_scope(query, None);
+            Statement::Query(query) => self.visit_query_in_new_scope(query, None),
+            Statement::Declare { stmts } if self.is_sqlserver => {
+                for declaration in stmts {
+                    if let Some(query) = &declaration.for_query {
+                        self.visit_query_in_new_scope(query, None);
+                    }
+                }
+            }
+            _ => {}
         }
     }
 

crates/dbx-core/tests/sql_analysis.rs

@@ -410,6 +410,152 @@ fn sqlserver_proc_identifiers_remain_identifiers_outside_create() {
     assert_eq!(analysis.columns[0].name, "proc");
 }
 
+#[test]
+fn sqlserver_cursor_declaration_analyzes_its_query_without_a_cursor_table() {
+    let sql = "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR
+               SELECT s.name FROM sys.schemas s WHERE s.schema_id > 0;
+               OPEN schema_cursor;
+               FETCH NEXT FROM schema_cursor INTO @schema;
+               CLOSE schema_cursor;
+               DEALLOCATE schema_cursor;";
+    let analysis = analyze_sql_references(sql, Some("sqlserver"))
+        .unwrap_or_else(|error| panic!("valid SQL Server cursor declaration should analyze: {error}"));
+
+    let tables: Vec<_> = analysis.tables.iter().map(|table| (table.schema.as_deref(), table.name.as_str())).collect();
+    assert_eq!(tables, vec![(Some("sys"), "schemas")]);
+    assert!(analysis.tables.iter().all(|table| table.name != "schema_cursor"));
+}
+
+#[test]
+fn sqlserver_reported_cursor_query_does_not_raise_parser_or_table_errors() {
+    let analysis = analyze_sql_references(
+        "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR
+         SELECT N'dbo' UNION ALL SELECT N'dev';",
+        Some("sqlserver"),
+    )
+    .expect("the reported SQL Server cursor declaration should analyze");
+
+    assert!(analysis.tables.is_empty());
+    assert!(analysis.columns.is_empty());
+}
+
+#[test]
+fn sqlserver_cursor_fallback_is_limited_to_the_reported_option_subset() {
+    for sql in [
+        "DECLARE plain_cursor CURSOR FOR SELECT name FROM dbo.reports;",
+        "DECLARE local_cursor CURSOR LOCAL FOR SELECT name FROM dbo.reports;",
+        "DECLARE fast_cursor CURSOR FAST_FORWARD FOR SELECT name FROM dbo.reports;",
+        "DECLARE global_cursor CURSOR GLOBAL FAST_FORWARD FOR SELECT name FROM dbo.reports;",
+        "DECLARE json_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT (SELECT TOP 1 a.note FROM dbo.audit a FOR JSON PATH) AS payload FROM dbo.reports;",
+    ] {
+        let analysis = analyze_sql_references(sql, Some("sqlserver"))
+            .unwrap_or_else(|error| panic!("the reported cursor option subset should analyze: {error}"));
+        assert!(analysis.tables.iter().any(|table| table.name == "reports"));
+    }
+
+    for sql in [
+        "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD;",
+        "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR UPDATE dbo.reports SET name = 'invalid';",
+        "DECLARE report_cursor CURSOR SCROLL FAST_FORWARD FOR SELECT name FROM dbo.reports;",
+        "DECLARE report_cursor CURSOR LOCAL KEYSET FOR SELECT name FROM dbo.reports;",
+        "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD READ_ONLY FOR SELECT name FROM dbo.reports;",
+        "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name INTO #report_copy FROM dbo.reports;",
+        "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports FOR BROWSE;",
+    ] {
+        if analyze_sql_references(sql, Some("sqlserver")).is_ok() {
+            panic!("unsupported or invalid cursor syntax must not be suppressed: {sql}");
+        }
+    }
+}
+
+#[test]
+fn sqlserver_cursor_uses_native_lifecycle_statements_and_preserves_query_spans() {
+    for sql in [
+        "OPEN schema_cursor",
+        "FETCH NEXT FROM schema_cursor INTO @schema",
+        "CLOSE schema_cursor",
+        "DEALLOCATE schema_cursor",
+    ] {
+        analyze_sql_references(sql, Some("sqlserver"))
+            .unwrap_or_else(|error| panic!("sqlparser should parse ordinary cursor lifecycle SQL {sql:?}: {error}"));
+    }
+
+    let analysis = analyze_sql_references(
+        "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR\n    SELECT r.name FROM dbo.reports r OPTION (RECOMPILE);",
+        Some("sqlserver"),
+    )
+    .expect("cursor option and query-hint fallbacks should compose");
+    assert_eq!(analysis.tables[0].name, "reports");
+    assert_eq!(analysis.tables[0].span.start_line, 2);
+    assert_eq!(analysis.tables[0].span.start_column, 28);
+    assert_eq!(analysis.columns[0].name, "name");
+    assert_eq!(analysis.columns[0].span.start_line, 2);
+    assert_eq!(analysis.columns[0].span.start_column, 14);
+
+    analyze_sql_references(
+        "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports OPTION (RECOMPILE);\
+         ALTER TABLE dbo.demo ADD first_flag BIT NULL, second_flag BIT NULL;",
+        Some("sqlserver"),
+    )
+    .expect("cursor, query-hint, and ALTER TABLE fallbacks should compose");
+}
+
+#[test]
+fn sqlserver_cursor_fallback_is_dialect_scoped_and_preserves_other_statements() {
+    let cursor_sql = "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports;";
+    analyze_sql_references(cursor_sql, Some("postgres"))
+        .expect_err("other dialects must not inherit SQL Server cursor handling");
+
+    let analysis =
+        analyze_sql_references("DECLARE @schema SYSNAME; SELECT u.name FROM dbo.users u;", Some("sqlserver"))
+            .expect("ordinary SQL Server variable declarations and table references should remain parseable");
+    assert_eq!(analysis.tables.len(), 1);
+    assert_eq!(analysis.tables[0].schema.as_deref(), Some("dbo"));
+    assert_eq!(analysis.tables[0].name, "users");
+}
+
+#[test]
+fn sqlserver_reported_cursor_batch_analyzes_with_control_flow_and_go() {
+    use dbx_core::sql::split_sql_batches;
+
+    let sql = "IF SCHEMA_ID(N'dev') IS NULL
+    EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo');
+GO
+
+IF COL_LENGTH(N'dbo.sys_user', N'highStandarUser') IS NULL
+    ALTER TABLE dbo.sys_user ADD highStandarUser INT NOT NULL DEFAULT 0;
+GO
+
+DECLARE @schema SYSNAME;
+DECLARE @sql NVARCHAR(MAX);
+DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR
+    SELECT N'dbo' UNION ALL SELECT N'dev';
+
+OPEN schema_cursor;
+FETCH NEXT FROM schema_cursor INTO @schema;
+
+WHILE @@FETCH_STATUS = 0
+BEGIN
+    IF OBJECT_ID(QUOTENAME(@schema) + N'.dashboard', N'U') IS NULL
+    BEGIN
+        SET @sql = N'CREATE TABLE dbo.dashboard (id INT);';
+        EXEC sys.sp_executesql @sql;
+    END;
+    FETCH NEXT FROM schema_cursor INTO @schema;
+END;
+
+CLOSE schema_cursor;
+DEALLOCATE schema_cursor;
+GO";
+
+    let batches = split_sql_batches(sql);
+    assert_eq!(batches.len(), 3);
+    for batch in batches {
+        analyze_sql_references(&batch, Some("sqlserver"))
+            .unwrap_or_else(|error| panic!("the reported SQL Server batch should analyze after removing GO: {error}"));
+    }
+}
+
 #[test]
 fn sqlserver_alter_table_single_add_supports_multiple_columns() {
     for sql in [

packages/app-tests/sqlSemanticDiagnostics.test.ts

@@ -444,6 +444,56 @@ test("selects complete SQL statements intersecting the visible viewport for diag
   assert.equal(ranges[0]?.to, sql.indexOf(";\nSELECT * FROM third"));
 });
 
+test("analyzes SQL Server control flow as complete GO batches", () => {
+  const sql = `IF SCHEMA_ID(N'dev') IS NULL
+    EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo');
+GO
+
+DECLARE @schema SYSNAME;
+DECLARE @sql NVARCHAR(MAX);
+DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR
+    SELECT N'dbo' UNION ALL SELECT N'dev';
+OPEN schema_cursor;
+FETCH NEXT FROM schema_cursor INTO @schema;
+WHILE @@FETCH_STATUS = 0
+BEGIN
+    IF OBJECT_ID(QUOTENAME(@schema) + N'.dashboard', N'U') IS NULL
+    BEGIN
+        SET @sql = N'CREATE TABLE dbo.dashboard (id INT);';
+        EXEC sys.sp_executesql @sql;
+    END;
+    FETCH NEXT FROM schema_cursor INTO @schema;
+END;
+CLOSE schema_cursor;
+DEALLOCATE schema_cursor;
+GO`;
+
+  const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver");
+
+  assert.equal(ranges.length, 2);
+  assert.equal(ranges[0]?.sql, "IF SCHEMA_ID(N'dev') IS NULL\n    EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo')");
+  assert.equal(ranges[1]?.from, sql.indexOf("DECLARE @schema"));
+  assert.equal(ranges[1]?.to, sql.lastIndexOf(";\nGO"));
+  assert.match(ranges[1]?.sql ?? "", /DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD/);
+  assert.match(ranges[1]?.sql ?? "", /WHILE @@FETCH_STATUS = 0[\s\S]*FETCH NEXT FROM schema_cursor INTO @schema;\nEND/);
+  assert.doesNotMatch(ranges[1]?.sql ?? "", /^\s*GO\s*$/m);
+
+  const commentedGo = "DECLARE c CURSOR LOCAL FAST_FORWARD FOR SELECT 1;\n/*\nGO\n*/\nOPEN c;";
+  assert.equal(sqlSemanticDiagnosticRangesForViewport(commentedGo, [{ from: 0, to: commentedGo.length }], "sqlserver").length, 1);
+});
+
+test("keeps ordinary SQL Server statements viewport-local", () => {
+  const sql = "SELECT id FROM first;\nSELECT id, missing_field FROM second;\nSELECT id FROM third;";
+  const visibleFrom = sql.indexOf("missing_field");
+
+  const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: visibleFrom, to: visibleFrom + 1 }], "sqlserver");
+
+  assert.deepEqual(
+    ranges.map((range) => range.sql),
+    ["SELECT id, missing_field FROM second"],
+  );
+});
+
 test("skips Oracle PL/SQL blocks when selecting semantic diagnostic ranges", () => {
   const sql = `DECLARE
   v_order_count NUMBER;