[QA] Raise PHPStan to level 8 by chr-hertel · Pull Request #492 · modelcontextprotocol/php-sdk

chr-hertel · 2026-09-01T20:44:39Z

Raises phpstan.dist.neon from level 6 to 8 and fixes every resulting error — no new baseline entries, no ignore comments.

Bugs this turned up

Tool laundered malformed input schemas. A member of the wrong type was published as written — properties: "bad" went out on the wire as "bad". fromArray() reads untrusted data, so both properties and required are now refused when they are not what they claim, rather than coerced into a tool contract nobody declared.
CompletionCompleteRequest never validated argument.name / argument.value. A request missing either completed against an empty prefix instead of being rejected. Both are now required strings. An empty value is still the legitimate "offer me everything" query.
Discoverer::getClassFromFile() returned type names it could not load, which then threw out of new ReflectionClass. It returns null now, and also recognises interfaces and traits.
ReferenceHandler could not take [$object, 'method'] even though the Handler type alias has always declared array{0: object|string, 1: string} — the object went to getClassInstance(string).
Unchecked false returns from file_get_contents, stream_get_contents, proc_open, preg_split and json_encode, several of which would have surfaced as a TypeError deep inside content or transport code.
ProtocolVersion::handshakeVersions() / modernVersions() promised a non-empty list while deriving it by filtering; the invariant is now explicit instead of implied. ElicitRequest::getParams() states its mode invariants the same way.

Type-model corrections

Behaviour-preserving, but they are what made level 8 reachable:

Page is generic, so RegistryInterface::getTools() returns Page<Tool> and the list handlers type-check against their results.
Tool::$inputSchema no longer claims properties and required are always present — they are not, and forcing them changed serialization.
ServerCapabilities / ClientCapabilities serialize \stdClass, not bare object.
Session::set() / forget() walk the key with array_pop + foreach instead of array_shift in a while, which is the same traversal with a provable last segment.

`fromArray()` parameters

The fromArray() methods in src/Schema declared narrow array shapes for data they parse off the wire and validate at runtime, so the shapes were unenforceable at every call site: a decoded JSON element is array<mixed, mixed>, and no runtime check turns that into a sealed shape.

They now read FooData|array<mixed> — the union keeps the documented shape visible while accepting what callers actually have. Note this documents rather than constrains: as before, neither arm rejects a malformed literal.

The confirmation that the shapes were fiction is in the tests: 33 pre-existing @phpstan-ignore argument.type comments existed only so tests could feed malformed data to these parsers. Widening made every one of them unmatched, and they are deleted here.

Tests

New: CompletionCompleteRequestTest (reference and argument shapes end to end), ListCompletionProviderTest (including the numeric→string coercion), malformed-schema rejections in ToolTest, the [$object, 'method'] and non-callable handler paths in ReferenceHandlerTest, the unloadable-class skip in DiscoveryTest, and Client::readResource('').

Test edits elsewhere are narrowing, not weakening — assertNotNull / assertInstanceOf before a member access, JSON_THROW_ON_ERROR where a string|false was fed to json_decode.

make cs, make phpstan and make tests are green.

Copilot

🟡 Changes recommended

The advertised providerClass completion path still throws during attribute construction, and completion argument errors identify the wrong field.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Raises PHPStan from level 6 to 8, correcting type models, unsafe return handling, and several runtime defects across discovery, transport, schema, and registry code.

Changes:

Strengthens static types and wire-data parsing.
Handles fallible PHP APIs and nullable values safely.
Fixes discovery, completion-provider, handler, and schema behavior.

File summaries

