From 57a116d21969beacfa57454c28d5f64183dfbb31 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Tue, 8 Sep 2026 17:59:37 +0200 Subject: [PATCH 1/7] Add Docblock Check CI to spot invalid @param/@return array types Runs honestype: instruments src and rules, collects docblock type mismatches while the test suite runs, then lists them PHPStan-style and fails the job when any are found. Also fixes one surfaced mismatch: configure() @param is string[], matching its Assert::allString and list values. --- .github/workflows/docblock_check.yaml | 65 +++++++++++++++++++ .../StringClassNameToClassConstantRector.php | 2 +- .../NodeAnalyzer/CallTypesResolver.php | 9 ++- 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/docblock_check.yaml diff --git a/.github/workflows/docblock_check.yaml b/.github/workflows/docblock_check.yaml new file mode 100644 index 00000000000..ebefcf6b7e5 --- /dev/null +++ b/.github/workflows/docblock_check.yaml @@ -0,0 +1,65 @@ +name: Docblock Check + +on: + pull_request: null + +env: + COMPOSER_ROOT_VERSION: "dev-main" + DOCBLOCK_CHECK_LOG: "/tmp/docblock-check.log" + +jobs: + docblock_check: + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - uses: actions/checkout@v5 + + - + uses: shivammathur/setup-php@v2 + with: + php-version: 8.4 + coverage: none + ini-values: zend.assertions=1 + + - uses: "ramsey/composer-install@v4" + + - + uses: actions/setup-go@v5 + with: + go-version: "1.26" + + - + name: Install honestype docblock checker + run: | + GOPROXY=direct GOSUMDB=off go install github.com/rectorphp/honestype@main + echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" + + - + name: Instrument sources + run: | + honestype instrument src + honestype instrument rules + + # Load the helper for every PHP process, including any parallel test + # workers which do not inherit a -d auto_prepend_file flag. + - + name: Enable runtime helper globally + run: | + INI="$(php -i | grep '^Loaded Configuration File' | awk '{print $NF}')" + echo "auto_prepend_file=$GITHUB_WORKSPACE/src/docblock_check.php" | sudo tee -a "$INI" + + - + name: Run tests to collect docblock type mismatches + run: vendor/bin/phpunit tests rules-tests utils/phpstan/tests || true + + - + name: Restore sources + if: always() + run: | + git checkout -- src rules + rm -f src/docblock_check.php rules/docblock_check.php + + - + name: List docblock type mismatches + run: honestype report "$DOCBLOCK_CHECK_LOG" diff --git a/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php b/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php index 9029e7b5b46..bfc2fa1e6a2 100644 --- a/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php +++ b/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php @@ -104,7 +104,7 @@ public function refactor(Node $node): ClassConstFetch|null } /** - * @param array $configuration + * @param string[] $configuration */ public function configure(array $configuration): void { diff --git a/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php b/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php index 857b8fcfeaa..c5b441ec67c 100644 --- a/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php +++ b/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php @@ -47,6 +47,11 @@ public function resolveStrictTypesFromCalls(array $calls): array return []; } + // must be int + if (! is_int($position)) { + continue; + } + /** @var Arg $arg */ $staticTypesByArgumentPosition[$position][] = $this->resolveStrictArgValueType($arg); } @@ -110,8 +115,8 @@ private function correctSelfType(Type $argValueType): Type } /** - * @param array $staticTypesByArgumentPosition - * @return array + * @param array $staticTypesByArgumentPosition + * @return array */ private function unionToSingleType(array $staticTypesByArgumentPosition, bool $removeMixedArray = false): array { From 840da8296e229164e66c0d487b8f4d69ef994722 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 8 Sep 2026 22:21:06 +0200 Subject: [PATCH 2/7] require stmts in array --- src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index f05f87dc7e8..b632b3b3594 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -461,6 +461,8 @@ private function nodeScopeResolverProcessNodes( MutatingScope $mutatingScope, callable $nodeCallback ): void { + Assert::allIsInstanceOf($stmts, Stmt::class); + try { $this->nodeScopeResolver->processNodes($stmts, $mutatingScope, $nodeCallback); } catch (ParserErrorsException|ParserException|ShouldNotHappenException|UndefinedVariableException) { From 7ddf143b2da03701caee173180663459a37d000a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 8 Sep 2026 22:31:04 +0200 Subject: [PATCH 3/7] fix property hook block body scope resolution Claude-Session: https://claude.ai/code/session_01X2RJNwg9ij4Z8b725WTVB6 --- src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index b632b3b3594..bc4b6a8591f 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -631,7 +631,7 @@ private function processProperty(Property $property, MutatingScope $mutatingScop /** @var Stmt[] $stmts */ $stmts = $hook->body instanceof Expr ? [new Expression($hook->body)] - : [$hook->body]; + : $hook->body; $this->nodeScopeResolverProcessNodes($stmts, $mutatingScope, $nodeCallback); } } From 88121a957343ea12b74f12eba2342dec2d58cf99 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 8 Sep 2026 22:58:32 +0200 Subject: [PATCH 4/7] fix order-dependent test state leaks in tests/ Claude-Session: https://claude.ai/code/session_01X2RJNwg9ij4Z8b725WTVB6 --- rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php | 2 +- .../NodeAnalyzer/ClassMethodParamTypeCompleter.php | 4 ++-- src/PhpParser/NodeTraverser/RectorNodeTraverser.php | 2 ++ tests/Config/RectorConfigTest.php | 5 +++++ tests/Configuration/ConfigurationFactoryTest.php | 7 +++++++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php b/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php index c5b441ec67c..f315ff24203 100644 --- a/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php +++ b/rules/TypeDeclaration/NodeAnalyzer/CallTypesResolver.php @@ -35,7 +35,7 @@ public function __construct( /** * @param MethodCall[]|StaticCall[] $calls - * @return array + * @return array */ public function resolveStrictTypesFromCalls(array $calls): array { diff --git a/rules/TypeDeclaration/NodeAnalyzer/ClassMethodParamTypeCompleter.php b/rules/TypeDeclaration/NodeAnalyzer/ClassMethodParamTypeCompleter.php index 104ac4f9695..91aeb065f16 100644 --- a/rules/TypeDeclaration/NodeAnalyzer/ClassMethodParamTypeCompleter.php +++ b/rules/TypeDeclaration/NodeAnalyzer/ClassMethodParamTypeCompleter.php @@ -25,7 +25,7 @@ public function __construct( } /** - * @param array $classParameterTypes + * @param array $classParameterTypes */ public function complete(ClassMethod $classMethod, array $classParameterTypes, int $maxUnionTypes): ?ClassMethod { @@ -73,7 +73,7 @@ public function complete(ClassMethod $classMethod, array $classParameterTypes, i private function shouldSkipArgumentStaticType( ClassMethod $classMethod, Type $argumentStaticType, - int $position, + int|string $position, int $maxUnionTypes ): bool { if ($argumentStaticType instanceof MixedType) { diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index 6dfdc9a44b6..fdaf7000c51 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -203,6 +203,8 @@ private function traverseNode(Node $node): void */ private function traverseArray(array $nodes): array { + Assert::allIsInstanceOf($nodes, Node::class); + $doNodes = []; foreach ($nodes as $i => $node) { if (! $node instanceof Node) { diff --git a/tests/Config/RectorConfigTest.php b/tests/Config/RectorConfigTest.php index 0f054548c7c..04123037f37 100644 --- a/tests/Config/RectorConfigTest.php +++ b/tests/Config/RectorConfigTest.php @@ -27,6 +27,11 @@ protected function setUp(): void // the registered-rule lists so the assertions hold whether this class // runs alone or batched into one warm process by a parallel runner RectorConfig::resetRecreated(); + + // the container is shared across tests, so its per-rule dedupe maps persist and would + // swallow a re-registration; reset them so REGISTERED_RECTOR_RULES fills regardless of order + self::getContainer()->resetRuleConfigurations(); + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, []); SimpleParameterProvider::setParameter(Option::ROOT_STANDALONE_REGISTERED_RULES, []); } diff --git a/tests/Configuration/ConfigurationFactoryTest.php b/tests/Configuration/ConfigurationFactoryTest.php index 0085c8c8ac2..adae31167c1 100644 --- a/tests/Configuration/ConfigurationFactoryTest.php +++ b/tests/Configuration/ConfigurationFactoryTest.php @@ -15,6 +15,13 @@ final class ConfigurationFactoryTest extends AbstractLazyTestCase { + protected function tearDown(): void + { + SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, false); + SimpleParameterProvider::setParameter(Option::SOURCE, []); + SimpleParameterProvider::setParameter(Option::PATHS, []); + } + public function test(): void { $configurationFactory = $this->make(ConfigurationFactory::class); From 8b57ac98b4d79072ae718b0d6dbda759c485791a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 9 Sep 2026 09:40:19 +0200 Subject: [PATCH 5/7] traverse accepts null --- .github/workflows/docblock_check.yaml | 3 +-- phpstan.neon | 3 +++ rules/Php70/Rector/List_/EmptyListRector.php | 2 +- src/PhpParser/NodeTraverser/RectorNodeTraverser.php | 4 +--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docblock_check.yaml b/.github/workflows/docblock_check.yaml index ebefcf6b7e5..66694083144 100644 --- a/.github/workflows/docblock_check.yaml +++ b/.github/workflows/docblock_check.yaml @@ -38,8 +38,7 @@ jobs: - name: Instrument sources run: | - honestype instrument src - honestype instrument rules + honestype instrument src rules tests # Load the helper for every PHP process, including any parallel test # workers which do not inherit a -d auto_prepend_file flag. diff --git a/phpstan.neon b/phpstan.neon index 43d0fb1241f..99adf237a33 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -336,6 +336,9 @@ parameters: - tests/debug_functions.php - src/Util/Reflection/PrivatesAccessor.php + # it may return array in case of empty array( , , ), very rare and would nullify all other types + - '#Method Rector\\PhpParser\\NodeTraverser\\RectorNodeTraverser\:\:traverseArray\(\) should return array but returns array#' + # checks for rector always autoloaded rules only - identifier: symplify.forbiddenFuncCall diff --git a/rules/Php70/Rector/List_/EmptyListRector.php b/rules/Php70/Rector/List_/EmptyListRector.php index 4eda277099d..8d96b2a06de 100644 --- a/rules/Php70/Rector/List_/EmptyListRector.php +++ b/rules/Php70/Rector/List_/EmptyListRector.php @@ -51,7 +51,7 @@ public function getNodeTypes(): array /** * @param List_ $node */ - public function refactor(Node $node): ?Node + public function refactor(Node $node): ?List_ { foreach ($node->items as $item) { if ($item instanceof ArrayItem) { diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index fdaf7000c51..11f01d4f664 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -198,13 +198,11 @@ private function traverseNode(Node $node): void } /** - * @param Node[] $nodes + * @param array $nodes The null can be in case of empty list(, , ) * @return Node[] */ private function traverseArray(array $nodes): array { - Assert::allIsInstanceOf($nodes, Node::class); - $doNodes = []; foreach ($nodes as $i => $node) { if (! $node instanceof Node) { From 2fe9a42170e6d057f43d0ddf613318696121c6c3 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 9 Sep 2026 13:09:34 +0200 Subject: [PATCH 6/7] Skip RectorNodeTraverser::traverseArray() as a docblock false positive Claude-Session: https://claude.ai/code/session_01YVdy9rGZefFP8PGM5ge585 --- .github/workflows/docblock_check.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docblock_check.yaml b/.github/workflows/docblock_check.yaml index 66694083144..5b09d3c6fed 100644 --- a/.github/workflows/docblock_check.yaml +++ b/.github/workflows/docblock_check.yaml @@ -61,4 +61,6 @@ jobs: - name: List docblock type mismatches - run: honestype report "$DOCBLOCK_CHECK_LOG" + # RectorNodeTraverser::traverseArray() legitimately yields null + # elements at runtime; skip it as a known false positive. + run: honestype report -skip 'RectorNodeTraverser::traverseArray()' "$DOCBLOCK_CHECK_LOG" From 852cccaa814c1463c004f4f1bd30ab79a9e8aec1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 9 Sep 2026 14:03:00 +0200 Subject: [PATCH 7/7] Pin honestype to commit with -skip report flag support Claude-Session: https://claude.ai/code/session_01NCbNJRKiq4dcrCcvTGecCh --- .github/workflows/docblock_check.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docblock_check.yaml b/.github/workflows/docblock_check.yaml index 5b09d3c6fed..3a23ec47bc7 100644 --- a/.github/workflows/docblock_check.yaml +++ b/.github/workflows/docblock_check.yaml @@ -32,7 +32,7 @@ jobs: - name: Install honestype docblock checker run: | - GOPROXY=direct GOSUMDB=off go install github.com/rectorphp/honestype@main + GOPROXY=direct GOSUMDB=off go install github.com/rectorphp/honestype@6546e5dc237fc5787bc77dcb40dcbd271f683a7b echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" -