alamb commented on code in PR #1587:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1587#discussion_r1882362868
##########
src/parser/mod.rs:
##########
@@ -3516,37 +3578,50 @@ impl<'a> Parser<'a> {
Ok(keyword)
} else {
let keywords: Vec<String> = keywords.iter().map(|x|
format!("{x:?}")).collect();
- self.expected(
+ self.expected_ref(
&format!("one of {}", keywords.join(" or ")),
- self.peek_token(),
+ self.peek_token_ref(),
)
}
}
/// If the current token is the `expected` keyword, consume the token.
/// Otherwise, return an error.
pub fn expect_keyword(&mut self, expected: Keyword) ->
Result<TokenWithSpan, ParserError> {
- if let Some(token) = self.parse_keyword_token(expected) {
- Ok(token)
+ if let Some(token) = self.parse_keyword_token_ref(expected) {
+ Ok(token.clone())
+ } else {
+ self.expected_ref(format!("{:?}", &expected).as_str(),
self.peek_token_ref())
+ }
+ }
+
+ /// If the current token is the `expected` keyword, consume the token.
+ /// Otherwise, return an error.
+ ///
+ /// This differs from expect_keyword only in that the matched keyword
Review Comment:
It might be really nice to add a link in the docs too:
```suggestion
/// This differs from [`Self::expect_keyword`] only in that the matched
keyword
```
##########
src/parser/mod.rs:
##########
@@ -186,6 +186,15 @@ impl std::error::Error for ParserError {}
// By default, allow expressions up to this deep before erroring
const DEFAULT_REMAINING_DEPTH: usize = 50;
+// A constant EOF token that can be referenced.
Review Comment:
💯
This might be useful enough to make `pub const` as well (so users of the
crate can do the same thing)
##########
src/parser/mod.rs:
##########
@@ -3516,37 +3578,50 @@ impl<'a> Parser<'a> {
Ok(keyword)
} else {
let keywords: Vec<String> = keywords.iter().map(|x|
format!("{x:?}")).collect();
- self.expected(
+ self.expected_ref(
&format!("one of {}", keywords.join(" or ")),
- self.peek_token(),
+ self.peek_token_ref(),
)
}
}
/// If the current token is the `expected` keyword, consume the token.
/// Otherwise, return an error.
pub fn expect_keyword(&mut self, expected: Keyword) ->
Result<TokenWithSpan, ParserError> {
Review Comment:
Maybe as a follow on PR we could mark this API as deprecated to locate other
uses of this API as well as help downstream consumers upgrade 🤔
https://github.com/apache/datafusion-sqlparser-rs/blob/5de5312406fae3f69b92b12dd63c68d7fce3ed74/src/tokenizer.rs#L604
##########
src/parser/mod.rs:
##########
@@ -3376,22 +3407,26 @@ impl<'a> Parser<'a> {
matched
}
+ pub fn next_token(&mut self) -> TokenWithSpan {
Review Comment:
Can we please add doc comments to these methods as well?
Maybe we can also mention "see `next_token_ref` for a faster, non copying
API"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
