fix+feat: codebase review improvements — 9 bug fixes, expanded linter, LSP/DAP fixes

.gitlab-ci.yml

@@ -154,7 +154,7 @@ cargo-build:
   stage: build
   image: rust:latest
   script:
-    - cargo build --release
+    - cargo build --workspace --all-targets --release
   artifacts:
     paths:
       - target/release/

src/bridges/VclTotalBridge.res

@@ -49,18 +49,7 @@ type checkReport = {
 /// against a schema — that requires the TypeLL server.
 let checkQuery = (queryStr: string): result<checkReport, string> => {
   switch VclTotalParser.parse(queryStr) {
-  | Error(msg) =>
-    Ok({
-      valid: false,
-      maxLevel: 0,
-      maxLevelName: "Parse-time safety",
-      queryPath: "VCL (Slipstream)",
-      levelDiagnostics: [`Parse error: ${msg}`],
-      explanation: `Parse failed: ${msg}`,
-      ast: None,
-      effects: [],
-      usage: "omega",
-    })
+  | Error(msg) => Error(`Parse failed: ${msg}`)
   | Ok(stmt) =>
     // Determine max level from present clauses
     let maxLevel = VclTotalAst.safetyLevelToInt(stmt.requestedLevel)
@@ -119,20 +108,30 @@ let checkQuery = (queryStr: string): result<checkReport, string> => {
   }
 }
 
+/// Escape a string for safe inclusion in a JSON string value.
+let escapeJsonString = (s: string): string =>
+  s
+  ->String.replaceAll("\\", "\\\\")
+  ->String.replaceAll("\"", "\\\"")
+  ->String.replaceAll("\n", "\\n")
+  ->String.replaceAll("\r", "\\r")
+  ->String.replaceAll("\t", "\\t")
+
 /// Encode a checkReport as JSON for transport to TypeLL server.
 let reportToJson = (report: checkReport): string => {
   let effectsJson = report.effects
-    ->Array.map(e => `"${e}"`)
+    ->Array.map(e => `"${escapeJsonString(e)}"`)
     ->Array.join(", ")
 
   let diagnosticsJson = report.levelDiagnostics
-    ->Array.map(d => {
-      let escaped = d->String.replaceAll("\"", "\\\"")
-      `"${escaped}"`
-    })
+    ->Array.map(d => `"${escapeJsonString(d)}"`)
     ->Array.join(", ")
 
-  `{"valid":${report.valid ? "true" : "false"},"maxLevel":${Int.toString(report.maxLevel)},"maxLevelName":"${report.maxLevelName}","queryPath":"${report.queryPath}","effects":[${effectsJson}],"usage":"${report.usage}","levelDiagnostics":[${diagnosticsJson}]}`
+  let maxLevelName = escapeJsonString(report.maxLevelName)
+  let queryPath = escapeJsonString(report.queryPath)
+  let usage = escapeJsonString(report.usage)
+
+  `{"valid":${report.valid ? "true" : "false"},"maxLevel":${Int.toString(report.maxLevel)},"maxLevelName":"${maxLevelName}","queryPath":"${queryPath}","effects":[${effectsJson}],"usage":"${usage}","levelDiagnostics":[${diagnosticsJson}]}`
 }
 
 /// Parse a query and return just the safety level (quick check).

src/core/Checker.idr

@@ -132,29 +132,6 @@ extractFieldRefs (EAggregate _ e _)     = extractFieldRefs e
 extractFieldRefs (EParam _ _)           = []
 extractFieldRefs EStar                  = []
 extractFieldRefs (ESubquery sub)        = statementFieldRefs sub
-  where
-    ||| Collect field references from all clauses of a statement.
-    ||| Gathers from: selectItems, whereClause, groupBy, having, orderBy.
-    statementFieldRefs : Statement -> List FieldRef
-    statementFieldRefs stmt =
-      let selRefs : List FieldRef
-          selRefs = concatMap selItemRefs (selectItems stmt)
-          whereRefs : List FieldRef
-          whereRefs = maybe [] extractFieldRefs (whereClause stmt)
-          groupRefs : List FieldRef
-          groupRefs = groupBy stmt
-          havingRefs : List FieldRef
-          havingRefs = maybe [] extractFieldRefs (having stmt)
-          orderRefs : List FieldRef
-          orderRefs = map fst (orderBy stmt)
-      in selRefs ++ whereRefs ++ groupRefs ++ havingRefs ++ orderRefs
-
-    ||| Extract field references from a single SELECT item.
-    selItemRefs : SelectItem -> List FieldRef
-    selItemRefs (SelField ref)       = [ref]
-    selItemRefs (SelModality _)      = []
-    selItemRefs (SelAggregate _ e)   = extractFieldRefs e
-    selItemRefs SelStar              = []
 
 ||| Collect all field references from every clause of a statement.
 ||| Delegates to extractFieldRefs for each expression-bearing clause.

src/errors/VclTotalError.res

@@ -20,6 +20,7 @@ type errorCode =
   | TypeError         // Level 2: incompatible types in comparison
   | NullError         // Level 3: nullable field used without guard
   | InjectionAttempt  // Level 4: raw string literal in unsafe position
+  | ResultTypeError   // Level 5: unresolved result type (TAny in select)
   | CardinalityViolation // Level 6: unbounded result without LIMIT
   | EffectViolation   // Level 7: undeclared effect
   | TemporalBoundsExceeded // Level 8: version constraint violation
@@ -43,11 +44,12 @@ let errorCodeToInt = (code: errorCode): int =>
   | TypeError => 3
   | NullError => 4
   | InjectionAttempt => 5
-  | CardinalityViolation => 6
-  | EffectViolation => 7
-  | TemporalBoundsExceeded => 8
-  | LinearityViolation => 9
-  | InternalError => 10
+  | ResultTypeError => 6
+  | CardinalityViolation => 7
+  | EffectViolation => 8
+  | TemporalBoundsExceeded => 9
+  | LinearityViolation => 10
+  | InternalError => 11
   }
 
 /// Convert an ABI integer tag to an error code.
@@ -58,11 +60,12 @@ let errorCodeFromInt = (n: int): option<errorCode> =>
   | 3 => Some(TypeError)
   | 4 => Some(NullError)
   | 5 => Some(InjectionAttempt)
-  | 6 => Some(CardinalityViolation)
-  | 7 => Some(EffectViolation)
-  | 8 => Some(TemporalBoundsExceeded)
-  | 9 => Some(LinearityViolation)
-  | 10 => Some(InternalError)
+  | 6 => Some(ResultTypeError)
+  | 7 => Some(CardinalityViolation)
+  | 8 => Some(EffectViolation)
+  | 9 => Some(TemporalBoundsExceeded)
+  | 10 => Some(LinearityViolation)
+  | 11 => Some(InternalError)
   | _ => None
   }
 

