diff --git a/.github/workflows/phpstan-static-analysis.yml b/.github/workflows/phpstan-static-analysis.yml index 3258b56ff3baa..1c461a9ac54dd 100644 --- a/.github/workflows/phpstan-static-analysis.yml +++ b/.github/workflows/phpstan-static-analysis.yml @@ -19,6 +19,9 @@ on: - 'phpstan.neon.dist' - 'tests/phpstan/base.neon' - 'tests/phpstan/baselines/**' + # These files configure the tests of the PHPStan extensions. + - 'tests/phpstan/phpunit.xml.dist' + - 'tests/phpstan/Tests/tests.neon' # Confirm any changes to relevant workflow files. - '.github/workflows/phpstan-static-analysis.yml' - '.github/workflows/reusable-phpstan-static-analysis-v1.yml' diff --git a/.github/workflows/reusable-phpstan-static-analysis-v1.yml b/.github/workflows/reusable-phpstan-static-analysis-v1.yml index 321f98189cdf9..8b30babe2e44e 100644 --- a/.github/workflows/reusable-phpstan-static-analysis-v1.yml +++ b/.github/workflows/reusable-phpstan-static-analysis-v1.yml @@ -30,6 +30,7 @@ jobs: # - Logs debug information. # - Installs Composer dependencies. # - Make Composer packages available globally. + # - Runs the tests of the WordPress-specific PHPStan extensions. # - Builds WordPress. # - Configures caching for PHPStan static analysis scans. # - Runs PHPStan static analysis. @@ -75,6 +76,13 @@ jobs: - name: Make Composer packages available globally run: echo "${PWD}/vendor/bin" >> "$GITHUB_PATH" + # The extensions in `tests/phpstan` are tested against fixtures rather than against + # core, so a defect in one is reported here as a failing test rather than as a + # change in what the analysis below reports. + - name: Run the tests of the PHPStan extensions + if: ${{ hashFiles('tests/phpstan/phpunit.xml.dist') != '' }} + run: composer run phpstan:test + # The directory holds more than the analysis results. PHPStan also stores what it read out of # each source file there, the docblocks and signatures it found, keyed by that file's contents # and nothing else. The extensions in `tests/phpstan` change what reading a file yields without diff --git a/composer.json b/composer.json index e20f4c7806b6e..81037a4be45cb 100644 --- a/composer.json +++ b/composer.json @@ -77,6 +77,7 @@ "scripts": { "phpstan": "@php ./vendor/bin/phpstan analyse --memory-limit=2G", "phpstan:baselines": [ "Composer\\Config::disableProcessTimeout", "@php ./tests/phpstan/generate-baselines.php" ], + "phpstan:test": "@php ./vendor/phpunit/phpunit/phpunit -c tests/phpstan/phpunit.xml.dist", "compat": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcompat.xml.dist --report=summary,source", "format": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf --report=summary,source", "lint": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary,source", diff --git a/package.json b/package.json index d8e955f0d9809..130c7b9278843 100644 --- a/package.json +++ b/package.json @@ -154,6 +154,7 @@ "test:visual": "wp-scripts test-playwright --config tests/visual-regression/playwright.config.js", "typecheck:php": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan --", "typecheck:php:baselines": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:baselines --", + "typecheck:php:test": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:test --", "gutenberg:copy": "node tools/gutenberg/copy.js", "gutenberg:verify": "node tools/gutenberg/utils.js", "gutenberg:download": "node tools/gutenberg/download.js && npm run build:gutenberg" diff --git a/tests/phpstan/README.md b/tests/phpstan/README.md index e263cd409036d..fbf8317779ba9 100644 --- a/tests/phpstan/README.md +++ b/tests/phpstan/README.md @@ -186,6 +186,24 @@ These identifiers are specific to WordPress, and can be ignored or baselined lik | `wordpress.hookDocReferenceHookMissing` | The referenced file exists, but documents no hook of that name. Either the reference is stale, or the canonical docblock has moved. | | `wordpress.hookParamCountMismatch` | The call passes a different number of arguments than the docblock documents `@param` tags for. Passing fewer risks an `ArgumentCountError` in a callback registered for the documented count; passing more silently drops the extra argument and leaves the documentation misleading. | +### Testing the extensions + +The extensions have tests of their own under [`Tests/`](Tests), built on PHPStan's test harness. The two rules are run over fixture files and the errors they report are compared with the ones expected. The visitors and the return type extension are checked through the types PHPStan infers for the `assertType()` calls in their fixtures. Reference comments resolve against a small WordPress tree of fixtures in [`Tests/data/wordpress`](Tests/data/wordpress) rather than against `src`, so a broken reference can be tested without one existing in core. + +Run them with: + +```bash +npm run typecheck:php:test +``` + +or, outside the Docker environment: + +```bash +composer phpstan:test +``` + +They also run in CI, ahead of the analysis itself. + ## Ignoring and baselining errors As we adopt PHPStan iteratively, you may be faced with false positives due to legacy code, or code that is not worth changing at this time. diff --git a/tests/phpstan/Tests/ApplyFiltersDynamicFunctionReturnTypeExtensionTest.php b/tests/phpstan/Tests/ApplyFiltersDynamicFunctionReturnTypeExtensionTest.php new file mode 100644 index 0000000000000..b4169ad0cdc8e --- /dev/null +++ b/tests/phpstan/Tests/ApplyFiltersDynamicFunctionReturnTypeExtensionTest.php @@ -0,0 +1,49 @@ + + */ + public function data_file_asserts(): iterable { + yield from self::gatherAssertTypes( __DIR__ . '/data/apply-filters.php' ); + yield from self::gatherAssertTypes( __DIR__ . '/data/ApplyFiltersTestClass.php' ); + } + + /** + * @dataProvider data_file_asserts + * + * @param string $assert_type Kind of assertion. + * @param string $file Fixture file. + * @param mixed ...$args Assertion arguments. + */ + public function test_file_asserts( string $assert_type, string $file, ...$args ): void { + $this->assertFileAsserts( $assert_type, $file, ...$args ); + } + + /** + * Returns the configuration registering the extension and the visitors it relies on. + * + * @return list + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } +} diff --git a/tests/phpstan/Tests/GlobalDocBlockVisitorTest.php b/tests/phpstan/Tests/GlobalDocBlockVisitorTest.php new file mode 100644 index 0000000000000..afadddefdadd2 --- /dev/null +++ b/tests/phpstan/Tests/GlobalDocBlockVisitorTest.php @@ -0,0 +1,49 @@ + + */ + public function data_file_asserts(): iterable { + yield from self::gatherAssertTypes( __DIR__ . '/data/global-docblock.php' ); + yield from self::gatherAssertTypes( __DIR__ . '/data/GlobalDocBlockTestClass.php' ); + } + + /** + * @dataProvider data_file_asserts + * + * @param string $assert_type Kind of assertion. + * @param string $file Fixture file. + * @param mixed ...$args Assertion arguments. + */ + public function test_file_asserts( string $assert_type, string $file, ...$args ): void { + $this->assertFileAsserts( $assert_type, $file, ...$args ); + } + + /** + * Returns the configuration registering the visitor. + * + * @return list + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } +} diff --git a/tests/phpstan/Tests/HashNotationTypeTest.php b/tests/phpstan/Tests/HashNotationTypeTest.php new file mode 100644 index 0000000000000..7ea3d4518af50 --- /dev/null +++ b/tests/phpstan/Tests/HashNotationTypeTest.php @@ -0,0 +1,47 @@ + + */ + public function data_file_asserts(): iterable { + yield from self::gatherAssertTypes( __DIR__ . '/data/hash-notation.php' ); + } + + /** + * @dataProvider data_file_asserts + * + * @param string $assert_type Kind of assertion. + * @param string $file Fixture file. + * @param mixed ...$args Assertion arguments. + */ + public function test_file_asserts( string $assert_type, string $file, ...$args ): void { + $this->assertFileAsserts( $assert_type, $file, ...$args ); + } + + /** + * Returns the configuration registering the visitor. + * + * @return list + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } +} diff --git a/tests/phpstan/Tests/HashNotationVisitorTest.php b/tests/phpstan/Tests/HashNotationVisitorTest.php new file mode 100644 index 0000000000000..931a52820cd70 --- /dev/null +++ b/tests/phpstan/Tests/HashNotationVisitorTest.php @@ -0,0 +1,548 @@ + $expected Tag lines the docblocks are expected to gain. + */ + public function test_derived_tags( string $code, array $expected ): void { + $this->assertSame( $expected, self::derived_tags( $code ) ); + } + + /** + * @return array}> + */ + public function data_derived_tags(): array { + return array( + 'param hash is open with optional keys' => array( + self::fn( + ' + * @param array $args { + * Arguments. + * + * @type string $post_type Post type. Default \'post\'. + * @type int $post_author Post author ID. + * } + ' + ), + array( '@phpstan-param array{post_type?: string, post_author?: int, ...} $args' ), + ), + 'param key marked required is required' => array( + self::fn( + ' + * @param array $args { + * Arguments. + * + * @type string $hostname Required. Server hostname. + * @type int $port Port. + * } + ' + ), + array( '@phpstan-param array{hostname: string, port?: int, ...} $args' ), + ), + 'return hash is sealed with required keys' => array( + self::fn( + ' + * @return array { + * @type string $name Name. + * @type int $count Count. Default 0. + * } + ' + ), + array( '@phpstan-return array{name: string, count: int}' ), + ), + 'numbered keys are required in a param hash' => array( + self::fn( + ' + * @param array $size { + * @type int $0 Width. + * @type int $1 Height. + * } + ', + '( $size )' + ), + array( '@phpstan-param array{0: int, 1: int, ...} $size' ), + ), + 'Optional. marks a return key optional' => array( + self::fn( + ' + * @return array { + * @type string $name Name. + * @type int $count Optional. Count. + * @type string $slash Optional self closing slash, which is prose. + * } + ' + ), + array( '@phpstan-return array{name: string, count?: int, slash: string}' ), + ), + 'Optional. marks a numbered param key optional' => array( + self::fn( + ' + * @param array $size { + * @type int $0 Width. + * @type int $1 Optional. Height. + * } + ', + '( $size )' + ), + array( '@phpstan-param array{0: int, 1?: int, ...} $size' ), + ), + 'a lone variadic entry describes the values' => array( + self::fn( + ' + * @param array $callbacks { + * @type callable ...$0 Callbacks. + * } + ', + '( $callbacks )' + ), + array( '@phpstan-param array $callbacks' ), + ), + 'a variadic entry beside named keys opens a return hash' => array( + self::fn( + ' + * @return array { + * @type string $name Name. + * @type mixed ...$1 The rest. + * } + ' + ), + array( '@phpstan-return array{name: string, ...}' ), + ), + 'nested hashes nest' => array( + self::fn( + ' + * @param array $args { + * @type array $labels { + * @type string $name Name. + * @type string $singular Singular name. + * } + * @type bool $public Whether public. + * } + ' + ), + array( '@phpstan-param array{labels?: array{name?: string, singular?: string, ...}, public?: bool, ...} $args' ), + ), + 'a nested hash in a return is sealed too' => array( + self::fn( + ' + * @return array { + * @type array $labels { + * @type string $name Name. + * } + * } + ' + ), + array( '@phpstan-return array{labels: array{name: string}}' ), + ), + 'keys that are not identifiers are quoted' => array( + self::fn( + ' + * @return array { + * @type string $mime-type MIME type. + * @type string $ext Extension. + * } + ' + ), + array( "@phpstan-return array{'mime-type': string, ext: string}" ), + ), + 'Array is array' => array( + self::fn( + ' + * @param Array $args { + * @type string $name Name. + * } + ' + ), + array( '@phpstan-param array{name?: string, ...} $args' ), + ), + 'the bare member of a union takes the shape' => array( + self::fn( + ' + * @param string|array $args { + * @type string $name Name. + * } + * @return array|WP_Error { + * @type string $name Name. + * } + ' + ), + array( + '@phpstan-param string|array{name?: string, ...} $args', + '@phpstan-return array{name: string}|WP_Error', + ), + ), + 'a class takes the shape through an intersection' => array( + self::fn( + ' + * @return stdClass { + * @type string $name Name. + * } + ' + ), + array( '@phpstan-return stdClass&object{name: string}' ), + ), + 'an intersection inside a union is parenthesized' => array( + self::fn( + ' + * @return stdClass|false { + * @type string $name Name. + * } + ' + ), + array( '@phpstan-return (stdClass&object{name: string})|false' ), + ), + 'a bare object return is a shape' => array( + self::fn( + ' + * @return object { + * @type string $name Name. + * } + ' + ), + array( '@phpstan-return object{name: string}' ), + ), + 'a bare object param would have to stay open, so is skipped' => array( + self::fn( + ' + * @param object $args { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'a class param stays open through the class' => array( + self::fn( + ' + * @param stdClass $args { + * @type string $name Name. + * } + ' + ), + array( '@phpstan-param stdClass&object{name?: string} $args' ), + ), + 'two bare members leave the target a guess' => array( + self::fn( + ' + * @return array|object { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'two classes and no bare member leave the target a guess' => array( + self::fn( + ' + * @return WP_Post|WP_Term { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'a type more specific than the hash is left alone' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'a PHPDoc keyword is not a class' => array( + self::fn( + ' + * @return mixed { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'a hand-written counterpart wins for its tag only' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * } + * @param array $other { + * @type string $name Name. + * } + * @phpstan-param array{name: non-empty-string} $args + * @return array { + * @type string $name Name. + * } + * @phpstan-return array{name: non-empty-string} + ', + '( $args, $other )' + ), + array( '@phpstan-param array{name?: string, ...} $other' ), + ), + 'a multi-line hand-written counterpart covers the name it ends with' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * } + * @phpstan-param array{ + * name: non-empty-string, + * } $args + ' + ), + array(), + ), + 'a conditional return naming a parameter is not a shape for it' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * } + * @phpstan-return ($args is array ? int : string) + ' + ), + array( '@phpstan-param array{name?: string, ...} $args' ), + ), + 'a by-reference parameter is skipped' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * } + ', + '( &$args )' + ), + array(), + ), + 'a param hash without a variable is skipped' => array( + self::fn( + ' + * @param array { + * @type string $name Name. + * } + ' + ), + array(), + ), + 'an unclosed hash is skipped' => array( + self::fn( + ' + * @param array $args { + * @type string $name Name. + * @return void + ' + ), + array(), + ), + 'an entry without a name is skipped' => array( + self::fn( + ' + * @param array $args { + * @type string Name. + * } + ' + ), + array(), + ), + 'an entry whose type does not parse is skipped' => array( + self::fn( + ' + * @param array $args { + * @type string, $name Name. + * } + ' + ), + array(), + ), + 'an entry with a nullable type is kept' => array( + self::fn( + ' + * @param array $args { + * @type ?string $name Name. + * @type (string|null)[] $names Names. + * } + ' + ), + array( '@phpstan-param array{name?: ?string, names?: (string|null)[], ...} $args' ), + ), + 'a var hash is left alone' => array( + ' array( + ' array( + self::fn( + ' + * @param array $args Arguments. See the @type tags elsewhere. + * @return array + ' + ), + array(), + ), + ); + } + + /** + * Tests that the rewritten docblock keeps everything it had, and gains its tags + * before the closing marker. + */ + public function test_rewritten_docblock_keeps_the_original_text(): void { + $code = self::fn( + ' + * Does a thing. + * + * @since 1.0.0 + * + * @param array $args { + * @type string $name Name. + * } + * @return void + ' + ); + + $docblocks = self::docblocks_after_visiting( $code ); + + $this->assertCount( 1, $docblocks ); + $this->assertStringContainsString( "Does a thing.\n", $docblocks[0] ); + $this->assertStringContainsString( "@since 1.0.0\n", $docblocks[0] ); + $this->assertStringContainsString( "@return void\n", $docblocks[0] ); + $this->assertStringEndsWith( " * @phpstan-param array{name?: string, ...} \$args\n */", $docblocks[0] ); + } + + /** + * Wraps a docblock body in a function declaration. + * + * @param string $body Docblock lines, each starting with ` * `. + * @param string $signature Parameter list. + * @return string + */ + private static function fn( string $body, string $signature = '( $args )' ): string { + return " + */ + private static function derived_tags( string $code ): array { + $before = self::phpstan_lines( self::docblocks( $code, false ) ); + $after = self::phpstan_lines( self::docblocks_after_visiting( $code ) ); + + return array_values( array_diff( $after, $before ) ); + } + + /** + * Returns the docblocks in a snippet after the visitor has run. + * + * @param string $code PHP source. + * @return list + */ + private static function docblocks_after_visiting( string $code ): array { + return self::docblocks( $code, true ); + } + + /** + * Returns the text of every docblock in a snippet. + * + * @param string $code PHP source. + * @param bool $visit Whether to run the visitor first. + * @return list + */ + private static function docblocks( string $code, bool $visit ): array { + $stmts = ( new ParserFactory() )->createForHostVersion()->parse( $code ); + self::assertNotNull( $stmts ); + + $collector = new class() extends NodeVisitorAbstract { + /** + * @var list + */ + public array $docblocks = array(); + + public function enterNode( Node $node ): ?Node { + $doc = $node->getDocComment(); + if ( null !== $doc ) { + $this->docblocks[] = $doc->getText(); + } + return null; + } + }; + + $traverser = new NodeTraverser(); + if ( $visit ) { + $traverser->addVisitor( new HashNotationVisitor() ); + } + $traverser->addVisitor( $collector ); + $traverser->traverse( $stmts ); + + return $collector->docblocks; + } + + /** + * Extracts the `@phpstan-*` tag lines from docblocks, without their furniture. + * + * @param list $docblocks Docblock texts. + * @return list + */ + private static function phpstan_lines( array $docblocks ): array { + $lines = array(); + foreach ( $docblocks as $docblock ) { + if ( preg_match_all( '#^\s*\*\s*(@phpstan-\S+ .*)$#m', $docblock, $matches ) > 0 ) { + foreach ( $matches[1] as $line ) { + $lines[] = rtrim( $line ); + } + } + } + return $lines; + } +} diff --git a/tests/phpstan/Tests/HookDocBlockTest.php b/tests/phpstan/Tests/HookDocBlockTest.php new file mode 100644 index 0000000000000..4a7975758ec5b --- /dev/null +++ b/tests/phpstan/Tests/HookDocBlockTest.php @@ -0,0 +1,104 @@ + + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } + + /** + * @dataProvider data_hook_names + * + * @param string $call Hook call expression. + * @param bool $identifiable Whether the hook name carries literal text. + * @param string $display How the name is rendered in messages. + */ + public function test_hook_names( string $call, bool $identifiable, string $display ): void { + $node = self::call( $call ); + + $this->assertSame( $identifiable, HookDocBlock::hasIdentifiableHookName( $node ) ); + $this->assertSame( $display, HookDocBlock::getHookNameDisplay( $node ) ); + } + + /** + * @return array + */ + public function data_hook_names(): array { + return array( + 'literal' => array( "apply_filters( 'the_content', \$c )", true, 'the_content' ), + 'interpolated with literal' => array( 'apply_filters( "pre_option_{$option}", $v )', true, 'pre_option_{$option}' ), + 'interpolated without literal' => array( 'apply_filters( "{$hook}", $v )', false, '{$hook}' ), + 'interpolated separators only' => array( 'apply_filters( "{$taxonomy}_{$field}", $v )', true, '{$taxonomy}_{$field}' ), + 'interpolated property fetch' => array( 'apply_filters( "manage_{$this->screen->id}_columns", $v )', true, 'manage_{...}_columns' ), + 'concatenated with literal' => array( "apply_filters( 'prefix_' . \$name, \$v )", true, 'prefix_$name' ), + 'concatenated variables' => array( 'apply_filters( $a . $b, $v )', false, '$a$b' ), + 'variable' => array( 'apply_filters( $hook_name, $v )', false, '$hook_name' ), + 'other expression' => array( 'apply_filters( wp_tests_name(), $v )', false, '...' ), + 'no arguments' => array( 'apply_filters()', false, '' ), + ); + } + + /** + * Tests that a filter resolving to a docblock without parameters is flagged, + * and an action is not. + */ + public function test_is_filter_missing_param_docs(): void { + $without = array( + 'kind' => 'inline', + 'resolved' => null, + 'paramCount' => 0, + 'problem' => null, + ); + $with = array_merge( $without, array( 'paramCount' => 1 ) ); + $unknown = array_merge( $without, array( 'paramCount' => null ) ); + + $this->assertTrue( HookDocBlock::isFilterMissingParamDocs( self::call( "apply_filters( 'a', \$v )" ), $without ) ); + $this->assertTrue( HookDocBlock::isFilterMissingParamDocs( self::call( "apply_filters_ref_array( 'a', \$v )" ), $without ) ); + $this->assertTrue( HookDocBlock::isFilterMissingParamDocs( self::call( "apply_filters_deprecated( 'a', \$v, '1.0' )" ), $without ) ); + $this->assertFalse( HookDocBlock::isFilterMissingParamDocs( self::call( "do_action( 'a', \$v )" ), $without ) ); + $this->assertFalse( HookDocBlock::isFilterMissingParamDocs( self::call( "apply_filters( 'a', \$v )" ), $with ) ); + $this->assertFalse( HookDocBlock::isFilterMissingParamDocs( self::call( "apply_filters( 'a', \$v )" ), $unknown ) ); + } + + /** + * Parses a hook call expression. + * + * @param string $call PHP expression. + * @return FuncCall + */ + private static function call( string $call ): FuncCall { + $stmts = ( new ParserFactory() )->createForHostVersion()->parse( 'findInstanceOf( $stmts, FuncCall::class ); + self::assertNotEmpty( $calls ); + + return $calls[0]; + } +} diff --git a/tests/phpstan/Tests/HookDocsVisitorTest.php b/tests/phpstan/Tests/HookDocsVisitorTest.php new file mode 100644 index 0000000000000..f238589d5c442 --- /dev/null +++ b/tests/phpstan/Tests/HookDocsVisitorTest.php @@ -0,0 +1,165 @@ + apply_filters( 'first_item', $value ), + 'second' => apply_filters( 'second_item', $value ), + ); + + /** + * Function docblock. + */ + function wp_tests_fn( $value ) { + return apply_filters( 'in_function', $value ); + } + + /** + * Argument docblock. + */ + wp_tests_fn( + /** + * Inner argument docblock. + */ + apply_filters( 'in_argument', $value ), + apply_filters( 'sibling_argument', $value ) + ); + + /** This filter is documented in wp-includes/canonical.php */ + $value = apply_filters( 'referenced', $value ); + PHP; + + $this->assertSame( + array( + 'on_statement' => 'Statement docblock.', + 'nested_in_statement' => 'Nested docblock.', + 'undocumented' => null, + 'in_condition' => 'Condition docblock.', + 'in_body' => null, + 'first_item' => 'First item docblock.', + 'second_item' => null, + 'in_function' => null, + 'in_argument' => 'Inner argument docblock.', + 'sibling_argument' => 'Argument docblock.', + 'referenced' => 'This filter is documented in wp-includes/canonical.php', + ), + self::attached_docblocks( $code ) + ); + } + + /** + * Tests that state does not leak from one traversal into the next. + */ + public function test_state_is_reset_between_traversals(): void { + $parser = ( new ParserFactory() )->createForHostVersion(); + $visitor = new HookDocsVisitor(); + $traverser = new NodeTraverser(); + $traverser->addVisitor( $visitor ); + + $documented = $parser->parse( "assertNotNull( $documented ); + $traverser->traverse( $documented ); + + $undocumented = $parser->parse( "assertNotNull( $undocumented ); + $traverser->traverse( $undocumented ); + + $calls = ( new NodeFinder() )->findInstanceOf( $undocumented, FuncCall::class ); + $this->assertCount( 1, $calls ); + $this->assertNull( $calls[0]->getAttribute( 'latestDocComment' ) ); + } + + /** + * Returns the summary line of the docblock attached to each hook call, keyed + * by hook name. + * + * @param string $code PHP source. + * @return array + */ + private static function attached_docblocks( string $code ): array { + $stmts = ( new ParserFactory() )->createForHostVersion()->parse( $code ); + self::assertNotNull( $stmts ); + + $traverser = new NodeTraverser(); + $traverser->addVisitor( new HookDocsVisitor() ); + $traverser->traverse( $stmts ); + + $result = array(); + foreach ( ( new NodeFinder() )->findInstanceOf( $stmts, FuncCall::class ) as $call ) { + $args = $call->getArgs(); + if ( ! isset( $args[0] ) || ! $args[0]->value instanceof String_ || 'wp_tests_fn' === (string) $call->name ) { + continue; + } + + $doc = $call->getAttribute( 'latestDocComment' ); + self::assertTrue( null === $doc || $doc instanceof Doc ); + + $result[ $args[0]->value->value ] = null === $doc ? null : self::summary( $doc ); + } + + return $result; + } + + /** + * Returns the first line of prose in a docblock. + * + * @param Doc $doc Docblock. + * @return string + */ + private static function summary( Doc $doc ): string { + $text = preg_replace( '#^/\*\*\s*|\s*\*/$#', '', $doc->getText() ); + $text = preg_replace( '#^\s*\*\s?#m', '', (string) $text ); + + return trim( (string) $text ); + } +} diff --git a/tests/phpstan/Tests/HookDocumentationRuleTest.php b/tests/phpstan/Tests/HookDocumentationRuleTest.php new file mode 100644 index 0000000000000..bce2fe671a7ba --- /dev/null +++ b/tests/phpstan/Tests/HookDocumentationRuleTest.php @@ -0,0 +1,131 @@ + + */ +final class HookDocumentationRuleTest extends RuleTestCase { + + /** + * Returns the rule under test, with its documentation resolved against the + * fixture WordPress root. + * + * @return HookDocumentationRule + */ + protected function getRule(): Rule { + return new HookDocumentationRule( self::getContainer()->getByType( HookDocBlock::class ) ); + } + + /** + * Returns the configuration registering the visitors and services the rule relies on. + * + * @return list + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } + + /** + * Tests the errors reported for a file outside the WordPress root, whose + * reference comments resolve against the root directly. + */ + public function test_rule(): void { + $missing = 'is not preceded by a docblock documenting the hook, nor by a "This filter/action is documented in " reference comment.'; + $no_params = 'is preceded by a docblock that documents no parameters. A filter is documented with a `@param` tag for the value being filtered, plus one for each further argument passed.'; + + $this->analyse( + array( __DIR__ . '/data/hook-documentation.php' ), + array( + array( 'apply_filters() call for hook "undocumented_filter" ' . $missing, 15 ), + array( 'do_action() call for hook "undocumented_action" ' . $missing, 17 ), + array( 'apply_filters() call for hook "paramless_filter" ' . $no_params, 34 ), + array( 'apply_filters() call for hook "var_annotated_filter" ' . $no_params, 37 ), + array( 'apply_filters() call for hook "documented_filter" references documentation in "wp-includes/missing.php", but no such file exists in the tree being analyzed.', 46 ), + array( 'apply_filters() call for hook "documented_filter" references documentation in "wp-includes/other.php", but no documented "documented_filter" hook is found there.', 49 ), + array( 'apply_filters() call for hook "only_referenced_here" references documentation in "wp-includes/canonical.php", but no documented "only_referenced_here" hook is found there.', 52 ), + array( 'apply_filters() call for hook "foo_bar" references documentation in "wp-includes/canonical.php", but no documented "foo_bar" hook is found there.', 61 ), + array( 'apply_filters() call for hook "{$prefix}_bar" references documentation in "wp-includes/canonical.php", but no documented "{$prefix}_bar" hook is found there.', 64 ), + array( 'apply_filters() call for hook "pre_option_{$option}" ' . $missing, 73 ), + array( 'apply_filters() call for hook "prefix_$context" ' . $missing, 75 ), + array( 'apply_filters() call for hook "second_item" ' . $missing, 93 ), + array( 'apply_filters_deprecated() call for hook "old_filter" ' . $missing, 103 ), + array( 'do_action_ref_array() call for hook "ref_action" ' . $missing, 105 ), + array( 'apply_filters() call for hook "inside_function" ' . $missing, 120 ), + ) + ); + } + + /** + * Tests that a reference is resolved by walking up from the file holding it, + * so a theme template can reference a sibling as well as a core file. + */ + public function test_references_resolve_relative_to_the_referencing_file(): void { + $this->analyse( + array( __DIR__ . '/data/wordpress/wp-content/themes/wptests/index.php' ), + array( + array( 'do_action() call for hook "wptests_missing_theme_hook" references documentation in "author.php", but no documented "wptests_missing_theme_hook" hook is found there.', 15 ), + array( 'apply_filters() call for hook "documented_filter" references documentation in "single.php", but no such file exists in the tree being analyzed.', 18 ), + ) + ); + } + + /** + * Tests that a reference written with a dot segment resolves inside the tree, + * and that one leading outside the tree is reported as missing. + */ + public function test_references_never_resolve_outside_the_root(): void { + $this->analyse( + array( __DIR__ . '/data/wordpress/wp-admin/admin.php' ), + array( + array( 'apply_filters() call for hook "outside_filter" references documentation in "../outside.php", but no such file exists in the tree being analyzed.', 19 ), + ) + ); + } + + /** + * Tests that each kind of problem is reported under its own identifier. + */ + public function test_error_identifiers(): void { + $errors = $this->gatherAnalyserErrors( array( __DIR__ . '/data/hook-documentation.php' ) ); + + $identifiers = array(); + foreach ( $errors as $error ) { + $identifiers[ (int) $error->getLine() ] = $error->getIdentifier(); + } + + $this->assertSame( + array( + 15 => 'wordpress.hookDocMissing', + 17 => 'wordpress.hookDocMissing', + 34 => 'wordpress.hookDocNoParams', + 37 => 'wordpress.hookDocNoParams', + 46 => 'wordpress.hookDocReferenceFileMissing', + 49 => 'wordpress.hookDocReferenceHookMissing', + 52 => 'wordpress.hookDocReferenceHookMissing', + 61 => 'wordpress.hookDocReferenceHookMissing', + 64 => 'wordpress.hookDocReferenceHookMissing', + 73 => 'wordpress.hookDocMissing', + 75 => 'wordpress.hookDocMissing', + 93 => 'wordpress.hookDocMissing', + 103 => 'wordpress.hookDocMissing', + 105 => 'wordpress.hookDocMissing', + 120 => 'wordpress.hookDocMissing', + ), + $identifiers + ); + } +} diff --git a/tests/phpstan/Tests/HookParamCountRuleTest.php b/tests/phpstan/Tests/HookParamCountRuleTest.php new file mode 100644 index 0000000000000..48561ccf63f1e --- /dev/null +++ b/tests/phpstan/Tests/HookParamCountRuleTest.php @@ -0,0 +1,78 @@ + + */ +final class HookParamCountRuleTest extends RuleTestCase { + + /** + * Returns the rule under test, with its documentation resolved against the + * fixture WordPress root. + * + * @return HookParamCountRule + */ + protected function getRule(): Rule { + return new HookParamCountRule( self::getContainer()->getByType( HookDocBlock::class ) ); + } + + /** + * Returns the configuration registering the visitors and services the rule relies on. + * + * @return list + */ + public static function getAdditionalConfigFiles(): array { + return array( __DIR__ . '/tests.neon' ); + } + + /** + * Tests the mismatches reported, and the calls left alone. + */ + public function test_rule(): void { + $this->analyse( + array( __DIR__ . '/data/hook-param-count.php' ), + array( + array( 'apply_filters() for hook "two_params_short" provides 1 argument, but the hook is documented with 2 parameters.', 22 ), + array( 'apply_filters() for hook "one_param_long" provides 2 arguments, but the hook is documented with 1 parameter.', 29 ), + array( 'do_action() for hook "no_params_extra" provides 1 argument, but its docblock documents no parameters.', 39 ), + array( 'do_action() for hook "no_params_extra_two" provides 2 arguments, but its docblock documents no parameters.', 44 ), + array( 'do_action_ref_array() for hook "ref_array_long" provides 2 arguments, but the hook is documented with 1 parameter.', 51 ), + array( 'apply_filters_deprecated() for hook "deprecated_short" provides 1 argument, but the hook is documented with 2 parameters.', 59 ), + array( 'apply_filters() for hook "spread_known_long" provides 3 arguments, but the hook is documented with 1 parameter.', 90 ), + array( 'apply_filters() for hook "documented_filter" provides 1 argument, but the hook is documented with 2 parameters.', 113 ), + array( 'do_action() for hook "documented_action" provides 2 arguments, but the hook is documented with 1 parameter.', 116 ), + array( 'apply_filters() for hook "index_template_hierarchy" provides 2 arguments, but the hook is documented with 1 parameter.', 119 ), + array( 'apply_filters() for hook "malformed_tag" provides 2 arguments, but the hook is documented with 1 parameter.', 135 ), + array( 'do_action() for hook "wp_tests_{$type}_fired" provides 2 arguments, but the hook is documented with 1 parameter.', 144 ), + array( 'apply_filters_ref_array() for hook "ref_array_filter_long" provides 2 arguments, but the hook is documented with 1 parameter.', 151 ), + array( 'do_action_deprecated() for hook "deprecated_action_long" provides 2 arguments, but the hook is documented with 1 parameter.', 158 ), + ) + ); + } + + /** + * Tests that every mismatch is reported under the same identifier. + */ + public function test_error_identifier(): void { + $errors = $this->gatherAnalyserErrors( array( __DIR__ . '/data/hook-param-count.php' ) ); + + $this->assertNotEmpty( $errors ); + + foreach ( $errors as $error ) { + $this->assertSame( 'wordpress.hookParamCountMismatch', $error->getIdentifier(), sprintf( 'Line %d', $error->getLine() ) ); + } + } +} diff --git a/tests/phpstan/Tests/bootstrap.php b/tests/phpstan/Tests/bootstrap.php new file mode 100644 index 0000000000000..725d7b3ed4897 --- /dev/null +++ b/tests/phpstan/Tests/bootstrap.php @@ -0,0 +1,17 @@ +" references against, in place of `src`. + * Read by tests.neon, which has no other way to learn where this directory is. + */ +putenv( 'WP_PHPSTAN_TESTS_WORDPRESS_ROOT=' . __DIR__ . '/data/wordpress' ); + +require_once dirname( __DIR__, 3 ) . '/vendor/autoload.php'; diff --git a/tests/phpstan/Tests/data/ApplyFiltersTestClass.php b/tests/phpstan/Tests/data/ApplyFiltersTestClass.php new file mode 100644 index 0000000000000..2210079675cbe --- /dev/null +++ b/tests/phpstan/Tests/data/ApplyFiltersTestClass.php @@ -0,0 +1,75 @@ +slug ) ? apply_filters( 'editable_slug', $tag->slug, $tag ) : 123; +assertType( '123|string', $slug ); + +/** + * Return type for `apply_filters_ref_array()`. + * + * @param string $foo Hello. + * @param int $bar World. + */ +$value = apply_filters_ref_array( 'foo', array( 'Hello', 123 ) ); +assertType( 'string', $value ); + +/** + * Return type for `apply_filters_deprecated()`. + * + * @param string $foo Hello. + * @param bool $bar World. + */ +$value = apply_filters_deprecated( 'foo', array( 'Hello', true ), '6.0.0' ); +assertType( 'string', $value ); + +/** + * An action is not a filter: its docblock does not type its return value, which + * stays whatever `do_action()` itself declares. + * + * @param string $foo Hello. + */ +$value = do_action( 'foo', 'Hello' ); +assertType( 'mixed', $value ); + +/** This filter is documented in wp-includes/missing.php */ +$value = apply_filters( 'documented_filter', 123 ); +assertType( 'mixed', $value ); + +/** This filter is documented in wp-includes/canonical.php */ +$value = apply_filters( 'documented_filter', 123, $context ); +assertType( 'string', $value ); + +/** This filter is documented in wp-includes/canonical.php */ +$value = apply_filters( 'not_documented_there', 123 ); +assertType( 'mixed', $value ); + +/** This filter is documented in wp-includes/canonical.php */ +$value = apply_filters( 'index_template_hierarchy', array() ); +assertType( 'array', $value ); + +/** This filter is documented in wp-includes/canonical.php */ +$value = apply_filters( 'documented_hash_filter', array() ); +assertType( 'array{post_type?: string, per_page?: int, ...}', $value ); + +/** This filter is documented in wp-includes/canonical.php */ +$value = apply_filters( $hook_name, 123 ); +assertType( 'mixed', $value ); diff --git a/tests/phpstan/Tests/data/functions.php b/tests/phpstan/Tests/data/functions.php new file mode 100644 index 0000000000000..dc594cf106a7c --- /dev/null +++ b/tests/phpstan/Tests/data/functions.php @@ -0,0 +1,19 @@ + $wp_tests_map A type containing whitespace. + */ +function wp_tests_documented_globals(): void { + global $wpdb, $pagenow, $wp_tests_ids, $wp_tests_map, $wp_tests_undocumented; + + assertType( 'wpdb', $wpdb ); + assertType( 'string', $pagenow ); + assertType( 'array|null', $wp_tests_ids ); + assertType( 'array', $wp_tests_map ); + assertType( 'mixed', $wp_tests_undocumented ); +} + +/** + * Documents nothing. + */ +function wp_tests_no_global_tags(): void { + global $wpdb; + + assertType( 'mixed', $wpdb ); +} + +function wp_tests_no_docblock(): void { + global $wpdb; + + assertType( 'mixed', $wpdb ); +} + +/** + * A hand-written `@var` on the statement is left alone, per variable. + * + * @global string $pagenow Current admin page. + * @global string $wp_tests_other Something else. + */ +function wp_tests_handwritten_var(): void { + /** @var int $pagenow */ + global $pagenow, $wp_tests_other; + + assertType( 'int', $pagenow ); + assertType( 'string', $wp_tests_other ); +} + +/** + * A `@global` tag on the statement itself takes precedence. + * + * @global string $pagenow Current admin page. + */ +function wp_tests_statement_level_tag(): void { + /** @global int $pagenow */ + global $pagenow; + + assertType( 'int', $pagenow ); +} + +/** + * A closure is its own frame, documented or not. + * + * @global string $pagenow Current admin page. + */ +function wp_tests_closure_frames(): void { + $undocumented = function (): void { + global $pagenow; + + assertType( 'mixed', $pagenow ); + }; + + /** + * A docblock on the assignment belongs to the statement, not to the closure. + * + * @global int $pagenow Current admin page, as the closure sees it. + */ + $documented_statement = function (): void { + global $pagenow; + + assertType( 'mixed', $pagenow ); + }; + + $documented_import = function (): void { + /** @global int $pagenow Current admin page, as the closure sees it. */ + global $pagenow; + + assertType( 'int', $pagenow ); + }; + + global $pagenow; + + assertType( 'string', $pagenow ); +} + +/** + * A tag for a variable the function does not import changes nothing. + * + * @global string $pagenow Current admin page. + */ +function wp_tests_unused_tag(): void { + global $wpdb; + + assertType( 'mixed', $wpdb ); +} + +/** @global \WP_Query $wp_query WordPress Query object. */ +global $wp_query; + +assertType( 'WP_Query', $wp_query ); + +global $wp_the_query; + +assertType( 'mixed', $wp_the_query ); diff --git a/tests/phpstan/Tests/data/hash-notation.php b/tests/phpstan/Tests/data/hash-notation.php new file mode 100644 index 0000000000000..a335fc203fb59 --- /dev/null +++ b/tests/phpstan/Tests/data/hash-notation.php @@ -0,0 +1,118 @@ + 'x', + 'count' => 1, + ); +} + +/** + * A hash on a class produces an intersection. + * + * @return stdClass { + * @type string $name Name. + * } + */ +function wp_tests_hash_object_return() { + $object = new stdClass(); + $object->name = 'x'; + + return $object; +} + +/** + * The bare member of a union takes the shape. + * + * @return array|WP_Error { + * @type string $name Name. + * } + */ +function wp_tests_hash_union_return() { + return array( 'name' => 'x' ); +} + +/** + * A hand-written `@phpstan-return` wins. + * + * @return array { + * @type string $name Name. + * } + * @phpstan-return array{name: non-empty-string} + */ +function wp_tests_hash_handwritten_return() { + return array( 'name' => 'x' ); +} + +/** + * A by-reference parameter keeps its declared type. + * + * @param array $args { + * @type string $name Name. + * } + */ +function wp_tests_hash_by_reference( &$args ) { + assertType( 'array', $args ); +} + +/** + * A hook documented with a hash is typed from the shape. + * + * @param array $args { + * @type string $post_type Post type. + * } + */ +function wp_tests_hash_hook( $args ) { + /** + * Filters the arguments. + * + * @param array $args { + * Arguments. + * + * @type string $post_type Post type. + * @type int $per_page Per page. + * } + */ + $args = apply_filters( 'wp_tests_hash_hook', $args ); + + assertType( 'array{post_type?: string, per_page?: int, ...}', $args ); +} + +/** + * The return shapes as a caller sees them. + */ +function wp_tests_hash_call_sites() { + assertType( 'array{name: string, count: int}', wp_tests_hash_param_and_return( array() ) ); + assertType( 'object{name: string}&stdClass', wp_tests_hash_object_return() ); + assertType( 'array{name: string}|WP_Error', wp_tests_hash_union_return() ); + assertType( 'array{name: non-empty-string}', wp_tests_hash_handwritten_return() ); +} diff --git a/tests/phpstan/Tests/data/hook-documentation.php b/tests/phpstan/Tests/data/hook-documentation.php new file mode 100644 index 0000000000000..6f6f4fabbaba9 --- /dev/null +++ b/tests/phpstan/Tests/data/hook-documentation.php @@ -0,0 +1,121 @@ + apply_filters( 'first_item', $value ), + 'second' => apply_filters( 'second_item', $value ), +); + +/** + * Filters a value passed through another call. + * + * @param string $value Value. + */ +$value = wp_tests_return_value( apply_filters( 'wrapped_filter', $value ) ); + +$value = apply_filters_deprecated( 'old_filter', array( $value ), '6.0.0' ); + +do_action_ref_array( 'ref_action', array( $value ) ); + +/** + * Fires a deprecated action. + * + * @param string $value Value. + */ +do_action_deprecated( 'old_action', array( $value ), '6.0.0' ); + +/** + * This docblock belongs to the function, not to the hook inside it. + * + * @param string $value Value. + */ +function wp_tests_function_docblock_does_not_reach_hook( $value ) { + return apply_filters( 'inside_function', $value ); +} diff --git a/tests/phpstan/Tests/data/hook-param-count.php b/tests/phpstan/Tests/data/hook-param-count.php new file mode 100644 index 0000000000000..05b0458cc019b --- /dev/null +++ b/tests/phpstan/Tests/data/hook-param-count.php @@ -0,0 +1,158 @@ + + + + + + Tests + + +