Skip to content

Enforce send operand, assignment target, and vec! bracket rules#72

Merged
StreamDemon merged 2 commits into
fix/parser-correctness-basefrom
fix/send-assign-vec-validation
Jul 2, 2026
Merged

Enforce send operand, assignment target, and vec! bracket rules#72
StreamDemon merged 2 commits into
fix/parser-correctness-basefrom
fix/send-assign-vec-validation

Conversation

@StreamDemon

@StreamDemon StreamDemon commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Three spec-mandated parse errors the parser silently accepted, from the same §16 review as PR #70:

  • send operand validation (§2.7): the send-statement now implements the statement-head rule exactly — send opens a send-statement only when the next token can begin an expression (a can_begin_expr predicate kept in sync with prefix()), and the operand must be a method call (handle.method(args), §8.2). send 42;, send free_fn(x);, and send (x); are now parse errors; send = x; and send.reset(); keep treating send as an ordinary identifier, per the deterministic rule.
  • Assignment-target validation (§16 assign_target): the left side of = must be a single identifier, a field access, an index, or a dereference. 1 + 2 = 3;, g() = 1;, a::b = 1;, x? = 1;, and self = x; now error with "invalid assignment target". (§16 lists IDENT, so self/Self alone and multi-segment paths are rejected.)
  • vec! bracket enforcement (§16 vec_literal): vec!(1) previously dropped the ! silently and re-parsed as a call of a plain vec binding. A missing [ after vec! is now a parse error; plain vec without ! remains an ordinary identifier.

Stacked-PR note: targets the integration branch fix/parser-correctness-base (PR #69), not main — sibling of PR #70. The unit-test module tail, corpus registration array, and one crates/AGENTS.md bullet will conflict textually with #70 when the second of the two merges; resolution is keep-both.

Known lenience (noted, not changed): parenthesized targets like (x) = y are accepted because the AST does not record parentheses; the grammar's assign_target list technically doesn't include them. Cheap to revisit if the spec ever cares.

Related Issue

None (review-driven; part of the parser correctness wave tracked in PR #69).

Spec Sections Affected

None changed — this PR implements §2.7's send-statement rule and §16's assign_target/vec_literal as written.

Checklist

  • Code follows the Sploosh design principles (one way to do it, explicit over implicit, etc.)
  • Documentation updated in relevant docs/ pages (crates/AGENTS.md subset contract)
  • Tests added or updated
  • All build targets still compile (if applicable)
  • Spec-only PR (skip Build Targets section if checked)

Build Targets Tested

  • N/A (docs/spec only) — no codegen targets exist yet; "build" = the Rust workspace.

Test Plan

  • 7 new unit tests: rejection paths (send non-method-call operands ×4, invalid assignment targets ×5, vec!(...)/vec!{...}) all assert on the specific error message; acceptance paths (send method calls incl. chained fields and turbofish, every assign_target shape incl. chained a = b = c, send-as-identifier before =/., plain vec binding).
  • New corpus fixture tests/corpus/send_assign.sp covering send statements and every accepted assign_target shape.
  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace (24 tests) green locally and via scripts/docker-check.ps1 -Build (Ubuntu parity).

Summary by cubic

Enforces spec-mandated parser rules and surfaces clear errors for invalid send, assignment, and vec! forms. send now requires a method call operand, assignment LHS is restricted to valid targets, and vec! requires [.

  • Bug Fixes
    • send opens a send-statement only when the next token can begin an expression; operand must be handle.method(args). send = x; and send.reset(); treat send as an identifier.
    • Assignment LHS must be an identifier, field access, index, or deref; rejects paths like a::b, calls, self, and other expressions.
    • vec! only binds to square brackets; vec!(...) and vec!{...} now error. Plain vec without ! remains an identifier.

Written for commit 90739fe. Summary will update on new commits.

Review in cubic

Three spec-mandated parse errors the parser silently accepted, found by
the same section-16 review that drove the pipe/impl/trait/dyn fixes:

- Send statements took any expression. Section 2.7 requires the operand
  to be a method call (`handle.method(args)`) and makes anything else a
  parse error inside the send-statement production. The statement-head
  rule is now implemented exactly: `send` opens a send-statement only
  when the next token can begin an expression, so `send = x;` and
  `send.reset();` keep treating `send` as an ordinary identifier.
- Assignment accepted any expression on the left (`1 + 2 = 3;` parsed).
  Section 16 restricts the left side to assign_target: a single
  identifier, a field access, an index, or a dereference. `self`/`Self`
  alone and multi-segment paths are rejected.
- `vec!(...)` silently dropped the `!` and re-parsed as a call of a
  plain `vec` binding. The vec_literal production only ever binds to
  square brackets, so a missing `[` after `vec!` is now a parse error.

Adds seven unit tests (acceptance and rejection paths for all three
rules), a send_assign.sp corpus fixture covering every accepted
assign_target shape, and the matching crates/AGENTS.md contract notes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Parser
    participant TokenStream
    participant Errors as Error Collector

    Note over Parser,Errors: NEW: Send-statement head rule (§2.7)
    Parser->>TokenStream: at_ident_text("send") & can_begin_expr(next)
    alt can_begin_expr is true
        Parser->>TokenStream: bump()
        Parser->>Parser: parse expression (expr(0))
        Parser->>Parser: is_method_call(expr)?
        alt is method call (handle.method(args))
            Parser->>TokenStream: expect Semi
            Parser->>Errors: No error
        else not a method call
            Parser->>Errors: error_at(span, "send operand must be a method call")
        end
    else can_begin_expr false (e.g. `=`, `.`)
        Parser->>Parser: treat `send` as identifier, parse as normal statement
    end

    Note over Parser,Errors: NEW: Assignment-target validation (§16)
    Parser->>Parser: parse LHS expression
    Parser->>TokenStream: eat("=")
    alt equals found
        Parser->>Parser: is_assign_target(lhs)?
        alt is valid target (ident, field, index, deref)
            Parser->>Parser: parse RHS expression
            Parser->>Parser: build Assign node
        else invalid target (e.g. binary, call, multi-path, self)
            Parser->>Errors: error_at(lhs.span, "invalid assignment target")
            Parser->>Parser: continue parsing (error recovery?)
        end
    else no equals
        Parser->>Parser: treat as other expression (no assignment)
    end

    Note over Parser,Errors: NEW: vec! bracket enforcement (§16)
    Parser->>TokenStream: parse prefix: `vec` ident
    Parser->>TokenStream: eat(Bang)
    alt bang found
        Parser->>TokenStream: eat(LBracket)?
        alt LBracket found
            Parser->>Parser: parse vec elements as before
        else no LBracket
            Parser->>Parser: error_here("expected `[` after `vec!`")
            Parser->>Parser: return None (abort prefix expr)
        end
    else no bang
        Parser->>Parser: treat `vec` as plain identifier expression
    end
Loading

Requires human review: Adds parse validation for send operand, assignment targets, and vec! brackets. Core parser logic changes warrant human review.

Re-trigger cubic

@StreamDemon StreamDemon merged commit e70552c into fix/parser-correctness-base Jul 2, 2026
@StreamDemon StreamDemon deleted the fix/send-assign-vec-validation branch July 2, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant