fix(sysml-derive): validate + fix SysML v2 output for primitive/DateTime field types - #196
Open
elasticdotventures wants to merge 6 commits into
Open
Conversation
…lues DateTime<Utc> previously emitted the literal, invalid text `DateTime<Utc>` as a SysML v2 attribute type -- SysML v2's grammar has no angle-bracket generic-parameter syntax, so this would fail to parse under any conformant SysML v2 tool. bool/usize/etc. also passed through as bare Rust keywords with no corresponding SysML type. Map DateTime<_> -> ScalarValues::String, bool -> ScalarValues::Boolean, unsigned ints -> ScalarValues::Natural, signed ints -> ScalarValues::Integer, floats -> ScalarValues::Rational. Any other single-type-argument generic is now a compile error instead of a silent invalid-syntax emission. Opaque domain types (NodeId, Confidence, Decimal) still pass through as bare names -- documented as an intentional modeling assumption, not a bug. Adds regression tests for the field types introduced by #184/#193 (Decimal, Confidence, bool, usize, DateTime<Utc>) that basic.rs never exercised. Closes ledgrrr#195.
…eal parser validation into ufo-types Ran SysmlV2Emitter's own output through the real sysml-v2-parser crate (docs/sysml-v2-parser-spike.md, an existing unmerged spike) and confirmed two bugs make it non-parseable: - The closing '}' was on the same line as a trailing '//' comment, so the comment swallowed it -- the block was never syntactically closed. - The emitter used SysML v1's 'block def' keyword. SysML v2 renamed this construct to 'part def'; 'block' is not a SysML v2 keyword at all. Fixed both. Added ufo_types::sysml -- a shared Constraint/Satisfies-based SysML v2 syntax validator wired to sysml-v2-parser (pinned to =0.54.0 per the spike's crate-health findings; not wasm32-compatible, so this must stay out of holon-viz's runtime dependency graph -- added to holon-viz only as a dev-dependency, used in a new round-trip test that feeds the emitter's own output through the real parser instead of just asserting on substrings. This is the concrete round-trip-closed signal the existing spike (PR #187) called out as the next step.
…yntax' into fix/sysml-block-scalar-type-mapping # Conflicts: # Cargo.lock
…al SysML v2 grammar Same bug holon-viz's SysmlV2Emitter had (ledgrrr#197): SysML v1 called this construct Block/'block def'; SysML v2 renamed it to 'part def', and 'block' is not a SysML v2 keyword at all. Confirmed via the newly-wired ufo_types::sysml::validate_sysml_v2 (real sysml-v2-parser crate, not a hand-rolled heuristic) that the fixed output actually parses. Adds tests/real_grammar_validation.rs: runs the actual generated sysml_block_def() text for Transaction/Requirement/ExtractedRow/ ModelProposal/WorkbookRow (mirroring the real production structs from #184/#193) through the real parser. This replaces the 'no angle brackets' manual check used to validate the earlier DateTime/bool/usize scalar mapping fix with genuine grammar validation, closing out ledgrrr#195.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
clippy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
…yntax' into fix/sysml-block-scalar-type-mapping
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #195 (top-priority follow-up flagged in retrospective review of the
systems-modeling epic).
Investigated what SysML v2 text
#[derive(SysmlBlock)]actually generatesfor the field types introduced by #184 (
Requirement/Decision/Cost,which use
DateTime<Utc>) and #193 (retrofit onto 8 pre-existing structs,which additionally introduced
rust_decimal::Decimal, a customConfidencetype,bool, andusize).Found a real bug:
DateTime<Utc>fields emitted the literal textattribute x : DateTime<Utc>;. SysML v2's textual grammar has noangle-bracket generic-parameter syntax at all — this is not "semantically
questionable," it's syntactically invalid under any conformant SysML v2
parser.
bool/usizefields emitted bare Rust keywords, which aren'tSysML v2 type names either (SysML v2's standard library scalar types live
under
ScalarValues::*).Fix
DateTime<_>->ScalarValues::String(SysML v2 has no nativedate/time scalar; the generic parameter is dropped rather than rendered,
since rendering it is what caused the invalid syntax).
bool->ScalarValues::Boolean, unsigned integers ->ScalarValues::Natural,signed integers ->
ScalarValues::Integer, floats ->ScalarValues::Rational.Vec/Option/DateTime) isnow a compile error instead of silently emitting the same class of
invalid
Outer<Inner>text — guards against the next person adding a newgeneric field type and reintroducing this silently.
Stringand opaque domain types (NodeId,Confidence,rust_decimal::Decimal) intentionally still pass through as baretype-name references — documented in the module doc as an intentional
modeling assumption (they're assumed to resolve to a sibling declaration
elsewhere in the model), not a bug needing a fix.
New tests in
crates/sysml-derive/tests/retrofitted_field_types.rscoverall of the above, including an explicit assertion that the generated text
never contains
</>.Verification
cargo test -p sysml-derive: 6/6 pass (2 existing + 4 new).cargo check --workspace --all-features: clean.ExtractedRow,ModelProposal,WorkbookRow) via a throwaway test before writing thereal regression tests, confirming the bug and the fix against real
struct shapes, not just synthetic ones.
Stacked on
feat/sysml-derive-spike(#183) since that's wheresysml-deriveitself lives; will merge-forward through #184/#185/#186/#190/#193 the same way the two generated-artifact CI-drift fixes were
propagated earlier in this epic.