From 567aa31c07c82908cb12989636dab88d001057ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 3 Jun 2025 19:12:50 +0200 Subject: [PATCH] feat(errors): improve error messages with example values Add token type examples to make error messages more helpful. Created an ExpectedToken enum to replace string literals for better type safety, added example values for each token type, and enhanced error display to show concrete examples of valid syntax. --- notes.org | 25 +++++++++++- src/command.rs | 39 ++++++++++++++++++- src/error_display.rs | 36 +++++++++++++---- src/parser.rs | 22 +++++------ ...parser__tests__parse_insert_command-2.snap | 2 +- ...parser__tests__parse_insert_command-3.snap | 2 +- ...ser__tests__parse_missing_semicolon-2.snap | 2 +- ...arser__tests__parse_missing_semicolon.snap | 2 +- 8 files changed, 105 insertions(+), 25 deletions(-) diff --git a/notes.org b/notes.org index 8a40aac..6ea2e37 100644 --- a/notes.org +++ b/notes.org @@ -235,7 +235,7 @@ i will use rustyline, since it seems like the most feature-complete insert ** DONE parse row insert * DONE separate statements with semicolons -* TODO this error message could be better +* DONE this error message could be better #+begin example Error: unexpected token ╭─[ :1:24 ] @@ -247,8 +247,29 @@ Error: unexpected token │ Note: expected token type to be one of ["semicolon"] ───╯ #+end example -* TODO correct all instances of in locations +** plan +1. Create an example mapping system + - Define a mapping of token types to example values + - Example: "integer" → "42", "string" → "example", "semicolon" → ";" +2. Enhance CommandParseError + - Add a method to generate user-friendly error messages + - Include both the expected token type and concrete examples +3. Implementation approach + - Create a static lookup table or function that returns examples + - Extend existing error handling to include examples in messages + - Make sure the examples follow SQL syntax conventions +4. Error display refinement + - Update error_display.rs to include these examples + - Format error messages to show both what was expected and example syntax +5. Testing + - Add tests that verify the error messages include helpful examples + - Ensure examples are contextually appropriate + +This will make errors like "expected semicolon" more helpful by showing "expected semicolon (example: ;)". +TODO * TODO correct all instances of in locations * TODO meta-commands must be followed by end-of-file +* TODO project code documentation +* TODO project usage documentation * DONE in case of parse error, skip until next semicolon to better recover * TODO serialize/deserialize row to/from raw bytes ** TODO look for best practices for creating binary formats diff --git a/src/command.rs b/src/command.rs index d19aeaa..43204a6 100644 --- a/src/command.rs +++ b/src/command.rs @@ -47,10 +47,47 @@ impl Command { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExpectedToken { + Integer, + String, + Semicolon, + Statement, + MetaCommand, + EndOfFile, +} + +impl ExpectedToken { + /// Returns an example value for this token type + pub fn example(&self) -> &'static str { + match self { + ExpectedToken::Integer => "42", + ExpectedToken::String => "\"example\"", + ExpectedToken::Semicolon => ";", + ExpectedToken::Statement => "select", + ExpectedToken::MetaCommand => ".exit", + ExpectedToken::EndOfFile => "", + } + } +} + +impl std::fmt::Display for ExpectedToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ExpectedToken::Integer => write!(f, "integer"), + ExpectedToken::String => write!(f, "string"), + ExpectedToken::Semicolon => write!(f, "semicolon"), + ExpectedToken::Statement => write!(f, "statement"), + ExpectedToken::MetaCommand => write!(f, "meta command"), + ExpectedToken::EndOfFile => write!(f, "end of file"), + } + } +} + #[derive(Debug)] pub enum CommandParseError { Scan(ScanError), - UnexpectedToken(Token, &'static [&'static str]), + UnexpectedToken(Token, &'static [ExpectedToken]), } impl From for Command { diff --git a/src/error_display.rs b/src/error_display.rs index 45b237c..92c8936 100644 --- a/src/error_display.rs +++ b/src/error_display.rs @@ -11,19 +11,41 @@ impl OSDBError for CommandParseError { CommandParseError::Scan(x) => { x.display(file, input); } - CommandParseError::UnexpectedToken(token, items) => { + CommandParseError::UnexpectedToken(token, expected_tokens) => { let location = (file, Into::>::into(&token.location)); - Report::build(ReportKind::Error, location.clone()) + + let mut report = Report::build(ReportKind::Error, location.clone()) .with_message("unexpected token") .with_label( Label::new(location.clone()) .with_color(Color::Red) .with_message(format!("found {token}")), - ) - .with_note(format!("expected token type to be one of {items:?}")) - .finish() - .eprint((file, Source::from(input))) - .unwrap() + ); + + // If we have expected tokens, show an example for the first one + if let Some(first_expected) = expected_tokens.get(0) { + let example = first_expected.example(); + + // Add a note with all expected types + let expected_types: Vec<_> = expected_tokens + .iter() + .map(|t| format!("{}", t)) + .collect(); + + // Use singular form when there's only one expected token type + match expected_types.as_slice() { + [single_type] => { + report = report.with_note(format!("expected: {}", single_type)); + }, + _ => { + report = report.with_note(format!("expected one of: {}", expected_types.join(", "))); + } + } + report = + report.with_help(format!("try a token of the expected type: {example}")) + } + + report.finish().eprint((file, Source::from(input))).unwrap() } } } diff --git a/src/parser.rs b/src/parser.rs index 585f5f8..912aba1 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,7 +1,7 @@ use std::collections::VecDeque; use crate::{ - command::{Command, CommandParseError}, + command::{Command, CommandParseError, ExpectedToken}, statements::Statement, tokens::{Location, Token, TokenData, tokenize}, }; @@ -37,7 +37,7 @@ fn expect_semicolon(tokens: &mut VecDeque) -> Result<(), C } _ => Err(CommandParseError::UnexpectedToken( next_token.clone(), - &["semicolon"], + &[ExpectedToken::Semicolon], )), } } else { @@ -51,7 +51,7 @@ fn expect_semicolon(tokens: &mut VecDeque) -> Result<(), C data: TokenData::EndOfFile, lexeme: String::new(), }, - &["semicolon"], + &[ExpectedToken::Semicolon], )) } } @@ -101,14 +101,14 @@ pub fn parse(file: String, input: String) -> Result, Vec { errs.push(CommandParseError::UnexpectedToken( token, - &["statement", "meta command", "eof"], + &[ExpectedToken::Statement, ExpectedToken::MetaCommand, ExpectedToken::EndOfFile], )); skip_to_next_statement(&mut tokens); } TokenData::String(_) => { errs.push(CommandParseError::UnexpectedToken( token, - &["statement", "meta command", "eof"], + &[ExpectedToken::Statement, ExpectedToken::MetaCommand, ExpectedToken::EndOfFile], )); skip_to_next_statement(&mut tokens); } @@ -135,13 +135,13 @@ fn parse_insert_command( data: TokenData::EndOfFile, lexeme: String::new(), }, - &["integer"], + &[ExpectedToken::Integer], ) })?; let id = match id_token.data { TokenData::Int(id) => id, - _ => return Err(CommandParseError::UnexpectedToken(id_token, &["integer"])), + _ => return Err(CommandParseError::UnexpectedToken(id_token, &[ExpectedToken::Integer])), }; // Parse the username (string) @@ -155,7 +155,7 @@ fn parse_insert_command( data: TokenData::EndOfFile, lexeme: String::new(), }, - &["string"], + &[ExpectedToken::String], ) })?; @@ -164,7 +164,7 @@ fn parse_insert_command( _ => { return Err(CommandParseError::UnexpectedToken( username_token, - &["string"], + &[ExpectedToken::String], )); } }; @@ -180,13 +180,13 @@ fn parse_insert_command( data: TokenData::EndOfFile, lexeme: String::new(), }, - &["string"], + &[ExpectedToken::String], ) })?; let email = match email_token.data { TokenData::String(email) => email, - _ => return Err(CommandParseError::UnexpectedToken(email_token, &["string"])), + _ => return Err(CommandParseError::UnexpectedToken(email_token, &[ExpectedToken::String])), }; // Check for semicolon after the insert command diff --git a/src/snapshots/osdb__parser__tests__parse_insert_command-2.snap b/src/snapshots/osdb__parser__tests__parse_insert_command-2.snap index 72d9ec6..a4a6547 100644 --- a/src/snapshots/osdb__parser__tests__parse_insert_command-2.snap +++ b/src/snapshots/osdb__parser__tests__parse_insert_command-2.snap @@ -17,7 +17,7 @@ Err( lexeme: "\"not_an_id\"", }, [ - "integer", + Integer, ], ), ], diff --git a/src/snapshots/osdb__parser__tests__parse_insert_command-3.snap b/src/snapshots/osdb__parser__tests__parse_insert_command-3.snap index a4fbebd..8a505fc 100644 --- a/src/snapshots/osdb__parser__tests__parse_insert_command-3.snap +++ b/src/snapshots/osdb__parser__tests__parse_insert_command-3.snap @@ -15,7 +15,7 @@ Err( lexeme: ";", }, [ - "string", + String, ], ), ], diff --git a/src/snapshots/osdb__parser__tests__parse_missing_semicolon-2.snap b/src/snapshots/osdb__parser__tests__parse_missing_semicolon-2.snap index a6787ca..9802ae3 100644 --- a/src/snapshots/osdb__parser__tests__parse_missing_semicolon-2.snap +++ b/src/snapshots/osdb__parser__tests__parse_missing_semicolon-2.snap @@ -15,7 +15,7 @@ Err( lexeme: "", }, [ - "semicolon", + Semicolon, ], ), ], diff --git a/src/snapshots/osdb__parser__tests__parse_missing_semicolon.snap b/src/snapshots/osdb__parser__tests__parse_missing_semicolon.snap index ad0856f..92022f3 100644 --- a/src/snapshots/osdb__parser__tests__parse_missing_semicolon.snap +++ b/src/snapshots/osdb__parser__tests__parse_missing_semicolon.snap @@ -15,7 +15,7 @@ Err( lexeme: "", }, [ - "semicolon", + Semicolon, ], ), ],