diff --git a/CHANGELOG.md b/CHANGELOG.md index 468c1e9d..d9a18c44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.9.1 +----- + +* Fix tools with an array shape parameter whose keys are nullable or unions (e.g. `array{name: string, nickname: string|null}`) rejecting valid arguments with `-32602`: a `null` inside the shape no longer turns the parameter into `array|null`, and such keys get their own types instead of `object`. + 0.9.0 ----- diff --git a/src/Capability/Discovery/SchemaGenerator.php b/src/Capability/Discovery/SchemaGenerator.php index 949ed013..e9f02b36 100644 --- a/src/Capability/Discovery/SchemaGenerator.php +++ b/src/Capability/Discovery/SchemaGenerator.php @@ -556,7 +556,7 @@ private function parseParametersInfo(\ReflectionMethod|\ReflectionFunction $refl $allowsNull = true; } elseif ($hasDefault && null === $defaultValue) { $allowsNull = true; - } elseif (str_contains($typeString, 'null') || 'mixed' === strtolower($typeString)) { + } elseif (self::hasNullBranch($typeString) || 'mixed' === strtolower($typeString)) { $allowsNull = true; } @@ -609,7 +609,7 @@ private function getParameterTypeString(\ReflectionParameter $rp, ?Param $paramT // Otherwise, use the DocBlock type if it was valid and non-generic if (null !== $docBlockType && !$isDocBlockTypeGeneric) { // Consider if DocBlock adds nullability missing from reflection - if (false !== stripos($docBlockType, 'null') && $reflectionTypeString && false === stripos($reflectionTypeString, 'null') && !str_ends_with($reflectionTypeString, '|null')) { + if (self::hasNullBranch($docBlockType) && $reflectionTypeString && false === stripos($reflectionTypeString, 'null') && !str_ends_with($reflectionTypeString, '|null')) { // If reflection didn't capture null, but docblock did, append |null (if not already mixed) if ('mixed' !== $reflectionTypeString) { return $reflectionTypeString.'|null'; @@ -733,6 +733,17 @@ public static function splitUnion(string $type): array return array_values(array_filter($branches, static fn (string $branch): bool => '' !== $branch)); } + /** + * Whether `null` is one of the top-level alternatives of a type, as in + * `string|null`. A `null` inside a generic or a shape, as in + * `array{name: string|null}`, belongs to that inner type and does not + * make the whole type nullable. + */ + private static function hasNullBranch(string $type): bool + { + return \in_array('null', array_map(strtolower(...), self::splitUnion($type)), true); + } + /** * Maps a PHP type string (potentially a union) to an array of JSON Schema type names. * @@ -936,9 +947,16 @@ private function parsePropertyDefinition(string $propDefinition, array &$propert ], ]; } - // Simple type + // Simple type, or a union of them such as `string|null` or `?string` else { - $properties[$propName] = ['type' => $this->mapSimpleTypeToJsonSchema($propType)]; + if (str_starts_with($propType, '?')) { + $propType = 'null|'.substr($propType, 1); + } + $types = array_values(array_unique(array_map( + fn (string $branch): string => 'null' === strtolower($branch) ? 'null' : $this->mapSimpleTypeToJsonSchema($branch), + self::splitUnion($propType), + ))); + $properties[$propName] = ['type' => 1 === \count($types) ? $types[0] : $types]; } } } diff --git a/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php b/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php index 613369b2..971d6870 100644 --- a/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php +++ b/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php @@ -319,6 +319,22 @@ public function nullableTypedArrays( ): void { } + /** + * A `null` or `|` inside an array shape belongs to one of its keys. + * + * @param array{name: string, nickname: string|null} $nullUnionMember Shape with a nullable key + * @param array{name: string, nickname: ?string} $nullableMember Shape with a nullable key + * @param array{id: int|string} $unionMember Shape with a union key + * @param array{id: int}|null $nullableShape Nullable shape + */ + public function arrayShapesWithUnionMembers( + array $nullUnionMember, + array $nullableMember, + array $unionMember, + $nullableShape, + ): void { + } + // ===== NULLABLE AND OPTIONAL SCENARIOS ===== /** diff --git a/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php b/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php index 32817f64..ffd1257d 100644 --- a/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php +++ b/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php @@ -293,6 +293,39 @@ public function testRecoversItemsTypeForNullableTypedArrays(): void $this->assertEqualsCanonicalizing(['nullableStrings'], $schema['required']); } + public function testKeepsNullableAndUnionKeysOfArrayShapesInsideTheShape(): void + { + $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'arrayShapesWithUnionMembers'); + $schema = $this->schemaGenerator->generate($method); + // The `null` of a key must neither make the parameter nullable nor turn it back into a plain array. + $this->assertEquals([ + 'type' => 'object', + 'description' => 'Shape with a nullable key', + 'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['string', 'null']]], + 'required' => ['name', 'nickname'], + ], $schema['properties']['nullUnionMember']); + $this->assertEquals([ + 'type' => 'object', + 'description' => 'Shape with a nullable key', + 'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['null', 'string']]], + 'required' => ['name', 'nickname'], + ], $schema['properties']['nullableMember']); + $this->assertEquals([ + 'type' => 'object', + 'description' => 'Shape with a union key', + 'properties' => ['id' => ['type' => ['integer', 'string']]], + 'required' => ['id'], + ], $schema['properties']['unionMember']); + // A top-level `|null` still makes the shape nullable. + $this->assertEquals([ + 'type' => ['object', 'null'], + 'description' => 'Nullable shape', + 'properties' => ['id' => ['type' => 'integer']], + 'required' => ['id'], + ], $schema['properties']['nullableShape']); + $this->assertEqualsCanonicalizing(['nullUnionMember', 'nullableMember', 'unionMember', 'nullableShape'], $schema['required']); + } + public function testHandlesNullableTypeHintsAndOptionalParameters(): void { $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'nullableAndOptional');