fix(syntax): skip shebangs, treat unused brackets as constituents, extend Scheme's # table

packages/core/syntax/src/sexpr/parser.rs

@@ -848,7 +848,7 @@ impl<'a> Parser<'a> {
                 }
                 self.close_list()?;
             }
-            byte if DialectReaderPolicy::is_raw_delimiter(byte) => {
+            byte if self.policy.is_token_delimiter(byte) => {
                 return Err(self.raw_delimiter_error());
             }
             b'"' => self.atom_string_with_prefixes(prefixes)?,
@@ -1030,7 +1030,7 @@ impl<'a> Parser<'a> {
                                 position: self.pos.get(),
                             });
                         }
-                        byte if DialectReaderPolicy::is_raw_delimiter(byte) => {
+                        byte if self.policy.is_token_delimiter(byte) => {
                             return Err(self.raw_delimiter_error());
                         }
                         b'"' => self.skip_string()?,

packages/core/syntax/src/sexpr/tests/parser.rs

@@ -3636,32 +3636,35 @@ fn racket_string_terminates_the_token_before_it() {
     }
 }
 
-/// Which Racket-only dispatch forms Scheme still refuses.
-///
-/// This began as `scheme_reader_is_unchanged_by_the_racket_split`, pinning
-/// that the Racket split left Scheme byte-identical. Four entries have since
-/// moved out of the refusal list below and into
-/// [`scheme_reads_r6rs_syntax_quotation`]: `#'`, `` #` ``, `#,` and `#,@` are
-/// R6RS 4.3.5 lexical syntax, the old doc comment flagged them as a *known
-/// gap* awaiting "its own Scheme corpus audit", and that audit has now been
-/// done — Guile 2.2.7's own `read` accepts all four, and refusing them cost
-/// 23 files of a 2356-file corpus. They were moved rather than deleted: the
-/// list below is still the load-bearing claim that Scheme is not Racket.
-///
-/// Everything remaining is genuinely Racket-only, verified by feeding each
-/// spelling to Guile 2.2.7, which rejects every one.
+/// Which of the forms Racket gained are Racket's alone, and which Scheme
+/// shares.
+///
+/// This test was written when Racket was split out of `classify_scheme`, and it
+/// pinned every one of those forms as still refused for Scheme. Four of them
+/// were pinned as a *known gap* rather than an intended reading, and the
+/// comment said so: "`#'`, `` #` ``, `#,` and `#,@` are R6RS lexical syntax
+/// that Guile, Chez and Chicken all accept, so this pins a known gap; closing
+/// it is a change to Scheme's reader needing its own Scheme corpus audit."
+///
+/// That audit has been run — 6185 deduplicated files from seven Scheme
+/// implementations, with Guile's own `read` as the oracle — so those four move
+/// here from the refused list below, along with `#"…"`. See
+/// [`DialectReaderPolicy::classify_scheme`] for the measurements. Everything
+/// still in the refused list is genuinely Racket-only, and stays refused.
 #[test]
 fn scheme_still_refuses_racket_only_dispatch() {
     for input in [
-        "(f #\"bytes\")",
         "(f #rx\"a\")",
         "(f #px\"a\")",
         "(f #hash((a . 1)))",
         "(f #s(pt 1))",
         "(f #&x)",
+        // `#{sym}` is Chez's gensym syntax and `#{sym}#` Guile's symbol escape
+        // — two different terminators for the same opener, which is why
+        // Scheme refuses both rather than guessing an extent.
         "(f #{1})",
-        // `#[` is a vector in Racket but not in Scheme: `classify_scheme`'s
-        // dispatch table admits `#(` alone.
+        // `#[` is a vector in Racket, and Gauche's char-set literal in Scheme.
+        // `classify_scheme`'s dispatch table admits `#(` alone.
         "(f #[1 2])",
         "(f #3(0))",
         "#<<END\nx\nEND\n",
@@ -3671,6 +3674,32 @@ fn scheme_still_refuses_racket_only_dispatch() {
             "{input} should still be refused for Scheme"
         );
     }
+    // R6RS 4.3.5's `syntax`/`quasisyntax`/`unsyntax` and the bytevector
+    // string. These four moved out of the list above; the audit their old
+    // comment asked for found `#'` alone in 184 corpus files.
+    for (input, children) in [
+        ("(f #'x)", 2usize),
+        ("(f #`x)", 2),
+        ("(f #,x)", 2),
+        ("(f #,@x)", 2),
+        ("(f #\"bytes\")", 2),
+    ] {
+        let tree = SyntaxTree::parse_with_dialect(input, Dialect::Scheme)
+            .unwrap_or_else(|error| panic!("{input}: {error}"));
+        assert_eq!(
+            tree.root_view().children[0].children.len(),
+            children,
+            "{input}"
+        );
+    }
+    // `#'x` keeps its child visible to rename and every lint rule, which is
+    // the whole reason it is a `ReaderPrefix` and the other three are not.
+    let input = "(f #'target)";
+    let tree = SyntaxTree::parse_with_dialect(input, Dialect::Scheme).expect("valid");
+    assert_eq!(
+        tree.root_view().children[0].children[1].span.slice(input),
+        "#'target"
+    );
     // `#u8(…)` is R7RS and stays; Racket's own `read-dispatch` has no `#\u`
     // clause, so it is refused there (pinned above) and kept here.
     let input = "(f #u8(1 2))";
@@ -3854,26 +3883,80 @@ fn scheme_reads_guile_nil_but_not_every_hash_n() {
 
 /// What Scheme deliberately still refuses, and why it is not a gap.
 ///
-/// Each of these is a *per-implementation* extension: Gambit's namespaced
-/// `##car`, Gauche's `#/re/`, `#"interpolated"` and `#[char-set]`, Guile's
-/// `#{extended symbol}#`. Between them they are every remaining failure
-/// cluster in the corpus (349 of 2352 files). They are refused on purpose —
-/// Guile's reader rejects the Gauche and Gambit spellings and vice versa, so
-/// admitting them into one `Dialect::Scheme` would make this reader accept a
-/// language no implementation actually reads. Separating them needs a
-/// per-implementation flavour of the dialect, which is a larger change.
+/// Each of these is a *per-implementation* extension: Gauche's `#/re/` and
+/// `#[char-set]`, Guile's `#{extended symbol}#`. Guile's reader rejects the
+/// Gauche spellings and Gauche rejects Guile's, so admitting them into one
+/// `Dialect::Scheme` would make this reader accept a language no
+/// implementation actually reads. Separating them needs a per-implementation
+/// flavour of the dialect, which is a larger change.
+///
+/// Gambit's `##name` and Gauche's `#"interpolated"` used to be refused here
+/// too; see [`scheme_reads_gambit_namespace_qualified_identifiers`] and
+/// [`scheme_still_refuses_racket_only_dispatch`] for why they moved.
 #[test]
 fn scheme_refuses_per_implementation_extensions() {
-    for input in [
-        "(##car x)",
-        "(f #/re/)",
-        "(f #\"interpolated ~x\")",
-        "(f #[a-z])",
-        "(f #{sym}#)",
-    ] {
+    for input in ["(f #/re/)", "(f #[a-z])", "(f #{sym}#)"] {
         assert!(
             SyntaxTree::parse_with_dialect(input, Dialect::Scheme).is_err(),
             "{input} is implementation-specific and must still refuse"
         );
     }
 }
+
+/// `##name` is Gambit's namespace-qualified identifier, and with it Gerbil's
+/// whole runtime and Chicken/Cyclone's `##core#lambda`. Guile's reader
+/// rejects `##car`, so the oracle cannot adjudicate this arm; it rests on
+/// Gambit's documented namespace syntax and on the fact that the alternative
+/// is not "read it correctly" but "fail the file". Chez's `#%name` /
+/// `#3%name` primitive references are pinned alongside it, since both are
+/// identifiers handed whole to the atom scanner rather than reader macros.
+#[test]
+fn scheme_reads_gambit_namespace_qualified_identifiers() {
+    for (input, children, second) in [
+        ("(f ##car)", 2usize, "##car"),
+        ("(f ##core#lambda)", 2, "##core#lambda"),
+        ("(f #%$tlc-next)", 2, "#%$tlc-next"),
+        ("(f #3%vector-copy)", 2, "#3%vector-copy"),
+        ("(f #2%apply)", 2, "#2%apply"),
+    ] {
+        let tree = SyntaxTree::parse_with_dialect(input, Dialect::Scheme)
+            .unwrap_or_else(|error| panic!("{input}: {error}"));
+        let form = &tree.root_view().children[0];
+        assert_eq!(form.children.len(), children, "{input}");
+        assert_eq!(form.children[1].span.slice(input), second, "{input}");
+    }
+}
+
+/// `##` does not leak into dialects that never asked for it, and does not
+/// regress the dialects that already read it their own way.
+///
+/// Four of the nine already read `##car` as an ordinary token before this
+/// change: Clojure because `##Inf`/`##NaN` are its symbolic values, Fennel
+/// because `#` is its `hashfn` shorthand, and LFE and Hy through their own
+/// tables. Asserting a refusal for them would fail on behaviour this change
+/// never touched.
+#[test]
+fn scheme_hash_hash_stays_scoped_to_scheme() {
+    for dialect in [
+        Dialect::CommonLisp,
+        Dialect::EmacsLisp,
+        Dialect::Carp,
+        // Racket reads `##car` as `#` dispatching on `#`, which is its own
+        // "bad syntax" — the same refusal, through a different table.
+        Dialect::Racket,
+        // Janet's `#` is a line comment, so `##car)` comments out the closing
+        // paren and the list never closes. A different error, still an error.
+        Dialect::Janet,
+    ] {
+        assert!(
+            SyntaxTree::parse_with_dialect("(f ##car)", dialect).is_err(),
+            "{dialect:?} should not have gained Gambit's `##`"
+        );
+    }
+    for dialect in [Dialect::Clojure, Dialect::Lfe, Dialect::Hy, Dialect::Fennel] {
+        assert!(
+            SyntaxTree::parse_with_dialect("(f ##car)", dialect).is_ok(),
+            "{dialect:?} read `##car` before this change and must still"
+        );
+    }
+}

packages/feature/form-transform/src/replace_forms/domain/tests.rs

@@ -128,7 +128,14 @@ fn supports_each_known_dialect_with_its_reader_semantics() {
 fn dialect_specific_reader_semantics_do_not_leak() {
     assert!(SyntaxTree::parse_with_dialect(r"(list #\))", Dialect::Clojure).is_err());
     assert!(SyntaxTree::parse_with_dialect(r"[?\)]", Dialect::EmacsLisp).is_ok());
-    assert!(SyntaxTree::parse_with_dialect(r"[?\)]", Dialect::CommonLisp).is_err());
+    // `[`, `]` and `?` are constituent characters in Common Lisp (CLHS 2.4.2),
+    // not delimiters, and `\)` is a single-escaped `)` -- so this reads as one
+    // ordinary symbol rather than failing. It happens to *also* parse in
+    // Emacs Lisp, just as a different construct (a vector containing the
+    // character literal for `)`); that difference in what it means, not
+    // whether it parses, is what keeps Emacs Lisp's reader semantics from
+    // leaking into Common Lisp's.
+    assert!(SyntaxTree::parse_with_dialect(r"[?\)]", Dialect::CommonLisp).is_ok());
     assert!(
         SyntaxTree::parse_with_dialect(r#"(vector #inst "2020-01-01")"#, Dialect::CommonLisp)
             .is_err()

packages/feature/lint-concurrency/src/lib.rs

@@ -274,15 +274,21 @@ mod engine_pass_tests {
     }
 
     /// `future-promise-never-realized` has no dual-syntax source at all: it
-    /// requires a `[…]` binding vector, and the Common Lisp reader rejects `[`
-    /// outright rather than parsing it as a list. So its Common Lisp silence is
-    /// pinned two ways — the Clojure spelling is not readable as Common Lisp,
-    /// and the Common Lisp `let` spelling that *is* head-matched by the rule's
-    /// `HeadFilter::Heads` reaches no finding.
+    /// requires a `[…]` binding vector, which Common Lisp has no `let` syntax
+    /// for. `[` and `]` are constituent characters in Common Lisp (CLHS
+    /// 2.4.2), not delimiters, so the Clojure spelling still *parses* as
+    /// Common Lisp — just not as anything shaped like the `let` this rule
+    /// looks for, since `[f` and `]` read as ordinary symbols rather than
+    /// opening and closing a binding form. So its Common Lisp silence is
+    /// pinned two ways — the Clojure spelling does not produce a finding once
+    /// read as Common Lisp, and the Common Lisp `let` spelling that *is*
+    /// head-matched by the rule's `HeadFilter::Heads` reaches no finding
+    /// either.
     #[test]
     fn the_clojure_only_future_rule_is_silent_on_common_lisp() {
         let clojure = "(let [f (future (risky))] (other-work))";
-        assert!(SyntaxTree::parse_with_dialect(clojure, Dialect::CommonLisp).is_err());
+        assert!(SyntaxTree::parse_with_dialect(clojure, Dialect::CommonLisp).is_ok());
+        assert_eq!(fired(clojure, Dialect::CommonLisp), Vec::<&str>::new());
 
         let common_lisp = "(let ((f (future (risky)))) (other-work))";
         assert_eq!(fired(common_lisp, Dialect::CommonLisp), Vec::<&str>::new());

packages/feature/lint-contract-annotation/src/clojure_pre_post_vacuous/domain.rs

@@ -417,12 +417,14 @@ mod tests {
     /// as clean.
     #[test]
     fn a_non_clojure_dialect_is_reported_as_unmodelled() {
-        // `[` is not readable as Common Lisp at all, so the Clojure spelling
-        // cannot even be parsed as one. The Common Lisp shape closest to it
-        // still reaches no finding.
+        // `[`, `]`, `{` and `}` are constituent characters in Common Lisp, not
+        // delimiters, so the Clojure spelling still parses as Common Lisp —
+        // just as a run of ordinary symbols rather than a `defn` this rule
+        // recognizes. The Common Lisp shape closest to it still reaches no
+        // finding.
         assert!(
             SyntaxTree::parse_with_dialect("(defn f [x] {:pre [true]} x)", Dialect::CommonLisp)
-                .is_err()
+                .is_ok()
         );
 
         let tree =

packages/feature/lint-contract-annotation/src/lib.rs

@@ -350,22 +350,22 @@ mod engine_pass_tests {
     }
 
     /// The Clojure rule, the other way round. It has no dual-syntax source: it
-    /// needs a `[…]` parameter vector and a `{…}` condition map, and neither
-    /// the Common Lisp nor the Scheme reader accepts those. So its silence
-    /// elsewhere is pinned two ways — the Clojure spelling is not readable at
-    /// all as those dialects, and every reader that *does* accept the bytes
-    /// reaches no finding.
+    /// needs a `[…]` parameter vector and a `{…}` condition map, which neither
+    /// the Scheme reader nor Common Lisp's own `let`/`defn` grammar has a
+    /// binding-vector or map-literal reading for. Common Lisp's reader does
+    /// still accept the bytes — `[`, `]`, `{` and `}` are CLHS-constituent
+    /// characters, not delimiters, so the trigger source reads as a run of
+    /// ordinary symbols rather than failing outright — which is exactly why
+    /// it joins the "whichever reader accepts these bytes must still find
+    /// nothing" loop below instead of a dialect it cannot even parse as.
     #[test]
     fn the_clojure_only_rule_fires_on_clojure_and_on_nothing_else() {
         for (rule, source) in CLOJURE_TRIGGERS {
             assert_eq!(fired(source, Dialect::Clojure), vec![rule]);
-            assert!(
-                !parses_as(source, Dialect::CommonLisp),
-                "{rule}'s trigger must not even read as Common Lisp"
-            );
             // Whichever of the remaining readers accept these bytes must still
             // find nothing; the ones that reject them cannot report either.
             for dialect in [
+                Dialect::CommonLisp,
                 Dialect::Racket,
                 Dialect::Scheme,
                 Dialect::EmacsLisp,

packages/feature/remove-unused/src/definition_report/domain.rs

@@ -523,7 +523,12 @@ mod tests {
     fn parses_reader_syntax_with_the_input_dialect() {
         let text = "(defun el-reader () [?\\)])\n";
         assert!(SyntaxTree::parse_with_dialect(text, Dialect::EmacsLisp).is_ok());
-        assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_err());
+        // `[`, `]` and `?` are Common Lisp constituent characters and `\)` a
+        // single-escaped `)`, so this also parses as Common Lisp -- just as a
+        // symbol-heavy form rather than a character literal in a vector. The
+        // point of this test is that removal reads `text` with the *input*
+        // dialect (Emacs Lisp) rather than either reader's opinion of it.
+        assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_ok());
         let files = vec![parsed_file("reader.el", Dialect::EmacsLisp, text)];
 
         let reports = collect_unused_definition_candidates(&files).expect("report must build");

packages/feature/remove-unused/src/remove_unused_definition/domain/tests/basic.rs

@@ -343,7 +343,12 @@ fn uses_the_input_dialect_for_reader_syntax_during_removal() {
     let text = "(defun el-reader () [?\\)])\n";
     let form = "(defun el-reader () [?\\)])";
     assert!(SyntaxTree::parse_with_dialect(text, Dialect::EmacsLisp).is_ok());
-    assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_err());
+    // `[`, `]` and `?` are Common Lisp constituent characters and `\)` a
+    // single-escaped `)`, so this also parses as Common Lisp -- just as a
+    // symbol-heavy form rather than a character literal in a vector. The
+    // point of this test is that removal reads `text` with the *input*
+    // dialect (Emacs Lisp) rather than either reader's opinion of it.
+    assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_ok());
     let mut item = definition(text, form, "el-reader", DefinitionCategory::Function);
     item.package = None;
     let request = RemoveUnusedDefinitionsRequest {