Skip to content

Commit ffbdcbf

Browse files
TimelordUKclaude
andcommitted
feat: improve parentheses error detection and messages
- Add parentheses depth tracking to Parser struct - Detect and report unclosed parentheses with count of missing closing parens - Detect and report extra closing parentheses with no matching opening paren - Add balance check at end of parsing to catch all imbalanced cases - Add comprehensive test suite for parentheses errors - Improve error messages from generic "Expected RightParen" to specific: - "Unclosed parenthesis - missing N closing parentheses" - "Unexpected closing parenthesis - no matching opening parenthesis" This addresses the user's request for better detection and reporting of parentheses errors, which are the most common SQL syntax errors. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e59bfe0 commit ffbdcbf

File tree

4 files changed

+199
-4
lines changed

4 files changed

+199
-4
lines changed

data/test_numeric_columns.csv

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Borough,Major Class Description,Minor Class Description,202202,202203,202204,202205,202206,202207,202208,202209,202210,202211,202212,202301,202302,202303,202304,202305,202306,202307,202308,202309,202310,202311,202312,202401
2+
Barking and Dagenham,Burglary,Aggravated Burglary,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,1.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,1.0,0.0,0.0,0.0
3+
Barking and Dagenham,Burglary,Burglary - Business and Community,5.0,5.0,3.0,3.0,2.0,3.0,1.0,5.0,4.0,2.0,7.0,6.0,1.0,3.0,6.0,9.0,3.0,4.0,4.0,4.0,6.0,7.0,4.0,2.0
4+
Barking and Dagenham,Burglary,Burglary - Residential,1.0,1.0,1.0,2.0,1.0,2.0,0.0,3.0,3.0,1.0,1.0,2.0,0.0,1.0,1.0,0.0,0.0,4.0,2.0,5.0,1.0,1.0,1.0,0.0
5+
Barking and Dagenham,Criminal Damage,Criminal Damage To Dwelling,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,1.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0
6+
Barking and Dagenham,Criminal Damage,Criminal Damage To Motor Vehicle,2.0,1.0,4.0,2.0,3.0,0.0,3.0,0.0,1.0,4.0,1.0,0.0,1.0,3.0,2.0,4.0,3.0,1.0,0.0,3.0,1.0,1.0,1.0,2.0

data/test_sorting.csv

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Category,Product,Price,Quantity
2+
Electronics,Laptop,999.99,5
3+
Electronics,Phone,699.99,10
4+
Electronics,Tablet,499.99,8
5+
Clothing,Shirt,29.99,20
6+
Clothing,Pants,59.99,15
7+
Clothing,Shoes,89.99,12
8+
Books,Fiction,14.99,50
9+
Books,NonFiction,24.99,30
10+
Books,TextBook,99.99,10

sql-cli/src/recursive_parser.rs

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ pub struct Parser {
366366
current_token: Token,
367367
in_method_args: bool, // Track if we're parsing method arguments
368368
columns: Vec<String>, // Known column names for context-aware parsing
369+
paren_depth: i32, // Track parentheses nesting depth
369370
}
370371

371372
impl Parser {
@@ -377,6 +378,7 @@ impl Parser {
377378
current_token,
378379
in_method_args: false,
379380
columns: Vec::new(),
381+
paren_depth: 0,
380382
}
381383
}
382384

