From a4221be4513dd01fecfeeaa59b037b30ad11774f Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Thu, 12 Nov 2026 00:23:50 +0100 Subject: [PATCH 1/2] [Server] Build tool schemas with symfony/json-schema SchemaGenerator describes the parameters of a handler with the component's generateForParameters() instead of parsing type strings itself. It keeps the MCP-specific parts: injectable parameters, reserved parameter names and the #[Schema] attribute. --- .github/workflows/pipeline.yaml | 6 - CHANGELOG.md | 12 + composer.json | 6 +- phpstan.dist.neon | 6 - .../Capability/Discovery/Discoverer.php | 2 +- .../Capability/Discovery/DocBlockParser.php | 23 - .../Capability/Discovery/SchemaGenerator.php | 1077 ++--------------- .../Loader/ReflectedElementLoader.php | 2 +- .../HttpComplexToolSchemaTest-tools_list.json | 24 +- ...tpDiscoveryUserProfileTest-tools_list.json | 7 +- .../HttpSchemaShowcaseTest-tools_list.json | 2 +- .../Discovery/DocBlockParserTest.php | 19 - .../Discovery/SchemaGeneratorFixture.php | 52 +- .../SchemaGeneratorNullableMemberTest.php | 3 +- .../SchemaGeneratorObjectFixture.php | 106 ++ .../Discovery/SchemaGeneratorTest.php | 324 +++-- .../Discovery/SchemaGeneratorUnionTest.php | 98 +- 17 files changed, 405 insertions(+), 1364 deletions(-) create mode 100644 tests/Unit/Server/Capability/Discovery/SchemaGeneratorObjectFixture.php diff --git a/.github/workflows/pipeline.yaml b/.github/workflows/pipeline.yaml index 184107cd2..2ea086fbc 100644 --- a/.github/workflows/pipeline.yaml +++ b/.github/workflows/pipeline.yaml @@ -20,12 +20,6 @@ jobs: dependencies: ['lowest', 'highest'] symfony-version: [''] include: - - symfony-version: '5.4.*' - dependencies: 'highest' - php: '8.2' - - symfony-version: '6.4.*' - dependencies: 'highest' - php: '8.2' - symfony-version: '7.4.*' dependencies: 'highest' php: '8.4' diff --git a/CHANGELOG.md b/CHANGELOG.md index eb0b7062f..2a7d0edb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.11.0 +------ + +### Server + +* Discovery + * [BC Break] Generate tool input and output schemas with `symfony/json-schema`. Unions are described with `anyOf`, `null` comes last in `type`, `default: null` is no longer emitted, array shapes are closed with `additionalProperties: false`, string-keyed maps get `additionalProperties`, and a class parameter is described by its properties. + * [BC Break] `SchemaGenerator` takes a `Symfony\Component\JsonSchema\SchemaGeneratorInterface`, a `TypeResolverInterface` and a `Configuration` instead of a `DocBlockParser`. + * [BC Break] Describe `array` and `array` as a JSON array, so a JSON object is rejected. Use `array` for a map. + * [BC Break] Remove `DocBlockParser::getParamTypeString()` and `DocBlockParser::getParamDescription()`. + * Add the `outputSchemaFromReturnType` argument of `SchemaGenerator`, to describe the output of a tool that returns an object by its class. + 0.10.0 ------ diff --git a/composer.json b/composer.json index 50abc999c..a7a717bfb 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ "opis/json-schema": "^2.4", "php-http/discovery": "^1.20", "phpdocumentor/reflection-docblock": "^6.0", + "phpstan/phpdoc-parser": "^2.0", "psr/clock": "^1.0", "psr/container": "^1.1 || ^2.0", "psr/event-dispatcher": "^1.0", @@ -32,6 +33,8 @@ "psr/http-server-middleware": "^1.0", "psr/log": "^1.0 || ^2.0 || ^3.0", "symfony/deprecation-contracts": "^2.5 || ^3.0", + "symfony/json-schema": "^8.2@beta", + "symfony/type-info": "^7.4.6 || ^8.0.6", "symfony/uid": "^5.4 || ^6.4 || ^7.3 || ^8.0" }, "require-dev": { @@ -60,7 +63,8 @@ "symfony/finder": "^5.4 || ^6.4 || ^7.3 || ^8.0", "symfony/http-client": "^5.4 || ^6.4 || ^7.3 || ^8.0", "symfony/lock": "^5.4 || ^6.4 || ^7.3 || ^8.0", - "symfony/process": "^5.4 || ^6.4 || ^7.3 || ^8.0" + "symfony/process": "^5.4 || ^6.4 || ^7.3 || ^8.0", + "symfony/validator": "^7.4 || ^8.0" }, "conflict": { "phpstan/phpstan": "2.3.0" diff --git a/phpstan.dist.neon b/phpstan.dist.neon index 0524c6120..c69447c30 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -16,9 +16,3 @@ parameters: - identifier: missingType.iterableValue path: tests/ - - # These errors should actually be fixed, but are ignored for now - - - identifier: missingType.iterableValue - path: src/Server/Capability/Discovery/SchemaGenerator.php - count: 12 diff --git a/src/Server/Capability/Discovery/Discoverer.php b/src/Server/Capability/Discovery/Discoverer.php index 5e9955ced..053a5e2f1 100644 --- a/src/Server/Capability/Discovery/Discoverer.php +++ b/src/Server/Capability/Discovery/Discoverer.php @@ -62,7 +62,7 @@ public function __construct( } $this->docBlockParser = $docBlockParser ?? new DocBlockParser(logger: $this->logger); - $this->schemaGenerator = $schemaGenerator ?? new SchemaGenerator($this->docBlockParser); + $this->schemaGenerator = $schemaGenerator ?? new SchemaGenerator(); } /** diff --git a/src/Server/Capability/Discovery/DocBlockParser.php b/src/Server/Capability/Discovery/DocBlockParser.php index 2a9c643a1..d16f6ceba 100644 --- a/src/Server/Capability/Discovery/DocBlockParser.php +++ b/src/Server/Capability/Discovery/DocBlockParser.php @@ -99,27 +99,4 @@ public function getParamTags(?DocBlock $docBlock): array return $paramTags; } - - /** - * Gets the description string from a Param tag. - */ - public function getParamDescription(?Param $paramTag): ?string - { - return $paramTag ? (trim((string) $paramTag->getDescription()) ?: null) : null; - } - - /** - * Gets the type string from a Param tag. - */ - public function getParamTypeString(?Param $paramTag): ?string - { - if ($paramTag && $paramTag->getType()) { - $typeFromTag = trim((string) $paramTag->getType()); - if (!empty($typeFromTag)) { - return ltrim($typeFromTag, '\\'); - } - } - - return null; - } } diff --git a/src/Server/Capability/Discovery/SchemaGenerator.php b/src/Server/Capability/Discovery/SchemaGenerator.php index 4b26e7a1d..bc493ddb3 100644 --- a/src/Server/Capability/Discovery/SchemaGenerator.php +++ b/src/Server/Capability/Discovery/SchemaGenerator.php @@ -16,1069 +16,164 @@ use Mcp\Server\Capability\Registry\InjectableParameters; use Mcp\Server\Exception\BadMethodCallException; use Mcp\Server\Exception\InvalidArgumentException; -use phpDocumentor\Reflection\DocBlock\Tags\Param; +use Symfony\Component\JsonSchema\Configuration; +use Symfony\Component\JsonSchema\Schema as JsonSchema; +use Symfony\Component\JsonSchema\SchemaGenerator as JsonSchemaGenerator; +use Symfony\Component\JsonSchema\SchemaGeneratorInterface as JsonSchemaGeneratorInterface; +use Symfony\Component\TypeInfo\Type\ObjectType; +use Symfony\Component\TypeInfo\TypeResolver\TypeResolver; +use Symfony\Component\TypeInfo\TypeResolver\TypeResolverInterface; /** - * Generates JSON Schema for method parameters with intelligent Schema attribute handling. + * Generates tool schemas with symfony/json-schema. * - * Priority system: - * 1. Schema attributes (method-level and parameter-level) - * 2. Reflection type information - * 3. DocBlock type information - * - * @phpstan-import-type SchemaAttributeData from Schema - * - * @phpstan-type ParameterInfo array{ - * name: string, - * doc_block_tag: Param|null, - * reflection_param: \ReflectionParameter, - * reflection_type_object: \ReflectionType|null, - * type_string: string, - * description: string|null, - * required: bool, - * allows_null: bool, - * default_value: mixed|null, - * has_default: bool, - * is_variadic: bool, - * parameter_schema: array - * } - * @phpstan-type InferredParameterSchema array{ - * type?: string|array, - * description?: string, - * default?: mixed, - * enum?: array, - * items?: array, - * } - * - * @author Kyrian Obikwelu + * The component describes the parameters, this class adds what is MCP specific: injectable + * parameters, reserved names and the {@see Schema} attribute. */ final class SchemaGenerator implements SchemaGeneratorInterface { + private readonly JsonSchemaGeneratorInterface $generator; + private readonly TypeResolverInterface $typeResolver; + public function __construct( - private readonly DocBlockParser $docBlockParser, + ?JsonSchemaGeneratorInterface $generator = null, + ?TypeResolverInterface $typeResolver = null, + private readonly Configuration $configuration = new Configuration(), + private readonly bool $outputSchemaFromReturnType = false, ) { + $this->generator = $generator ?? JsonSchemaGenerator::create(); + $this->typeResolver = $typeResolver ?? TypeResolver::create(); } /** - * Generates a JSON Schema object (as a PHP array) for parameters. - * * @return array */ public function generate(\Reflector $reflection): array { - if ($reflection instanceof \ReflectionClass) { - throw new BadMethodCallException('Schema generation from ReflectionClass is not implemented yet. Use ReflectionMethod or ReflectionFunction instead.'); - } + $reflection = self::assertFunction($reflection); - if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) { - throw new BadMethodCallException(\sprintf('Schema generation from %s is not supported. Use ReflectionMethod or ReflectionFunction instead.', $reflection::class)); - } - - $methodSchema = $this->extractMethodLevelSchema($reflection); - - if ($methodSchema && isset($methodSchema['definition'])) { + $methodSchema = self::schemaAttribute($reflection); + if (isset($methodSchema['definition'])) { return $methodSchema['definition']; } - $parametersInfo = $this->parseParametersInfo($reflection); - - return $this->buildSchemaFromParameters($parametersInfo, $methodSchema); - } - - /** - * Generates a JSON Schema object (as a PHP array) for a method's or function's return type. - * - * Only returns an outputSchema if explicitly provided in the McpTool attribute. - * Per MCP spec, outputSchema should only be present when explicitly provided. - * - * @return ?array - */ - public function generateOutputSchema(\Reflector $reflection): ?array - { - if ($reflection instanceof \ReflectionClass) { - throw new BadMethodCallException('Schema generation from ReflectionClass is not implemented yet. Use ReflectionMethod or ReflectionFunction instead.'); - } - - if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) { - throw new BadMethodCallException(\sprintf('Schema generation from %s is not supported. Use ReflectionMethod or ReflectionFunction instead.', $reflection::class)); - } - - // Only return outputSchema if explicitly provided in McpTool attribute - $mcpToolAttrs = $reflection->getAttributes(McpTool::class, \ReflectionAttribute::IS_INSTANCEOF); - if ($mcpToolAttrs) { - $mcpToolInstance = $mcpToolAttrs[0]->newInstance(); - - return $mcpToolInstance->outputSchema; - } - - return null; - } - - /** - * Extracts method-level or function-level Schema attribute. - * - * @return SchemaAttributeData - */ - private function extractMethodLevelSchema(\ReflectionFunctionAbstract $reflection): ?array - { - $schemaAttrs = $reflection->getAttributes(Schema::class, \ReflectionAttribute::IS_INSTANCEOF); - if (empty($schemaAttrs)) { - return null; - } - - /** @var Schema $schemaAttr */ - $schemaAttr = $schemaAttrs[0]->newInstance(); - - return $schemaAttr->toArray(); - } - - /** - * Extracts parameter-level Schema attribute. - * - * @return SchemaAttributeData - */ - private function extractParameterLevelSchema(\ReflectionParameter $parameter): array - { - $schemaAttrs = $parameter->getAttributes(Schema::class, \ReflectionAttribute::IS_INSTANCEOF); - if (empty($schemaAttrs)) { - return []; - } - - /** @var Schema $schemaAttr */ - $schemaAttr = $schemaAttrs[0]->newInstance(); - - return $schemaAttr->toArray(); - } - - /** - * Builds the final schema from parameter information and method-level schema. - * - * @param ParameterInfo[] $parametersInfo - * @param SchemaAttributeData $methodSchema - * - * @return array - */ - private function buildSchemaFromParameters(array $parametersInfo, ?array $methodSchema): array - { - $schema = [ - 'type' => 'object', - 'properties' => [], - 'required' => [], - ]; - - // Apply method-level schema as base - if ($methodSchema) { - $schema = array_merge($schema, $methodSchema); - if (!isset($schema['type'])) { - $schema['type'] = 'object'; - } - if (!isset($schema['properties'])) { - $schema['properties'] = []; - } - if (!isset($schema['required'])) { - $schema['required'] = []; + $parameters = []; + foreach ($reflection->getParameters() as $parameter) { + $type = $parameter->getType(); + if ($type instanceof \ReflectionNamedType && !$type->isBuiltin() && InjectableParameters::supports($type->getName())) { + continue; } - } - - foreach ($parametersInfo as $paramInfo) { - $paramName = $paramInfo['name']; - - $methodLevelParamSchema = $schema['properties'][$paramName] ?? null; - - $paramSchema = $this->buildParameterSchema($paramInfo, $methodLevelParamSchema); - $schema['properties'][$paramName] = $paramSchema; - - if ($paramInfo['required'] && !\in_array($paramName, $schema['required'])) { - $schema['required'][] = $paramName; - } elseif (!$paramInfo['required'] && ($key = array_search($paramName, $schema['required'])) !== false) { - unset($schema['required'][$key]); - $schema['required'] = array_values($schema['required']); // Re-index + if (\in_array(strtolower($parameter->name), ['_session', '_request'], true)) { + throw new InvalidArgumentException(\sprintf('Handler "%s" has parameter named "%s" which is not allowed. Please change the name of that parameter.', self::handlerName($reflection), $parameter->name)); } - } - // Clean up empty properties - if (empty($schema['properties'])) { - $schema['properties'] = new \stdClass(); - } - if (empty($schema['required'])) { - unset($schema['required']); + $parameters[$parameter->name] = $parameter; } - return $schema; - } + $schema = self::toArray($this->generator->generateForParameters(array_values($parameters), $this->configuration)->flatten()); + unset($schema['$schema']); - /** - * Builds the final schema for a single parameter by merging all three levels. - * - * @param ParameterInfo $paramInfo - * @param array|null $methodLevelParamSchema - */ - private function buildParameterSchema(array $paramInfo, ?array $methodLevelParamSchema): array - { - if ($paramInfo['is_variadic']) { - return $this->ensureArrayItems($this->buildVariadicParameterSchema($paramInfo)); - } + $methodProperties = $methodSchema['properties'] ?? []; + $methodRequired = $methodSchema['required'] ?? []; + unset($methodSchema['properties'], $methodSchema['required']); + $schema = [...$schema, ...$methodSchema ?? []]; - $inferredSchema = $this->buildInferredParameterSchema($paramInfo); - - // Method-level takes precedence over inferred schema - $mergedSchema = $inferredSchema; - if ($methodLevelParamSchema) { - $mergedSchema = array_merge($inferredSchema, $methodLevelParamSchema); + $schema['properties'] = [...$schema['properties'] ?? [], ...array_diff_key($methodProperties, $parameters)]; + if ($required = array_values(array_unique([...$schema['required'] ?? [], ...array_diff($methodRequired, array_keys($parameters))]))) { + $schema['required'] = $required; } - // Parameter-level takes highest precedence - $parameterLevelSchema = $paramInfo['parameter_schema']; - if (!empty($parameterLevelSchema)) { - // A complete definition replaces the schema, as it does at method level. - if (isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition'])) { - $mergedSchema = $parameterLevelSchema['definition']; + foreach ($parameters as $name => $parameter) { + $property = (array) $schema['properties'][$name]; + if (isset($methodProperties[$name])) { + $property = [...$property, ...$methodProperties[$name]]; + } - // The default comes from the signature, not from the schema. - if (!\array_key_exists('default', $mergedSchema) && \array_key_exists('default', $inferredSchema)) { - $mergedSchema['default'] = $inferredSchema['default']; - } + $parameterSchema = self::schemaAttribute($parameter) ?? []; + if (isset($parameterSchema['definition'])) { + $property = $parameterSchema['definition'] + array_intersect_key($property, ['default' => true]); + } elseif (isset($parameterSchema['type'])) { + $property = [...array_intersect_key($property, ['description' => true, 'default' => true]), ...$parameterSchema]; } else { - $mergedSchema = array_merge($mergedSchema, $parameterLevelSchema); + $property = [...$property, ...$parameterSchema]; } - } - - // Run after all merges so that when a Schema attribute reshapes the parameter - // (e.g. to `object`), the array `items` invariant is only enforced on what is - // genuinely still an array. - return $this->ensureObjectDefault($this->ensureArrayItems($mergedSchema)); - } - /** - * An empty PHP array encodes as `[]`, which a client could not send back to an object-only parameter. - * - * @param array $schema - * - * @return array - */ - private function ensureObjectDefault(array $schema): array - { - $types = (array) ($schema['type'] ?? []); - - if ([] === ($schema['default'] ?? null) && \in_array('object', $types, true) && !\in_array('array', $types, true)) { - $schema['default'] = new \stdClass(); + $schema['properties'][$name] = $property ?: new \stdClass(); } - return $schema; - } - - /** - * Guarantees an array-typed schema always declares `items`. - * - * `items` is optional in JSON Schema, but some strict clients reject an array schema - * without it. When no element type could be inferred, default to the empty schema `{}` - * (matches anything) — represented as `new \stdClass()` so it serializes to `{}` rather - * than `[]`. - * - * @param array $schema - * - * @return array - */ - private function ensureArrayItems(array $schema): array - { - $type = $schema['type'] ?? null; - $isArray = 'array' === $type || (\is_array($type) && \in_array('array', $type, true)); - - if ($isArray && !isset($schema['items'])) { - $schema['items'] = new \stdClass(); + if ([] === $schema['properties']) { + $schema['properties'] = new \stdClass(); } return $schema; } - /** - * Builds parameter schema from inferred type and docblock information only. - * Returns empty array for variadic parameters (handled separately). - * - * @param ParameterInfo $paramInfo - * - * @return InferredParameterSchema - */ - private function buildInferredParameterSchema(array $paramInfo): array + public function generateOutputSchema(\Reflector $reflection): ?array { - $paramSchema = []; - - // Variadic parameters are handled separately - if ($paramInfo['is_variadic']) { - return []; - } + $reflection = self::assertFunction($reflection); - // Infer JSON Schema types - $jsonTypes = $this->inferParameterTypes($paramInfo); - - if (1 === \count($jsonTypes)) { - $paramSchema['type'] = $jsonTypes[0]; - } elseif (\count($jsonTypes) > 1) { - $paramSchema['type'] = $jsonTypes; + foreach ($reflection->getAttributes(McpTool::class, \ReflectionAttribute::IS_INSTANCEOF) as $attribute) { + if (null !== $outputSchema = $attribute->newInstance()->outputSchema) { + return $outputSchema; + } } - // Add description from docblock - if ($paramInfo['description']) { - $paramSchema['description'] = $paramInfo['description']; + if (!$this->outputSchemaFromReturnType) { + return null; } - // Add default value only if parameter actually has a default - if ($paramInfo['has_default']) { - $paramSchema['default'] = $paramInfo['default_value']; + // Only classes: `array` says nothing about the shape and scalars are not objects before 2026-07-28. + $type = $this->typeResolver->resolve($reflection); + if (!$type instanceof ObjectType) { + return null; } - // Handle enums - $paramSchema = $this->applyEnumConstraints($paramSchema, $paramInfo); + $schema = self::toArray($this->generator->generate($type, $this->configuration)->flatten()); + unset($schema['$schema']); - // Handle array items - return $this->applyArrayConstraints($paramSchema, $paramInfo); + return $schema; } /** - * Builds schema for variadic parameters. - * - * @param ParameterInfo $paramInfo + * Empty schemas come back as ArrayObject, the SDK keeps them as stdClass. * * @return array */ - private function buildVariadicParameterSchema(array $paramInfo): array - { - $paramSchema = ['type' => 'array']; - - // Add description from docblock before parameter-level schema is merged in, - // so a definition's own description takes precedence (matches buildParameterSchema). - if ($paramInfo['description']) { - $paramSchema['description'] = $paramInfo['description']; - } - - // Apply parameter-level Schema attributes first - if (!empty($paramInfo['parameter_schema'])) { - $parameterLevelSchema = $paramInfo['parameter_schema']; - - $paramSchema = isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition']) - ? $parameterLevelSchema['definition'] - : array_merge($paramSchema, $parameterLevelSchema); - - // Ensure type is always array for variadic - $paramSchema['type'] = 'array'; - } - - // If no items specified by Schema attribute, infer from type - if (!isset($paramSchema['items'])) { - $itemJsonTypes = $this->mapPhpTypeToJsonSchemaType($paramInfo['type_string']); - $nonNullItemTypes = array_filter($itemJsonTypes, static fn ($t) => 'null' !== $t); - - if (1 === \count($nonNullItemTypes)) { - $paramSchema['items'] = ['type' => $nonNullItemTypes[0]]; - } - } - - return $paramSchema; - } - - /** - * Infers JSON Schema types for a parameter. - * - * @param ParameterInfo $paramInfo - */ - private function inferParameterTypes(array $paramInfo): array - { - $jsonTypes = $this->mapPhpTypeToJsonSchemaType($paramInfo['type_string']); - - if ($paramInfo['allows_null'] && 'mixed' !== strtolower($paramInfo['type_string']) && !\in_array('null', $jsonTypes)) { - $jsonTypes[] = 'null'; - } - - if (\count($jsonTypes) > 1) { - // Sort but ensure null comes first for consistency - $nullIndex = array_search('null', $jsonTypes); - if (false !== $nullIndex) { - unset($jsonTypes[$nullIndex]); - sort($jsonTypes); - array_unshift($jsonTypes, 'null'); - } else { - sort($jsonTypes); - } - } - - return $jsonTypes; - } - - /** - * Applies enum constraints to parameter schema. - */ - private function applyEnumConstraints(array $paramSchema, array $paramInfo): array + private static function toArray(JsonSchema $schema): array { - $reflectionType = $paramInfo['reflection_type_object']; - - if (!($reflectionType instanceof \ReflectionNamedType) || $reflectionType->isBuiltin() || !enum_exists($reflectionType->getName())) { - return $paramSchema; - } - - $enumClass = $reflectionType->getName(); - $enumReflection = new \ReflectionEnum($enumClass); - $backingTypeReflection = $enumReflection->getBackingType(); - - if ($enumReflection->isBacked() && $backingTypeReflection instanceof \ReflectionNamedType) { - $paramSchema['enum'] = array_column($enumClass::cases(), 'value'); - $jsonBackingType = match ($backingTypeReflection->getName()) { - 'int' => 'integer', - 'string' => 'string', - default => null, + $convert = static function (mixed $value) use (&$convert): mixed { + return match (true) { + $value instanceof \ArrayObject => new \stdClass(), + \is_array($value) => array_map($convert, $value), + default => $value, }; - - if ($jsonBackingType) { - if (isset($paramSchema['type']) && \is_array($paramSchema['type']) && \in_array('null', $paramSchema['type'])) { - $paramSchema['type'] = [$jsonBackingType, 'null']; - $paramSchema['enum'][] = null; - } else { - $paramSchema['type'] = $jsonBackingType; - } - } - } else { - // Non-backed enum - use names as enum values - $paramSchema['enum'] = array_column($enumClass::cases(), 'name'); - if (isset($paramSchema['type']) && \is_array($paramSchema['type']) && \in_array('null', $paramSchema['type'])) { - $paramSchema['type'] = ['string', 'null']; - $paramSchema['enum'][] = null; - } else { - $paramSchema['type'] = 'string'; - } - } - - return $paramSchema; - } - - /** - * Applies array-specific constraints to parameter schema. - */ - private function applyArrayConstraints(array $paramSchema, array $paramInfo): array - { - if (!isset($paramSchema['type'])) { - return $paramSchema; - } - - $typeString = $paramInfo['type_string']; - $allowsNull = $paramInfo['allows_null']; - - // Handle object-like arrays using array{} syntax - if (preg_match('/^array\s*{/i', $typeString)) { - $objectSchema = $this->inferArrayItemsType($typeString); - if (\is_array($objectSchema) && isset($objectSchema['properties'])) { - $paramSchema = array_merge($paramSchema, $objectSchema); - $paramSchema['type'] = $allowsNull ? ['object', 'null'] : 'object'; - } - } - // Handle regular arrays - elseif (\in_array('array', $this->mapPhpTypeToJsonSchemaType($typeString))) { - // `inferArrayItemsType()` expects a single array type, not a union — for - // `string[]|int` it would see the whole string, match nothing, and lose - // the element-type constraint. Isolate the branch(es) that are arrays. - $arrayBranches = array_values(array_filter( - self::splitUnion($typeString), - fn (string $branch): bool => \in_array('array', $this->mapPhpTypeToJsonSchemaType($branch), true), - )); - if ([] === $arrayBranches) { - $arrayBranches = [$typeString]; - } - - // Several array branches (`string[]|int[]`) can disagree on their - // element type. Applying just one would reject values valid under - // the other, so `items` only applies when every array branch agrees. - $itemsTypes = array_map($this->inferArrayItemsType(...), $arrayBranches); - $itemsType = 1 === \count(array_unique(array_map(json_encode(...), $itemsTypes))) ? $itemsTypes[0] : 'any'; - - if ('any' !== $itemsType) { - if (\is_string($itemsType)) { - $paramSchema['items'] = ['type' => $itemsType]; - } else { - if (!isset($itemsType['type']) && isset($itemsType['properties'])) { - $itemsType = array_merge(['type' => 'object'], $itemsType); - } - $paramSchema['items'] = $itemsType; - } - } - - // Only when the parameter is an array and nothing else. A union - // like `string[]|int` keeps both branches: `items` constrains the - // instance only when it *is* an array, so the two coexist and - // overwriting the type here would drop the scalar branch — - // rejecting a value the handler accepts. - $otherTypes = array_values(array_diff( - (array) $paramSchema['type'], - ['array', 'null'], - )); - - if ([] === $otherTypes) { - $paramSchema['type'] = $allowsNull ? ['array', 'null'] : 'array'; - - if (\is_array($paramSchema['type'])) { - sort($paramSchema['type']); - } - } elseif ($allowsNull && !\in_array('null', (array) $paramSchema['type'], true)) { - $paramSchema['type'] = [...(array) $paramSchema['type'], 'null']; - sort($paramSchema['type']); - } - } - - return $paramSchema; - } - - /** - * Parses detailed information about a method's parameters. - * - * @return ParameterInfo[] - */ - private function parseParametersInfo(\ReflectionMethod|\ReflectionFunction $reflection): array - { - $docBlock = $this->docBlockParser->parseDocBlock($reflection->getDocComment()); - $paramTags = $this->docBlockParser->getParamTags($docBlock); - $parametersInfo = []; - - foreach ($reflection->getParameters() as $rp) { - $reflectionType = $rp->getType(); - - if ($reflectionType instanceof \ReflectionNamedType && !$reflectionType->isBuiltin() - && InjectableParameters::supports($reflectionType->getName()) - ) { - continue; - } - - $paramName = $rp->getName(); - if (\in_array(strtolower($paramName), ['_session', '_request'], true)) { - $handlerName = $reflection instanceof \ReflectionMethod ? $reflection->class.'::'.$reflection->name : $reflection->name; - throw new InvalidArgumentException(\sprintf('Handler "%s" has parameter named "%s" which is not allowed. Please change the name of that parameter.', $handlerName, $paramName)); - } - $paramTag = $paramTags['$'.$paramName] ?? null; - - $typeString = $this->getParameterTypeString($rp, $paramTag); - $description = $this->docBlockParser->getParamDescription($paramTag); - $hasDefault = $rp->isDefaultValueAvailable(); - $defaultValue = $hasDefault ? $rp->getDefaultValue() : null; - $isVariadic = $rp->isVariadic(); - - $parameterSchema = $this->extractParameterLevelSchema($rp); - - if ($defaultValue instanceof \BackedEnum) { - $defaultValue = $defaultValue->value; - } - - if ($defaultValue instanceof \UnitEnum) { - $defaultValue = $defaultValue->name; - } - - $allowsNull = false; - if ($reflectionType && $reflectionType->allowsNull()) { - $allowsNull = true; - } elseif ($hasDefault && null === $defaultValue) { - $allowsNull = true; - } elseif (self::hasNullBranch($typeString) || 'mixed' === strtolower($typeString)) { - $allowsNull = true; - } - - $parametersInfo[] = [ - 'name' => $paramName, - 'doc_block_tag' => $paramTag, - 'reflection_param' => $rp, - 'reflection_type_object' => $reflectionType, - 'type_string' => $typeString, - 'description' => $description, - 'required' => !$rp->isOptional(), - 'allows_null' => $allowsNull, - 'default_value' => $defaultValue, - 'has_default' => $hasDefault, - 'is_variadic' => $isVariadic, - 'parameter_schema' => $parameterSchema, - ]; - } - - return $parametersInfo; - } - - /** - * Determines the type string for a parameter, prioritizing DocBlock. - */ - private function getParameterTypeString(\ReflectionParameter $rp, ?Param $paramTag): string - { - $docBlockType = $this->docBlockParser->getParamTypeString($paramTag); - $isDocBlockTypeGeneric = false; - - if (null !== $docBlockType) { - if (\in_array(strtolower($docBlockType), ['mixed', 'unknown', ''])) { - $isDocBlockTypeGeneric = true; - } - } else { - $isDocBlockTypeGeneric = true; // No tag or no type in tag implies generic - } - - $reflectionType = $rp->getType(); - $reflectionTypeString = null; - if ($reflectionType) { - $reflectionTypeString = $this->getTypeStringFromReflection($reflectionType, $rp->allowsNull()); - } - - // Prioritize Reflection if DocBlock type is generic AND Reflection provides a more specific type - if ($isDocBlockTypeGeneric && null !== $reflectionTypeString && 'mixed' !== $reflectionTypeString) { - return $reflectionTypeString; - } - - // 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 (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'; - } - } - - return $docBlockType; - } - - // Fallback to Reflection type even if it was generic ('mixed') - if (null !== $reflectionTypeString) { - return $reflectionTypeString; - } - - // Default to 'mixed' if nothing else found - return 'mixed'; - } - - /** - * Converts a ReflectionType object into a type string representation. - */ - private function getTypeStringFromReflection(?\ReflectionType $type, bool $nativeAllowsNull): string - { - if (null === $type) { - return 'mixed'; - } - - $types = []; - if ($type instanceof \ReflectionUnionType) { - foreach ($type->getTypes() as $innerType) { - $types[] = $this->getTypeStringFromReflection($innerType, $innerType->allowsNull()); - } - if ($nativeAllowsNull) { - $types = array_filter($types, static fn ($t) => 'null' !== strtolower($t)); - } - $typeString = implode('|', array_unique(array_filter($types))); - } elseif ($type instanceof \ReflectionIntersectionType) { - foreach ($type->getTypes() as $innerType) { - $types[] = $this->getTypeStringFromReflection($innerType, false); - } - $typeString = implode('&', array_unique(array_filter($types))); - } elseif ($type instanceof \ReflectionNamedType) { - $typeString = $type->getName(); - } else { - return 'mixed'; - } - - $typeString = match (strtolower($typeString)) { - 'bool' => 'boolean', - 'int' => 'integer', - 'float', 'double' => 'number', - 'str' => 'string', - default => $typeString, - }; - - $isNullable = $nativeAllowsNull; - if ($type instanceof \ReflectionNamedType && 'mixed' === $type->getName()) { - $isNullable = true; - } - - if ($type instanceof \ReflectionUnionType && !$nativeAllowsNull) { - foreach ($type->getTypes() as $innerType) { - if ($innerType instanceof \ReflectionNamedType && 'null' === strtolower($innerType->getName())) { - $isNullable = true; - break; - } - } - } - - if ($isNullable && 'mixed' !== $typeString && false === stripos($typeString, 'null')) { - if (!str_ends_with($typeString, '|null') && !str_ends_with($typeString, '&null')) { - $typeString .= '|null'; - } - } - - // Remove leading backslash from class names, but handle built-ins like 'int' or unions like 'int|string' - if (str_contains($typeString, '\\')) { - $parts = preg_split('/([|&])/', $typeString, -1, \PREG_SPLIT_DELIM_CAPTURE); - if (false !== $parts) { - $processedParts = array_map(static fn ($part) => str_starts_with($part, '\\') ? ltrim($part, '\\') : $part, $parts); - $typeString = implode('', $processedParts); - } - } - - return $typeString ?: 'mixed'; - } - - /** - * Splits a type string on its top-level `|`, leaving generics and shapes - * intact. - * - * `int|string` splits; `array` and `array{a: int|string}` do - * not, because the `|` there is a parameter of the outer type rather than - * an alternative to it. - * - * @return list - */ - public static function splitUnion(string $type): array - { - return self::splitTopLevel($type, '|'); - } - - /** - * Splits a type string on a separator outside of generics, shapes, parentheses and quoted literals. - * - * @return list - */ - private static function splitTopLevel(string $type, string $separator): array - { - $parts = []; - $depth = 0; - $current = ''; - $quote = null; - $escaped = false; - - foreach (str_split($type) as $character) { - if (null !== $quote) { - if ($escaped) { - $escaped = false; - } elseif ('\\' === $character) { - $escaped = true; - } elseif ($quote === $character) { - $quote = null; - } - } elseif ('\'' === $character || '"' === $character) { - $quote = $character; - } elseif (\in_array($character, ['<', '{', '('], true)) { - ++$depth; - } elseif (\in_array($character, ['>', '}', ')'], true)) { - $depth = max(0, $depth - 1); - } elseif ($separator === $character && 0 === $depth) { - $parts[] = trim($current); - $current = ''; - - continue; - } - - $current .= $character; - } - - $parts[] = trim($current); - - return array_values(array_filter($parts, static fn (string $part): bool => '' !== $part)); - } - - /** - * 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. - * - * @return string[] - */ - private function mapPhpTypeToJsonSchemaType(string $phpTypeString): array - { - $normalizedType = strtolower(trim($phpTypeString)); - - if (str_starts_with($normalizedType, '?')) { - $normalizedType = 'null|'.substr($normalizedType, 1); - } - - // PRIORITY 1: Handle unions before anything else. A `|` at the top - // level joins alternatives; one inside `<>` or `{}` belongs to a - // generic (`array`) and is not a union of the whole type. - // Checked first because a branch may itself be an array shape — and - // `string[]|int` read as "an array" silently loses the `int`. - $branches = self::splitUnion($normalizedType); - - if (\count($branches) > 1) { - $jsonTypes = []; - foreach ($branches as $branch) { - $jsonTypes = array_merge($jsonTypes, $this->mapPhpTypeToJsonSchemaType($branch)); - } - - return array_values(array_unique($jsonTypes)); - } - - // PRIORITY 2: Check for array{} syntax which should be treated as object - if (preg_match('/^array\s*{/i', $normalizedType)) { - return ['object']; - } - - // PRIORITY 3: Treat PHPStan/Psalm integer ranges and other pseudo-types - // (`non-empty-string`, `positive-int`, literals, ...) as their base type - if (null !== $pseudoTypes = self::mapPseudoTypeToJsonSchemaTypes($normalizedType)) { - return $pseudoTypes; - } - - // The key type of a generic array decides between a JSON array and a JSON object. - if (preg_match('/^(?:non-empty-)?array\s*<.*>$/s', $normalizedType)) { - return (array) $this->mapTypeToSchema($normalizedType)['type']; - } - - // PRIORITY 4: Check for array syntax (T[] or generics) - if ( - str_contains($normalizedType, '[]') - || preg_match('/^(array|list|non-empty-list|iterable|collection) ['string'], - 'int', 'integer' => ['integer'], - 'float', 'double', 'number' => ['number'], - 'bool', 'boolean' => ['boolean'], - 'array' => ['array'], - 'object', 'stdclass' => ['object'], - 'null' => ['null'], - 'resource', 'callable' => ['object'], - 'mixed' => [], - 'void', 'never' => [], - default => ['object'], }; - } - - /** - * Maps a PHPStan/Psalm pseudo-type or a literal type to the JSON Schema - * types of its base type, e.g. `non-empty-string` to `string`, - * `positive-int` or `int<0, max>` to `integer` and `'a'` to `string`. - * - * @return list|null null when the type is not a known pseudo-type - */ - private static function mapPseudoTypeToJsonSchemaTypes(string $type): ?array - { - $type = strtolower(trim($type)); - - if (preg_match('/^int\s*<\s*[^<>]+\s*>$/', $type)) { - return ['integer']; - } - - if (preg_match('/^class-string\s*<[^<>]+>$/', $type)) { - return ['string']; - } - - // Literal types: 'foo', "foo", 42, -1.5 - if (preg_match('/^(\'(?:[^\'\\\\]|\\\\.)*\'|"(?:[^"\\\\]|\\\\.)*")$/s', $type)) { - return ['string']; - } - if (preg_match('/^-?\d+$/', $type)) { - return ['integer']; - } - if (preg_match('/^-?\d*\.\d+$/', $type)) { - return ['number']; - } - - return match ($type) { - 'non-empty-string', 'non-falsy-string', 'truthy-string', 'numeric-string', 'literal-string', - 'non-empty-literal-string', 'lowercase-string', 'non-empty-lowercase-string', 'uppercase-string', - 'non-empty-uppercase-string', 'class-string', 'interface-string', 'trait-string', 'enum-string', - 'callable-string' => ['string'], - 'positive-int', 'negative-int', 'non-positive-int', 'non-negative-int', 'non-zero-int' => ['integer'], - 'true', 'false' => ['boolean'], - 'array-key' => ['integer', 'string'], - 'numeric' => ['integer', 'number', 'string'], - default => null, - }; - } - - /** - * Infers the 'items' schema type for an array based on DocBlock type hints. - * - * @return string|array - */ - private function inferArrayItemsType(string $phpTypeString): string|array - { - $normalizedType = trim($phpTypeString); - - // Strip a top-level nullable union (e.g. `string[]|null`, `null|int[]`) so the - // element type is still recovered; array nullability is handled separately via - // `allows_null`. Internal unions such as `array` are left untouched. - $normalizedType = trim((string) preg_replace('/^null\s*\|\s*|\s*\|\s*null$/i', '', $normalizedType)); - - // Case 1: Simple T[] syntax (e.g., string[], int[], bool[], etc.) - if (preg_match('/^(\\??)([\w\\\\]+)\\s*\\[\\]$/i', $normalizedType, $matches)) { - $itemType = strtolower($matches[2]); - - return $this->mapSimpleTypeToJsonSchema($itemType); - } - - // Case 2: Generic array syntax (e.g., array, array, etc.) - if (preg_match('/^(\\??)(?:array|list|non-empty-array|non-empty-list)\s*<\s*((?:[\w\\\\|?-]|\'[^\']*\'|"[^"]*")+)\s*>$/i', $normalizedType, $matches)) { - $itemSchema = $this->mapTypeToSchema(strtolower($matches[2])); - if (!isset($itemSchema['type'])) { - return 'any'; - } - - return \is_string($itemSchema['type']) && 1 === \count($itemSchema) ? $itemSchema['type'] : $itemSchema; - } - // Case 3: Nested array> syntax or T[][] syntax - if ( - preg_match('/^(\\??)array\s*<\s*array\s*<\s*([\w\\\\|?]+)\s*>\s*>$/i', $normalizedType, $matches) - || preg_match('/^(\\??)([\w\\\\]+)\s*\[\]\[\]$/i', $normalizedType, $matches) - ) { - // Return a schema for array with items being arrays - return [ - 'type' => 'array', - 'items' => $this->mapTypeToSchema(strtolower($matches[2])) ?: new \stdClass(), - ]; - } - - // Case 4: Object-like array syntax (e.g., array{name: string, age: int}) - if (preg_match('/^(\\??)array\s*\{(.+)\}$/is', $normalizedType, $matches)) { - return $this->parseObjectLikeArray($matches[2]); - } - - return 'any'; + return $convert($schema->jsonSerialize()); } - /** - * Parses object-like array syntax into a JSON Schema object. - * - * @return array{ - * type: 'object', - * properties?: array, - * required?: array - * } - */ - private function parseObjectLikeArray(string $propertiesStr): array + private static function assertFunction(\Reflector $reflection): \ReflectionMethod|\ReflectionFunction { - $properties = []; - $required = []; - - foreach (self::splitTopLevel($propertiesStr, ',') as $propDefinition) { - $this->parsePropertyDefinition($propDefinition, $properties, $required); - } - - if (!empty($properties)) { - return [ - 'type' => 'object', - 'properties' => $properties, - 'required' => $required, - ]; + if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) { + throw new BadMethodCallException(\sprintf('Schema generation from %s is not supported. Use ReflectionMethod or ReflectionFunction instead.', $reflection::class)); } - return ['type' => 'object']; + return $reflection; } /** - * Parses a single property definition from an object-like array syntax. + * @return array|null */ - private function parsePropertyDefinition(string $propDefinition, array &$properties, array &$required): void + private static function schemaAttribute(\ReflectionFunctionAbstract|\ReflectionParameter $reflection): ?array { - // Match property name and type - if (preg_match('/^([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\s*:\s*(.+)$/i', $propDefinition, $matches)) { - $propName = $matches[1]; - $propType = trim($matches[2]); + $attributes = $reflection->getAttributes(Schema::class, \ReflectionAttribute::IS_INSTANCEOF); - // Add to required properties - $required[] = $propName; - - // `mixed` accepts anything, which an empty schema expresses; it must encode as `{}`, not `[]`. - $properties[$propName] = $this->mapTypeToSchema($propType) ?: new \stdClass(); - } + return $attributes ? $attributes[0]->newInstance()->toArray() : null; } - /** - * Maps a type inside a shape or generic, e.g. `string`, `int[]|null` or `array{x: int}`, to a JSON Schema. - * - * @return array - */ - private function mapTypeToSchema(string $type): array + private static function handlerName(\ReflectionFunctionAbstract $reflection): string { - if (str_starts_with($type, '?')) { - $type = 'null|'.substr($type, 1); - } - - $branches = self::splitUnion($type); - if (\count($branches) > 1) { - $nonNull = array_values(array_filter($branches, static fn (string $branch): bool => 'null' !== strtolower($branch))); - $schemas = array_map($this->mapTypeToSchema(...), $nonNull); - // A branch without a type (`mixed`) accepts anything, and so does the union. - foreach ($schemas as $branchSchema) { - if (!isset($branchSchema['type'])) { - return []; - } - } - $types = array_values(array_unique(array_merge(...array_map(static fn (array $schema): array => (array) $schema['type'], $schemas)))); - if (\count($nonNull) < \count($branches)) { - $types[] = 'null'; - } - - // Constraints of several branches could reject what another accepts, so only a lone non-null branch keeps them. - $schema = 1 === \count($schemas) ? $schemas[0] : []; - $schema['type'] = 1 === \count($types) ? $types[0] : $types; - - return $schema; - } - - if (preg_match('/^array\s*\{(.+)\}$/is', $type, $matches)) { - return $this->parseObjectLikeArray($matches[1]); - } - - // `array`: only string keys rule out a JSON array; sparse int keys arrive as a JSON object - if (preg_match('/^(?:non-empty-)?array\s*<\s*((?:\'[^\']*\'|"[^"]*"|[^<>{},\'"]|<[^<>]*>)+?)\s*,\s*(.+?)\s*>$/is', $type, $matches)) { - $keyTypes = $this->mapPhpTypeToJsonSchemaType($matches[1]); - $valueSchema = $this->mapTypeToSchema($matches[2]); - $items = ['items' => $valueSchema ?: new \stdClass()]; - $additionalProperties = ['additionalProperties' => $valueSchema ?: true]; - - return ['string'] === $keyTypes - ? ['type' => 'object'] + $additionalProperties - : ['type' => ['array', 'object']] + $items + $additionalProperties; - } - - // `array`, `list`, `non-empty-list`, `non-empty-array` and `T[]` - if ( - preg_match('/^(?:array|list|non-empty-array|non-empty-list)\s*<\s*(.+?)\s*>$/is', $type, $matches) - || preg_match('/^([\w\\\\]+)\s*\[\]$/i', $type, $matches) - ) { - return ['type' => 'array', 'items' => $this->mapTypeToSchema($matches[1]) ?: new \stdClass()]; - } - - if (\in_array(strtolower($type), ['list', 'non-empty-array', 'non-empty-list'], true)) { - return ['type' => 'array']; - } - - if ('mixed' === strtolower($type)) { - return []; - } - - if (null !== $pseudoTypes = self::mapPseudoTypeToJsonSchemaTypes($type)) { - return ['type' => 1 === \count($pseudoTypes) ? $pseudoTypes[0] : $pseudoTypes]; - } - - return ['type' => 'null' === strtolower($type) ? 'null' : $this->mapSimpleTypeToJsonSchema($type)]; - } - - /** - * Helper method to map basic PHP types to JSON Schema types. - */ - private function mapSimpleTypeToJsonSchema(string $type): string - { - return match (strtolower($type)) { - 'string' => 'string', - 'int', 'integer' => 'integer', - 'bool', 'boolean' => 'boolean', - 'float', 'double', 'number' => 'number', - 'array' => 'array', - 'object', 'stdclass' => 'object', - default => \in_array(strtolower($type), ['datetime', 'datetimeinterface']) ? 'string' : 'object', - }; + return $reflection instanceof \ReflectionMethod ? $reflection->class.'::'.$reflection->name : $reflection->name; } } diff --git a/src/Server/Capability/Registry/Loader/ReflectedElementLoader.php b/src/Server/Capability/Registry/Loader/ReflectedElementLoader.php index 60fff503e..49e6f9ae3 100644 --- a/src/Server/Capability/Registry/Loader/ReflectedElementLoader.php +++ b/src/Server/Capability/Registry/Loader/ReflectedElementLoader.php @@ -96,7 +96,7 @@ public function __construct( public function load(RegistryInterface $registry): void { $docBlockParser = new DocBlockParser(logger: $this->logger); - $schemaGenerator = $this->schemaGenerator ?? new SchemaGenerator($docBlockParser); + $schemaGenerator = $this->schemaGenerator ?? new SchemaGenerator(); // Register Tools foreach ($this->tools as $data) { diff --git a/tests/Interop/Server/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json b/tests/Interop/Server/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json index bff62c43c..75699e865 100644 --- a/tests/Interop/Server/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json +++ b/tests/Interop/Server/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json @@ -26,11 +26,10 @@ }, "time": { "type": [ - "null", - "string" + "string", + "null" ], - "description": "the time of the event (HH:MM), optional", - "default": null + "description": "the time of the event (HH:MM), optional" }, "priority": { "type": "string", @@ -48,7 +47,6 @@ "null" ], "description": "an optional list of attendee email addresses", - "default": null, "items": { "type": "string" } @@ -60,16 +58,22 @@ }, "labels": { "type": "object", + "additionalProperties": { + "type": "string" + }, "description": "labels of the event by name", "default": {} }, "metadata": { - "type": [ - "null", - "object" + "anyOf": [ + { + "type": "object" + }, + { + "type": "null" + } ], - "description": "free-form data stored with the event, exactly as sent", - "default": null + "description": "free-form data stored with the event, exactly as sent" } }, "required": [ diff --git a/tests/Interop/Server/Http/snapshots/HttpDiscoveryUserProfileTest-tools_list.json b/tests/Interop/Server/Http/snapshots/HttpDiscoveryUserProfileTest-tools_list.json index 08b8198d1..ac5f82955 100644 --- a/tests/Interop/Server/Http/snapshots/HttpDiscoveryUserProfileTest-tools_list.json +++ b/tests/Interop/Server/Http/snapshots/HttpDiscoveryUserProfileTest-tools_list.json @@ -51,11 +51,10 @@ }, "customMessage": { "type": [ - "null", - "string" + "string", + "null" ], - "description": "an optional custom message part", - "default": null + "description": "an optional custom message part" } }, "required": [ diff --git a/tests/Interop/Server/Http/snapshots/HttpSchemaShowcaseTest-tools_list.json b/tests/Interop/Server/Http/snapshots/HttpSchemaShowcaseTest-tools_list.json index f79bfeba8..f312cf57f 100644 --- a/tests/Interop/Server/Http/snapshots/HttpSchemaShowcaseTest-tools_list.json +++ b/tests/Interop/Server/Http/snapshots/HttpSchemaShowcaseTest-tools_list.json @@ -264,11 +264,11 @@ }, "attendees": { "type": "array", - "default": [], "items": { "type": "string", "format": "email" }, + "default": [], "description": "List of attendee email addresses", "maxItems": 20 } diff --git a/tests/Unit/Server/Capability/Discovery/DocBlockParserTest.php b/tests/Unit/Server/Capability/Discovery/DocBlockParserTest.php index 8998abfcb..9c36809f9 100644 --- a/tests/Unit/Server/Capability/Discovery/DocBlockParserTest.php +++ b/tests/Unit/Server/Capability/Discovery/DocBlockParserTest.php @@ -11,8 +11,6 @@ namespace Mcp\Tests\Unit\Server\Capability\Discovery; -use Composer\InstalledVersions; -use Composer\Semver\VersionParser; use Mcp\Server\Capability\Discovery\DocBlockParser; use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\DocBlock\Tags\Deprecated; @@ -60,38 +58,21 @@ public function testGetParamTagsReturnsStructuredParamInfo(): void $this->assertInstanceOf(Param::class, $params['$param1']); $this->assertEquals('param1', $params['$param1']->getVariableName()); - $this->assertEquals('string', $this->parser->getParamTypeString($params['$param1'])); - $this->assertEquals('description for string param', $this->parser->getParamDescription($params['$param1'])); $this->assertInstanceOf(Param::class, $params['$param2']); $this->assertEquals('param2', $params['$param2']->getVariableName()); - $this->assertEquals('int|null', $this->parser->getParamTypeString($params['$param2'])); - $this->assertEquals('description for nullable int param', $this->parser->getParamDescription($params['$param2'])); $this->assertInstanceOf(Param::class, $params['$param3']); $this->assertEquals('param3', $params['$param3']->getVariableName()); - $this->assertEquals('bool', $this->parser->getParamTypeString($params['$param3'])); - $this->assertEquals('nothing to say', $this->parser->getParamDescription($params['$param3'])); $this->assertInstanceOf(Param::class, $params['$param4']); $this->assertEquals('param4', $params['$param4']->getVariableName()); - $this->assertEquals('mixed', $this->parser->getParamTypeString($params['$param4'])); - $this->assertEquals('Missing type', $this->parser->getParamDescription($params['$param4'])); $this->assertInstanceOf(Param::class, $params['$param5']); $this->assertEquals('param5', $params['$param5']->getVariableName()); - // Remove if when dropping support for phpdocumentor/reflection-docblock:^5.6 - if (InstalledVersions::satisfies(new VersionParser(), 'phpdocumentor/reflection-docblock', '^6.0')) { - $this->assertEquals('array', $this->parser->getParamTypeString($params['$param5'])); - } else { - $this->assertEquals('array', $this->parser->getParamTypeString($params['$param5'])); - } - $this->assertEquals('array description', $this->parser->getParamDescription($params['$param5'])); $this->assertInstanceOf(Param::class, $params['$param6']); $this->assertEquals('param6', $params['$param6']->getVariableName()); - $this->assertEquals('stdClass', $this->parser->getParamTypeString($params['$param6'])); - $this->assertEquals('object param', $this->parser->getParamDescription($params['$param6'])); } public function testGetTagsByNameReturnsSpecificTags(): void diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorFixture.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorFixture.php index 14f105c39..05bea1fab 100644 --- a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorFixture.php +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorFixture.php @@ -360,27 +360,19 @@ public function pseudoTypes( } /** - * The key type of a map inside an array shape decides between a JSON array and a JSON object. + * A map inside an array shape is a JSON object for string keys and a JSON array otherwise. * - * @param array{map: array} $stringKey - * @param array{map: array} $pseudoStringKey - * @param array{map: array, int>} $genericStringKey - * @param array{map: array} $intKey - * @param array{map: array, int>} $rangeKey - * @param array{map: array} $arrayKey - * @param array{map: array} $unionKey - * @param array{map: array} $arrayKeyMixedValue - * @param array{map: array|null} $nullableArrayKey - * @param array{map: array} $shapeValue + * @param array{map: array} $stringKey + * @param array{map: array} $intKey + * @param array{map: array} $arrayKey + * @param array{map: array} $arrayKeyMixedValue + * @param array{map: array|null} $nullableArrayKey + * @param array{map: array} $shapeValue */ public function arrayShapeMapKeys( array $stringKey, - array $pseudoStringKey, - array $genericStringKey, array $intKey, - array $rangeKey, array $arrayKey, - array $unionKey, array $arrayKeyMixedValue, array $nullableArrayKey, array $shapeValue, @@ -411,36 +403,6 @@ public function nullableAndNonEmptyPseudoTypes( ): void { } - /** - * @param 'a|b'|'c' $pipeLiteral - * @param "a|b" $doubleQuotedPipeLiteral - * @param 'a[]' $bracketLiteral - * @param 'it\'s' $escapedLiteral - * @param array{operator: '<', limit: int} $lessThanLiteral - * @param array{operator: '>', limit: int} $greaterThanLiteral - * @param array{label: 'a,b', limit: int} $commaLiteral - * @param array{label: '{', limit: int} $braceLiteral - * @param list<'a,b'> $commaLiteralList - * @param array> $nestedMixed - * @param array> $nestedMixedUnion - * @param mixed[][] $nestedMixedBrackets - */ - public function quotedLiteralsAndNestedMixed( - string $pipeLiteral, - string $doubleQuotedPipeLiteral, - string $bracketLiteral, - string $escapedLiteral, - array $lessThanLiteral, - array $greaterThanLiteral, - array $commaLiteral, - array $braceLiteral, - array $commaLiteralList, - array $nestedMixed, - array $nestedMixedUnion, - array $nestedMixedBrackets, - ): void { - } - // ===== NULLABLE AND OPTIONAL SCENARIOS ===== /** diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorNullableMemberTest.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorNullableMemberTest.php index 205922e31..1b2348afc 100644 --- a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorNullableMemberTest.php +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorNullableMemberTest.php @@ -11,7 +11,6 @@ namespace Mcp\Tests\Unit\Server\Capability\Discovery; -use Mcp\Server\Capability\Discovery\DocBlockParser; use Mcp\Server\Capability\Discovery\SchemaGenerator; use Mcp\Server\Capability\Discovery\SchemaValidator; use PHPUnit\Framework\TestCase; @@ -63,7 +62,7 @@ public function testClassNamedNullIsNotNullable(): void */ private function schema(string $method): array { - return (new SchemaGenerator(new DocBlockParser())) + return (new SchemaGenerator()) ->generate(new \ReflectionMethod(NullableMemberFixture::class, $method)); } diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorObjectFixture.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorObjectFixture.php new file mode 100644 index 000000000..cad958859 --- /dev/null +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorObjectFixture.php @@ -0,0 +1,106 @@ + $tags + */ + public function __construct( + #[Assert\Email] + public string $email, + #[JsonSchemaConstraint(minimum: 18)] + public int $age, + public ?FixtureAddress $address = null, + public array $tags = [], + public ?\DateTimeImmutable $birthday = null, + ) { + } +} + +final class FixtureCategory +{ + public string $name; + + /** @var list */ + public array $children = []; +} + +final class SchemaGeneratorObjectFixture +{ + /** + * @param string $city the city to look up + * @param list $fields which fields to return + * @param array{lat: float, label?: string} $point + * @param array $weights + */ + public function scalars(string $city, array $fields, array $point, array $weights, int $days = 3, FixtureUnit $unit = FixtureUnit::Celsius): void + { + } + + /** + * @param FixtureUser $user the user to create + */ + public function dto(FixtureUser $user, RequestContext $context): void + { + } + + public function tree(FixtureCategory $root): void + { + } + + public function variadic(int ...$numbers): void + { + } + + public function untyped($anything): void // @phpstan-ignore missingType.parameter + { + } + + public function returnsDto(): FixtureUser + { + return new FixtureUser('a@b.c', 18); + } + + #[McpTool(outputSchema: ['type' => 'object', 'properties' => ['explicit' => ['type' => 'boolean']]])] + public function explicitOutput(): FixtureUser + { + return new FixtureUser('a@b.c', 18); + } + + public function returnsArray(): array + { + return []; + } +} diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php index 90e57d149..3ee53aa9c 100644 --- a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php @@ -11,8 +11,7 @@ namespace Mcp\Tests\Unit\Server\Capability\Discovery; -use Mcp\Server\Capability\Attribute\Schema; -use Mcp\Server\Capability\Discovery\DocBlockParser; +use Mcp\Schema\Tool; use Mcp\Server\Capability\Discovery\SchemaGenerator; use Mcp\Server\Capability\Discovery\SchemaValidator; use Mcp\Server\Exception\InvalidArgumentException; @@ -25,7 +24,7 @@ final class SchemaGeneratorTest extends TestCase protected function setUp(): void { - $this->schemaGenerator = new SchemaGenerator(new DocBlockParser()); + $this->schemaGenerator = new SchemaGenerator(); } public function testGeneratesEmptyPropertiesObjectForMethodWithNoParameters(): void @@ -47,7 +46,7 @@ public function testInfersBasicTypesFromPhpTypeHints(): void $this->assertEquals(['type' => 'integer'], $schema['properties']['age']); $this->assertEquals(['type' => 'boolean'], $schema['properties']['active']); $this->assertEquals(['type' => 'array', 'items' => new \stdClass()], $schema['properties']['tags']); - $this->assertEquals(['type' => ['null', 'object'], 'default' => null], $schema['properties']['config']); + $this->assertEquals(['anyOf' => [['type' => 'object'], ['type' => 'null']]], $schema['properties']['config']); $this->assertEqualsCanonicalizing(['name', 'age', 'active', 'tags'], $schema['required']); } @@ -192,7 +191,6 @@ public function testGeneratesSchemaFromIndividualParameterLevelSchemaAttributes( 'deviceToken' => ['type' => 'string', 'description' => 'Required if type is push'], ], 'required' => ['type'], - 'default' => null, ], $schema['properties']['notificationConfig']); $this->assertEqualsCanonicalizing(['recipientId', 'messageBody'], $schema['required']); } @@ -262,15 +260,16 @@ public function testMapsPhpStanAndPsalmPseudoTypesToTheirBaseType(): void $this->assertEquals(['type' => 'string', 'description' => 'Name'], $schema['properties']['name']); $this->assertEquals(['type' => 'string', 'description' => 'Class'], $schema['properties']['class']); $this->assertEquals(['type' => 'integer', 'description' => 'Count'], $schema['properties']['count']); - $this->assertEquals(['type' => ['integer', 'string'], 'description' => 'Key'], $schema['properties']['key']); + $this->assertEquals(['anyOf' => [['type' => 'integer'], ['type' => 'string']], 'description' => 'Key'], $schema['properties']['key']); $this->assertEquals(['type' => 'string', 'description' => 'Order'], $schema['properties']['order']); $this->assertEquals(['type' => 'integer', 'description' => 'Level'], $schema['properties']['level']); $this->assertEquals(['type' => 'array', 'description' => 'Tags', 'items' => ['type' => 'string']], $schema['properties']['tags']); $settings = $schema['properties']['settings']; $this->assertSame('object', $settings['type']); + $this->assertFalse($settings['additionalProperties']); $this->assertEquals(['type' => 'array', 'items' => ['type' => 'string']], $settings['properties']['tags']); - $this->assertEquals(['type' => ['array', 'object'], 'items' => ['type' => 'integer'], 'additionalProperties' => ['type' => 'integer']], $settings['properties']['ids']); + $this->assertEquals(['type' => 'array', 'items' => ['type' => 'integer']], $settings['properties']['ids']); $this->assertEquals(['type' => 'object', 'additionalProperties' => ['type' => 'integer']], $settings['properties']['meta']); // `mixed` accepts any value, serialized as the empty schema `{}` $this->assertSame('{}', json_encode($settings['properties']['any'])); @@ -305,33 +304,21 @@ public function testMapsTheKeyTypeOfAMapInsideAnArrayShape(string $parameter, ar public static function provideArrayShapeMapKeys(): iterable { $object = ['type' => 'object', 'additionalProperties' => ['type' => 'integer']]; - $either = ['type' => ['array', 'object'], 'items' => ['type' => 'integer'], 'additionalProperties' => ['type' => 'integer']]; + // keys other than `string` describe a list + $list = ['type' => 'array', 'items' => ['type' => 'integer']]; yield 'string' => ['stringKey', $object, ['{"x":1}'], ['{"x":"y"}', '"x"']]; - yield 'string pseudo-type' => ['pseudoStringKey', $object, ['{"x":1}'], ['{"x":"y"}', '"x"']]; - yield 'generic string pseudo-type' => ['genericStringKey', $object, ['{"stdClass":1}'], ['{"stdClass":"y"}', '"x"']]; - yield 'int' => ['intKey', $either, ['[1]', '[]', '{"5":1}'], ['["y"]', '{"5":"y"}', '"x"']]; - yield 'int range' => ['rangeKey', $either, ['[1]', '[]', '{"5":1}'], ['["y"]', '{"5":"y"}', '"x"']]; - yield 'array-key' => ['arrayKey', $either, ['{"x":1}', '[1]', '{}', '{"0":1}'], ['{"x":"y"}', '["y"]', '"x"', 'null']]; - yield 'int|string' => ['unionKey', $either, ['{"x":1}', '[1]', '{}', '{"0":1}'], ['{"x":"y"}', '["y"]', '"x"', 'null']]; - yield 'array-key with mixed value' => [ - 'arrayKeyMixedValue', - ['type' => ['array', 'object'], 'items' => new \stdClass(), 'additionalProperties' => true], - ['{"x":1}', '{"x":"y"}', '[1,"y"]', '{}'], - ['"x"', 'null'], - ]; - yield 'nullable array-key' => [ - 'nullableArrayKey', - ['type' => ['array', 'object', 'null']] + $either, - ['{"x":1}', '[1]', 'null'], - ['{"x":"y"}', '["y"]', '"x"'], - ]; + yield 'int' => ['intKey', $list, ['[1]', '[]'], ['{"5":1}', '["y"]', '"x"']]; + yield 'array-key' => ['arrayKey', $list, ['[1]', '[]'], ['{"x":1}', '["y"]', '"x"', 'null']]; + yield 'array-key with mixed value' => ['arrayKeyMixedValue', ['type' => 'array', 'items' => new \stdClass()], ['[1,"y"]', '[]'], ['{"x":1}', '"x"', 'null']]; + yield 'nullable array-key' => ['nullableArrayKey', ['type' => ['array', 'null'], 'items' => ['type' => 'integer']], ['[1]', 'null'], ['{"x":1}', '["y"]', '"x"']]; yield 'single argument with a shape value' => [ 'shapeValue', ['type' => 'array', 'items' => [ 'type' => 'object', 'properties' => ['a' => ['type' => 'integer'], 'b' => ['type' => 'integer']], 'required' => ['a', 'b'], + 'additionalProperties' => false, ]], ['[{"a":1,"b":2}]', '[]'], ['[{"a":1}]', '{"x":{"a":1,"b":2}}'], @@ -365,65 +352,17 @@ public function testMapsNullableAndNonEmptyPseudoTypes(string $parameter, array */ public static function provideNullableAndNonEmptyPseudoTypes(): iterable { - yield '?non-empty-string' => ['nullableString', ['type' => ['null', 'string']], ['"a"', 'null'], ['1']]; - yield '?positive-int' => ['nullableInt', ['type' => ['null', 'integer']], ['1', 'null'], ['"a"']]; - yield '?array-key' => ['nullableArrayKey', ['type' => ['null', 'integer', 'string']], ['1', '"a"', 'null'], ['true']]; + yield '?non-empty-string' => ['nullableString', ['type' => ['string', 'null']], ['"a"', 'null'], ['1']]; + yield '?positive-int' => ['nullableInt', ['type' => ['integer', 'null']], ['1', 'null'], ['"a"']]; + yield '?array-key' => ['nullableArrayKey', ['anyOf' => [['type' => 'integer'], ['type' => 'string'], ['type' => 'null']]], ['1', '"a"', 'null'], ['true']]; yield '?list' => ['nullableList', ['type' => ['array', 'null'], 'items' => ['type' => 'string']], ['["a"]', 'null'], ['[1]', '"a"']]; - yield 'non-empty-array' => ['stringKeyMap', ['type' => 'object'], ['{"a":1}'], ['["a"]', '"a"']]; - yield 'non-empty-array|null' => ['nullableStringKeyMap', ['type' => ['null', 'object']], ['{"a":1}', 'null'], ['["a"]']]; - yield 'non-empty-array' => ['intKeyMap', ['type' => ['array', 'object'], 'items' => new \stdClass()], ['[1]', '{"5":1}'], ['"a"']]; - yield 'non-empty-array' => ['arrayKeyMap', ['type' => ['array', 'object'], 'items' => new \stdClass()], ['{"a":1}', '[1]'], ['"a"']]; + yield 'non-empty-array' => ['stringKeyMap', ['type' => 'object', 'additionalProperties' => ['type' => 'integer']], ['{"a":1}'], ['["a"]', '"a"']]; + yield 'non-empty-array|null' => ['nullableStringKeyMap', ['type' => ['object', 'null'], 'additionalProperties' => ['type' => 'integer']], ['{"a":1}', 'null'], ['["a"]']]; + yield 'non-empty-array' => ['intKeyMap', ['type' => 'array', 'items' => ['type' => 'integer']], ['[1]'], ['{"5":1}', '"a"']]; + yield 'non-empty-array' => ['arrayKeyMap', ['type' => 'array', 'items' => ['type' => 'integer']], ['[1]'], ['{"a":1}', '"a"']]; yield 'non-empty-array' => ['singleArgument', ['type' => 'array', 'items' => ['type' => 'integer']], ['[1]'], ['["a"]', '{"a":1}']]; } - /** - * @param array $expected - * @param list $valid JSON arguments that must pass validation - * @param list $invalid JSON arguments that must fail validation - */ - #[DataProvider('provideQuotedLiteralsAndNestedMixed')] - public function testMapsQuotedLiteralsAndNestedMixed(string $parameter, array $expected, array $valid, array $invalid): void - { - $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'quotedLiteralsAndNestedMixed'); - $schema = $this->schemaGenerator->generate($method)['properties'][$parameter]; - - $this->assertEquals($expected, $schema); - - $validator = new SchemaValidator(); - foreach ($valid as $argument) { - $this->assertSame([], $validator->validateAgainstJsonSchema(json_decode($argument, true), $schema), $argument); - } - foreach ($invalid as $argument) { - $this->assertNotSame([], $validator->validateAgainstJsonSchema(json_decode($argument, true), $schema), $argument); - } - } - - /** - * @return iterable, list, list}> - */ - public static function provideQuotedLiteralsAndNestedMixed(): iterable - { - $shape = static fn (string $key): array => [ - 'type' => 'object', - 'properties' => [$key => ['type' => 'string'], 'limit' => ['type' => 'integer']], - 'required' => [$key, 'limit'], - ]; - $nested = ['type' => 'array', 'items' => ['type' => 'array', 'items' => new \stdClass()]]; - - yield 'pipe in a literal' => ['pipeLiteral', ['type' => 'string'], ['"a|b"', '"c"'], ['1']]; - yield 'pipe in a double quoted literal' => ['doubleQuotedPipeLiteral', ['type' => 'string'], ['"a|b"'], ['1']]; - yield 'brackets in a literal' => ['bracketLiteral', ['type' => 'string'], ['"a[]"'], ['["a"]']]; - yield 'escaped quote in a literal' => ['escapedLiteral', ['type' => 'string'], ['"it\'s"'], ['1']]; - yield 'less than in a shape literal' => ['lessThanLiteral', $shape('operator'), ['{"operator":"<","limit":1}'], ['{"operator":"<"}', '{"operator":1,"limit":1}']]; - yield 'greater than in a shape literal' => ['greaterThanLiteral', $shape('operator'), ['{"operator":">","limit":1}'], ['{"operator":">"}']]; - yield 'comma in a shape literal' => ['commaLiteral', $shape('label'), ['{"label":"a,b","limit":1}'], ['{"label":"a,b"}']]; - yield 'brace in a shape literal' => ['braceLiteral', $shape('label'), ['{"label":"{","limit":1}'], ['{"label":"{"}']]; - yield 'comma in a list literal' => ['commaLiteralList', ['type' => 'array', 'items' => ['type' => 'string']], ['["a,b"]'], ['[1]']]; - yield 'nested mixed' => ['nestedMixed', $nested, ['[[1,"x"]]', '[]'], ['[1]', '"x"']]; - yield 'nested mixed union' => ['nestedMixedUnion', $nested, ['[[1,"x"]]'], ['[1]']]; - yield 'nested mixed brackets' => ['nestedMixedBrackets', $nested, ['[[1,"x"]]'], ['[1]']]; - } - public function testGeneratesCorrectSchemaForEnumParameters(): void { $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'enumParameters'); @@ -431,7 +370,7 @@ public function testGeneratesCorrectSchemaForEnumParameters(): void $this->assertEquals(['type' => 'string', 'description' => 'Backed string enum', 'enum' => ['A', 'B']], $schema['properties']['stringEnum']); $this->assertEquals(['type' => 'integer', 'description' => 'Backed int enum', 'enum' => [1, 2]], $schema['properties']['intEnum']); $this->assertEquals(['type' => 'string', 'description' => 'Unit enum', 'enum' => ['Yes', 'No']], $schema['properties']['unitEnum']); - $this->assertEquals(['type' => ['string', 'null'], 'enum' => ['A', 'B', null], 'default' => null], $schema['properties']['nullableEnum']); + $this->assertEquals(['type' => ['string', 'null'], 'enum' => ['A', 'B', null]], $schema['properties']['nullableEnum']); $this->assertEquals(['type' => 'integer', 'enum' => [1, 2], 'default' => 1], $schema['properties']['enumWithDefault']); $this->assertEqualsCanonicalizing(['stringEnum', 'intEnum', 'unitEnum'], $schema['required']); } @@ -446,7 +385,7 @@ public function testGeneratesCorrectSchemaForArrayTypeDeclarations(): void $this->assertSame('{}', json_encode($schema['properties']['genericArray']['items'])); $this->assertEquals(['type' => 'array', 'description' => 'Array of strings', 'items' => ['type' => 'string']], $schema['properties']['stringArray']); $this->assertEquals(['type' => 'array', 'description' => 'Array of integers', 'items' => ['type' => 'integer']], $schema['properties']['intArray']); - $this->assertEquals(['type' => 'object', 'description' => 'Mixed array map'], $schema['properties']['mixedMap']); + $this->assertEquals(['type' => 'object', 'description' => 'Mixed array map', 'additionalProperties' => new \stdClass()], $schema['properties']['mixedMap']); $this->assertArrayHasKey('type', $schema['properties']['objectLikeArray']); $this->assertEquals('object', $schema['properties']['objectLikeArray']['type']); $this->assertArrayHasKey('properties', $schema['properties']['objectLikeArray']); @@ -461,7 +400,7 @@ public function testRecoversItemsTypeForNullableTypedArrays(): void $schema = $this->schemaGenerator->generate($method); // A `|null` suffix must not erase the element type: `string[]|null` keeps `items: {type: string}`. $this->assertEquals(['type' => ['array', 'null'], 'description' => 'Nullable list of strings', 'items' => ['type' => 'string']], $schema['properties']['nullableStrings']); - $this->assertEquals(['type' => ['array', 'null'], 'description' => 'Nullable list of integers', 'default' => null, 'items' => ['type' => 'integer']], $schema['properties']['nullableInts']); + $this->assertEquals(['type' => ['array', 'null'], 'description' => 'Nullable list of integers', 'items' => ['type' => 'integer']], $schema['properties']['nullableInts']); $this->assertEqualsCanonicalizing(['nullableStrings'], $schema['required']); } @@ -475,18 +414,21 @@ public function testKeepsNullableAndUnionKeysOfArrayShapesInsideTheShape(): void 'description' => 'Shape with a nullable key', 'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['string', 'null']]], 'required' => ['name', 'nickname'], + 'additionalProperties' => false, ], $schema['properties']['nullUnionMember']); $this->assertEquals([ 'type' => 'object', 'description' => 'Shape with a nullable key', 'properties' => ['name' => ['type' => 'string'], 'nickname' => ['type' => ['string', 'null']]], 'required' => ['name', 'nickname'], + 'additionalProperties' => false, ], $schema['properties']['nullableMember']); $this->assertEquals([ 'type' => 'object', 'description' => 'Shape with a union key', - 'properties' => ['id' => ['type' => ['integer', 'string']]], + 'properties' => ['id' => ['anyOf' => [['type' => 'integer'], ['type' => 'string']]]], 'required' => ['id'], + 'additionalProperties' => false, ], $schema['properties']['unionMember']); // A top-level `|null` still makes the shape nullable. $this->assertEquals([ @@ -494,6 +436,7 @@ public function testKeepsNullableAndUnionKeysOfArrayShapesInsideTheShape(): void 'description' => 'Nullable shape', 'properties' => ['id' => ['type' => 'integer']], 'required' => ['id'], + 'additionalProperties' => false, ], $schema['properties']['nullableShape']); $this->assertEqualsCanonicalizing(['nullUnionMember', 'nullableMember', 'unionMember', 'nullableShape'], $schema['required']); } @@ -502,11 +445,11 @@ public function testHandlesNullableTypeHintsAndOptionalParameters(): void { $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'nullableAndOptional'); $schema = $this->schemaGenerator->generate($method); - $this->assertEquals(['type' => ['null', 'string'], 'description' => 'Nullable string'], $schema['properties']['nullableString']); - $this->assertEquals(['type' => ['null', 'integer'], 'description' => 'Nullable integer', 'default' => null], $schema['properties']['nullableInt']); + $this->assertEquals(['type' => ['string', 'null'], 'description' => 'Nullable string'], $schema['properties']['nullableString']); + $this->assertEquals(['type' => ['integer', 'null'], 'description' => 'Nullable integer'], $schema['properties']['nullableInt']); $this->assertEquals(['type' => 'string', 'default' => 'default'], $schema['properties']['optionalString']); $this->assertEquals(['type' => 'boolean', 'default' => true], $schema['properties']['optionalBool']); - $this->assertEquals(['type' => 'array', 'default' => [], 'items' => new \stdClass()], $schema['properties']['optionalArray']); + $this->assertEquals(['type' => 'array', 'items' => new \stdClass(), 'default' => []], $schema['properties']['optionalArray']); $this->assertEqualsCanonicalizing(['nullableString'], $schema['required']); } @@ -514,8 +457,8 @@ public function testGeneratesSchemaForPhpUnionTypes(): void { $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'unionTypes'); $schema = $this->schemaGenerator->generate($method); - $this->assertEquals(['type' => ['integer', 'string'], 'description' => 'String or integer'], $schema['properties']['stringOrInt']); - $this->assertEquals(['type' => ['null', 'boolean', 'string'], 'description' => 'Bool, string or null'], $schema['properties']['multiUnion']); + $this->assertEquals(['description' => 'String or integer', 'anyOf' => [['type' => 'integer'], ['type' => 'string']]], $schema['properties']['stringOrInt']); + $this->assertEquals(['description' => 'Bool, string or null', 'anyOf' => [['type' => 'boolean'], ['type' => 'string'], ['type' => 'null']]], $schema['properties']['multiUnion']); $this->assertEqualsCanonicalizing(['stringOrInt', 'multiUnion'], $schema['required']); } @@ -687,82 +630,129 @@ public function testGenerateOutputSchemaForComplexNestedSchema(): void ], $schema); } - public function testEmptyDefaultOfAMapIsAnObject(): void - { - $handler = new class { - /** - * @param array $map - * @param array|null $nullableMap - * @param array $mapOrList - * @param string[] $list - */ - public function handle(array $map = [], ?array $nullableMap = [], array $mapOrList = [], array $list = []): void - { - } - }; - - $properties = $this->schemaGenerator->generate(new \ReflectionMethod($handler, 'handle'))['properties']; - - $this->assertSame('{}', json_encode($properties['map']['default'])); - $this->assertSame('{}', json_encode($properties['nullableMap']['default'])); - $this->assertSame('[]', json_encode($properties['mapOrList']['default'])); - $this->assertSame('[]', json_encode($properties['list']['default'])); - } - - public function testEmptyDefaultFollowsTheFinalType(): void - { - $handler = new class { - /** - * @param array $toArray - * @param array $toDefinition - */ - public function handle( - #[Schema(type: 'array')] - array $toArray = [], - #[Schema(definition: ['type' => 'array'])] - array $toDefinition = [], - #[Schema(type: 'object')] - array $toObject = [], - ): void { - } - - /** - * @param array $map - */ - #[Schema(properties: ['map' => ['type' => 'array']])] - public function method(array $map = []): void - { - } - }; - - $properties = $this->schemaGenerator->generate(new \ReflectionMethod($handler, 'handle'))['properties']; - - $this->assertSame('[]', json_encode($properties['toArray']['default'])); - $this->assertSame('[]', json_encode($properties['toDefinition']['default'])); - $this->assertSame('{}', json_encode($properties['toObject']['default'])); - - $properties = $this->schemaGenerator->generate(new \ReflectionMethod($handler, 'method'))['properties']; - - $this->assertSame('[]', json_encode($properties['map']['default'])); - } - - public function testListOfGenericArraysIsAList(): void - { - $handler = new class { - /** - * @param array[] $maps - * @param array[]|null $nullableMaps - * @param array[] $lists - */ - public function handle($maps, $nullableMaps, $lists): void - { - } - }; - - $properties = $this->schemaGenerator->generate(new \ReflectionMethod($handler, 'handle'))['properties']; - - $this->assertSame('array', $properties['maps']['type']); - $this->assertSame(['array', 'null'], $properties['nullableMaps']['type']); - $this->assertSame('array', $properties['lists']['type']); + public function testDescribesParametersFromNativeTypesAndDocBlocks(): void + { + $this->assertSchema([ + 'type' => 'object', + 'properties' => [ + 'city' => ['type' => 'string', 'description' => 'the city to look up'], + 'fields' => ['type' => 'array', 'items' => ['type' => 'string'], 'description' => 'which fields to return'], + 'point' => [ + 'type' => 'object', + 'properties' => ['label' => ['type' => 'string'], 'lat' => ['type' => 'number']], // sorted by TypeInfo + 'required' => ['lat'], + 'additionalProperties' => false, + ], + 'weights' => ['type' => 'object', 'additionalProperties' => ['type' => 'integer']], + 'days' => ['type' => 'integer', 'default' => 3], + 'unit' => ['type' => 'string', 'enum' => ['c', 'f'], 'default' => 'c'], + ], + 'required' => ['city', 'fields', 'point', 'weights'], + ], 'scalars'); + } + + public function testDescribesObjectParametersFromTheirProperties(): void + { + $this->assertSchema([ + 'type' => 'object', + 'properties' => [ + 'user' => [ + 'type' => 'object', + 'properties' => [ + 'email' => ['type' => 'string', 'format' => 'email'], + 'age' => ['type' => 'integer', 'minimum' => 18], + 'address' => ['anyOf' => [ + [ + 'type' => 'object', + 'properties' => [ + 'street' => ['type' => 'string'], + 'country' => ['type' => 'string', 'default' => 'DE', 'minLength' => 2, 'maxLength' => 2], + ], + 'required' => ['street'], + ], + ['type' => 'null'], + ]], + 'tags' => ['type' => 'array', 'items' => ['type' => 'string'], 'default' => []], + 'birthday' => ['type' => ['string', 'null'], 'format' => 'date-time'], + ], + 'required' => ['email', 'age'], + 'description' => 'the user to create', + ], + ], + 'required' => ['user'], + ], 'dto'); + } + + public function testHoistsDefinitionsOfCyclicClassesToTheRoot(): void + { + $category = [ + 'type' => 'object', + 'properties' => [ + 'name' => ['type' => 'string'], + 'children' => ['type' => 'array', 'items' => ['$ref' => '#/$defs/FixtureCategory'], 'default' => []], + ], + ]; + + $this->assertSchema([ + 'type' => 'object', + 'properties' => ['root' => $category], + 'required' => ['root'], + '$defs' => ['FixtureCategory' => $category], + ], 'tree'); + } + + public function testDescribesVariadicParametersAsLists(): void + { + $this->assertSchema([ + 'type' => 'object', + 'properties' => ['numbers' => ['type' => 'array', 'items' => ['type' => 'integer']]], + ], 'variadic'); + } + + public function testAcceptsAnythingForUntypedParameters(): void + { + $this->assertSchema([ + 'type' => 'object', + 'properties' => ['anything' => new \stdClass()], + 'required' => ['anything'], + ], 'untyped'); + } + + public function testOutputSchemaFromReturnTypeIsOptIn(): void + { + $method = new \ReflectionMethod(SchemaGeneratorObjectFixture::class, 'returnsDto'); + + $this->assertNull($this->schemaGenerator->generateOutputSchema($method)); + + $schema = (new SchemaGenerator(outputSchemaFromReturnType: true))->generateOutputSchema($method); + $this->assertNotNull($schema); + $this->assertSame('object', $schema['type']); + $this->assertSame(['email', 'age'], $schema['required']); + } + + public function testExplicitOutputSchemaWins(): void + { + $generator = new SchemaGenerator(outputSchemaFromReturnType: true); + + $this->assertSame( + ['type' => 'object', 'properties' => ['explicit' => ['type' => 'boolean']]], + $generator->generateOutputSchema(new \ReflectionMethod(SchemaGeneratorObjectFixture::class, 'explicitOutput')), + ); + $this->assertNull($generator->generateOutputSchema(new \ReflectionMethod(SchemaGeneratorObjectFixture::class, 'returnsArray'))); + } + + /** + * @param array $expected + */ + private function assertSchema(array $expected, string $method): void + { + $schema = $this->schemaGenerator->generate(new \ReflectionMethod(SchemaGeneratorObjectFixture::class, $method)); + + $this->assertSame( + json_encode($expected, \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES), + json_encode($schema, \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES), + ); + + new Tool('probe', null, $schema, null, null); } } diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorUnionTest.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorUnionTest.php index 859f2b034..3d1d314a9 100644 --- a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorUnionTest.php +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorUnionTest.php @@ -11,111 +11,35 @@ namespace Mcp\Tests\Unit\Server\Capability\Discovery; -use Mcp\Server\Capability\Discovery\DocBlockParser; use Mcp\Server\Capability\Discovery\SchemaGenerator; use Mcp\Server\Capability\Discovery\SchemaValidator; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; class SchemaGeneratorUnionTest extends TestCase { - /** - * @return iterable}> - */ - public static function unions(): iterable - { - yield 'not a union' => ['string', ['string']]; - yield 'simple union' => ['int|string', ['int', 'string']]; - yield 'three branches' => ['int|string|bool', ['int', 'string', 'bool']]; - yield 'array branch alongside a scalar' => ['string[]|int', ['string[]', 'int']]; - yield 'generic branch alongside a scalar' => ['array|int', ['array', 'int']]; - - // The `|` here parameterises the outer type; splitting it would turn - // one type into two that do not exist. - yield 'union inside a generic' => ['array', ['array']]; - yield 'union inside a shape' => ['array{a: int|string}', ['array{a: int|string}']]; - yield 'nested generics' => ['array>', ['array>']]; - yield 'shape beside a scalar' => ['array{a: int|string}|null', ['array{a: int|string}', 'null']]; - - yield 'whitespace is trimmed' => ['int | string', ['int', 'string']]; - yield 'empty branches are dropped' => ['int||string', ['int', 'string']]; - } - - /** - * @param list $expected - */ - #[DataProvider('unions')] - #[TestDox('a type string splits on its top-level union only')] - public function testSplitUnion(string $type, array $expected): void - { - $this->assertSame($expected, SchemaGenerator::splitUnion($type)); - } - - /** - * @return iterable}> - */ - public static function mappedTypes(): iterable - { - yield 'scalar union' => ['int|string', ['integer', 'string']]; - - // The regression: the `[]` check ran before the union split, so this - // came back as ['array'] and the `int` branch vanished. - yield 'array beside a scalar keeps both' => ['string[]|int', ['array', 'integer']]; - yield 'scalar before an array keeps both' => ['int|string[]', ['integer', 'array']]; - yield 'generic beside a scalar keeps both' => ['array|bool', ['array', 'boolean']]; - - yield 'a generic union is still one array' => ['array', ['array']]; - yield 'a shape is still one object' => ['array{a: int|string}', ['object']]; - - yield 'a literal stays a string' => ["'a[]'", ['string']]; - yield 'an array of literals stays an array' => ["'asc'[]", ['array']]; - yield 'an array of bracket literals stays an array' => ['"a[]"[]', ['array']]; - } - - /** - * @param list $expected - */ - #[DataProvider('mappedTypes')] - #[TestDox('a union maps to every JSON Schema type its branches need')] - public function testMappedTypes(string $type, array $expected): void - { - $generator = new SchemaGenerator(new DocBlockParser()); - - $map = new \ReflectionMethod($generator, 'mapPhpTypeToJsonSchemaType'); - - $this->assertSame($expected, $map->invoke($generator, $type)); - } - #[TestDox('a generated schema keeps both branches of an array-or-scalar union')] public function testGeneratedUnionSchemaKeepsBothBranches(): void { - $schema = (new SchemaGenerator(new DocBlockParser())) + $schema = (new SchemaGenerator()) ->generate(new \ReflectionMethod(UnionFixture::class, 'handle')); - // `items` constrains the instance only when it *is* an array, so it - // coexists with the scalar branch instead of excluding it. - $this->assertSame(['array', 'integer'], $schema['properties']['mixedish']['type']); - // The array branch keeps its own element-type constraint rather than - // losing it to the union as a whole (`items` would otherwise be `{}`). - $this->assertSame(['type' => 'string'], $schema['properties']['mixedish']['items']); - - $this->assertSame(['integer', 'string'], $schema['properties']['scalars']['type']); - $this->assertSame('array', $schema['properties']['arrayOnly']['type']); + $this->assertSame([['type' => 'array', 'items' => ['type' => 'string']], ['type' => 'integer']], $schema['properties']['mixedish']['anyOf']); + $this->assertSame([['type' => 'integer'], ['type' => 'string']], $schema['properties']['scalars']['anyOf']); + $this->assertSame(['type' => 'string'], $schema['properties']['arrayOnly']['items']); $this->assertSame(['array', 'null'], $schema['properties']['nullableArray']['type']); - // Two array branches with different element types can't both be - // enforced without rejecting a value that is valid under one of - // them, so neither constraint is applied (falls back to the "matches - // anything" empty schema `ensureArrayItems()` defaults every array to). - $this->assertSame('array', $schema['properties']['disagreeingArrays']['type']); - $this->assertEquals(new \stdClass(), $schema['properties']['disagreeingArrays']['items']); + // Each array branch keeps its own element type. + $this->assertSame([ + ['type' => 'array', 'items' => ['type' => 'integer']], + ['type' => 'array', 'items' => ['type' => 'string']], + ], $schema['properties']['disagreeingArrays']['anyOf']); } #[TestDox('a value from either branch validates against the generated schema')] public function testBothBranchesValidate(): void { - $schema = (new SchemaGenerator(new DocBlockParser())) + $schema = (new SchemaGenerator()) ->generate(new \ReflectionMethod(UnionFixture::class, 'handle')); $validator = new SchemaValidator(); @@ -127,8 +51,8 @@ public function testBothBranchesValidate(): void // A float is in neither branch, so it is still refused. $this->assertNotSame([], $validator->validateAgainstJsonSchema([...$base, 'mixedish' => 1.5], $schema)); - // Neither element type is enforced, so both branches' values pass. $this->assertSame([], $validator->validateAgainstJsonSchema([...$base, 'mixedish' => 42, 'disagreeingArrays' => ['a', 'b']], $schema)); $this->assertSame([], $validator->validateAgainstJsonSchema([...$base, 'mixedish' => 42, 'disagreeingArrays' => [1, 2]], $schema)); + $this->assertNotSame([], $validator->validateAgainstJsonSchema([...$base, 'mixedish' => 42, 'disagreeingArrays' => [1, 'a']], $schema)); } } From 0a684e516ad41685e30583139a2b663df76bf036 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 12 Nov 2026 00:24:26 +0100 Subject: [PATCH 2/2] [Server] Describe tool inputs and outputs with their direction Input schemas only list the properties a client can send (Direction::Input), and output schemas require the properties that are always present (Direction::Output). A dialect without schema URI makes stripping $schema unnecessary, and definitions keep the plain class name since each schema is a document of its own. --- CHANGELOG.md | 2 +- .../Capability/Discovery/SchemaGenerator.php | 16 ++++++++++++---- .../Capability/Discovery/SchemaGeneratorTest.php | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a7d0edb5..a32b86a0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Discovery * [BC Break] Generate tool input and output schemas with `symfony/json-schema`. Unions are described with `anyOf`, `null` comes last in `type`, `default: null` is no longer emitted, array shapes are closed with `additionalProperties: false`, string-keyed maps get `additionalProperties`, and a class parameter is described by its properties. - * [BC Break] `SchemaGenerator` takes a `Symfony\Component\JsonSchema\SchemaGeneratorInterface`, a `TypeResolverInterface` and a `Configuration` instead of a `DocBlockParser`. + * [BC Break] `SchemaGenerator` takes a `Symfony\Component\JsonSchema\SchemaGeneratorInterface`, a `TypeResolverInterface` and the input and output `Configuration` instead of a `DocBlockParser`. * [BC Break] Describe `array` and `array` as a JSON array, so a JSON object is rejected. Use `array` for a map. * [BC Break] Remove `DocBlockParser::getParamTypeString()` and `DocBlockParser::getParamDescription()`. * Add the `outputSchemaFromReturnType` argument of `SchemaGenerator`, to describe the output of a tool that returns an object by its class. diff --git a/src/Server/Capability/Discovery/SchemaGenerator.php b/src/Server/Capability/Discovery/SchemaGenerator.php index bc493ddb3..01480ba64 100644 --- a/src/Server/Capability/Discovery/SchemaGenerator.php +++ b/src/Server/Capability/Discovery/SchemaGenerator.php @@ -17,6 +17,9 @@ use Mcp\Server\Exception\BadMethodCallException; use Mcp\Server\Exception\InvalidArgumentException; use Symfony\Component\JsonSchema\Configuration; +use Symfony\Component\JsonSchema\Dialect; +use Symfony\Component\JsonSchema\Direction; +use Symfony\Component\JsonSchema\NullSyntax; use Symfony\Component\JsonSchema\Schema as JsonSchema; use Symfony\Component\JsonSchema\SchemaGenerator as JsonSchemaGenerator; use Symfony\Component\JsonSchema\SchemaGeneratorInterface as JsonSchemaGeneratorInterface; @@ -35,11 +38,18 @@ final class SchemaGenerator implements SchemaGeneratorInterface private readonly JsonSchemaGeneratorInterface $generator; private readonly TypeResolverInterface $typeResolver; + /** + * Each schema is a document of its own, so definitions are named after their class only. + * + * @param Configuration $configuration The configuration of input schemas + * @param Configuration $outputConfiguration The configuration of output schemas + */ public function __construct( ?JsonSchemaGeneratorInterface $generator = null, ?TypeResolverInterface $typeResolver = null, - private readonly Configuration $configuration = new Configuration(), + private readonly Configuration $configuration = new Configuration(new Dialect('#/$defs/', NullSyntax::Union), definitionName: '', direction: Direction::Input), private readonly bool $outputSchemaFromReturnType = false, + private readonly Configuration $outputConfiguration = new Configuration(new Dialect('#/$defs/', NullSyntax::Union), definitionName: '', direction: Direction::Output), ) { $this->generator = $generator ?? JsonSchemaGenerator::create(); $this->typeResolver = $typeResolver ?? TypeResolver::create(); @@ -72,7 +82,6 @@ public function generate(\Reflector $reflection): array } $schema = self::toArray($this->generator->generateForParameters(array_values($parameters), $this->configuration)->flatten()); - unset($schema['$schema']); $methodProperties = $methodSchema['properties'] ?? []; $methodRequired = $methodSchema['required'] ?? []; @@ -129,8 +138,7 @@ public function generateOutputSchema(\Reflector $reflection): ?array return null; } - $schema = self::toArray($this->generator->generate($type, $this->configuration)->flatten()); - unset($schema['$schema']); + $schema = self::toArray($this->generator->generate($type, $this->outputConfiguration)->flatten()); return $schema; } diff --git a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php index 3ee53aa9c..f90b085c5 100644 --- a/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php +++ b/tests/Unit/Server/Capability/Discovery/SchemaGeneratorTest.php @@ -727,7 +727,7 @@ public function testOutputSchemaFromReturnTypeIsOptIn(): void $schema = (new SchemaGenerator(outputSchemaFromReturnType: true))->generateOutputSchema($method); $this->assertNotNull($schema); $this->assertSame('object', $schema['type']); - $this->assertSame(['email', 'age'], $schema['required']); + $this->assertSame(['email', 'age', 'address', 'tags', 'birthday'], $schema['required']); } public function testExplicitOutputSchemaWins(): void