diff --git a/CHANGELOG.md b/CHANGELOG.md index b546e73..c084dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- OpenAPI 3.0 `nullable: true` is no longer discarded when a schema is + reached through `$ref`. `ScalarSiblingMerger::merge()` combined the `$ref` + stub's `nullable` flag with the resolved target's using a logical AND, and + because `Schema::$nullable` defaults to `false` a bare `{$ref: ...}` stub + always evaluated `false && true` — erasing the target's nullability, so a + legitimate `null` was rejected by `TypeValidator`. The flag now merges with + OR: per OpenAPI 3.0 a `nullable` sibling next to `$ref` can only widen the + target, and there is no spelling for "narrow this to non-nullable" (#64). + ## [0.7.0] Preparation for the 1.0.0 stable release. This section tracks work that diff --git a/src/Schema/Model/Internal/ScalarSiblingMerger.php b/src/Schema/Model/Internal/ScalarSiblingMerger.php index d8f8b79..c662de1 100644 --- a/src/Schema/Model/Internal/ScalarSiblingMerger.php +++ b/src/Schema/Model/Internal/ScalarSiblingMerger.php @@ -38,7 +38,7 @@ public function merge(SiblingMergeContext $context): array return [ 'format' => $this->mergeFormat($resolved->format, $sibling->format), 'type' => $this->mergeType($resolved->type, $sibling->type), - 'nullable' => $sibling->nullable && $resolved->nullable, + 'nullable' => $sibling->nullable || $resolved->nullable, 'const' => $sibling->hasConst ? $sibling->const : $resolved->const, 'hasConst' => $sibling->hasConst || $resolved->hasConst, 'multipleOf' => $this->mergeNullableIdentical($resolved->multipleOf, $sibling->multipleOf), diff --git a/tests/Integration/NullableRefSiblingTest.php b/tests/Integration/NullableRefSiblingTest.php new file mode 100644 index 0000000..cceba6b --- /dev/null +++ b/tests/Integration/NullableRefSiblingTest.php @@ -0,0 +1,126 @@ +validator()->validateSchema(['p' => null], '#/components/schemas/InlineProp'); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function bare_ref_to_nullable_schema_accepts_null(): void + { + $this->validator()->validateSchema(['p' => null], '#/components/schemas/RefProp'); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function ref_restating_nullable_accepts_null(): void + { + $this->validator()->validateSchema(['p' => null], '#/components/schemas/RefPropRestatingNullable'); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function ref_to_type_union_with_null_accepts_null(): void + { + $this->validator()->validateSchema(['p' => null], '#/components/schemas/RefPropTypeUnion'); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function nullable_sibling_widens_non_nullable_target(): void + { + $this->validator()->validateSchema(['p' => null], '#/components/schemas/RefPropNullableSibling'); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function bare_ref_to_non_nullable_schema_still_rejects_null(): void + { + $this->expectException(ValidationException::class); + + $this->validator()->validateSchema(['p' => null], '#/components/schemas/RefPropNonNullable'); + } + + #[Test] + public function bare_ref_to_nullable_schema_still_rejects_wrong_type(): void + { + $this->expectException(ValidationException::class); + + $this->validator()->validateSchema(['p' => 42], '#/components/schemas/RefProp'); + } + + private function validator(): OpenApiValidatorInterface + { + return OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC)->build(); + } +} diff --git a/tests/Integration/PostRefactorBehavioralSnapshotTest.php b/tests/Integration/PostRefactorBehavioralSnapshotTest.php index 35c0b1b..f61fd06 100644 --- a/tests/Integration/PostRefactorBehavioralSnapshotTest.php +++ b/tests/Integration/PostRefactorBehavioralSnapshotTest.php @@ -355,23 +355,25 @@ public function validator_compiler_combined_schema_enforces_all_supported_keywor } // --------------------------------------------------------------------- - // Zone 2: SchemaSiblingMerger — AND-merge semantics + // Zone 2: SchemaSiblingMerger — merge semantics // --------------------------------------------------------------------- #[Test] - public function sibling_merger_nullable_AND_semantics_sibling_false_blocks_null(): void + public function sibling_merger_nullable_widening_semantics_sibling_false_keeps_null(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: false); $merged = new SchemaSiblingMerger()->merge($resolved, $sibling); - // AND semantics: both must allow null for the merge to allow null. - self::assertFalse($merged->nullable); + // Widening semantics: a `nullable` sibling next to `$ref` can only add + // null to the target. `nullable: false` is indistinguishable from an + // omitted flag, so it never removes the target's nullability. + self::assertTrue($merged->nullable); } #[Test] - public function sibling_merger_nullable_AND_semantics_both_true_allows_null(): void + public function sibling_merger_nullable_widening_semantics_both_true_allows_null(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: true); diff --git a/tests/Unit/Schema/Model/Internal/ScalarSiblingMergerTest.php b/tests/Unit/Schema/Model/Internal/ScalarSiblingMergerTest.php index e6f7d5b..ffde949 100644 --- a/tests/Unit/Schema/Model/Internal/ScalarSiblingMergerTest.php +++ b/tests/Unit/Schema/Model/Internal/ScalarSiblingMergerTest.php @@ -34,18 +34,29 @@ public function merge_returns_overrides_for_eight_scalar_fields(): void } #[Test] - public function merge_nullable_uses_and_semantics_sibling_false_blocks_null(): void + public function merge_nullable_keeps_resolved_nullable_when_sibling_omits_it(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: false); $overrides = new ScalarSiblingMerger()->merge(new SiblingMergeContext($resolved, $sibling)); - self::assertFalse($overrides['nullable']); + self::assertTrue($overrides['nullable']); + } + + #[Test] + public function merge_nullable_sibling_widens_non_nullable_resolved(): void + { + $resolved = new Schema(type: 'string', nullable: false); + $sibling = new Schema(nullable: true); + + $overrides = new ScalarSiblingMerger()->merge(new SiblingMergeContext($resolved, $sibling)); + + self::assertTrue($overrides['nullable']); } #[Test] - public function merge_nullable_and_semantics_both_true_allows_null(): void + public function merge_nullable_both_true_allows_null(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: true); @@ -55,6 +66,17 @@ public function merge_nullable_and_semantics_both_true_allows_null(): void self::assertTrue($overrides['nullable']); } + #[Test] + public function merge_nullable_stays_false_when_neither_side_is_nullable(): void + { + $resolved = new Schema(type: 'string', nullable: false); + $sibling = new Schema(nullable: false); + + $overrides = new ScalarSiblingMerger()->merge(new SiblingMergeContext($resolved, $sibling)); + + self::assertFalse($overrides['nullable']); + } + #[Test] public function merge_format_returns_null_when_formats_differ(): void { diff --git a/tests/Unit/Schema/Model/SchemaSiblingMergerTest.php b/tests/Unit/Schema/Model/SchemaSiblingMergerTest.php index 98c8790..493ba3c 100644 --- a/tests/Unit/Schema/Model/SchemaSiblingMergerTest.php +++ b/tests/Unit/Schema/Model/SchemaSiblingMergerTest.php @@ -108,29 +108,29 @@ public function merge_upper_bound_inherits_resolved_when_sibling_null(): void } #[Test] - public function merge_nullable_and_when_resolved_rejects_null(): void + public function merge_nullable_widens_when_only_sibling_allows_null(): void { $resolved = new Schema(type: 'string', nullable: false); $sibling = new Schema(nullable: true); $merged = new SchemaSiblingMerger()->merge($resolved, $sibling); - self::assertFalse($merged->nullable); + self::assertTrue($merged->nullable); } #[Test] - public function merge_nullable_and_when_sibling_rejects_null(): void + public function merge_nullable_keeps_resolved_when_sibling_omits_nullable(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: false); $merged = new SchemaSiblingMerger()->merge($resolved, $sibling); - self::assertFalse($merged->nullable); + self::assertTrue($merged->nullable); } #[Test] - public function merge_nullable_and_when_both_allow_null(): void + public function merge_nullable_when_both_allow_null(): void { $resolved = new Schema(type: 'string', nullable: true); $sibling = new Schema(nullable: true);