@@ -387,17 +389,59 @@ impl Parser {
387389

388390
fn consume(&mut self, expected: Token) -> Result<(), String> {
389391
if std::mem::discriminant(&self.current_token) == std::mem::discriminant(&expected) {
392+
// Track parentheses depth
393+
match &expected {
394+
Token::LeftParen => self.paren_depth += 1,
395+
Token::RightParen => {
396+
self.paren_depth -= 1;
397+
// Check for extra closing parenthesis
398+
if self.paren_depth < 0 {
399+
return Err(
400+
"Unexpected closing parenthesis - no matching opening parenthesis"
401+
.to_string(),
402+
);
403+
}
404+
}
405+
_ => {}
406+
}
407+
390408
self.current_token = self.lexer.next_token();
391409
Ok(())
392410
} else {
393-
Err(format!(
394-
"Expected {:?}, found {:?}",
395-
expected, self.current_token
396-
))
411+
// Provide better error messages for common cases
412+
let error_msg = match (&expected, &self.current_token) {
413+
(Token::RightParen, Token::Eof) if self.paren_depth > 0 => {
414+
format!(
415+
"Unclosed parenthesis - missing {} closing parenthes{}",
416+
self.paren_depth,
417+
if self.paren_depth == 1 { "is" } else { "es" }
418+
)
419+
}
420+
(Token::RightParen, _) if self.paren_depth > 0 => {
421+
format!(
422+
"Expected closing parenthesis but found {:?} (currently {} unclosed parenthes{})",
423+
self.current_token,
424+
self.paren_depth,
425+
if self.paren_depth == 1 { "is" } else { "es" }
426+
)
427+
}
428+
_ => format!("Expected {:?}, found {:?}", expected, self.current_token),
429+
};
430+
Err(error_msg)
397431
}
398432
}
399433

400434
fn advance(&mut self) {
435+
// Track parentheses depth when advancing
436+
match &self.current_token {
437+
Token::LeftParen => self.paren_depth += 1,
438+
Token::RightParen => {
439+
self.paren_depth -= 1;
440+
// Note: We don't check for < 0 here because advance() is used
441+
// in contexts where we're not necessarily expecting a right paren
442+
}
443+
_ => {}
444+
}
401445
self.current_token = self.lexer.next_token();
402446
}
403447

@@ -451,6 +495,19 @@ impl Parser {
451495
None
452496
};
453497

498+
// Check for balanced parentheses at the end of parsing
499+
if self.paren_depth > 0 {
500+
return Err(format!(
501+
"Unclosed parenthesis - missing {} closing parenthes{}",
502+
self.paren_depth,
503+
if self.paren_depth == 1 { "is" } else { "es" }
504+
));
505+
} else if self.paren_depth < 0 {
506+
return Err(
507+
"Extra closing parenthesis found - no matching opening parenthesis".to_string(),
508+
);
509+
}
510+
454511
Ok(SelectStatement {
455512
columns,
456513
from_table,
@@ -583,6 +640,13 @@ impl Parser {
583640
self.advance();
584641
Some(LogicalOp::Or)
585642
}
643+
Token::RightParen if self.paren_depth <= 0 => {
644+
// Unexpected closing parenthesis
645+
return Err(
646+
"Unexpected closing parenthesis - no matching opening parenthesis"
647+
.to_string(),
648+
);
649+
}
586650
_ => None,
587651
};
588652

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
use sql_cli::recursive_parser::{format_ast_tree, Parser};
2+
3+
#[test]
4+
fn test_unclosed_parenthesis_in_where() {
5+
// Missing closing parenthesis
6+
let query = "SELECT * FROM table WHERE (price > 100";
7+
let mut parser = Parser::new(query);
8+
let result = parser.parse();
9+
10+
assert!(result.is_err(), "Should fail with unclosed parenthesis");
11+
let error = result.unwrap_err();
12+
println!("Error for unclosed paren: {}", error);
13+
14+
// Check AST formatting shows helpful error
15+
let ast = format_ast_tree(query);
16+
assert!(ast.contains("PARSE ERROR"));
17+
}
18+
19+
#[test]
20+
fn test_extra_closing_parenthesis() {
21+
// Extra closing parenthesis
22+
let query = "SELECT * FROM table WHERE price > 100)";
23+
let mut parser = Parser::new(query);
24+
let result = parser.parse();
25+
26+
assert!(
27+
result.is_err(),
28+
"Should fail with extra closing parenthesis"
29+
);
30+
let error = result.unwrap_err();
31+
println!("Error for extra closing paren: {}", error);
32+
}
33+
34+
#[test]
35+
fn test_mismatched_parentheses_in_complex_query() {
36+
// Missing closing paren in first condition
37+
let query = "SELECT * FROM table WHERE (price > 100 AND quantity < 50) OR (category = 'Books'";
38+
let mut parser = Parser::new(query);
39+
let result = parser.parse();
40+
41+
assert!(result.is_err(), "Should fail with mismatched parentheses");
42+
let error = result.unwrap_err();
43+
println!("Error for mismatched parens: {}", error);
44+
}
45+
46+
#[test]
47+
fn test_nested_unclosed_parentheses() {
48+
// Nested parentheses with missing close
49+
let query = "SELECT * FROM table WHERE ((price > 100 AND quantity < 50)";
50+
let mut parser = Parser::new(query);
51+
let result = parser.parse();
52+
53+
assert!(
54+
result.is_err(),
55+
"Should fail with nested unclosed parentheses"
56+
);
57+
let error = result.unwrap_err();
58+
println!("Error for nested unclosed: {}", error);
59+
}
60+
61+
#[test]
62+
fn test_method_call_unclosed() {
63+
// Method call with unclosed parenthesis
64+
let query = "SELECT * FROM table WHERE name.Contains('test'";
65+
let mut parser = Parser::new(query);
66+
let result = parser.parse();
67+
68+
assert!(result.is_err(), "Should fail with unclosed method call");
69+
let error = result.unwrap_err();
70+
println!("Error for unclosed method: {}", error);
71+
}
72+
73+
#[test]
74+
fn test_in_clause_unclosed() {
75+
// IN clause with unclosed parenthesis
76+
let query = "SELECT * FROM table WHERE category IN ('Books', 'Electronics'";
77+
let mut parser = Parser::new(query);
78+
let result = parser.parse();
79+
80+
assert!(result.is_err(), "Should fail with unclosed IN clause");
81+
let error = result.unwrap_err();
82+
println!("Error for unclosed IN: {}", error);
83+
}
84+
85+
#[test]
86+
fn test_between_with_unclosed_paren() {
87+
// BETWEEN inside unclosed parenthesis
88+
let query = "SELECT * FROM table WHERE (price BETWEEN 50 AND 100";
89+
let mut parser = Parser::new(query);
90+
let result = parser.parse();
91+
92+
assert!(
93+
result.is_err(),
94+
"Should fail with unclosed BETWEEN parenthesis"
95+
);
96+
let error = result.unwrap_err();
97+
println!("Error for unclosed BETWEEN: {}", error);
98+
}
99+
100+
#[test]
101+
fn test_complex_query_missing_paren() {
102+
// Complex real-world example with missing paren
103+
let query = "SELECT * FROM trades WHERE (price > 100 AND quantity < 50 OR (category = 'Tech' AND date > '2024-01-01')";
104+
let mut parser = Parser::new(query);
105+
let result = parser.parse();
106+
107+
// This might actually parse but incorrectly due to precedence
108+
if result.is_err() {
109+
println!("Error: {}", result.unwrap_err());
110+
} else {
111+
println!(
112+
"Warning: Query parsed but might have incorrect precedence due to missing parenthesis"
113+
);
114+
}
115+
}

0 commit comments

Comments
 (0)