Skip to content

Commit

Permalink
Nicer parse errors using miette
Browse files Browse the repository at this point in the history
I noticed that the parse errors were a bit hard to read - only the nearest token and the line/col offsets were printed.

I made a first attempt at improving the errors using [miette](https://github.com/zkat/miette).
- Added derive for `miette::Diagnostic` to both the parser's error type and LimboError.
- Added miette dependency to both sqlite3_parser and core. The `fancy` feature is only enabled for CLI.

Some future improvements that can be made further:
- Add spans to AST nodes so that errors can better point to the correct token. See upstream issue: gwenn/lemon-rs#33
- Construct more errors with offset information. I noticed that most parser errors are constructed with `None` as the offset.

Comparisons.
Before:
```
❯ cargo run --package limbo --bin limbo database.db --output-mode pretty
...
limbo> selet * from a;
[2025-01-05T11:22:55Z ERROR sqlite3Parser] near "Token([115, 101, 108, 101, 116])": syntax error
Parse error: near "selet": syntax error at (1, 6)
```

After:
```
❯ cargo run --package limbo --bin limbo database.db --output-mode pretty
...
limbo> selet * from a;
[2025-01-05T12:25:52Z ERROR sqlite3Parser] near "Token([115, 101, 108, 101, 116])": syntax error

  × near "selet": syntax error at (1, 6)
   ╭────
 1 │ selet * from a
   ·     ▲
   ·     ╰── syntax error
   ╰────

```
  • Loading branch information
Samyak2 committed Jan 5, 2025
1 parent fdbf62d commit c09a0bc
Show file tree
Hide file tree
Showing 10 changed files with 224 additions and 59 deletions.
102 changes: 102 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ limbo_core = { path = "../core" }
rustyline = "12.0.0"
ctrlc = "3.4.4"
csv = "1.3.1"
miette = { version = "7.4.0", features = ["fancy"] }
10 changes: 8 additions & 2 deletions cli/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,10 @@ impl Limbo {
break;
}
Err(err) => {
let _ = self.write_fmt(format_args!("{}", err));
let _ = self.write_fmt(format_args!(
"{:?}",
miette::Error::from(err).with_source_code(sql.to_owned())
));
break;
}
}
Expand All @@ -595,7 +598,10 @@ impl Limbo {
},
Ok(None) => {}
Err(err) => {
let _ = self.write_fmt(format_args!("{}", err));
let _ = self.write_fmt(format_args!(
"{:?}",
miette::Error::from(err).with_source_code(sql.to_owned())
));
}
}
// for now let's cache flush always
Expand Down
1 change: 1 addition & 0 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ rand = "0.8.5"
bumpalo = { version = "3.16.0", features = ["collections", "boxed"] }
limbo_macros = { path = "../macros" }
uuid = { version = "1.11.0", features = ["v4", "v7"], optional = true }
miette = "7.4.0"

[target.'cfg(not(target_family = "windows"))'.dev-dependencies]
pprof = { version = "0.14.0", features = ["criterion", "flamegraph"] }
Expand Down
5 changes: 3 additions & 2 deletions core/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use thiserror::Error;

#[derive(Debug, Error)]
#[derive(Debug, Error, miette::Diagnostic)]
pub enum LimboError {
#[error("Corrupt database: {0}")]
Corrupt(String),
Expand All @@ -10,7 +10,8 @@ pub enum LimboError {
InternalError(String),
#[error("Parse error: {0}")]
ParseError(String),
#[error("Parse error: {0}")]
#[error(transparent)]
#[diagnostic(transparent)]
LexerError(#[from] sqlite3_parser::lexer::sql::Error),
#[error("Conversion error: {0}")]
ConversionError(String),
Expand Down
1 change: 1 addition & 0 deletions vendored/sqlite3-parser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fallible-iterator = "0.3"
bitflags = "2.0"
uncased = "0.9.10"
indexmap = "2.0"
miette = "7.4.0"

[dev-dependencies]
env_logger = { version = "0.11", default-features = false }
Expand Down
6 changes: 6 additions & 0 deletions vendored/sqlite3-parser/src/lexer/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ impl<S: Splitter> Scanner<S> {
pub fn column(&self) -> usize {
self.column
}

/// Current byte offset in the source string
pub fn offset(&self) -> usize {
self.offset
}

/// Associated splitter
pub fn splitter(&self) -> &S {
&self.splitter
Expand Down
Loading

0 comments on commit c09a0bc

Please sign in to comment.