From 47d4ab5309ebae524f11f0244df5298f733abe0d Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:19:47 +0000 Subject: [PATCH 1/4] fix: quote all lexer keywords in `fmt` --- prqlc/prqlc-parser/src/lexer/mod.rs | 29 +++++++++++------------ prqlc/prqlc/src/codegen/ast.rs | 23 +++++++++--------- web/book/src/reference/syntax/keywords.md | 2 ++ 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/prqlc/prqlc-parser/src/lexer/mod.rs b/prqlc/prqlc-parser/src/lexer/mod.rs index 05767633ff1b..75ce55bcd07e 100644 --- a/prqlc/prqlc-parser/src/lexer/mod.rs +++ b/prqlc/prqlc-parser/src/lexer/mod.rs @@ -190,22 +190,21 @@ fn multi_char_operators<'a>() -> impl Parser<'a, ParserInput<'a>, TokenKind, Par )) } +/// Words the lexer reserves; an identifier that collides with one of these has +/// to be written in backticks. +/// +/// This is the single source of truth — `prqlc`'s codegen reads it to decide +/// when to quote a name, and the syntax highlighting grammars under +/// `grammars/` and the playground's `prql-syntax.js` mirror it by hand. +pub const KEYWORDS: [&str; 10] = [ + "let", "into", "case", "prql", "type", "module", "internal", "func", "import", "enum", +]; + fn keyword<'a>() -> impl Parser<'a, ParserInput<'a>, TokenKind, ParserError<'a>> { - choice(( - just("let"), - just("into"), - just("case"), - just("prql"), - just("type"), - just("module"), - just("internal"), - just("func"), - just("import"), - just("enum"), - )) - .to_slice() - .then_ignore(end_expr()) - .map(|s: &str| TokenKind::Keyword(s.to_string())) + choice(KEYWORDS.map(just)) + .to_slice() + .then_ignore(end_expr()) + .map(|s: &str| TokenKind::Keyword(s.to_string())) } fn param<'a>() -> impl Parser<'a, ParserInput<'a>, TokenKind, ParserError<'a>> { diff --git a/prqlc/prqlc/src/codegen/ast.rs b/prqlc/prqlc/src/codegen/ast.rs index 8f8daa83d1bd..1183a3327058 100644 --- a/prqlc/prqlc/src/codegen/ast.rs +++ b/prqlc/prqlc/src/codegen/ast.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; -use std::collections::HashSet; use std::sync::OnceLock; +use prqlc_parser::lexer; use regex::Regex; use super::{WriteOpt, WriteSource}; @@ -328,15 +328,6 @@ impl WriteSource for pr::Ident { } } -fn keywords() -> &'static HashSet<&'static str> { - static KEYWORDS: OnceLock> = OnceLock::new(); - KEYWORDS.get_or_init(|| { - HashSet::from_iter([ - "let", "into", "case", "prql", "type", "module", "internal", "func", - ]) - }) -} - fn valid_prql_ident() -> &'static Regex { static VALID_PRQL_IDENT: OnceLock = OnceLock::new(); VALID_PRQL_IDENT.get_or_init(|| { @@ -347,7 +338,7 @@ fn valid_prql_ident() -> &'static Regex { } pub fn write_ident_part(s: &str) -> Cow<'_, str> { - if valid_prql_ident().is_match(s) && !keywords().contains(s) { + if valid_prql_ident().is_match(s) && !lexer::KEYWORDS.contains(&s) { s.into() } else { format!("`{s}`").into() @@ -784,6 +775,16 @@ let `case` = 5 ); } + /// Every lexer keyword needs quoting, not just the ones we thought to list + /// by hand — `import` and `enum` were missing from the codegen's own copy + /// of the list, so `let `import` = 5` formatted to unparseable source. + #[test] + fn test_every_keyword_is_quoted() { + for keyword in lexer::KEYWORDS { + assert_is_formatted(&format!("let `{keyword}` = 5")); + } + } + /// Named arguments need their backticks too. Unlike the declaration names /// above, dropping them produces output that still parses — as a different /// call, since the unquoted name splits into a positional argument. diff --git a/web/book/src/reference/syntax/keywords.md b/web/book/src/reference/syntax/keywords.md index bdb4506f3953..ed91bcc0cf95 100644 --- a/web/book/src/reference/syntax/keywords.md +++ b/web/book/src/reference/syntax/keywords.md @@ -93,9 +93,11 @@ PRQL uses following keywords: - **`into`** - variable definition [_more..._](../declarations/variables.md) - **`case`** - flow control [_more..._](../syntax/case.md) - **`type`** - type declaration +- **`enum`** - enumeration type declaration - **`func`** - explicit function declaration [_more..._](../declarations/functions.md) - **`module`** - used internally +- **`import`** - used internally - **`internal`** - used internally - **`true`** - boolean [_more..._](./literals.md#booleans) - **`false`** - boolean [_more..._](./literals.md#booleans) From 2427c35299f5b8a54e0c25751ddbd0c1c717ddf3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:21:46 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- prqlc/prqlc/src/codegen/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prqlc/prqlc/src/codegen/ast.rs b/prqlc/prqlc/src/codegen/ast.rs index 1183a3327058..8840df069e54 100644 --- a/prqlc/prqlc/src/codegen/ast.rs +++ b/prqlc/prqlc/src/codegen/ast.rs @@ -777,7 +777,7 @@ let `case` = 5 /// Every lexer keyword needs quoting, not just the ones we thought to list /// by hand — `import` and `enum` were missing from the codegen's own copy - /// of the list, so `let `import` = 5` formatted to unparseable source. + /// of the list, so `let `import` = 5` formatted to unparsable source. #[test] fn test_every_keyword_is_quoted() { for keyword in lexer::KEYWORDS { From d8111fc276294a98dfcab109e9910df72228e817 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:41:48 +0000 Subject: [PATCH 3/4] fix: quote reserved literals and expression-position idents in `fmt` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `true` / `false` / `null` are reserved by the lexer (`boolean()` and `null()` both end with `end_expr()`) but produce `TokenKind::Literal` rather than `TokenKind::Keyword`, so they were absent from `lexer::KEYWORDS` and `write_ident_part` left them unquoted. Adds a sibling `lexer::RESERVED_LITERALS` and rejects both lists. Separately, `ExprKind::Ident` wrote idents via `pr::Ident`'s `Display` impl, which carries its own copy of the quoting rule and knows nothing about reserved words — so expression-position names lost their backticks even for keywords already in `KEYWORDS`. Routes it through `write_ident_part` like every other codegen path. --- prqlc/prqlc-parser/src/lexer/mod.rs | 18 +++++--- prqlc/prqlc-parser/src/lexer/test.rs | 15 +++++++ prqlc/prqlc/src/codegen/ast.rs | 45 +++++++++++++------ .../integration__queries__fmt__distinct.snap | 4 +- ...gration__queries__fmt__set_ops_remove.snap | 2 +- 5 files changed, 63 insertions(+), 21 deletions(-) diff --git a/prqlc/prqlc-parser/src/lexer/mod.rs b/prqlc/prqlc-parser/src/lexer/mod.rs index 75ce55bcd07e..d1b2159d0d4e 100644 --- a/prqlc/prqlc-parser/src/lexer/mod.rs +++ b/prqlc/prqlc-parser/src/lexer/mod.rs @@ -190,16 +190,24 @@ fn multi_char_operators<'a>() -> impl Parser<'a, ParserInput<'a>, TokenKind, Par )) } -/// Words the lexer reserves; an identifier that collides with one of these has -/// to be written in backticks. +/// Words the lexer reserves as [`TokenKind::Keyword`]; an identifier that +/// collides with one of these has to be written in backticks. /// -/// This is the single source of truth — `prqlc`'s codegen reads it to decide -/// when to quote a name, and the syntax highlighting grammars under -/// `grammars/` and the playground's `prql-syntax.js` mirror it by hand. +/// [`RESERVED_LITERALS`] is the other half — together the two are the single +/// source of truth for which names need quoting. `prqlc`'s codegen reads both +/// to decide when to quote a name, while the syntax highlighting grammars under +/// `grammars/`, the playground's `prql-syntax.js`, and the docs in +/// `web/book/src/reference/syntax/keywords.md` track them by hand. pub const KEYWORDS: [&str; 10] = [ "let", "into", "case", "prql", "type", "module", "internal", "func", "import", "enum", ]; +/// Words the lexer reserves as [`TokenKind::Literal`] rather than +/// [`TokenKind::Keyword`] — see [`boolean`] and [`null`]. They're reserved just +/// as firmly as [`KEYWORDS`], since both parsers end with [`end_expr`], so an +/// identifier that collides with one also has to be written in backticks. +pub const RESERVED_LITERALS: [&str; 3] = ["true", "false", "null"]; + fn keyword<'a>() -> impl Parser<'a, ParserInput<'a>, TokenKind, ParserError<'a>> { choice(KEYWORDS.map(just)) .to_slice() diff --git a/prqlc/prqlc-parser/src/lexer/test.rs b/prqlc/prqlc-parser/src/lexer/test.rs index b18d84250dc4..491a692a46e8 100644 --- a/prqlc/prqlc-parser/src/lexer/test.rs +++ b/prqlc/prqlc-parser/src/lexer/test.rs @@ -562,3 +562,18 @@ fn raw_string_delimiters() { ) "#); } + +/// `RESERVED_LITERALS` is hand-written next to `boolean()` / `null()`, so pin +/// it to what the lexer actually does: each entry must lex as a `Literal` +/// rather than an `Ident`, which is what makes it need backticks in codegen. +#[test] +fn test_reserved_literals_lex_as_literals() { + for word in crate::lexer::RESERVED_LITERALS { + let tokens = lexer().parse(word).output().unwrap().to_vec(); + assert!( + matches!(tokens[0].kind, TokenKind::Literal(_)), + "`{word}` lexed as {:?}, expected a Literal", + tokens[0].kind + ); + } +} diff --git a/prqlc/prqlc/src/codegen/ast.rs b/prqlc/prqlc/src/codegen/ast.rs index 8840df069e54..70350449f3ef 100644 --- a/prqlc/prqlc/src/codegen/ast.rs +++ b/prqlc/prqlc/src/codegen/ast.rs @@ -91,7 +91,7 @@ impl WriteSource for pr::ExprKind { use pr::ExprKind::*; match &self { - Ident(ident) => Some(ident.to_string()), + Ident(ident) => Some(write_ident(ident)), Pipeline(pipeline) => SeparatedExprs { inline: " | ", @@ -318,14 +318,23 @@ impl WriteSource for pr::Ident { let width = self.path.iter().map(|p| p.len() + 1).sum::() + self.name.len(); opt.consume_width(width as u16)?; - let mut r = String::new(); - for part in &self.path { - r += &write_ident_part(part); - r += "."; - } - r += &write_ident_part(&self.name); - Some(r) + Some(write_ident(self)) + } +} + +/// Write a dotted identifier, quoting each part that needs it. +/// +/// `pr::Ident`'s `Display` impl looks similar but has its own copy of the +/// "needs backticks" rule which doesn't know about reserved words, so codegen +/// must not reach for `to_string()` here. +fn write_ident(ident: &pr::Ident) -> String { + let mut r = String::new(); + for part in &ident.path { + r += &write_ident_part(part); + r += "."; } + r += &write_ident_part(&ident.name); + r } fn valid_prql_ident() -> &'static Regex { @@ -338,7 +347,8 @@ fn valid_prql_ident() -> &'static Regex { } pub fn write_ident_part(s: &str) -> Cow<'_, str> { - if valid_prql_ident().is_match(s) && !lexer::KEYWORDS.contains(&s) { + let reserved = lexer::KEYWORDS.contains(&s) || lexer::RESERVED_LITERALS.contains(&s); + if valid_prql_ident().is_match(s) && !reserved { s.into() } else { format!("`{s}`").into() @@ -775,13 +785,22 @@ let `case` = 5 ); } - /// Every lexer keyword needs quoting, not just the ones we thought to list + /// Every reserved word needs quoting, not just the ones we thought to list /// by hand — `import` and `enum` were missing from the codegen's own copy /// of the list, so `let `import` = 5` formatted to unparsable source. + /// + /// `true` / `false` / `null` lex as literals rather than keywords, and were + /// missing for the same reason. In declaration position they fail loudly + /// like `import`; in expression position they're worse, since the output + /// still parses — as a literal instead of the column that was written. #[test] - fn test_every_keyword_is_quoted() { - for keyword in lexer::KEYWORDS { - assert_is_formatted(&format!("let `{keyword}` = 5")); + fn test_every_reserved_word_is_quoted() { + for word in lexer::KEYWORDS + .iter() + .chain(lexer::RESERVED_LITERALS.iter()) + { + assert_is_formatted(&format!("let `{word}` = 5")); + assert_is_formatted(&format!("from t\nselect {{`{word}`}}")); } } diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__distinct.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__distinct.snap index 48957dc12cad..825bc298f6ed 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__distinct.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__distinct.snap @@ -5,5 +5,5 @@ input_file: prqlc/prqlc/tests/integration/queries/distinct.prql --- from tracks select {album_id, genre_id} -group tracks.`*` (take 1) -sort tracks.`*` +group tracks.* (take 1) +sort tracks.* diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__set_ops_remove.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__set_ops_remove.snap index 3401bcc505e8..481775b95785 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__set_ops_remove.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__fmt__set_ops_remove.snap @@ -5,7 +5,7 @@ input_file: prqlc/prqlc/tests/integration/queries/set_ops_remove.prql --- let distinct = func rel -> ( from t = _param.rel - group {t.`*`} (take 1) + group {t.*} (take 1) ) from_text format:json '{ "columns": ["a"], "data": [[1], [2], [2], [3]] }' From b932806cbfc7268f4279477c291f3399f73766e0 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:54:21 +0000 Subject: [PATCH 4/4] docs: avoid private intra-doc links in `RESERVED_LITERALS` `boolean`, `null` and `end_expr` are private fns, so linking them from a `pub const`'s docs trips `rustdoc::private_intra_doc_links`, which is an error under the workflow's `RUSTDOCFLAGS: -Dwarnings`. Plain code spans keep the pointer without breaking `cargo doc`. --- prqlc/prqlc-parser/src/lexer/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prqlc/prqlc-parser/src/lexer/mod.rs b/prqlc/prqlc-parser/src/lexer/mod.rs index d1b2159d0d4e..8b8c4e53328e 100644 --- a/prqlc/prqlc-parser/src/lexer/mod.rs +++ b/prqlc/prqlc-parser/src/lexer/mod.rs @@ -203,8 +203,8 @@ pub const KEYWORDS: [&str; 10] = [ ]; /// Words the lexer reserves as [`TokenKind::Literal`] rather than -/// [`TokenKind::Keyword`] — see [`boolean`] and [`null`]. They're reserved just -/// as firmly as [`KEYWORDS`], since both parsers end with [`end_expr`], so an +/// [`TokenKind::Keyword`] — see `boolean()` and `null()`. They're reserved just +/// as firmly as [`KEYWORDS`], since both parsers end with `end_expr()`, so an /// identifier that collides with one also has to be written in backticks. pub const RESERVED_LITERALS: [&str; 3] = ["true", "false", "null"];