highlight syntax errors

Author: lovasoa

CHANGELOG.md

@@ -37,6 +37,8 @@
       ```
     - Before, you would usually build the link manually with `CONCAT('/product.sql?product=', product_name)`, which would fail if the product name contained special characters like '&'. The new `sqlpage.link` function takes care of encoding the parameters correctly.
  - Calls to `json_object` are now accepted as arguments to SQLPage functions. This allows you to pass complex data structures to functions such as `sqlpage.fetch`, `sqlpage.run_sql`, and `sqlpage.link`. 
+ - Better syntax error messages, with a short quotation of the part of the SQL file that caused the error:
+  - ![syntax error](https://github.com/user-attachments/assets/86ab5628-87bd-4dea-b6fe-64ea19afcdc3)
 
 ## 0.24.0 (2024-06-23)
  - in the form component, searchable `select` fields now support more than 50 options. They used to display only the first 50 options. 

src/webserver/database/error_highlighting.rs

@@ -0,0 +1,60 @@
+use std::fmt::Write;
+
+/// Display a database error with a highlighted line and character offset.
+#[must_use]
+pub fn display_db_error(context: &str, query: &str, db_err: sqlx::error::Error) -> anyhow::Error {
+    let mut msg = format!("{context}:\n");
+    let offset = if let sqlx::error::Error::Database(db_err) = &db_err {
+        db_err.offset()
+    } else {
+        None
+    };
+    if let Some(mut offset) = offset {
+        for (line_no, line) in query.lines().enumerate() {
+            if offset > line.len() {
+                offset -= line.len() + 1;
+            } else {
+                highlight_line_offset(&mut msg, line, offset);
+                write!(msg, "line {}, character {offset}", line_no + 1).unwrap();
+                break;
+            }
+        }
+    } else {
+        write!(msg, "{}", query.lines().next().unwrap_or_default()).unwrap();
+    }
+    anyhow::Error::new(db_err).context(msg)
+}
+
+/// Highlight a line with a character offset.
+pub fn highlight_line_offset(msg: &mut String, line: &str, offset: usize) {
+    writeln!(msg, "{line}").unwrap();
+    writeln!(msg, "{}⬆️", " ".repeat(offset)).unwrap();
+}
+
+/// Highlight an error given a line and a character offset
+/// line and `col_num` are 1-based
+pub fn quote_source_with_highlight(source: &str, line_num: u64, col_num: u64) -> String {
+    let mut msg = String::new();
+    let mut current_line_num: u64 = 1; // 1-based line number
+    let col_num_usize = usize::try_from(col_num)
+        .unwrap_or_default()
+        .saturating_sub(1);
+    for line in source.lines() {
+        current_line_num += 1;
+        if current_line_num + 1 == line_num || current_line_num == line_num + 1 {
+            writeln!(msg, "{line}").unwrap();
+        } else if current_line_num == line_num {
+            highlight_line_offset(&mut msg, line, col_num_usize);
+        } else if current_line_num > line_num + 1 {
+            break;
+        }
+    }
+    msg
+}
+
+#[test]
+fn test_quote_source_with_highlight() {
+    let source = "SELECT *\nFROM table\nWHERE <syntax error>";
+    let expected = "FROM table\nWHERE <syntax error>\n     ⬆️\n";
+    assert_eq!(quote_source_with_highlight(source, 3, 6), expected);
+}

src/webserver/database/execute_queries.rs

@@ -16,7 +16,7 @@ use crate::webserver::http::SingleOrVec;
 use crate::webserver::http_request_info::RequestInfo;
 
 use super::syntax_tree::{extract_req_param, StmtParam};
-use super::{highlight_sql_error, Database, DbItem};
+use super::{error_highlighting::display_db_error, Database, DbItem};
 use sqlx::any::{AnyArguments, AnyQueryResult, AnyRow, AnyStatement, AnyTypeInfo};
 use sqlx::pool::PoolConnection;
 use sqlx::{Any, Arguments, Column, Either, Executor, Row as _, Statement, ValueRef};
@@ -33,7 +33,7 @@ impl Database {
             .prepare_with(query, param_types)
             .await
             .map(|s| s.to_owned())
-            .map_err(|e| highlight_sql_error("Failed to prepare SQL statement", query, e))
+            .map_err(|e| display_db_error("Failed to prepare SQL statement", query, e))
     }
 }
 
@@ -214,7 +214,7 @@ fn parse_single_sql_result(sql: &str, res: sqlx::Result<Either<AnyQueryResult, A
             log::debug!("Finished query with result: {:?}", res);
             DbItem::FinishedQuery
         }
-        Err(err) => DbItem::Error(highlight_sql_error(
+        Err(err) => DbItem::Error(display_db_error(
             "Failed to execute SQL statement",
             sql,
             err,

src/webserver/database/migrations.rs

@@ -1,5 +1,5 @@
+use super::error_highlighting::display_db_error;
 use super::Database;
-use crate::webserver::database::highlight_sql_error;
 use crate::MIGRATIONS_DIR;
 use anyhow;
 use anyhow::Context;
@@ -35,7 +35,7 @@ pub async fn apply(config: &crate::app_config::AppConfig, db: &Database) -> anyh
         match err {
             MigrateError::Execute(n, source) => {
                 let migration = migrator.iter().find(|&m| m.version == n).unwrap();
-                highlight_sql_error("Error in the SQL migration", &migration.sql, source).context(
+                display_db_error("Error in the SQL migration", &migration.sql, source).context(
                     format!("Failed to apply migration {}", DisplayMigration(migration)),
                 )
             }

src/webserver/database/mod.rs

@@ -6,6 +6,7 @@ mod sql;
 mod sqlpage_functions;
 mod syntax_tree;
 
+mod error_highlighting;
 mod sql_to_json;
 
 pub use sql::{make_placeholder, ParsedSqlFile};
@@ -21,36 +22,6 @@ pub enum DbItem {
     Error(anyhow::Error),
 }
 
-#[must_use]
-pub fn highlight_sql_error(
-    context: &str,
-    query: &str,
-    db_err: sqlx::error::Error,
-) -> anyhow::Error {
-    use std::fmt::Write;
-    let mut msg = format!("{context}:\n");
-    let offset = if let sqlx::error::Error::Database(db_err) = &db_err {
-        db_err.offset()
-    } else {
-        None
-    };
-    if let Some(mut offset) = offset {
-        for (line_no, line) in query.lines().enumerate() {
-            if offset > line.len() {
-                offset -= line.len() + 1;
-            } else {
-                writeln!(msg, "{line}").unwrap();
-                writeln!(msg, "{}⬆️", " ".repeat(offset)).unwrap();
-                write!(msg, "line {}, character {offset}", line_no + 1).unwrap();
-                break;
-            }
-        }
-    } else {
-        write!(msg, "{}", query.lines().next().unwrap_or_default()).unwrap();
-    }
-    anyhow::Error::new(db_err).context(msg)
-}
-
 impl std::fmt::Display for Database {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{:?}", self.connection.any_kind())

src/webserver/database/sql.rs

@@ -3,8 +3,8 @@ use super::sqlpage_functions::functions::SqlPageFunctionName;
 use super::sqlpage_functions::{are_params_extractable, func_call_to_param};
 use super::syntax_tree::StmtParam;
 use crate::file_cache::AsyncFromStrWithState;
+use crate::webserver::database::error_highlighting::quote_source_with_highlight;
 use crate::{AppState, Database};
-use anyhow::Context;
 use async_trait::async_trait;
 use sqlparser::ast::{
     BinaryOperator, CastKind, CharacterLength, DataType, Expr, Function, FunctionArg,
@@ -16,7 +16,6 @@ use sqlparser::parser::{Parser, ParserError};
 use sqlparser::tokenizer::Token::{SemiColon, EOF};
 use sqlparser::tokenizer::Tokenizer;
 use sqlx::any::AnyKind;
-use std::fmt::Write;
 use std::ops::ControlFlow;
 use std::str::FromStr;
 
@@ -92,21 +91,28 @@ fn parse_sql<'a>(
     log::trace!("Parsing SQL: {sql}");
     let tokens = Tokenizer::new(dialect, sql)
         .tokenize_with_location()
-        .with_context(|| "SQLPage's SQL parser could not tokenize the sql file")?;
+        .map_err(|err| {
+            let location = err.location;
+            anyhow::Error::new(err).context(format!("The SQLPage parser couldn't understand the SQL file. Tokenization failed. Please check for syntax errors:\n{}", quote_source_with_highlight(sql, location.line, location.column)))
+        })?;
     let mut parser = Parser::new(dialect).with_tokens_with_locations(tokens);
     let db_kind = kind_of_dialect(dialect);
     Ok(std::iter::from_fn(move || {
-        parse_single_statement(&mut parser, db_kind)
+        parse_single_statement(&mut parser, db_kind, sql)
     }))
 }
 
-fn parse_single_statement(parser: &mut Parser<'_>, db_kind: AnyKind) -> Option<ParsedStatement> {
+fn parse_single_statement(
+    parser: &mut Parser<'_>,
+    db_kind: AnyKind,
+    source_sql: &str,
+) -> Option<ParsedStatement> {
     if parser.peek_token() == EOF {
         return None;
     }
     let mut stmt = match parser.parse_statement() {
         Ok(stmt) => stmt,
-        Err(err) => return Some(syntax_error(err, parser)),
+        Err(err) => return Some(syntax_error(err, parser, source_sql)),
     };
     log::debug!("Parsed statement: {stmt}");
     let mut semicolon = false;
@@ -145,25 +151,13 @@ fn parse_single_statement(parser: &mut Parser<'_>, db_kind: AnyKind) -> Option<P
     }))
 }
 
-fn syntax_error(err: ParserError, parser: &mut Parser) -> ParsedStatement {
-    let mut err_msg = String::with_capacity(128);
-    parser.prev_token(); // go back to the token that caused the error
-    for i in 0..32 {
-        let next_token = parser.next_token();
-        if i == 0 {
-            writeln!(
-                &mut err_msg,
-                "SQLPage found a syntax error on line {}, character {}:",
-                next_token.location.line, next_token.location.column
-            )
-            .unwrap();
-        }
-        if next_token == EOF {
-            break;
-        }
-        write!(&mut err_msg, "{next_token} ").unwrap();
-    }
-    ParsedStatement::Error(anyhow::Error::from(err).context(err_msg))
+fn syntax_error(err: ParserError, parser: &Parser, sql: &str) -> ParsedStatement {
+    dbg!((&err, &parser.peek_token_no_skip(), &sql));
+    let location = parser.peek_token_no_skip().location;
+    ParsedStatement::Error(anyhow::Error::from(err).context(format!(
+        "The SQLPage parser couldn't understand the SQL file. Parsing failed. Please check for syntax errors:\n{}",
+        quote_source_with_highlight(sql, location.line, location.column)
+    )))
 }
 
 fn dialect_for_db(db_kind: AnyKind) -> Box<dyn Dialect> {
@@ -856,7 +850,8 @@ mod test {
     #[test]
     fn test_sqlpage_function_with_argument() {
         for &(dialect, kind) in ALL_DIALECTS {
-            let mut ast = parse_stmt("select sqlpage.fetch($x)", dialect);
+            let sql = "select sqlpage.fetch($x)";
+            let mut ast = parse_stmt(sql, dialect);
             let parameters = ParameterExtractor::extract_parameters(&mut ast, kind);
             assert_eq!(
                 parameters,
@@ -874,7 +869,7 @@ mod test {
         let sql = "set x = $y";
         for &(dialect, db_kind) in ALL_DIALECTS {
             let mut parser = Parser::new(dialect).try_with_sql(sql).unwrap();
-            let stmt = parse_single_statement(&mut parser, db_kind);
+            let stmt = parse_single_statement(&mut parser, db_kind, sql);
             if let Some(ParsedStatement::SetVariable {
                 variable,
                 value: StmtWithParams { query, params, .. },