src/interface/dap/src/main.rs

@@ -165,23 +165,35 @@ fn handle_client(stream: TcpStream) -> Result<(), Box<dyn std::error::Error>> {
                 })?
             }
             "continue" => {
-                // TODO: Execute the query using database-mcp cartridge
-                // For now, return a dummy response
-                let query = if let Some(args) = &request.arguments {
-                    args.get("query").and_then(|q| q.as_str()).unwrap_or("").to_string()
+                let query = request.arguments
+                    .as_ref()
+                    .and_then(|args| args.get("query"))
+                    .and_then(|q| q.as_str())
+                    .unwrap_or("")
+                    .to_string();
+
+                if query.is_empty() {
+                    serde_json::to_string(&DapResponse {
+                        seq: 9,
+                        r#type: "response".to_string(),
+                        request_seq: request.seq,
+                        command: "continue".to_string(),
+                        success: false,
+                        message: Some("Missing or invalid 'query' argument".to_string()),
+                        body: None,
+                    })?
                 } else {
-                    "".to_string()
-                };
-                let result = execute_vql_query(&query);
-                serde_json::to_string(&DapResponse {
-                    seq: 9,
-                    r#type: "response".to_string(),
-                    request_seq: request.seq,
-                    command: "continue".to_string(),
-                    success: true,
-                    message: Some(format!("Query executed: {}", result)),
-                    body: None,
-                })?
+                    let result = execute_vql_query(&query);
+                    serde_json::to_string(&DapResponse {
+                        seq: 9,
+                        r#type: "response".to_string(),
+                        request_seq: request.seq,
+                        command: "continue".to_string(),
+                        success: true,
+                        message: Some(format!("Query executed: {}", result)),
+                        body: None,
+                    })?
+                }
             }
             "disconnect" => {
                 serde_json::to_string(&DapResponse {

src/interface/fmt/src/lib.rs

@@ -13,11 +13,20 @@
 /// All lines are trimmed of leading/trailing whitespace before processing.
 pub fn format_vqlut(content: &str) -> String {
     let mut formatted = String::new();
-    let keywords = ["SELECT", "FROM", "WHERE", "GROUP", "ORDER", "HAVING", "LIMIT"];
+    let keywords = [
+        "SELECT", "FROM", "WHERE", "GROUP", "ORDER", "HAVING", "LIMIT",
+        "OFFSET", "EFFECTS", "PROOF", "CONSUME",
+    ];
 
     for line in content.lines() {
         let trimmed = line.trim();
-        if keywords.iter().any(|&kw| trimmed.starts_with(kw)) {
+        if keywords.iter().any(|&kw| {
+            trimmed.starts_with(kw)
+                && trimmed
+                    .as_bytes()
+                    .get(kw.len())
+                    .map_or(true, |&b| !b.is_ascii_alphanumeric() && b != b'_')
+        }) {
             formatted.push_str("  ");
         }
         formatted.push_str(trimmed);

src/interface/fmt/src/main.rs

@@ -27,7 +27,7 @@ fn main() {
     let content = fs::read_to_string(&input_path).expect("Unable to read file");
 
     // Format the content (basic indentation for now)
-    let formatted = vqlut_fmt::format_vqlut(&content);
+    let formatted = vcltotal_fmt::format_vqlut(&content);
 
     // Write the output file
     let output_path = args.output.unwrap_or(input_path);

src/interface/lint/src/lib.rs

@@ -4,7 +4,7 @@
 //! VCL-total Linting Library
 //!
 //! Provides linting capabilities for VCL-total query files.
-//! Checks for missing semicolons and lowercase keywords.
+//! Checks for missing semicolons, lowercase keywords, SELECT *, and OFFSET without LIMIT.
 
 /// A single lint issue found in VCL-total content.
 #[derive(Debug)]
@@ -19,8 +19,10 @@ pub struct LintIssue {
 ///
 /// Current checks:
 /// - Missing semicolons at the end of non-empty lines
-/// - Lowercase keywords (select, from, where, group, order, having, limit)
+/// - Lowercase keywords (SQL and VCL-total extension keywords)
 ///   when surrounded by spaces (` keyword `)
+/// - `SELECT *` usage (prefer explicit columns for Level 5 result typing)
+/// - `OFFSET` without `LIMIT` (likely a mistake)
 pub fn lint_vqlut(content: &str) -> Vec<LintIssue> {
     let mut issues = Vec::new();
 
@@ -36,16 +38,63 @@ pub fn lint_vqlut(content: &str) -> Vec<LintIssue> {
         }
     }
 
-    // Check for uppercase keywords
+    // Check for lowercase keywords: flag keywords that appear (case-insensitive)
+    // but are not already uppercase in the original text.
+    let keywords = [
+        "select", "from", "where", "group", "order", "having", "limit",
+        "offset", "effects", "proof", "consume", "usage",
+    ];
     for (i, line) in content.lines().enumerate() {
         let line_num = i + 1;
-        let keywords = ["select", "from", "where", "group", "order", "having", "limit"];
         for keyword in keywords {
-            if line.to_lowercase().contains(&format!(" {} ", keyword)) {
+            let lower_pattern = format!(" {} ", keyword);
+            if line.to_lowercase().contains(&lower_pattern) {
+                let upper_pattern = format!(" {} ", keyword.to_uppercase());
+                if !line.contains(&upper_pattern) {
+                    issues.push(LintIssue {
+                        line: line_num,
+                        message: format!(
+                            "Keyword '{}' should be uppercase",
+                            keyword.to_uppercase()
+                        ),
+                    });
+                }
+            }
+        }
+    }
+
+    // Check for SELECT * (prefer explicit columns for Level 5 result typing)
+    let upper_content = content.to_uppercase();
+    for (i, line) in upper_content.lines().enumerate() {
+        let line_num = i + 1;
+        let trimmed = line.trim();
+        if trimmed.starts_with("SELECT") && trimmed.contains("SELECT *") {
+            issues.push(LintIssue {
+                line: line_num,
+                message: "Prefer explicit column list over SELECT * for result-type safety (Level 5)".to_string(),
+            });
+        }
+    }
+
+    // Check for OFFSET without LIMIT (likely a mistake — OFFSET is meaningless without LIMIT)
+    let has_offset = upper_content.lines().any(|l| {
+        let t = l.trim();
+        t.starts_with("OFFSET") || t.contains(" OFFSET ")
+    });
+    let has_limit = upper_content.lines().any(|l| {
+        let t = l.trim();
+        t.starts_with("LIMIT") || t.contains(" LIMIT ")
+    });
+    if has_offset && !has_limit {
+        // Report on the line containing OFFSET
+        for (i, line) in upper_content.lines().enumerate() {
+            let trimmed = line.trim();
+            if trimmed.starts_with("OFFSET") || trimmed.contains(" OFFSET ") {
                 issues.push(LintIssue {
-                    line: line_num,
-                    message: format!("Keyword '{}' should be uppercase", keyword.to_uppercase()),
+                    line: i + 1,
+                    message: "OFFSET without LIMIT has no effect".to_string(),
                 });
+                break;
             }
         }
     }

src/interface/lint/src/main.rs

@@ -23,7 +23,7 @@ fn main() {
     let content = fs::read_to_string(&input_path).expect("Unable to read file");
 
     // Lint the content
-    let issues = vqlut_lint::lint_vqlut(&content);
+    let issues = vcltotal_lint::lint_vqlut(&content);
 
     // Print the issues
     for issue in &issues {