File	Description
`phpstan.dist.neon`	Enables PHPStan level 8.
`src/Client.php`	Tightens connection and URI validation.
`src/Client/Stateless/ToolCatalog.php`	Handles malformed tool entries safely.
`src/Client/Transport/HttpTransport.php`	Narrows fibers and nullable streams.
`src/Client/Transport/StdioTransport.php`	Validates process and fiber values.
`src/Capability/Attribute/CompletionProvider.php`	Documents provider source types.
`src/Capability/Attribute/Schema.php`	Corrects numeric schema types.
`src/Capability/Completion/ListCompletionProvider.php`	Normalizes scalar completion values.
`src/Capability/Discovery/Discoverer.php`	Fixes discovery and provider resolution.
`src/Capability/Discovery/SchemaGenerator.php`	Improves reflected type handling.
`src/Capability/Discovery/SchemaGeneratorInterface.php`	Reuses the tool schema type.
`src/Capability/Discovery/SchemaValidator.php`	Safely formats validation errors.
`src/Capability/Formatter/ResourceResultFormatter.php`	Handles failed file reads.
`src/Capability/Registry.php`	Adds generic pagination types.
`src/Capability/Registry/Loader/ReflectedElementLoader.php`	Supports named-function handlers safely.
`src/Capability/Registry/ReferenceHandler.php`	Validates callable handlers.
`src/Capability/Registry/ResourceTemplateReference.php`	Handles failed URI splitting.
`src/Capability/RegistryInterface.php`	Adds typed page returns.
`src/Schema/ClientCapabilities.php`	Corrects serialized object types.
`src/Schema/Content/AudioContent.php`	Broadens wire-data input typing.
`src/Schema/Content/BlobResourceContents.php`	Handles stream and file failures.
`src/Schema/Content/EmbeddedResource.php`	Corrects MIME and serialization handling.
`src/Schema/Content/ImageContent.php`	Handles failed image reads.
`src/Schema/Content/PromptMessage.php`	Broadens wire-data input typing.
`src/Schema/Content/ResourceLink.php`	Corrects serialized discriminator typing.
`src/Schema/Content/SamplingMessage.php`	Broadens wire-data input typing.
`src/Schema/Content/TextContent.php`	Broadens wire-data input typing.
`src/Schema/Content/TextResourceContents.php`	Broadens wire-data input typing.
`src/Schema/Content/ToolUseContent.php`	Corrects serialized discriminator typing.
`src/Schema/Elicitation/AbstractSchemaDefinition.php`	Refines serialized base shape.
`src/Schema/Elicitation/BooleanSchemaDefinition.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/ElicitationSchema.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/EnumSchemaDefinition.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/MultiSelectEnumSchemaDefinition.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/NumberSchemaDefinition.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/StringSchemaDefinition.php`	Broadens wire-data input typing.
`src/Schema/Elicitation/TitledEnumSchemaDefinition.php`	Validates and normalizes enum pairs.
`src/Schema/Elicitation/TitledMultiSelectEnumSchemaDefinition.php`	Validates and normalizes enum pairs.
`src/Schema/Enum/ProtocolVersion.php`	Guards empty revision groups.
`src/Schema/Extension/Apps/UiToolMeta.php`	Normalizes visibility as a list.
`src/Schema/Icon.php`	Broadens wire-data input typing.
`src/Schema/Implementation.php`	Safely hydrates icon data.
`src/Schema/JsonRpc/Error.php`	Broadens wire-data input typing.
`src/Schema/JsonRpc/Response.php`	Validates and narrows result members.
`src/Schema/Page.php`	Makes pages generic by item type.
`src/Schema/Request/CallToolRequest.php`	Corrects argument object typing.
`src/Schema/Request/CompletionCompleteRequest.php`	Validates completion arguments.
`src/Schema/Request/ElicitRequest.php`	Omits impossible null parameters.
`src/Schema/ResourceDefinition.php`	Broadens wire-data input typing.
`src/Schema/ResourceTemplate.php`	Broadens wire-data input typing.
`src/Schema/Result/ElicitResult.php`	Broadens wire-data input typing.
`src/Schema/Result/InitializeResult.php`	Broadens wire-data input typing.
`src/Schema/Result/ListRootsResult.php`	Broadens wire-data input typing.
`src/Schema/Root.php`	Broadens wire-data input typing.
`src/Schema/ServerCapabilities.php`	Corrects serialized object types.
`src/Schema/Tool.php`	Refines and normalizes schema types.
`src/Schema/ToolChoice.php`	Validates mixed wire input.
`src/Server/Builder.php`	Supplies default discovery patterns.
`src/Server/ClientGateway.php`	Narrows sampling message conversion.
`src/Server/Protocol.php`	Narrows dispatched event types.
`src/Server/Session/Session.php`	Simplifies nested key traversal.
`src/Server/Stateless/StatelessProtocol.php`	Allows nullable error identifiers.
`src/Server/Subscription/PublishingEventDispatcher.php`	Corrects listener callable typing.
`src/Server/Subscription/RegistryChangePublisher.php`	Corrects listener return typing.
`src/Server/Transport/Http/Middleware/OAuthProxyMiddleware.php`	Makes JSON encoding fail explicitly.
`src/Server/Transport/ManagesTransportCallbacks.php`	Marks the response finder nullable.
`src/Server/Transport/StdioTransport.php`	Validates limits and narrows fibers.
`src/Server/Transport/StreamableHttpTransport.php`	Safely captures and handles fibers.
`src/Server/Wire/InboundClassifier.php`	Narrows nullable version handling.
`examples/client/stdio_elicitation.php`	Handles URL elicitation and null titles.
`examples/server/bootstrap.php`	Selects shutdown behavior by result type.
`examples/server/mcp-apps/WeatherApp.php`	Handles template read failures.
`examples/server/oauth-microsoft/McpElements.php`	Handles failed timestamp parsing.
`tests/Conformance/Elements.php`	Uses throwing JSON encoding.
`tests/Inspector/Http/HttpSchemaShowcaseTest.php`	Handles failed regex normalization.
`tests/Inspector/InspectorSnapshotTestCase.php`	Asserts snapshot reads succeed.
`tests/Inspector/Stdio/StdioCustomDependenciesTest.php`	Handles failed regex normalization.
`tests/Integration/HandshakeTest.php`	Narrows nullable server information.
`tests/Integration/SamplingTest.php`	Narrows sampling request data.
`tests/Integration/SamplingToolsTest.php`	Narrows tool-sampling requests.
`tests/Unit/Capability/Discovery/DiscoveryTest.php`	Adds static-analysis assertions.
`tests/Unit/Capability/Discovery/DocBlockParserTest.php`	Narrows nullable descriptions.
`tests/Unit/Capability/Discovery/SchemaValidatorTest.php`	Uses throwing JSON encoding.
`tests/Unit/Capability/Formatter/PromptResultFormatterTest.php`	Narrows formatted resource content.
`tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php`	Narrows closure handlers.
`tests/Unit/Capability/Registry/Loader/DiscoveryLoaderTest.php`	Narrows closure handlers.
`tests/Unit/Capability/Registry/Loader/ExplicitElementLoaderTest.php`	Narrows resource references.
`tests/Unit/Capability/Registry/Loader/ReflectedElementLoaderResourceTitleTest.php`	Narrows resource references.
`tests/Unit/Capability/RegistryTest.php`	Corrects mock and closure types.
`tests/Unit/Client/Transport/StdioTransportTest.php`	Asserts stream creation succeeds.
`tests/Unit/Schema/ClientCapabilitiesTest.php`	Narrows serialized capabilities.
`tests/Unit/Schema/Content/ImageContentTest.php`	Uses safe serialization assertions.
`tests/Unit/Schema/Content/PromptMessageTest.php`	Uses throwing JSON encoding.
`tests/Unit/Schema/Content/ResourceLinkTest.php`	Narrows optional serialized fields.
`tests/Unit/Schema/Content/SamplingMessageTest.php`	Narrows content blocks and JSON.
`tests/Unit/Schema/Content/ToolResultContentTest.php`	Uses throwing JSON encoding.
`tests/Unit/Schema/Content/ToolUseContentTest.php`	Uses throwing JSON encoding.
`tests/Unit/Schema/Elicitation/BooleanSchemaDefinitionTest.php`	Removes obsolete PHPStan suppression.
`tests/Unit/Schema/Elicitation/ElicitationSchemaTest.php`	Updates mixed-data assertions.
`tests/Unit/Schema/Elicitation/EnumSchemaDefinitionTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/Elicitation/MultiSelectEnumSchemaDefinitionTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/Elicitation/NumberSchemaDefinitionTest.php`	Removes obsolete PHPStan suppression.
`tests/Unit/Schema/Elicitation/StringSchemaDefinitionTest.php`	Removes obsolete PHPStan suppression.
`tests/Unit/Schema/Elicitation/TitledEnumSchemaDefinitionTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/Elicitation/TitledMultiSelectEnumSchemaDefinitionTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/Extension/Apps/McpAppsTest.php`	Narrows optional serialized metadata.
`tests/Unit/Schema/Extension/CapabilitiesExtensionsTest.php`	Narrows extension objects.
`tests/Unit/Schema/IconTest.php`	Narrows nullable icon fields.
`tests/Unit/Schema/ImplementationTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/NonObjectOutputSchemaTest.php`	Updates optional schema assertions.
`tests/Unit/Schema/Request/CreateSamplingMessageRequestTest.php`	Narrows request payload fields.
`tests/Unit/Schema/ResourceDefinitionTest.php`	Narrows optional title assertions.
`tests/Unit/Schema/ResourceTemplateTest.php`	Narrows optional title assertions.
`tests/Unit/Schema/Result/CallToolResultTest.php`	Uses throwing JSON encoding.
`tests/Unit/Schema/Result/CreateSamplingMessageResultTest.php`	Narrows content and serialization.
`tests/Unit/Schema/Result/ElicitResultTest.php`	Removes obsolete PHPStan suppression.
`tests/Unit/Schema/Result/ListRootsResultTest.php`	Updates malformed-data and JSON tests.
`tests/Unit/Schema/ServerCapabilitiesTest.php`	Narrows serialized capability objects.
`tests/Unit/Schema/ToolChoiceTest.php`	Removes obsolete PHPStan suppressions.
`tests/Unit/Schema/ToolTest.php`	Narrows optional schema members.
`tests/Unit/Server/BuilderTest.php`	Narrows nullable configuration.
`tests/Unit/Server/Handler/Request/CallToolHandlerTest.php`	Narrows tool result types.
`tests/Unit/Server/Stateless/StatelessProtocolTest.php`	Narrows streams and frame data.
`tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php`	Uses throwing JSON encoding.
`tests/Unit/Server/Transport/Http/OAuth/JwtTokenValidatorTest.php`	Guards response sequence access.
`tests/Unit/Server/Transport/StdioTransportTest.php`	Asserts stream creation succeeds.

