From a3466c92b0a1c3b1b69ae1b3c901ef06b9a6055a Mon Sep 17 00:00:00 2001 From: StreamDemon Date: Thu, 2 Jul 2026 15:21:51 +0800 Subject: [PATCH 1/2] Enforce range non-associativity and item-modifier placement Two remaining over-acceptances from the section-16 review: - The precedence table marks `..`/`..=` as non-associative, but the parser gave them ordinary left-associative binding powers, so `a..b..c` silently parsed as `(a..b)..c`. A range operand may not itself be an unparenthesized range: the chain is now a parse error ("range operators cannot be chained; parenthesize one side"), while `(a..b)..c` stays legal because the parenthesized form is explicit. - Item modifiers were eaten before dispatch and never validated, so `pub async struct`, `offchain mod`, `pub impl`, and `pub actor` all parsed. Grammar places `offchain`/`async` only on fn_def, and omits `pub` from impl_block, actor_def, onchain_mod/event_def, and extern_block. Misplaced modifiers now error on the modifier's own span, and the item still parses afterward for error recovery. Adds four unit tests (chained-range rejection, single/parenthesized range acceptance, and both directions of the modifier matrix), a ranges_modifiers.sp corpus fixture, and the crates/AGENTS.md contract updates. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja --- crates/AGENTS.md | 14 +-- crates/sploosh-parser/src/lib.rs | 125 +++++++++++++++++++++++++- crates/sploosh-parser/tests/corpus.rs | 1 + tests/corpus/ranges_modifiers.sp | 22 +++++ 4 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 tests/corpus/ranges_modifiers.sp diff --git a/crates/AGENTS.md b/crates/AGENTS.md index bbe9a96..c02dcc1 100644 --- a/crates/AGENTS.md +++ b/crates/AGENTS.md @@ -23,11 +23,15 @@ the accepted list as **not yet implemented** — add a corpus fixture when it la - **Accepted:** every item form (`fn`, `struct`, `enum` incl. tuple/struct variants, `actor`, `mod`, `use`, `const`, `type`, `trait`, `impl`, - `onchain mod`, `extern`); types incl. generics, references, arrays/slices, - tuples, `fn`/`dyn`; expressions incl. calls, turbofish, field/index, - unary/binary (incl. `/`), `?`, `.await`, `|>`, `as`, struct literals (with the - §5.1 block-head restriction), `vec!`, and `if`/`else`; `let`, `return`, - `break`, `continue`, `send`, and expression/tail statements. + `onchain mod`, `extern`), with §16 modifier placement enforced — + `offchain`/`async` only on `fn` items, and `pub` rejected on `impl`, + `actor`, `onchain`, and `extern` items; types incl. generics, references, + arrays/slices, tuples, `fn`/`dyn`; expressions incl. calls, turbofish, + field/index, unary/binary (incl. `/`; `..`/`..=` are non-associative — + chained ranges are a parse error), `?`, `.await`, `|>`, `as`, struct + literals (with the §5.1 block-head restriction), `vec!`, and `if`/`else`; + `let`, `return`, `break`, `continue`, `send`, and expression/tail + statements. - **Not yet implemented:** `match`, `while`, `for`, `loop`, `if let`, and closures (their keywords lex but have no parse production); `spawn`, `select`, and `emit` (reserved keywords with no expression/statement production yet, so diff --git a/crates/sploosh-parser/src/lib.rs b/crates/sploosh-parser/src/lib.rs index 6d83a3f..88d1e3b 100644 --- a/crates/sploosh-parser/src/lib.rs +++ b/crates/sploosh-parser/src/lib.rs @@ -63,14 +63,42 @@ impl Parser { self.skip_doc_comments(); let start = self.peek()?.span.start; let attrs = self.attrs(); - let visibility = if self.eat_keyword(Keyword::Pub).is_some() { + let pub_token = self.eat_keyword(Keyword::Pub); + let offchain_token = self.eat_keyword(Keyword::Offchain); + let async_token = self.eat_keyword(Keyword::Async); + let visibility = if pub_token.is_some() { Visibility::Public } else { Visibility::Private }; - let is_offchain = self.eat_keyword(Keyword::Offchain).is_some(); - let is_async = self.eat_keyword(Keyword::Async).is_some(); - let kind = match self.peek_kind()? { + let is_offchain = offchain_token.is_some(); + let is_async = async_token.is_some(); + // §16: `offchain`/`async` prefix only `fn_def`; `pub` prefixes every + // item form except `impl_block`, `actor_def`, `onchain_mod`/`event_def`, + // and `extern_block`. + let next = self.peek_kind()?; + if !matches!(next, TokenKind::Keyword(Keyword::Fn)) { + if let Some(token) = &offchain_token { + self.error_at(token.span, "`offchain` applies only to `fn` items"); + } + if let Some(token) = &async_token { + self.error_at(token.span, "`async` applies only to `fn` items"); + } + } + if let Some(token) = &pub_token + && matches!( + next, + TokenKind::Keyword( + Keyword::Impl | Keyword::Actor | Keyword::Onchain | Keyword::Extern + ) + ) + { + self.error_at( + token.span, + "`pub` is not allowed on `impl`, `actor`, `onchain`, or `extern` items", + ); + } + let kind = match next { TokenKind::Keyword(Keyword::Fn) => ItemKind::Function(self.function_after_mods( visibility, is_async, @@ -676,6 +704,16 @@ impl Parser { span, } }; + // The precedence table marks `..`/`..=` non-associative: a range + // operand may not itself be an unparenthesized range. + if matches!(op, ".." | "..=") + && matches!( + self.peek_kind(), + Some(TokenKind::DotDot | TokenKind::DotDotEq) + ) + { + self.error_here("range operators cannot be chained; parenthesize one side"); + } } Some(lhs) } @@ -1162,6 +1200,10 @@ impl Parser { let span = self .peek() .map_or_else(|| self.prev_span(), |token| token.span); + self.error_at(span, message); + } + + fn error_at(&mut self, span: Span, message: impl Into) { self.errors.push(ParseError { message: message.into(), span, @@ -1332,4 +1374,79 @@ mod tests { }; assert_eq!(fields.len(), 1); } + + #[test] + fn range_operators_cannot_chain() { + for source in [ + "fn f() { let r = a..b..c; }", + "fn f() { let r = a..=b..c; }", + "fn f() { let r = a..b..=c; }", + ] { + let errors = parse_program(source).unwrap_err(); + assert!( + errors.iter().any(|err| err.message.contains("chained")), + "{source}: expected a range-chaining error, got {errors:?}" + ); + } + } + + #[test] + fn single_and_parenthesized_ranges_parse() { + assert!(parse_program("fn f() { let r = lo..hi; }").is_ok()); + assert!(parse_program("fn f() { let r = lo..=hi; }").is_ok()); + assert!(parse_program("fn f() { let r = (a..b)..c; }").is_ok()); + } + + #[test] + fn offchain_and_async_apply_only_to_fn_items() { + for source in [ + "offchain struct S { x: i64 }", + "async struct S { x: i64 }", + "pub async struct S { x: i64 }", + "async trait T {}", + "offchain mod m;", + ] { + let errors = parse_program(source).unwrap_err(); + assert!( + errors + .iter() + .any(|err| err.message.contains("applies only to `fn` items")), + "{source}: expected a modifier error, got {errors:?}" + ); + } + assert!(parse_program("pub offchain async fn f() {}").is_ok()); + assert!(parse_program("offchain fn g() {}").is_ok()); + } + + #[test] + fn pub_rejected_where_grammar_omits_it() { + for source in [ + "pub impl User {}", + "pub actor A { state: i64, }", + "pub onchain mod token {}", + "pub extern \"C\" { fn f(); }", + ] { + let errors = parse_program(source).unwrap_err(); + assert!( + errors + .iter() + .any(|err| err.message.contains("`pub` is not allowed")), + "{source}: expected a pub-placement error, got {errors:?}" + ); + } + for source in [ + "pub struct S { x: i64 }", + "pub enum E { A }", + "pub trait T {}", + "pub mod m;", + "pub use crate::api;", + "pub const X: i64 = 1;", + "pub type Y = i64;", + ] { + assert!( + parse_program(source).is_ok(), + "{source}: `pub` should be accepted here" + ); + } + } } diff --git a/crates/sploosh-parser/tests/corpus.rs b/crates/sploosh-parser/tests/corpus.rs index f40dc58..d4fb7c9 100644 --- a/crates/sploosh-parser/tests/corpus.rs +++ b/crates/sploosh-parser/tests/corpus.rs @@ -6,6 +6,7 @@ fn parses_corpus_files() { "tests/corpus/basic.sp", "tests/corpus/actor.sp", "tests/corpus/control_flow.sp", + "tests/corpus/ranges_modifiers.sp", ] { let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("../..") diff --git a/tests/corpus/ranges_modifiers.sp b/tests/corpus/ranges_modifiers.sp new file mode 100644 index 0000000..7f1d940 --- /dev/null +++ b/tests/corpus/ranges_modifiers.sp @@ -0,0 +1,22 @@ +/// Range expressions (`..`/`..=`, non-associative per the precedence table) +/// and §16 item-modifier placement: `pub` on declarations, `offchain`/`async` +/// on functions only. +pub offchain async fn window(lo: i64, hi: i64) -> i64 { + let half_open = lo..hi; + let inclusive = lo..=hi; + lo + hi +} + +pub struct Config { + pub retries: i64, +} + +pub mod api; + +pub use crate::api; + +pub const LIMIT: i64 = 8; + +pub type Pair = (i64, i64); + +pub trait Sink {} From f909490c23326c0ef07611a9b27dbd040c86d9d1 Mon Sep 17 00:00:00 2001 From: StreamDemon Date: Thu, 2 Jul 2026 15:34:30 +0800 Subject: [PATCH 2/2] Assert diagnostic spans in range-chain and modifier tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic review on PR #74: both tests validated message text only, while the PR promises span anchoring — the second range operator for chained ranges, the modifier's own token for misplaced modifiers. The tests now assert the error span alongside the message, so the anchoring cannot silently regress. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja --- crates/sploosh-parser/src/lib.rs | 35 +++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/crates/sploosh-parser/src/lib.rs b/crates/sploosh-parser/src/lib.rs index 88d1e3b..a7033a5 100644 --- a/crates/sploosh-parser/src/lib.rs +++ b/crates/sploosh-parser/src/lib.rs @@ -1383,9 +1383,14 @@ mod tests { "fn f() { let r = a..b..=c; }", ] { let errors = parse_program(source).unwrap_err(); + // The diagnostic must anchor to the second range operator + // (`..=` starts with `..`, so the second `..` match is its start). + let second_op = source.match_indices("..").nth(1).unwrap().0; assert!( - errors.iter().any(|err| err.message.contains("chained")), - "{source}: expected a range-chaining error, got {errors:?}" + errors + .iter() + .any(|err| err.message.contains("chained") && err.span.start == second_op), + "{source}: expected a range-chaining error at {second_op}, got {errors:?}" ); } } @@ -1399,19 +1404,23 @@ mod tests { #[test] fn offchain_and_async_apply_only_to_fn_items() { - for source in [ - "offchain struct S { x: i64 }", - "async struct S { x: i64 }", - "pub async struct S { x: i64 }", - "async trait T {}", - "offchain mod m;", + for (source, modifier) in [ + ("offchain struct S { x: i64 }", "offchain"), + ("async struct S { x: i64 }", "async"), + ("pub async struct S { x: i64 }", "async"), + ("async trait T {}", "async"), + ("offchain mod m;", "offchain"), ] { let errors = parse_program(source).unwrap_err(); + // The diagnostic must anchor to the misplaced modifier's own span. + let start = source.find(modifier).unwrap(); + let span = Span::new(start, start + modifier.len()); assert!( errors .iter() - .any(|err| err.message.contains("applies only to `fn` items")), - "{source}: expected a modifier error, got {errors:?}" + .any(|err| err.message.contains("applies only to `fn` items") + && err.span == span), + "{source}: expected a modifier error at {span:?}, got {errors:?}" ); } assert!(parse_program("pub offchain async fn f() {}").is_ok()); @@ -1427,11 +1436,13 @@ mod tests { "pub extern \"C\" { fn f(); }", ] { let errors = parse_program(source).unwrap_err(); + // `pub` heads each source, so the diagnostic must anchor to 0..3. assert!( errors .iter() - .any(|err| err.message.contains("`pub` is not allowed")), - "{source}: expected a pub-placement error, got {errors:?}" + .any(|err| err.message.contains("`pub` is not allowed") + && err.span == Span::new(0, 3)), + "{source}: expected a pub-placement error at 0..3, got {errors:?}" ); } for source in [