Skip to content

Commit

Permalink
Always import classes (#119)
Browse files Browse the repository at this point in the history
* Fix FQCN

* Add type checking for hook callbacks in `add_action` and `add_filter` (#107) (#121)

* Setup the hook callback rule and test.

* Add the wp-hooks library.

* Start adding tests.

* Preparing more tests.

* If we don't have enough arguments, bail out and let PHPStan handle the error.

* If the callback is not valid, bail out and let PHPStan handle the error:

* Simplify this.

* More valid test cases.

* Update some test line numbers.

* First pass at detecting mismatching accepted and expected argument counts.

* Remove some accidental duplicates.

* Fix some logic.

* Let's split this up before it gets out of hand.

* Reduce this down.

* More tidying up.

* We're going to be needing this in multiple methods soon.

* Prep for callback return type checking, not yet functional.

* Split action return type assertion into its own method.

* Bring this message more inline with those used by PHPStan.

* Add detection for filter callbacks with no return value.

* Don't need this just yet.

* Use early exit to reduce code nesting.

* Remove unused imports.

* Yoda comparisons are disallowed.

* Coding standards.

* Remove unused code.

* Use early return to reduce code nesting.

* Remove more unused code.

* Renaming.

* Use early return to reduce code nesting.

* Tidying up.

* Correct logic introduced during refactoring.

* Exclude "maybe" callables.

* More handling for parameter counts.

* More handling for optional parameters in hook callbacks.

* Make the tests more manageable.

* Tests for more callback types.

* Tidying up.

* This isn't used.

* More test cases.

* Tests for variadic callbacks.

* More specific handling for variadic callbacks.

* More tests.

* The hooks names are not relevant to the tests.

* Remove an `else`.

* I'm still not entirely sure why this works, but it does. Props @herndlm.

* Another test for an action with a return value.

* More variadic handling.

* Turns out PHPStan handles typed and untyped functions differently.

* Partly broken callbacks will trigger an error in PHPStan so we don't need to handle them.

* Terminology.

* Refactoring.

* PHP 7.2 compatibility.

* Reorder the processing so the return type is checked first.

* More tests.

Co-authored-by: Viktor Szépe <viktor@szepe.net>

Co-authored-by: John Blackbourn <john@humanmade.com>

* Fix

* Fix

Co-authored-by: John Blackbourn <john@humanmade.com>
  • Loading branch information
szepeviktor and johnbillion committed Nov 9, 2022
1 parent c02f76a commit f4d433c
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 10 deletions.
11 changes: 7 additions & 4 deletions src/HookDocsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
use PHPStan\PhpDoc\Tag\ParamTag;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Rules\RuleLevelHelper;
Expand Down Expand Up @@ -67,7 +70,7 @@ public function processNode(Node $node, Scope $scope): array
$this->currentNode = $node;
$this->currentScope = $scope;

if (!($name instanceof \PhpParser\Node\Name)) {
if (!($name instanceof Name)) {
return [];
}

Expand All @@ -91,7 +94,7 @@ public function processNode(Node $node, Scope $scope): array
* @param \PHPStan\PhpDoc\ResolvedPhpDocBlock $resolvedPhpDoc
* @return array<int,\PHPStan\Rules\RuleError>
*/
public function validateDocBlock(\PHPStan\PhpDoc\ResolvedPhpDocBlock $resolvedPhpDoc): array
public function validateDocBlock(ResolvedPhpDocBlock $resolvedPhpDoc): array
{
// Count all documented `@param` tag strings in the docblock.
$numberOfParamTagStrings = substr_count($resolvedPhpDoc->getPhpDocString(), '* @param ');
Expand Down Expand Up @@ -169,7 +172,7 @@ public function validateParamCount(int $numberOfParamTagStrings): void
*/
public function validateParamDocumentation(
int $numberOfParamTags,
\PHPStan\PhpDoc\ResolvedPhpDocBlock $resolvedPhpDoc
ResolvedPhpDocBlock $resolvedPhpDoc
): void {
$nodeArgs = $this->currentNode->getArgs();
$numberOfParams = count($nodeArgs) - 1;
Expand All @@ -182,7 +185,7 @@ public function validateParamDocumentation(
// We might have an invalid `@param` tag because it's named `$this`.
if (strpos($resolvedPhpDoc->getPhpDocString(), ' $this') !== false) {
foreach ($nodeArgs as $param) {
if (($param->value instanceof \PhpParser\Node\Expr\Variable) && $param->value->name === 'this') {
if (($param->value instanceof Variable) && $param->value->name === 'this') {
// PHPStan does not detect param tags named `$this`, it skips the tag.
// We can indirectly detect this by checking the actual parameter name,
// and if one of them is `$this` assume that's the problem.
Expand Down
3 changes: 2 additions & 1 deletion src/IsWpErrorRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Rules\RuleLevelHelper;
Expand Down Expand Up @@ -46,7 +47,7 @@ public function processNode(Node $node, Scope $scope): array
{
$name = $node->name;

if (! ($name instanceof \PhpParser\Node\Name)) {
if (! ($name instanceof Name)) {
return [];
}

Expand Down
3 changes: 2 additions & 1 deletion src/WpThemeGetDynamicMethodReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use WP_Theme;

class WpThemeGetDynamicMethodReturnTypeExtension implements \PHPStan\Type\DynamicMethodReturnTypeExtension
{
Expand All @@ -46,7 +47,7 @@ class WpThemeGetDynamicMethodReturnTypeExtension implements \PHPStan\Type\Dynami

public function getClass(): string
{
return 'WP_Theme';
return WP_Theme::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
Expand Down
3 changes: 2 additions & 1 deletion tests/HookDocsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace SzepeViktor\PHPStan\WordPress\Tests;

use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleLevelHelper;
use PHPStan\Type\FileTypeMapper;
use SzepeViktor\PHPStan\WordPress\HookDocsRule;
Expand All @@ -13,7 +14,7 @@
*/
class HookDocsRuleTest extends \PHPStan\Testing\RuleTestCase
{
protected function getRule(): \PHPStan\Rules\Rule
protected function getRule(): Rule
{
/** @var \PHPStan\Type\FileTypeMapper */
$fileTypeMapper = self::getContainer()->getByType(FileTypeMapper::class);
Expand Down
3 changes: 2 additions & 1 deletion tests/IsWpErrorRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace SzepeViktor\PHPStan\WordPress\Tests;

use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleLevelHelper;
use SzepeViktor\PHPStan\WordPress\IsWpErrorRule;

Expand All @@ -12,7 +13,7 @@
*/
class IsWpErrorRuleTest extends \PHPStan\Testing\RuleTestCase
{
protected function getRule(): \PHPStan\Rules\Rule
protected function getRule(): Rule
{
/** @var \PHPStan\Rules\RuleLevelHelper */
$ruleLevelHelper = self::getContainer()->getByType(RuleLevelHelper::class);
Expand Down
2 changes: 1 addition & 1 deletion tests/data/esc_sql.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
assertType('string', esc_sql('someValueToEscape'));

// Wrong type provided (esc_sql() returns an empty string in that case)
assertType('string', esc_sql(new \stdClass()));
assertType('string', esc_sql(new stdClass()));
3 changes: 2 additions & 1 deletion tests/data/get_post.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@

namespace SzepeViktor\PHPStan\WordPress\Tests;

use stdClass;
use function PHPStan\Testing\assertType;

/** @var \WP_Post $wpPostType */
$wpPostType = new \stdClass();
$wpPostType = new stdClass();

// Default output
assertType('WP_Post|null', get_post());
Expand Down

0 comments on commit f4d433c

Please sign in to comment.