Hook callback false positives for mixed return type (#130)

* Increase the memory limit during unit tests.

* Add some failing tests.

* More tests for `mixed`.

* Boop.

* Add a test for implicit mixed return type.

* Add another implicit return type test for an action callback.

* Update tests/data/hook-callback.php

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

* Implicit mixed is not the responsibility of this library.

* Add support for filters that return explicit mixed. Props @herndlm.

* Add support for actions that incorrectly return explicit mixed.

* Fix the implicit handling.

* Update composer.json

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

src/HookCallbackRule.php

@@ -17,6 +17,7 @@
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Rules\RuleLevelHelper;
 use PHPStan\Type\Constant\ConstantIntegerType;
+use PHPStan\Type\MixedType;
 use PHPStan\Type\VerbosityLevel;
 use PHPStan\Type\VoidType;
 
@@ -184,30 +185,43 @@ protected static function buildParameterCountMessage(int $minArgs, int $maxArgs,
     protected function validateActionReturnType(ParametersAcceptor $callbackAcceptor): void
     {
         $acceptedType = $callbackAcceptor->getReturnType();
+        $exception = new \SzepeViktor\PHPStan\WordPress\HookCallbackException(
+            sprintf(
+                'Action callback returns %s but should not return anything.',
+                $acceptedType->describe(VerbosityLevel::getRecommendedLevelByType($acceptedType))
+            )
+        );
+
+        if ($acceptedType instanceof MixedType && $acceptedType->isExplicitMixed()) {
+            throw $exception;
+        }
+
         $accepted = $this->ruleLevelHelper->accepts(
             new VoidType(),
             $acceptedType,
             true
         );
 
-        if ($accepted) {
-            return;
+        if (! $accepted) {
+            throw $exception;
         }
-
-        throw new \SzepeViktor\PHPStan\WordPress\HookCallbackException(
-            sprintf(
-                'Action callback returns %s but should not return anything.',
-                $acceptedType->describe(VerbosityLevel::getRecommendedLevelByType($acceptedType))
-            )
-        );
     }
 
     protected function validateFilterReturnType(ParametersAcceptor $callbackAcceptor): void
     {
         $returnType = $callbackAcceptor->getReturnType();
-        $isVoidSuperType = $returnType->isSuperTypeOf(new VoidType());
 
-        if (! $isVoidSuperType->yes()) {
+        if ($returnType instanceof MixedType) {
+            return;
+        }
+
+        $accepted = $this->ruleLevelHelper->accepts(
+            new VoidType(),
+            $returnType,
+            true
+        );
+
+        if (! $accepted) {
             return;
         }
 

tests/HookCallbackRuleTest.php

@@ -113,7 +113,7 @@ public function testRule(): void
                     96,
                 ],
                 [
-                    'Filter callback return statement is missing.',
+                    'Action callback returns mixed but should not return anything.',
                     99,
                 ],
             ]

tests/data/hook-callback.php

@@ -95,8 +95,8 @@
 // Action callback returns false but should not return anything (with incorrect number of accepted args).
 add_action('action', '__return_false', 10, 2);
 
-// Filter callback return statement is missing.
-add_filter('filter', __NAMESPACE__ . '\\no_return_value_untyped');
+// Action callback returns mixed but should not return anything.
+add_action('action', __NAMESPACE__ . '\\return_value_mixed');
 
 /**
  * Incorrect usage that's handled by PHPStan:
@@ -145,6 +145,7 @@
 
 // Callback function with no declared return type.
 add_action('action', __NAMESPACE__ . '\\return_value_untyped');
+add_filter('filter', __NAMESPACE__ . '\\no_return_value_untyped');
 
 /**
  * Correct usage:
@@ -214,6 +215,11 @@
 
 // Various callback types
 add_filter('filter', '__return_false');
+add_filter('filter', __NAMESPACE__ . '\\return_value_mixed');
+add_filter('filter', __NAMESPACE__ . '\\return_value_untyped');
+add_filter('filter', __NAMESPACE__ . '\\return_value_mixed_union');
+add_filter('filter', __NAMESPACE__ . '\\return_value_documented');
+add_filter('filter', __NAMESPACE__ . '\\return_value_untyped');
 add_filter('filter', __NAMESPACE__ . '\\return_value_typed');
 add_filter('filter', new TestInvokableTyped(), 10, 2);
 add_filter('filter', [new TestClassTyped, 'foo']);
@@ -240,6 +246,31 @@ function return_value_typed() : int {
 
 function no_return_value_untyped( $value ) {}
 
+/**
+ * @return mixed
+ */
+function return_value_mixed() {
+    return 123;
+}
+
+/**
+ * Return type documented as a union that includes mixed.
+ *
+ * This is added as a regression test for a bug.
+ *
+ * @return int|mixed
+ */
+function return_value_mixed_union() {
+    return 123;
+}
+
+/**
+ * @return int
+ */
+function return_value_documented() {
+    return 123;
+}
+
 function return_value_untyped() {
     return 123;
 }