From 62550867692a8da52392f16d008bcd242ab016b0 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 20:18:02 +0200 Subject: [PATCH 01/14] Confirm that docblocks are no longer detected. --- src/HookDocsRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HookDocsRule.php b/src/HookDocsRule.php index 4d386aab..d6a83825 100644 --- a/src/HookDocsRule.php +++ b/src/HookDocsRule.php @@ -79,7 +79,7 @@ public function processNode(Node $node, Scope $scope): array // A docblock is optional. if ($resolvedPhpDoc === null) { - return []; + return [RuleErrorBuilder::message('NO DOC BLOCK')->build()]; } return $this->validateDocBlock($resolvedPhpDoc); From 730e986e8dcdc0ddc5f457a2b164953db8511aba Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 22:03:53 +0200 Subject: [PATCH 02/14] Ensure the extension config is loaded during all tests. --- tests/HookDocsRuleTest.php | 6 ++++++ tests/IsWpErrorRuleTest.php | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/HookDocsRuleTest.php b/tests/HookDocsRuleTest.php index f8db4f23..10c5b558 100644 --- a/tests/HookDocsRuleTest.php +++ b/tests/HookDocsRuleTest.php @@ -79,4 +79,10 @@ public function testRule(): void ] ); } + + public static function getAdditionalConfigFiles(): array + { + // Path to your project's phpstan.neon, or extension.neon in case of custom extension packages. + return [dirname(__DIR__) . '/extension.neon']; + } } diff --git a/tests/IsWpErrorRuleTest.php b/tests/IsWpErrorRuleTest.php index 660021a5..9e91ff65 100644 --- a/tests/IsWpErrorRuleTest.php +++ b/tests/IsWpErrorRuleTest.php @@ -51,4 +51,10 @@ public function testRule(): void ] ); } + + public static function getAdditionalConfigFiles(): array + { + // Path to your project's phpstan.neon, or extension.neon in case of custom extension packages. + return [dirname(__DIR__) . '/extension.neon']; + } } From 53f748ce05e24ccd0eca2b2fcbbc3081148f3c19 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 22:05:06 +0200 Subject: [PATCH 03/14] =?UTF-8?q?Introduce=20a=20node=20visitor=20that=20n?= =?UTF-8?q?a=C3=AFvely=20tracks=20the=20most=20recent=20doc=20block.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- extension.neon | 4 ++++ src/HookDocBlock.php | 17 ++--------------- src/HookDocsVisitor.php | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 src/HookDocsVisitor.php diff --git a/extension.neon b/extension.neon index ff73ac94..4a332572 100644 --- a/extension.neon +++ b/extension.neon @@ -75,6 +75,10 @@ services: class: SzepeViktor\PHPStan\WordPress\TermExistsDynamicFunctionReturnTypeExtension tags: - phpstan.broker.dynamicFunctionReturnTypeExtension + - + class: SzepeViktor\PHPStan\WordPress\HookDocsVisitor + tags: + - phpstan.parser.richParserNodeVisitor rules: - SzepeViktor\PHPStan\WordPress\HookDocsRule - SzepeViktor\PHPStan\WordPress\IsWpErrorRule diff --git a/src/HookDocBlock.php b/src/HookDocBlock.php index e920f6a2..eacd765b 100644 --- a/src/HookDocBlock.php +++ b/src/HookDocBlock.php @@ -49,20 +49,7 @@ public function getNullableHookDocBlock(FuncCall $functionCall, Scope $scope): ? private static function getNullableNodeComment(FuncCall $node): ?\PhpParser\Comment\Doc { - $startLine = $node->getStartLine(); - - while ($node !== null && $node->getStartLine() === $startLine) { - // Fetch the docblock from the node. - $comment = $node->getDocComment(); - - if ($comment !== null) { - return $comment; - } - - /** @var \PhpParser\Node|null */ - $node = $node->getAttribute('parent'); - } - - return null; + /** @var \PhpParser\Comment\Doc|null */ + return $node->getAttribute('latestDocComment'); } } diff --git a/src/HookDocsVisitor.php b/src/HookDocsVisitor.php new file mode 100644 index 00000000..e788c977 --- /dev/null +++ b/src/HookDocsVisitor.php @@ -0,0 +1,32 @@ +getDocComment(); + + if ($doc !== null) { + $this->latestDocComment = $doc; + } + + $node->setAttribute('latestDocComment', $this->latestDocComment); + + return null; + } +} From 160d168573d6d31b410425117d19eff1cfa1a4e4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 22:49:00 +0200 Subject: [PATCH 04/14] More tests. --- tests/HookDocsRuleTest.php | 4 ++++ tests/data/hook-docs.php | 22 ++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/HookDocsRuleTest.php b/tests/HookDocsRuleTest.php index 10c5b558..c1bb0e8a 100644 --- a/tests/HookDocsRuleTest.php +++ b/tests/HookDocsRuleTest.php @@ -76,6 +76,10 @@ public function testRule(): void 'One or more @param tags has an invalid name or invalid syntax.', 206, ], + [ + 'Expected 2 @param tags, found 1.', + 217, + ], ] ); } diff --git a/tests/data/hook-docs.php b/tests/data/hook-docs.php index 597bbfb9..710130da 100644 --- a/tests/data/hook-docs.php +++ b/tests/data/hook-docs.php @@ -87,7 +87,7 @@ function wide_param_type(string $one) * @param \stdClass $instance Instance. * @param array $args Args */ -do_action('action', $this, $args); +do_action('action', $this, []); /** * This param tag renames the `$this` variable, which is fine, but still has a missing param tag. @@ -203,8 +203,26 @@ function incorrect_nullable_param_type(?string $one = null) * @param bool has_site_pending_automated_transfer( $this->blog_id ) * @param int $blog_id Blog identifier. */ -return apply_filters( +$value = apply_filters( 'filter', false, $this->blog_id ); + +/** + * This filter is wrapped inside another function call, which is weird but ok. Its param count is incorrect. + * + * @param int $number + */ +$value = intval(apply_filters('filter', 123, $foo)); + +/** + * This is a docblock for an unrelated function. + * + * It exists to ensure the undocumented filter below does not have its docblock inherited from this function. + * + * @param bool $yes + */ +function foo( bool $yes ) {} + +$value = apply_filters('filter', 123, $foo); From c5d31f1d7a6dd36a2cbe89a89b1e02af05c59389 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 22:49:06 +0200 Subject: [PATCH 05/14] Don't need this any more. --- src/HookDocsRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HookDocsRule.php b/src/HookDocsRule.php index d6a83825..4d386aab 100644 --- a/src/HookDocsRule.php +++ b/src/HookDocsRule.php @@ -79,7 +79,7 @@ public function processNode(Node $node, Scope $scope): array // A docblock is optional. if ($resolvedPhpDoc === null) { - return [RuleErrorBuilder::message('NO DOC BLOCK')->build()]; + return []; } return $this->validateDocBlock($resolvedPhpDoc); From 72bc01006371ec89470a6b95cdb51988495ee462 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 23:33:14 +0200 Subject: [PATCH 06/14] Reset the visitor state before traversing nodes. --- src/HookDocsVisitor.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/HookDocsVisitor.php b/src/HookDocsVisitor.php index e788c977..a6319e83 100644 --- a/src/HookDocsVisitor.php +++ b/src/HookDocsVisitor.php @@ -17,6 +17,13 @@ final class HookDocsVisitor extends \PhpParser\NodeVisitorAbstract */ protected $latestDocComment = null; + public function beforeTraverse(array $nodes): ?array + { + $this->latestDocComment = null; + + return null; + } + public function enterNode(Node $node): ?Node { $doc = $node->getDocComment(); From 675b9555f907107acd0f68a9ccda31b2718b3c87 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 23:39:43 +0200 Subject: [PATCH 07/14] Link the doc comment to the start line of the node. --- src/HookDocsVisitor.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/HookDocsVisitor.php b/src/HookDocsVisitor.php index a6319e83..9e39a580 100644 --- a/src/HookDocsVisitor.php +++ b/src/HookDocsVisitor.php @@ -12,6 +12,11 @@ final class HookDocsVisitor extends \PhpParser\NodeVisitorAbstract { + /** + * @var int|null + */ + protected $latestStartLine = null; + /** * @var \PhpParser\Comment\Doc|null */ @@ -19,6 +24,7 @@ final class HookDocsVisitor extends \PhpParser\NodeVisitorAbstract public function beforeTraverse(array $nodes): ?array { + $this->latestStartLine = null; $this->latestDocComment = null; return null; @@ -26,6 +32,12 @@ public function beforeTraverse(array $nodes): ?array public function enterNode(Node $node): ?Node { + if ($node->getStartLine() !== $this->latestStartLine) { + $this->latestDocComment = null; + } + + $this->latestStartLine = $node->getStartLine(); + $doc = $node->getDocComment(); if ($doc !== null) { From 4dbec1bafa612f1bd93f1885738b53592476b7ba Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 23:44:58 +0200 Subject: [PATCH 08/14] None of these linting rules are of any value. --- src/HookDocsVisitor.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/HookDocsVisitor.php b/src/HookDocsVisitor.php index 9e39a580..4655dcd8 100644 --- a/src/HookDocsVisitor.php +++ b/src/HookDocsVisitor.php @@ -12,16 +12,13 @@ final class HookDocsVisitor extends \PhpParser\NodeVisitorAbstract { - /** - * @var int|null - */ + // @var int|null protected $latestStartLine = null; - /** - * @var \PhpParser\Comment\Doc|null - */ + // @var \PhpParser\Comment\Doc|null protected $latestDocComment = null; + // phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter public function beforeTraverse(array $nodes): ?array { $this->latestStartLine = null; From 18af04a5310f0920f2def0bde7a14be09d92e0e8 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 23:48:44 +0200 Subject: [PATCH 09/14] Boop. --- src/HookDocsVisitor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HookDocsVisitor.php b/src/HookDocsVisitor.php index 4655dcd8..7465361c 100644 --- a/src/HookDocsVisitor.php +++ b/src/HookDocsVisitor.php @@ -12,10 +12,10 @@ final class HookDocsVisitor extends \PhpParser\NodeVisitorAbstract { - // @var int|null + /** @var int|null */ protected $latestStartLine = null; - // @var \PhpParser\Comment\Doc|null + /** @var \PhpParser\Comment\Doc|null */ protected $latestDocComment = null; // phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter From 3a5b4d4fa16fed9ba8dd854feb63b6fefa47f20c Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 26 Apr 2022 23:57:47 +0200 Subject: [PATCH 10/14] Add some debugging to CI. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 56bb67c2..71b5da7e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,7 @@ before_install: install: - "composer update --no-interaction" + - "composer show" script: - "composer test:syntax -- --no-progress" From 82e5901377dbf998b236a17f179fc05dfb075ccd Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 27 Apr 2022 00:18:39 +0200 Subject: [PATCH 11/14] Bish bash bosh. --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 71b5da7e..18c9ad8b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,8 +8,10 @@ os: dist: "bionic" php: + - "8.1" - "8.0" - "7.4" + - "7.2" - "7.1" cache: From 4f811b6d98c309e0570d7dc31a78d1775ac8176c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Tue, 26 Apr 2022 23:52:46 +0000 Subject: [PATCH 12/14] Skip PHPCS on PHP 8.1 --- .travis.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 18c9ad8b..1f64ea7c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,11 +8,18 @@ os: dist: "bionic" php: - - "8.1" - "8.0" - "7.4" - "7.2" - - "7.1" + +jobs: + include: + - php: "8.1" + script: + - "composer test:syntax -- --no-progress" + - "composer test:phpunit -- --verbose" + # - "composer test:cs -- -s" + - "composer test:phpstan -- --ansi --memory-limit=1G --no-progress" cache: directories: @@ -24,7 +31,6 @@ before_install: install: - "composer update --no-interaction" - - "composer show" script: - "composer test:syntax -- --no-progress" From b39adc9aeca8332b0fb6071fae03e64864ba42fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Wed, 27 Apr 2022 00:01:04 +0000 Subject: [PATCH 13/14] Drop PHP 7.1 support --- composer.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 14751727..8d026e45 100644 --- a/composer.json +++ b/composer.json @@ -11,17 +11,17 @@ "phpstan" ], "require": { - "php": "^7.1 || ^8.0", + "php": "^7.2 || ^8.0", "php-stubs/wordpress-stubs": "^4.7 || ^5.0", - "phpstan/phpstan": "^1.0", + "phpstan/phpstan": "^1.6", "symfony/polyfill-php73": "^1.12.0" }, "require-dev": { - "composer/composer": "^2.1.12", + "composer/composer": "^2.1.14", "dealerdirect/phpcodesniffer-composer-installer": "^0.7", "php-parallel-lint/php-parallel-lint": "^1.1", - "phpstan/phpstan-strict-rules": "^1.0", - "phpunit/phpunit": "^7 || ^9", + "phpstan/phpstan-strict-rules": "^1.2", + "phpunit/phpunit": "^8 || ^9", "szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset": "^0.6" }, "autoload": { From 94a355102207683ec5d4784fe908db1c52e37224 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Mon, 2 May 2022 21:06:53 +0200 Subject: [PATCH 14/14] Avoid namespace-clashing of test helpers --- tests/HookDocsRuleTest.php | 24 ++++++++++++------------ tests/data/apply_filters.php | 1 - tests/data/hook-docs.php | 9 +++------ tests/functions.php | 6 ++++-- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/HookDocsRuleTest.php b/tests/HookDocsRuleTest.php index c1bb0e8a..48007fc7 100644 --- a/tests/HookDocsRuleTest.php +++ b/tests/HookDocsRuleTest.php @@ -38,47 +38,47 @@ public function testRule(): void [ [ 'Expected 2 @param tags, found 1.', - 22, + 19, ], [ 'Expected 2 @param tags, found 3.', - 31, + 28, ], [ '@param string $one does not accept actual type of parameter: int|string.', - 43, + 40, ], [ '@param string $one does not accept actual type of parameter: int.', - 53, + 50, ], [ '@param tag must not be named $this. Choose a descriptive alias, for example $instance.', - 82, + 79, ], [ 'Expected 2 @param tags, found 1.', - 97, + 94, ], [ - '@param ChildTestClass $one does not accept actual type of parameter: ParentTestClass.', - 134, + '@param SzepeViktor\PHPStan\WordPress\Tests\ChildTestClass $one does not accept actual type of parameter: SzepeViktor\PHPStan\WordPress\Tests\ParentTestClass.', + 131, ], [ '@param string $one does not accept actual type of parameter: string|null.', - 155, + 152, ], [ 'One or more @param tags has an invalid name or invalid syntax.', - 170, + 167, ], [ 'One or more @param tags has an invalid name or invalid syntax.', - 206, + 203, ], [ 'Expected 2 @param tags, found 1.', - 217, + 214, ], ] ); diff --git a/tests/data/apply_filters.php b/tests/data/apply_filters.php index 698f42f1..1a07fbbb 100644 --- a/tests/data/apply_filters.php +++ b/tests/data/apply_filters.php @@ -9,7 +9,6 @@ use function PHPStan\Testing\assertType; use function apply_filters; -use function returnValue; $value = apply_filters('filter', 'Hello, World'); assertType('mixed', $value); diff --git a/tests/data/hook-docs.php b/tests/data/hook-docs.php index 710130da..e718e7b1 100644 --- a/tests/data/hook-docs.php +++ b/tests/data/hook-docs.php @@ -4,9 +4,6 @@ namespace SzepeViktor\PHPStan\WordPress\Tests; -use ChildTestClass; -use ParentTestClass; - // phpcs:disable Squiz.NamingConventions.ValidFunctionName.NotCamelCaps,Squiz.NamingConventions.ValidVariableName.NotCamelCaps,Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps $one = 1; @@ -109,7 +106,7 @@ function correct_inherited_param_type(ChildTestClass $one) /** * This param tag is for a super class of the variable, which is fine. * - * @param \ParentTestClass $one First parameter. + * @param ParentTestClass $one First parameter. */ $args = apply_filters('filter', $one); } @@ -119,7 +116,7 @@ function correct_interface_param_type(ChildTestClass $one) /** * This param tag is for the interface of the variable, which is fine. * - * @param \TestInterface $one First parameter. + * @param TestInterface $one First parameter. */ $args = apply_filters('filter', $one); } @@ -129,7 +126,7 @@ function incorrect_inherited_param_type(ParentTestClass $one) /** * This param tag is for a child class of the variable. Oh no. * - * @param \ChildTestClass $one First parameter. + * @param ChildTestClass $one First parameter. */ $args = apply_filters('filter', $one); } diff --git a/tests/functions.php b/tests/functions.php index 1b30d85f..269d7c1a 100644 --- a/tests/functions.php +++ b/tests/functions.php @@ -2,6 +2,8 @@ declare(strict_types=1); +namespace SzepeViktor\PHPStan\WordPress\Tests; + /** * Returns the passed value. * @@ -18,10 +20,10 @@ interface TestInterface { } -class ParentTestClass implements \TestInterface +class ParentTestClass implements TestInterface { } -class ChildTestClass extends \ParentTestClass +class ChildTestClass extends ParentTestClass { }