From 17b5c1a894c1333c20cd21820d96d6df156b385b Mon Sep 17 00:00:00 2001 From: takeokunn Date: Mon, 3 Aug 2026 19:33:06 +0900 Subject: [PATCH] fix(syntax): Janet quote prefix, and Clojure namespaced maps with space Two reader gaps in two dialects, each deferred by earlier work with a reproduction already recorded. **Janet `'`.** `parse.c`'s `root` puts `'` in the same PFLAG_READERMAC group as `,` `;` `~` `|`, and `popstate` expands it to `(quote x)` -- the CL shape, taking exactly one following form. paredit had no arm, so `'` glued onto whatever followed: `(a '" " b)` failed with an unterminated string. One arm in `classify_janet` fixes it. Oracle, against a Janet 1.41.3-dev build using `parse-all` and a node count blind to atom values and struct key order: files parsing 208/210 -> 210/210 agreeing with Janet 146/210 -> 202/210 trees changed 61 regressed 0 The 8 residuals are all pre-existing: 6 are the bare-`@` divergence (Janet reads `(@ define X)` as the *symbol* `@`, and the deficits match the bare-`@` counts exactly), 2 are the metric's blind spot, since Janet dedups struct keys at parse time. Differential fuzz 2116/2500 -> 2500/2500, with 0 cases rejected by Janet, so no agreement is coincidental. The generator excludes `@`, `{}`, long strings, and any reader-macro byte adjacent to a preceding token -- the constructs whose divergence predates this change. **Clojure `#:ns {...}`.** The premise this started from was slightly wrong: `#:foo{...}` already parsed. The gap is *whitespace* between the namespace and the brace, which `LispReader.java`'s `NamespaceMapReader` explicitly allows -- it reads the symbol, then `while(isWhitespace(...))` before requiring `{`. The failing line in the reported repro file is `(println #::it {:a #::it {}})`; its own tight `#::it{:a 1}` lines were fine. Clojure files: 577 scanned, 18 parse failures -> 16. The causes split 9 `#!` shebangs (babashka), **6 `#^` old-style metadata** -- a real third gap that was not in the brief and is left for a follow-up -- 2 namespaced maps, 1 deliberately-unreadable fixture. No JDK is available here, so the oracle is Clojure's own reader test suite (`test_clojure/reader.cljc:745-771`), which asserts `#:a{...}` is `#:a {...}`, `#::{...}` is `#:: {...}`, and that `#:::` and `#: s{:a 1}` throw. Shape-level failures 6 -> 0. Kept the existing `MultiDatum` model rather than making `#:ns` a `ReaderPrefix`: `ReaderPrefix` is payload-free with a `&'static str` spelling, and `#:foo` has no fixed one. Documented, after measuring, that this model makes namespaced maps **opaque** -- `#:foo{:a 1}` reports 0 atom occurrences, so map-literal rules see nothing inside. That is pre-existing; this change only widens which maps parse. **Other dialects: 7879 file-runs, 2 intended differences.** Pre/post canonical tree dumps byte-compared across emacs-lisp, scheme, racket, common-lisp, fennel, hy, carp, lfe and the legacy classifier. Lint findings that moved: Janet 304 -> 341, and two files *lost* a `leftover-print-debug` finding -- both false positives removed, since `'(do ... (print "there"))` is quoted data that was previously read as a live call. Clojure 350 -> 364, purely additive from the two newly-parsing files. Both repo fixtures lint byte-identically; no golden or pinned count moved. Two pre-existing formatter bugs were found and are reported rather than hidden, neither caused by this change: - `edit format` silently drops reader prefixes on binding/clause arguments. Cleanest repro is Common Lisp: `(let `(a b) x)` becomes `(let (a b) x)`. `BODY_FORMS` in `reindent.rs` applies a CL-shaped `("do", 2)` entry to Janet and Fennel too. - A reader prefix immediately before a closing delimiter is dropped in every dialect: `(a ')` becomes `(a)`. Janet refuses the same input. Pinned as a *stated* expectation so a future fix must update it deliberately. --- .../core/syntax/src/sexpr/reader_policy.rs | 90 ++++++- .../core/syntax/src/sexpr/tests/parser.rs | 236 ++++++++++++++++++ 2 files changed, 319 insertions(+), 7 deletions(-) diff --git a/packages/core/syntax/src/sexpr/reader_policy.rs b/packages/core/syntax/src/sexpr/reader_policy.rs index 3abe869c..5bc62004 100644 --- a/packages/core/syntax/src/sexpr/reader_policy.rs +++ b/packages/core/syntax/src/sexpr/reader_policy.rs @@ -488,7 +488,61 @@ impl DialectReaderPolicy { } } - fn clojure_namespaced_map_width(self, bytes: &[u8], pos: usize) -> Option { + /// Byte width of the `#:ns` dispatch introducing a namespaced map literal. + /// + /// The width covers the dispatch alone -- `#:foo`, `#::foo`, `#::` -- and + /// never the `{`. The brace is where the `MultiDatum` payload starts, and + /// `skip_form` consumes it; the two together become one opaque reader-form + /// node spanning the whole literal. + /// + /// That opacity is this model's known limitation, and it is unchanged here: + /// `#:foo{:a 1}` reports zero atom occurrences, so a rule looking for map + /// keys sees nothing inside a namespaced map. Representing the dispatch as + /// a [`ReaderPrefix`] on the `{...}` list -- the way `#{...}` keeps its + /// elements visible through `HashLiteral` -- is the shape that would fix + /// that, but `ReaderPrefix` is a payload-free enum whose `as_source` + /// returns a `&'static str`, and `#:foo` has no fixed spelling. Giving it + /// one is a change to the prefix representation itself, rippling through + /// the formatter and every prefix consumer, and it would rewrite the tree + /// of every `#:foo{...}` that already parses. This fix deliberately does + /// neither: it only widens *which* namespaced maps parse at all, and + /// leaves the shape of the ones that already did byte-identical. + /// + /// Clojure's `NamespaceMapReader` (`LispReader.java`) allows whitespace + /// between the namespace and the brace, and this must too: + /// + /// ```text + /// } else if(nextChar != '{') { // #:foo { } or #::foo { } + /// unread(r, nextChar); + /// sym = read(r, true, null, false, opts, pendingForms); + /// nextChar = read1(r); + /// while(isWhitespace(nextChar)) + /// nextChar = read1(r); + /// } + /// if(nextChar != '{') + /// throw Util.runtimeException("Namespaced map must specify a map"); + /// ``` + /// + /// Requiring the brace to touch the namespace made `#:foo {:a 1}` an + /// unsupported dispatch, and an unsupported dispatch fails the whole parse + /// -- so a single such literal silently dropped its entire file from every + /// lint run. `#::it {:a #::it {}}` in clj-kondo's own corpus is the case + /// that found it. + /// + /// `skip_form` skips trivia before reading a `MultiDatum` payload, so the + /// whitespace needs no representation in the width; it stays trivia, which + /// is what keeps a format round-trip byte-identical. + /// + /// Comments are deliberately not skipped. Clojure's loop advances over + /// `isWhitespace` only, so `#:foo ;; c` then `{}` is its "must specify a + /// map" error, and refusing it here agrees rather than inventing a rule. + /// `isWhitespace` counts a comma as whitespace and so does + /// [`Self::is_whitespace`], so `#:foo,{:a 1}` reads for both. + /// + /// Visible to the crate for the same reason [`Self::long_string_extent`] + /// is: the width is a shared decision worth pinning directly, rather than + /// only through documents that happen to exercise it. + pub(super) fn clojure_namespaced_map_width(self, bytes: &[u8], pos: usize) -> Option { let mut cursor = pos + 2; let auto_resolved = bytes.get(cursor) == Some(&b':'); if auto_resolved { @@ -496,15 +550,22 @@ impl DialectReaderPolicy { } let namespace_start = cursor; while let Some(&byte) = bytes.get(cursor) { - if byte == b'{' { - return (auto_resolved || cursor > namespace_start).then_some(cursor - pos); - } - if self.is_atom_boundary(bytes, cursor) { - return None; + if byte == b'{' || self.is_atom_boundary(bytes, cursor) { + break; } cursor += 1; } - None + let namespace_end = cursor; + // `#:{...}` and `#: {...}` are both "Namespaced map must specify a + // namespace" in Clojure. Only the auto-resolved `#::` may omit one. + if !auto_resolved && namespace_end == namespace_start { + return None; + } + let mut probe = namespace_end; + while matches!(bytes.get(probe), Some(&byte) if self.is_whitespace(byte)) { + probe += 1; + } + (bytes.get(probe) == Some(&b'{')).then_some(namespace_end - pos) } fn clojure_tagged_literal_width(self, bytes: &[u8], pos: usize) -> Option { @@ -532,6 +593,21 @@ impl DialectReaderPolicy { const fn classify_janet(self, byte: u8, next: Option) -> Option { match byte { + // Janet's `root` state lists `'` in the same `PFLAG_READERMAC` + // group as `,` `;` `~` `|` (`src/core/parse.c`), and `popstate` + // expands it to the two-element tuple `(quote
)` -- exactly + // one following datum, the same shape Common Lisp gives it. It was + // the only member of that group missing here. + // + // Its absence was not a cosmetic gap. `'` is not in Janet's + // `symchars` either, but `is_atom_boundary` does not know that, so + // with no reader-macro arm the quote glued onto whatever followed: + // `'foo` read as the single atom `'foo` rather than a quote prefix + // on `foo`, and `(a '" " b)` failed outright with "unterminated + // string" because the atom swallowed the opening quotation mark of + // the string after it. That accounted for both remaining parse + // failures over a 210-file Janet/spork corpus. + b'\'' => prefix(ReaderPrefix::Quote, 1), b';' => prefix(ReaderPrefix::UnquoteSplicing, 1), b'~' => prefix(ReaderPrefix::Quasiquote, 1), b',' => prefix(ReaderPrefix::Unquote, 1), diff --git a/packages/core/syntax/src/sexpr/tests/parser.rs b/packages/core/syntax/src/sexpr/tests/parser.rs index 4914454a..fa001fc9 100644 --- a/packages/core/syntax/src/sexpr/tests/parser.rs +++ b/packages/core/syntax/src/sexpr/tests/parser.rs @@ -1445,3 +1445,239 @@ fn backtick_is_still_quasiquote_outside_janet() { ); } } + +/// Janet's `root` state lists `'` in the same `PFLAG_READERMAC` group as `,` +/// `;` `~` `|` (`src/core/parse.c`), and `popstate` expands it to the tuple +/// `(quote )`. It was the one member of that group with no arm here, so +/// the quote glued onto whatever followed it. +/// +/// Every expectation below was read off Janet 1.41.3's own reader before it +/// was written here: the `janet_value` column is what +/// `janet -e '(pp (parse ...))'` prints, not a restatement of what this parser +/// happens to do. +#[test] +fn janet_quote_is_a_reader_prefix() { + struct Case { + input: &'static str, + /// Source text of each child of the single top-level list. + children: &'static [&'static str], + /// What Janet's own reader makes of the form, for the record. + janet_value: &'static str, + } + + let cases = [ + // The regression that found this: without a `'` arm the quote glued + // onto the opening `"` of the string after it and the whole document + // failed with "unterminated string starting at byte 6". This is + // `spork/spork/cjanet.janet:31` reduced. + Case { + input: r#"(a '" " b)"#, + children: &["a", r#"'" ""#, "b"], + janet_value: r#"(a (quote " ") b)"#, + }, + Case { + input: "(a 'foo b)", + children: &["a", "'foo", "b"], + janet_value: "(a (quote foo) b)", + }, + Case { + input: "(a '(b c))", + children: &["a", "'(b c)"], + janet_value: "(a (quote (b c)))", + }, + // `#` opens a *line comment* in Janet, so `'` before a brace is the + // other half of `janet/test/suite-peg.janet:405`: the `{` was being + // read as an opening brace of a struct that never closed. + Case { + input: r#"(a '"{" b)"#, + children: &["a", r#"'"{""#, "b"], + janet_value: r#"(a (quote "{") b)"#, + }, + // Stacked prefixes nest, they do not collapse. + Case { + input: "(a ''b)", + children: &["a", "''b"], + janet_value: "(a (quote (quote b)))", + }, + // A quote in front of the other Janet reader forms. + Case { + input: "(a '@{})", + children: &["a", "'@{}"], + janet_value: "(a (quote @{}))", + }, + Case { + input: "(a '`long`)", + children: &["a", "'`long`"], + janet_value: r#"(a (quote "long"))"#, + }, + Case { + input: "(a '[b c])", + children: &["a", "'[b c]"], + janet_value: "(a (quote [b c]))", + }, + ]; + + for case in cases { + let tree = SyntaxTree::parse_with_dialect(case.input, Dialect::Janet) + .unwrap_or_else(|error| panic!("{}: {error}", case.input)); + let form = &tree.root_view().children[0]; + let children = form + .children + .iter() + .map(|child| child.span.slice(case.input)) + .collect::>(); + assert_eq!( + children, case.children, + "{} (janet reads {})", + case.input, case.janet_value + ); + } + + // The prefix is recorded as `Quote`, not merely swallowed into the span. + let tree = SyntaxTree::parse_with_dialect("(a 'foo)", Dialect::Janet).expect("valid"); + assert_eq!( + tree.root_view().children[0].children[1].reader_prefixes, + vec![ReaderPrefix::Quote] + ); +} + +/// A dangling `'` at end of input is a truncated form, not the symbol `'`. +/// +/// Janet agrees: `janet -e "(pp (parse-all \"'\"))"` fails with "unexpected end +/// of source, opened at line 1, column 1". Before the `'` arm existed paredit +/// accepted it as a one-character atom. +#[test] +fn janet_dangling_quote_is_refused() { + let error = SyntaxTree::parse_with_dialect("'", Dialect::Janet) + .expect_err("a quote with nothing after it is not a form"); + assert!( + matches!(error, ParseError::MissingReaderForm(0)), + "{error:?}" + ); + + // `(a ')` is *not* refused, and that is a known defect rather than an + // intended rule. `form` accumulates prefixes, finds a closing delimiter + // rather than a datum, and calls `close_list` without ever consuming the + // prefixes it collected -- so the quote is dropped and the document parses + // clean. Janet refuses the same input ("mismatched delimiter )"). + // + // It is pinned here because it is *cross-dialect and pre-existing*: Common + // Lisp, Scheme, Clojure, Emacs Lisp, Fennel and the legacy reader all drop + // it identically, on this commit and before it. Fixing it means changing + // every dialect's behaviour, which is a separate change from adding a + // Janet reader arm; this test exists so that change has to update a stated + // expectation instead of a silent one. + let tree = SyntaxTree::parse_with_dialect("(a ')", Dialect::Janet) + .expect("known defect: the prefix is dropped rather than refused"); + let form = &tree.root_view().children[0]; + assert_eq!(form.children.len(), 1, "the quote is dropped, not recorded"); + assert!(form.children[0].reader_prefixes.is_empty()); +} + +/// `'` keeps meaning quote everywhere else, and this pins the other dialects so +/// a later edit to `classify_janet` cannot quietly widen. Fennel already had +/// its own `'` arm; the legacy reader and the named dialects share +/// `classify_quote_prefix`. +#[test] +fn janet_quote_arm_does_not_change_other_dialects() { + for dialect in [ + Dialect::CommonLisp, + Dialect::EmacsLisp, + Dialect::Scheme, + Dialect::Racket, + Dialect::Clojure, + Dialect::Fennel, + Dialect::Lfe, + Dialect::Hy, + Dialect::Carp, + Dialect::Unknown, + ] { + let input = "(f '(a b))"; + let tree = SyntaxTree::parse_with_dialect(input, dialect) + .unwrap_or_else(|error| panic!("{}: {error}", dialect.label())); + let form = &tree.root_view().children[0]; + let children = form + .children + .iter() + .map(|child| child.span.slice(input)) + .collect::>(); + assert_eq!(children, vec!["f", "'(a b)"], "{}", dialect.label()); + assert_eq!( + form.children[1].reader_prefixes, + vec![ReaderPrefix::Quote], + "{}", + dialect.label() + ); + } +} + +/// Clojure's `NamespaceMapReader` reads the namespace with a full `read`, then +/// skips `isWhitespace` before demanding the `{`, so `#:foo {:a 1}` is as legal +/// as `#:foo{:a 1}`. Requiring the brace to touch the namespace made the spaced +/// spelling an unsupported dispatch, which fails the whole parse -- so one such +/// literal silently dropped its entire file from every lint run. +/// +/// The read/throw split below is taken from Clojure's own reader test suite, +/// `test/clojure/test_clojure/reader.cljc` lines 745-771. +#[test] +fn clojure_namespaced_maps_allow_whitespace_before_the_brace() { + // Asserted equal to their tight-brace spellings by reader.cljc:745-752. + let must_read = [ + "#:a{1 nil, :b nil}", + "#:a {1 nil, :b nil}", + "#::{1 nil, :a nil}", + // reader.cljc:749 uses *two* spaces, so the skip is a loop. + "#:: {1 nil, :a nil}", + "#::s{1 nil, :a nil}", + "#::s {1 nil, :a nil}", + // `isWhitespace` in LispReader counts a comma, and so does this. + "#:a,{1 1}", + "#:a\n{1 1}", + ]; + for input in must_read { + SyntaxTree::parse_with_dialect(input, Dialect::Clojure) + .unwrap_or_else(|error| panic!("{input:?}: {error}")); + } + + // Refused by Clojure, and still refused here. `#: s{:a 1}` is + // reader.cljc:764 ("Namespaced map must specify a namespace"); the comment + // case is refused because LispReader's loop skips `isWhitespace` only, so + // a `;` is its "must specify a map" error rather than trivia. + let must_refuse = [ + "#:::", + "#: {:a 1}", + "#:{:a 1}", + "#:a b", + "#:a", + "#:a ;; c\n{:a 1}", + ]; + for input in must_refuse { + SyntaxTree::parse_with_dialect(input, Dialect::Clojure) + .expect_err(&format!("{input:?} is not a namespaced map")); + } +} + +/// The dispatch width covers `#:ns` alone, never the brace, and the whitespace +/// between them stays trivia. That is what keeps the spaced spelling formatting +/// back to itself, and it is the property the width return value encodes. +#[test] +fn clojure_namespaced_map_width_stops_at_the_namespace() { + let policy = DialectReaderPolicy::new(Dialect::Clojure); + let width = |input: &str| policy.clojure_namespaced_map_width(input.as_bytes(), 0); + + assert_eq!(width("#:foo{:a 1}"), Some(5)); + assert_eq!(width("#:foo {:a 1}"), Some(5)); + assert_eq!(width("#:foo {:a 1}"), Some(5)); + assert_eq!(width("#::foo{:a 1}"), Some(6)); + assert_eq!(width("#::foo {:a 1}"), Some(6)); + assert_eq!(width("#::{:a 1}"), Some(3)); + assert_eq!(width("#:: {:a 1}"), Some(3)); + // No namespace, and no auto-resolve marker to excuse it. + assert_eq!(width("#:{:a 1}"), None); + assert_eq!(width("#: {:a 1}"), None); + // A namespace with no map after it. + assert_eq!(width("#:foo bar"), None); + assert_eq!(width("#:foo"), None); + // Comments are not whitespace to Clojure's reader here. + assert_eq!(width("#:foo ;; c\n{:a 1}"), None); +}