Review details

Files reviewed: 126/126 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

Named function handlers remain unsupported and malformed tool schemas are silently rewritten.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 127/127 changed files
Comments generated: 2
Review effort level: Balanced

chr-hertel · 2026-09-07T23:58:13Z

+                    $handler = $data['handler'];
+                    $name = $data['name'] ?? ($handler instanceof \Closure ? 'closure_tool_'.spl_object_id($handler) : $reflection->getName());


You're right, and thanks — I had claimed a live TypeError here and that was wrong. Verified:

HandlerResolver::resolve('named_handler') => InvalidArgumentException: Invalid handler format. HandlerResolver::resolve('strlen') => InvalidArgumentException: Invalid handler format.

So ReflectionFunction only ever comes from a Closure and the getName() arm is unreachable. It stays only because spl_object_id() on Closure|list<object|string>|string cannot be proven at level 8 — it is narrowing, not a fix, and the PR description no longer claims otherwise.

Adding the function_exists() branch to the resolver is a new capability rather than a typing fix, so it belongs in its own PR (as the providerClass removal did in #499) — with the regression coverage for named functions you describe.

chr-hertel · 2026-09-07T23:58:05Z

+        if (\array_key_exists('properties', $normalized)) {
+            $properties = $normalized['properties'];
+            $normalized['properties'] = \is_array($properties) || $properties instanceof \stdClass ? $properties : new \stdClass();
+        }
+
+        if (\array_key_exists('required', $normalized)) {
+            $required = $normalized['required'];
+            $normalized['required'] = \is_array($required) ? $required : null;
+        }


Fixed in 971035b. Both members are now refused rather than replaced:

properties: string => Tool inputSchema "properties" must be an object. required: string => Tool inputSchema "required" must be a list of property names. required: null => OK properties absent => OK

Covered by ToolTest::provideMalformedInputSchemas. The type check moved into normalizeInputSchema() as well, so the method no longer assigns type back over whatever came in.

Fixes all 330 errors the bump surfaces across src/, tests/ and examples/, without new baseline entries or ignore comments.

chr-hertel requested review from CodeWithKyrian, Nyholm and soyuka as code owners September 1, 2026 20:44

chr-hertel requested a balanced review from Copilot September 1, 2026 23:46

Copilot started reviewing on behalf of chr-hertel September 1, 2026 23:47 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread src/Capability/Attribute/CompletionProvider.php Outdated

Comment thread src/Schema/Request/CompletionCompleteRequest.php

chr-hertel force-pushed the phpstan-level-8 branch 2 times, most recently from 0a28008 to 1874811 Compare September 5, 2026 00:43

chr-hertel mentioned this pull request Sep 5, 2026

[Capability] Remove CompletionProvider's dead providerClass argument #499

Merged

chr-hertel force-pushed the phpstan-level-8 branch 2 times, most recently from 319cbe2 to 255bacc Compare September 7, 2026 22:19

chr-hertel requested a balanced review from Copilot September 7, 2026 22:38

Copilot started reviewing on behalf of chr-hertel September 7, 2026 22:38 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

chr-hertel force-pushed the phpstan-level-8 branch 2 times, most recently from 1499b84 to 78f6842 Compare September 7, 2026 23:03

chr-hertel added Server Issues & PRs related to the Server component Client Issues & PRs related to the Client component Schema Issues & PRs related to the Schema component labels Sep 7, 2026

[QA] Raise PHPStan to level 8

971035b

Fixes all 330 errors the bump surfaces across src/, tests/ and examples/, without new baseline entries or ignore comments.

chr-hertel force-pushed the phpstan-level-8 branch from 78f6842 to 971035b Compare September 7, 2026 23:47

Navigation Menu

Uh oh!

[QA] Raise PHPStan to level 8 - #492

[QA] Raise PHPStan to level 8#492
chr-hertel wants to merge 1 commit into
mainfrom
phpstan-level-8

chr-hertel commented Sep 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

chr-hertel Sep 7, 2026

Uh oh!

chr-hertel Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		$handler = $data['handler'];
		$name = $data['name'] ?? ($handler instanceof \Closure ? 'closure_tool_'.spl_object_id($handler) : $reflection->getName());

Uh oh!

Conversation

chr-hertel commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bugs this turned up

Type-model corrections

fromArray() parameters

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

chr-hertel Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

chr-hertel Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chr-hertel commented Sep 1, 2026 •

edited

Loading

`fromArray()` parameters