feat(macros): add compile-time GTS ID validation to struct_to_gts_schema (#47)#51
Conversation
Fixes GlobalTypeSystem#47 The macro now validates schema_id format at compile time, catching invalid GTS IDs early. Validation mirrors the logic from gts/src/gts.rs: - Must start with 'gts.' prefix - Must be lowercase, no hyphens - Must end with '~' (type marker) - Each segment must have 5-6 tokens (vendor.package.namespace.type.vMAJOR[.MINOR]) - Version token must start with 'v' followed by integer Also fixes existing tests that used invalid GTS IDs: - pretty_test.rs: gts.test.pretty.v1~ -> gts.x.test.entities.pretty.v1~ - inheritance_tests.rs: x.app.content.v1 -> x.app.entities.content.v1 - base_parent_mismatch.rs: gts.x.wrong.parent.v1~ -> gts.x.wrong.parent.type.v1~
- Use tokens.contains(&"*") instead of iter().any() (clippy::manual_contains) - Fix GtsError::InvalidSegment -> GtsError::Segment (correct variant name) - Update version_missing_*.stderr to expect GTS validation error (GTS ID validation now catches missing version as "Too few tokens" before the version mismatch check runs)
|
@Artifizer please review |
Move GTS ID validation logic into a new lightweight gts-id crate to eliminate duplication between gts (runtime) and gts-macros (compile-time). Both crates now delegate to gts_id::validate_gts_id() as the single source of truth. Also fix segment GlobalTypeSystem#1 error messages to show expected format with gts. prefix (gts.vendor.package.namespace.type.vMAJOR[.MINOR]) while segments GlobalTypeSystem#2+ show without prefix, per reviewer feedback. Signed-off-by: Dmitry Efremov <Dmitry.Efremov@acronis.com>
Signed-off-by: Dmitry Efremov <Dmitry.Efremov@acronis.com>
📝 WalkthroughWalkthroughA new workspace crate Changes
Sequence Diagram(s)sequenceDiagram
actor Dev
participant Compiler
participant Macro as gts-macros (compile-time)
participant GtsId as gts-id (validator)
Dev->>Compiler: compile crate with #[struct_to_gts_schema(..., schema_id=...)]
Compiler->>Macro: expand struct_to_gts_schema (parse attributes)
Macro->>GtsId: validate_gts_id(schema_id, allow_wildcards=false)
GtsId-->>Macro: Ok(parsed_segments) / Err(GtsIdError)
alt Ok
Macro-->>Compiler: embed validated schema_id / proceed
else Err
Macro-->>Compiler: emit compile_error with mapped diagnostics
end
sequenceDiagram
participant Runtime
participant GtsCrate as gts
participant GtsId as gts-id (validator)
Runtime->>GtsCrate: create/parse GTS ID string
GtsCrate->>GtsId: validate_gts_id(id, allow_wildcards=...)
GtsId-->>GtsCrate: parsed segments or error
alt Ok
GtsCrate->>Runtime: return GtsId instance constructed from parsed data
else Err
GtsCrate->>Runtime: return mapped GtsError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Comment |
Signed-off-by: Dmitry Efremov <Dmitry.Efremov@acronis.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gts-id/src/lib.rs`:
- Around line 564-614: The tests currently use a wildcard `_` arm when asserting
error variants; change those to explicit enum-variant matches to satisfy clippy:
in tests that expect GtsIdError::Id (e.g., test_gts_id_start,
test_gts_id_uppercase, test_gts_id_hyphen) replace `_ => panic!(...)` with an
explicit GtsIdError::Segment { .. } => panic!(...) (or any other concrete non-Id
variant if different), and in test_gts_id_segment_error_carries_num_and_offset
replace `_ => panic!(...)` with an explicit GtsIdError::Id { .. } =>
panic!(...); keep the panic message unchanged and use the same pattern binding
style (`{ .. }`) so the arms remain exhaustive and clear.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gts-id/src/lib.rs`:
- Around line 172-192: The current logic skips validating the first four tokens
when ends_with_wildcard is true, letting invalid tokens like "gts.1bad.*" pass;
update the validation in the function handling segment parsing to always
validate tokens[0..4] with is_valid_segment_token, and separately allow a single
'*' only if it is the final token by explicitly rejecting '*' in any of the
first four positions (e.g., check tokens.iter().take(4) and return an error if
token == "*" or !is_valid_segment_token(token)); keep the existing error
messages (using token_name mapping) and ensure the code still permits a trailing
'*' but not wildcards in middle positions.
Tokens preceding a trailing '*' wildcard are now validated against naming rules. Previously 'gts.1bad.*' would bypass token validation entirely. Also reject '*' in non-final positions with a clear error. Signed-off-by: Dmitry Efremov <Dmitry.Efremov@acronis.com>
|
Reviewed by Claude — no actionable issues found. Both Artifizer's review comments addressed:
CodeRabbit's wildcard validation bypass — fixed (tokens before All CI checks green, 797 tests pass. |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #54 |
Summary
struct_to_gts_schemamacro, catching invalid IDs before runtimegts.prefix, token count (5-6), version format (vMAJOR[.MINOR]), and token naming rulesx.core.license_enforcer.integration.plugin.v1~has 5 name tokens instead of 4)gts/src/gts.rsfor consistencyCloses #47
Test plan
invalid_gts_id_too_many_tokens.rswith exact Issue struct_to_gts_schema to ensure if GTS string is valid #47 caseinvalid_gts_id_missing_prefix.rsfor prefix validation-D warningsSummary by CodeRabbit
New Features
Bug Fixes
Tests