diff --git a/crates/fhir-validator/packs/fhir_schemas_r4.json.gz b/crates/fhir-validator/packs/fhir_schemas_r4.json.gz index 3dcc46f71..d09ced9a1 100644 Binary files a/crates/fhir-validator/packs/fhir_schemas_r4.json.gz and b/crates/fhir-validator/packs/fhir_schemas_r4.json.gz differ diff --git a/crates/fhir-validator/packs/fhir_schemas_r4b.json.gz b/crates/fhir-validator/packs/fhir_schemas_r4b.json.gz index f29898cab..24203db50 100644 Binary files a/crates/fhir-validator/packs/fhir_schemas_r4b.json.gz and b/crates/fhir-validator/packs/fhir_schemas_r4b.json.gz differ diff --git a/crates/fhir-validator/packs/fhir_schemas_r5.json.gz b/crates/fhir-validator/packs/fhir_schemas_r5.json.gz index 272da8a31..92d7c2f07 100644 Binary files a/crates/fhir-validator/packs/fhir_schemas_r5.json.gz and b/crates/fhir-validator/packs/fhir_schemas_r5.json.gz differ diff --git a/crates/fhir-validator/packs/fhir_schemas_r6.json.gz b/crates/fhir-validator/packs/fhir_schemas_r6.json.gz index bbd3eeb05..ecad655f8 100644 Binary files a/crates/fhir-validator/packs/fhir_schemas_r6.json.gz and b/crates/fhir-validator/packs/fhir_schemas_r6.json.gz differ diff --git a/crates/fhir-validator/src/converter/mod.rs b/crates/fhir-validator/src/converter/mod.rs index ad5214981..ac28d9446 100644 --- a/crates/fhir-validator/src/converter/mod.rs +++ b/crates/fhir-validator/src/converter/mod.rs @@ -107,9 +107,51 @@ pub(crate) struct Ed { pub rest: serde_json::Map, } +impl Ed { + /// The value-domain type an element's own type resolution must yield when the + /// FHIR type model — not the (inconsistent) `structuredefinition-fhir-type` + /// extension — is authoritative. Returns `Some("string")` for any element + /// deriving from `Element.id`. + /// + /// # Why (issue #424) + /// + /// FHIR defines `Element.id` as type `string` ("any string value that does + /// not contain spaces"), whereas `Resource.id` is the constrained `id` token + /// (`[A-Za-z0-9\-\.]{1,64}`). The spec's own `type[].code` for both is the + /// FHIRPath `System.String`; the concrete FHIR type is carried only as a + /// `structuredefinition-fhir-type` extension hint — which HL7 populates + /// inconsistently across versions: R4B stamps `id` on *every* `.id` + /// (including `Element.id`), R5 stamps it on `ElementDefinition.id`, while + /// R4/R6 say `string`. Trusting that hint makes the `id` regex reject valid + /// element ids that contain `[x]` (choice elements), `:` (named slices), or + /// exceed 64 characters — i.e. it rejects StructureDefinitions the spec + /// itself publishes. + /// + /// HL7's own `fhir.schema.json` types every `Element.id` (and + /// `ElementDefinition.id`) as `string` in all four versions, confirming the + /// extension is the outlier. Keying on `base.path == "Element.id"` fixes the + /// whole class — `ElementDefinition.id`, `Element.id`, and every datatype + /// `.id` — without relaxing genuine resource-id validation + /// (`base.path == "Resource.id"` is left to honor the extension, correctly + /// yielding `id`) and without disturbing `Extension.url` (`uri`). It is a + /// no-op for the already-correct R4 and R6 packs. + pub(crate) fn value_type_override(&self) -> Option<&'static str> { + match self.base.as_ref().and_then(|b| b.path.as_deref()) { + Some("Element.id") => Some("string"), + _ => None, + } + } +} + #[derive(Debug, Deserialize)] pub(crate) struct EdBase { pub max: Option, + /// The path of the base element this ED derives from (e.g. `Element.id`, + /// `Resource.id`, `Extension.url`). Used to resolve the value-domain type of + /// FHIRPath `System.*`-coded elements structurally rather than trusting the + /// (inconsistent) `structuredefinition-fhir-type` extension — see + /// [`Ed::value_type_override`]. + pub path: Option, } #[derive(Debug, Deserialize)] diff --git a/crates/fhir-validator/src/converter/tree.rs b/crates/fhir-validator/src/converter/tree.rs index 747a91dd2..302b8b913 100644 --- a/crates/fhir-validator/src/converter/tree.rs +++ b/crates/fhir-validator/src/converter/tree.rs @@ -240,7 +240,13 @@ fn apply_element_content(element: &mut Node, ed: &Ed, warnings: &mut Vec } 1 => { let t = &ed.types[0]; - element.schema.type_ = Some(t.effective_code()); + // `Element.id`-derived elements are `string`, not the `id` the + // (inconsistent) fhir-type extension may claim (#424). + element.schema.type_ = Some( + ed.value_type_override() + .map(str::to_string) + .unwrap_or_else(|| t.effective_code()), + ); if !t.target_profile.is_empty() { element.schema.refers = Some(t.target_profile.clone()); } @@ -251,7 +257,11 @@ fn apply_element_content(element: &mut Node, ed: &Ed, warnings: &mut Vec "{}: multiple types without [x]; using the first", ed.path )); - element.schema.type_ = Some(ed.types[0].effective_code()); + element.schema.type_ = Some( + ed.value_type_override() + .map(str::to_string) + .unwrap_or_else(|| ed.types[0].effective_code()), + ); } } apply_value_keywords(&mut element.schema, ed, warnings); diff --git a/crates/fhir-validator/tests/converter_tests.rs b/crates/fhir-validator/tests/converter_tests.rs index 33fd00767..cf49f0b82 100644 --- a/crates/fhir-validator/tests/converter_tests.rs +++ b/crates/fhir-validator/tests/converter_tests.rs @@ -231,6 +231,121 @@ fn carries_informational_mirrors_and_short_labels() { assert_eq!(note["mustSupport"], Value::Null); } +// ── issue #424: Element.id-derived ids are `string`, not `id` ────────────── +// +// The FHIRPath `System.String` code carries a `structuredefinition-fhir-type` +// extension that HL7 populates inconsistently (R4B stamps `id` on every `.id`, +// R5 on `ElementDefinition.id`). Taken literally, the `id` regex +// `[A-Za-z0-9\-\.]{1,64}` rejects element ids with `[x]`, `:`, or > 64 chars — +// i.e. StructureDefinitions the spec itself publishes. The converter now keys +// the value-domain type on the base element: `Element.id` → `string`, while +// `Resource.id` and `Extension.url` keep honoring the extension. + +const FHIR_TYPE_EXT: &str = "http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type"; +const SYSTEM_STRING: &str = "http://hl7.org/fhirpath/System.String"; + +/// Converts a one-field complex-type SD and returns that field's schema `type`. +/// The field derives from `base_path` and its single `type[]` carries `code` +/// plus a `structuredefinition-fhir-type` extension of `ext_value`. +fn field_type(field: &str, base_path: &str, code: &str, ext_value: &str) -> Value { + let sd = json!({ + "resourceType": "StructureDefinition", + "url": "http://example.org/StructureDefinition/T", + "name": "T", "kind": "complex-type", "derivation": "specialization", "type": "T", + "snapshot": { "element": [ + { "path": "T", "min": 0, "max": "*" }, + { + "path": format!("T.{field}"), + "min": 0, "max": "1", + "base": { "path": base_path, "min": 0, "max": "1" }, + "type": [{ + "extension": [{ "url": FHIR_TYPE_EXT, "valueUrl": ext_value }], + "code": code + }] + } + ]} + }); + let conversion = convert(&sd).expect("conversion"); + serde_json::to_value(&conversion.schema).unwrap()["elements"][field]["type"].clone() +} + +#[test] +fn element_id_is_string_despite_id_type_extension() { + // The bug: R4B/R5 stamp `valueUrl: "id"` on Element.id. Must resolve to + // `string` so the restrictive `id` regex is never applied to element ids. + assert_eq!( + field_type("id", "Element.id", SYSTEM_STRING, "id"), + json!("string") + ); +} + +#[test] +fn resource_id_keeps_id_type() { + // Resource ids are the genuine constrained `id` token; the fix must NOT + // relax them. A `Resource.id`-based field keeps honoring the extension. + assert_eq!( + field_type("id", "Resource.id", SYSTEM_STRING, "id"), + json!("id") + ); +} + +#[test] +fn extension_url_keeps_uri_type() { + // Extension.url is `uri` in every version's fhir.schema.json; the fix must + // not demote it to `string`. + assert_eq!( + field_type("url", "Extension.url", SYSTEM_STRING, "uri"), + json!("uri") + ); +} + +#[test] +fn element_id_override_is_independent_of_the_extension_value() { + // Whatever the (inconsistent) extension claims, an Element.id-derived field + // is `string`: `id` (the R4B/R5 bug) and `string` (R4/R6) both land there. + assert_eq!( + field_type("id", "Element.id", SYSTEM_STRING, "string"), + json!("string") + ); + assert_eq!( + field_type("id", "Element.id", SYSTEM_STRING, "id"), + json!("string") + ); +} + +#[test] +fn multiple_types_without_choice_take_the_first_and_warn() { + // A non-`[x]` element with more than one type is malformed but tolerated: + // the converter warns and uses the first type. This also exercises the + // `value_type_override` fall-through on the multi-type arm — the field is + // not `Element.id`-derived, so the override yields None and the first + // type's effective code (`string`) is used. + let sd = json!({ + "resourceType": "StructureDefinition", + "url": "http://example.org/StructureDefinition/T", + "name": "T", "kind": "complex-type", "derivation": "specialization", "type": "T", + "snapshot": { "element": [ + { "path": "T", "min": 0, "max": "*" }, + { + "path": "T.weird", + "min": 0, "max": "1", + "type": [{ "code": "string" }, { "code": "integer" }] + } + ]} + }); + let conversion = convert(&sd).expect("conversion"); + let value = serde_json::to_value(&conversion.schema).unwrap(); + assert_eq!(value["elements"]["weird"]["type"], json!("string")); + assert!( + conversion + .warnings + .iter() + .any(|w| w.contains("multiple types without [x]")), + "expected a multiple-types warning, got: {:?}", + conversion.warnings + ); +} + /// `ordered: true` slicing carries each slice's declaration ordinal as /// `order` — without it the engine's ordered check has nothing to compare and /// silently passes (`engine/slicing.rs`). diff --git a/crates/fhir-validator/tests/pack_smoke.rs b/crates/fhir-validator/tests/pack_smoke.rs index aa6ce5e5c..e590c6214 100644 --- a/crates/fhir-validator/tests/pack_smoke.rs +++ b/crates/fhir-validator/tests/pack_smoke.rs @@ -248,3 +248,69 @@ fn r4_pack_validates_known_good_and_bad_resources() { serde_json::to_string_pretty(&outcome.errors).unwrap() ); } + +/// Regression for issue #424: a StructureDefinition whose `element.id`s carry a +/// choice suffix (`[x]`) or a slice qualifier (`:`) must NOT trip a +/// primitive-value error. Before the converter fix, R4B/R5 typed +/// `ElementDefinition.id` as the constrained `id` primitive, whose regex +/// `[A-Za-z0-9\-\.]{1,64}` rejects exactly these ids — so the spec's own +/// profiles failed validation. R4 was always clean (typed `string`), so it +/// serves as the control that the assertion is meaningful. +/// +/// Version-gated behind R4B/R5 (that is where the bug lived), so it runs in +/// CI's `cargo test --workspace --all-features` but not in the coverage job, +/// which builds default features (R4 only). +#[cfg(any(feature = "R4B", feature = "R5"))] +#[test] +fn structuredefinition_element_ids_with_choice_or_slice_are_not_rejected() { + // A trimmed but structurally-valid StructureDefinition whose element ids + // include a choice base (`Observation.value[x]`) and a named slice + // (`Observation.category:vital-signs`). These are the exact shapes the `id` + // regex rejected. + let sd = json!({ + "resourceType": "StructureDefinition", + "url": "http://example.org/StructureDefinition/choice-and-slice", + "name": "ChoiceAndSlice", + "status": "active", + "kind": "resource", + "abstract": false, + "type": "Observation", + "baseDefinition": "http://hl7.org/fhir/StructureDefinition/Observation", + "derivation": "constraint", + "differential": { "element": [ + { "id": "Observation.value[x]", "path": "Observation.value[x]", "min": 0, "max": "1" }, + { + "id": "Observation.category:vital-signs", + "path": "Observation.category", + "sliceName": "vital-signs", + "min": 0, "max": "1" + } + ]} + }); + + let versions = [ + #[cfg(feature = "R4B")] + FhirVersion::R4B, + #[cfg(feature = "R5")] + FhirVersion::R5, + ]; + for version in versions { + let validator = Validator::new(core_registry(version)); + let outcome = validator.validate_sync(&sd, &ValidationOptions::default()); + + // The precise #424 failure: a primitive-value error on an element id. + let id_value_errors: Vec<_> = outcome + .errors + .iter() + .filter(|e| { + e.kind == helios_fhir_validator::ErrorKind::PrimitiveValue + && e.path.ends_with(".id") + }) + .collect(); + assert!( + id_value_errors.is_empty(), + "{version:?}: element ids with [x]/: must not fail primitive-value validation, got: {}", + serde_json::to_string_pretty(&id_value_errors).unwrap() + ); + } +}