Repository navigation
feat(rules): convert a rule-content-v1 statement into the public model - #534
Conversation
Add rules::statement::statement_from_content. It turns one condition or action list item into a Statement, and refuses unknown ids, wrong argument counts, internal sentinels, unrepresentable nodes (including ambiguous enum members) and spans with distinct structured errors. The rule-content-v1 format id and schema are unchanged. Fixes #531
Teakowa
left a comment
There was a problem hiding this comment.
One defect against the repo's contract rules; everything else I checked holds.
Arity check reimplements the established path instead of sharing it (crates/workshop-rs/src/rules/statement.rs, Converter::check_arity). It copies the rule in values::validate::validate_call_signature (undeclared-signature skip, folds_player_variable_pair +1, required_param_count..param_count, variadic). workshop-rs/AGENTS.md "Sibling paths share checks" requires reusing the established path's checks and adding a test that both paths agree on the same inputs. The PR body states the rule is the same, but nothing enforces it: when validation changes (for example a new fold case), conversion silently diverges and accepts or refuses differently from canonical validation. No agreement test exists.
Required correction, smallest level: extract the count rule into one CatalogEntry helper taking the supplied count (and the fold flag), call it from both validate_call_signature and check_arity, and add one test that feeds the same call (including a folded player-variable pair, a variadic and an undeclared-signature entry) through conversion and canonical validation and asserts both accept or both refuse.
Checked and fine: fmt, clippy -D warnings, full workspace tests and catalog check pass locally; CI is green (stable, 1.88.0, public API, catalog, real-project). Round-trip passes over practice-range (en-US, zh-CN) and corpus fixtures; 5 distinct error variants with JSON pointers; spans (top-level and nested) rejected; ambiguous enum and raw action refused; additive API; docs updated; no emission or edit code (#532).
…nd statement conversion Extract CatalogEntry::rejected_arity so canonical validation and statement_from_content apply one rule, and add an agreement test over a folded player-variable pair, a variadic entry and an undeclared-signature entry.
Teakowa
left a comment
There was a problem hiding this comment.
LGTM: the argument-count rule now lives once in CatalogEntry::rejected_arity and both validate_call_signature and statement_from_content call it. The new agreement test checks accept and reject for every count on a folded player-variable pair, a variadic entry and an undeclared-signature entry. fmt, clippy -D warnings and the full workspace tests pass in a fresh worktree on 99d1ec4, scope is unchanged, and all 5 CI checks are green.
Fixes #531
Adds
rules::statement::statement_from_content(catalog, node), the input direction ofrule-content-v1. It converts one condition ({"value": node}) or action ({"call", "args"}) list item, with optional"disabled": true, into aStatement. A node fromRule::contentis valid input unchanged. The format id and schema are unchanged.StatementContentErrorhas distinct variants, each naming the node by JSON pointer:UnknownId,ArgumentCount,InternalSentinel,Unrepresentable,Spans. Argument counts follow the same rule canonical validation applies. An ambiguous enum node and an opaquerawWorkshopActionare refused asUnrepresentable.Verification:
content(statement_from_content(content(s))) == content(s)over the practice-range exports and the corpus fixtures. The practice-range files are parsed in en-US and zh-CN, and the program is emitted and re-parsed in every locale that can render it (emission skips locales missing a spelling; en-US and zh-CN are asserted). Round-trip is verified only for en-US and zh-CN because the catalog lacks spellings in the other 14 locales; statement content is locale-independent.-D warnings, full tests,workshop-catalog-gen checkon stable and 1.88.0.cargo semver-checksreports two failures (SettingsListElement.variant,LookupMatch::Builtin.description). Both are in code this PR does not touch, and no failure names the new module.Argument-count checking is one shared
CatalogEntry::rejected_arityused by canonical validation and by conversion, with an agreement test over a folded player-variable pair, a variadic entry and an undeclared-signature entry.Not in scope: emission and edits (#532).