Improving the MethodMustReturnType rule

Author: Florian Krämer

data/MethodMustReturnType/ReturnTypeTestClass.php

@@ -2,10 +2,15 @@
 
 class ReturnTypeTestClass
 {
-    public function mustReturnInt(): void {}
-    public function mustReturnNullableString(): string {}
-    public function mustReturnVoid(): int {}
-    public function mustReturnSpecificObject(): OtherObject {}
+    // Invalid cases (should trigger errors)
+    public function mustReturnInt(): void { return; }
+    public function mustReturnNullableString(): string { return 'test'; }
+    public function mustReturnVoid(): int { return 1; }
+    public function mustReturnVoidLegacy(): int { return 1; }
+    public function mustReturnSpecificObject(): OtherObject { return new OtherObject(); }
+    public function mustReturnOneOf(): float { return 1.0; }
+    public function mustReturnAllOf(): int { return 1; }
+    public function mustReturnOneOfNullable(): string { return 'test'; }
 }
 
 class SomeObject {}

data/MethodMustReturnType/UnionTypeTestClass.php

@@ -0,0 +1,17 @@
+<?php
+
+class UnionTypeTestClass
+{
+    // Test cases for oneOf functionality
+    public function validOneOfInt(): int { return 1; }
+    public function validOneOfString(): string { return 'test'; }
+    public function validOneOfBool(): bool { return true; }
+    
+    // Test cases for allOf functionality (simplified)
+    public function validAllOfInt(): int { return 1; }
+    public function validAllOfString(): string { return 'test'; }
+    
+    // Invalid cases
+    public function invalidOneOf(): float { return 1.0; }
+    public function invalidAllOf(): bool { return true; }
+} 

docs/Rules.md

@@ -7,6 +7,7 @@ Add them to your `phpstan.neon` configuration file under the section `services`.
 Ensures that the nesting level of `if` and `try-catch` statements does not exceed a specified limit.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\CleanCode\ControlStructureNestingRule
@@ -21,6 +22,7 @@ Ensures that the nesting level of `if` and `try-catch` statements does not excee
 Checks that methods do not have more than a specified number of arguments.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\CleanCode\TooManyArgumentsRule
@@ -35,6 +37,7 @@ Checks that methods do not have more than a specified number of arguments.
 Ensures that classes matching specified patterns are declared as `readonly`.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\ClassMustBeReadonlyRule
@@ -53,6 +56,7 @@ The constructor takes an array of namespace dependencies. The key is the namespa
 In the example below nothing from `App\Domain` can depend on anything from `App\Controller`.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\DependencyConstraintsRule
@@ -69,6 +73,7 @@ In the example below nothing from `App\Domain` can depend on anything from `App\
 Ensures that classes matching specified patterns are declared as `final`.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\ClassMustBeFinalRule
@@ -83,6 +88,7 @@ Ensures that classes matching specified patterns are declared as `final`.
 Ensures that classes inside namespaces matching a given regex must have names matching at least one of the provided patterns.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\ClassnameMustMatchPatternRule
@@ -104,6 +110,7 @@ Ensures that classes inside namespaces matching a given regex must have names ma
 Ensures that specific exception types are not caught in catch blocks. This is useful for preventing the catching of overly broad exception types like `Exception`, `Error`, or `Throwable`.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\CatchExceptionOfTypeNotAllowedRule
@@ -118,6 +125,7 @@ Ensures that specific exception types are not caught in catch blocks. This is us
 Ensures that methods returning boolean values follow a specific naming convention. By default, boolean methods should start with `is`, `has`, `can`, `should`, `was`, or `will`.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\MethodsReturningBoolMustFollowNamingConventionRule
@@ -132,6 +140,7 @@ Ensures that methods returning boolean values follow a specific naming conventio
 Ensures that methods matching a class and method name pattern have a specific signature, including parameter types, names, and count.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\MethodSignatureMustMatchRule
@@ -151,16 +160,18 @@ Ensures that methods matching a class and method name pattern have a specific si
         tags:
             - phpstan.rules.rule
 ```
+
 - `pattern`: Regex for `ClassName::methodName`.
 - `minParameters`/`maxParameters`: Minimum/maximum number of parameters.
 - `signature`: List of expected parameter types and (optionally) name patterns.
 - `visibilityScope`: Optional visibility scope (e.g., `public`, `protected`, `private`).
 
 ## Method Must Return Type Rule {#method-must-return-type-rule}
 
-Ensures that methods matching a class and method name pattern have a specific return type, nullability, or are void.
+Ensures that methods matching a class and method name pattern have a specific return type, nullability, or are void. Supports union types with "oneOf" and "allOf" configurations.
 
 **Configuration Example:**
+
 ```neon
     -
         class: Phauthentic\PHPStanRules\Architecture\MethodMustReturnTypeRule
@@ -182,13 +193,28 @@ Ensures that methods matching a class and method name pattern have a specific re
                     pattern: '/^MyClass::reset$/'
                     type: 'void'
                     nullable: false
-                    void: true
+                    void: false
+                    objectTypePattern: null
+                -
+                    pattern: '/^MyClass::getValue$/'
+                    nullable: false
+                    void: false
+                    oneOf: ['int', 'string', 'bool']
+                    objectTypePattern: null
+                -
+                    pattern: '/^MyClass::getUnionType$/'
+                    nullable: false
+                    void: false
+                    allOf: ['int', 'string']
                     objectTypePattern: null
         tags:
             - phpstan.rules.rule
 ```
+
 - `pattern`: Regex for `ClassName::methodName`.
-- `type`: Expected return type (`int`, `string`, `object`, etc.).
+- `type`: Expected return type (`int`, `string`, `object`, `void`, etc.). When using `oneOf` or `allOf`, this field is optional.
 - `nullable`: Whether the return type must be nullable.
-- `void`: Whether the method must return void.
+- `void`: Legacy field for void return types (use `type: 'void'` instead).
 - `objectTypePattern`: Regex for object return types (if `type` is `object`).
+- `oneOf`: Array of types where one must match (for union types).
+- `allOf`: Array of types where all must be present in the union type.

src/Architecture/MethodMustReturnTypeRule.php

@@ -20,6 +20,7 @@
  * - Checks if the method returns the expected type or object, is nullable or void.
  * - Check if the types of the parameters match the expected types.
  * - If an object type is expected, it can match a specific class or a pattern.
+ * - Supports union types with "oneOf" (one type must match) and "allOf" (all types must match).
  */
 class MethodMustReturnTypeRule implements Rule
 {
@@ -31,14 +32,18 @@ class MethodMustReturnTypeRule implements Rule
     private const ERROR_MESSAGE_OBJECT_TYPE_PATTERN = 'Method %s must return an object matching pattern %s, %s given.';
     private const ERROR_MESSAGE_OBJECT_TYPE = 'Method %s must return an object type.';
     private const ERROR_MESSAGE_TYPE_MISMATCH = 'Method %s must have return type %s, %s given.';
+    private const ERROR_MESSAGE_ONE_OF_MISMATCH = 'Method %s must have one of the return types: %s, %s given.';
+    private const ERROR_MESSAGE_ALL_OF_MISMATCH = 'Method %s must have all of the return types: %s, %s given.';
 
     /**
      * @param array<array{
      *     pattern: string,
-     *     type: string,
+     *     type?: string,
      *     nullable: bool,
      *     void: bool,
      *     objectTypePattern: string|null,
+     *     oneOf?: array<string>,
+     *     allOf?: array<string>,
      * }> $returnTypePatterns
      */
     public function __construct(
@@ -81,7 +86,8 @@ public function processNode(Node $node, Scope $scope): array
 
                 // Check for missing return type
                 if ($this->shouldErrorOnMissingReturnType($returnType)) {
-                    $errors[] = $this->buildMissingReturnTypeError($fullName, $patternConfig['type'], $method->getLine());
+                    $expectedType = $this->getExpectedTypeDescription($patternConfig);
+                    $errors[] = $this->buildMissingReturnTypeError($fullName, $expectedType, $method->getLine());
                     continue;
                 }
 
@@ -92,18 +98,32 @@ public function processNode(Node $node, Scope $scope): array
                     $errors[] = $this->buildNullabilityError($fullName, $patternConfig['nullable'], $method->getLine());
                 }
 
-                // Check for type
-                if ($patternConfig['type'] === 'object') {
-                    if ($returnTypeNode instanceof Name) {
-                        $objectType = $returnTypeNode->toString();
-                        if ($this->shouldErrorOnObjectTypePattern($patternConfig, $objectType)) {
-                            $errors[] = $this->buildObjectTypePatternError($fullName, $patternConfig['objectTypePattern'], $objectType, $method->getLine());
+                // Check for union types (oneOf/allOf)
+                if (isset($patternConfig['oneOf'])) {
+                    if ($this->shouldErrorOnOneOf($patternConfig['oneOf'], $returnType)) {
+                        $errors[] = $this->buildOneOfError($fullName, $patternConfig['oneOf'], $returnType, $method->getLine());
+                        continue;
+                    }
+                } elseif (isset($patternConfig['allOf'])) {
+                    if ($this->shouldErrorOnAllOf($patternConfig['allOf'], $returnType)) {
+                        $errors[] = $this->buildAllOfError($fullName, $patternConfig['allOf'], $returnType, $method->getLine());
+                        continue;
+                    }
+                } else {
+                    // Check for single type
+                    $expectedType = $patternConfig['type'] ?? 'void';
+                    if ($expectedType === 'object') {
+                        if ($returnTypeNode instanceof Name) {
+                            $objectType = $returnTypeNode->toString();
+                            if ($this->shouldErrorOnObjectTypePattern($patternConfig, $objectType)) {
+                                $errors[] = $this->buildObjectTypePatternError($fullName, $patternConfig['objectTypePattern'], $objectType, $method->getLine());
+                            }
+                        } else {
+                            $errors[] = $this->buildObjectTypeError($fullName, $method->getLine());
                         }
-                    } else {
-                        $errors[] = $this->buildObjectTypeError($fullName, $method->getLine());
+                    } elseif ($this->shouldErrorOnTypeMismatch($returnType, $expectedType)) {
+                        $errors[] = $this->buildTypeMismatchError($fullName, $expectedType, $returnType, $method->getLine());
                     }
-                } elseif ($this->shouldErrorOnTypeMismatch($returnType, $patternConfig['type'])) {
-                    $errors[] = $this->buildTypeMismatchError($fullName, $patternConfig['type'], $returnType, $method->getLine());
                 }
             }
         }
@@ -134,6 +154,17 @@ private function shouldErrorOnMissingReturnType(?string $returnType): bool
         return $returnType === null;
     }
 
+    private function getExpectedTypeDescription(array $patternConfig): string
+    {
+        if (isset($patternConfig['oneOf'])) {
+            return 'one of: ' . implode(', ', $patternConfig['oneOf']);
+        }
+        if (isset($patternConfig['allOf'])) {
+            return 'all of: ' . implode(', ', $patternConfig['allOf']);
+        }
+        return $patternConfig['type'] ?? 'void';
+    }
+
     private function buildMissingReturnTypeError(string $fullName, string $expectedType, int $line)
     {
         return RuleErrorBuilder::message(
@@ -167,6 +198,114 @@ private function buildNullabilityError(string $fullName, bool $expectedNullable,
         ->build();
     }
 
+    private function shouldErrorOnOneOf(array $expectedTypes, ?string $returnType): bool
+    {
+        if ($returnType === null) {
+            return true;
+        }
+
+        foreach ($expectedTypes as $expectedType) {
+            if ($this->isTypeMatch($returnType, $expectedType)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private function buildOneOfError(string $fullName, array $expectedTypes, ?string $actualType, int $line)
+    {
+        return RuleErrorBuilder::message(
+            sprintf(
+                self::ERROR_MESSAGE_ONE_OF_MISMATCH,
+                $fullName,
+                implode(', ', $expectedTypes),
+                $actualType ?? 'no return type'
+            )
+        )
+        ->identifier(self::IDENTIFIER)
+        ->line($line)
+        ->build();
+    }
+
+    private function shouldErrorOnAllOf(array $expectedTypes, ?string $returnType): bool
+    {
+        if ($returnType === null) {
+            return true;
+        }
+
+        // For allOf, we need to check if the return type is a union type that contains all expected types
+        // This is a simplified implementation - in practice, you might need more sophisticated union type parsing
+        $actualTypes = $this->parseUnionType($returnType);
+
+        foreach ($expectedTypes as $expectedType) {
+            $found = false;
+            foreach ($actualTypes as $actualType) {
+                if ($this->isTypeMatch($actualType, $expectedType)) {
+                    $found = true;
+                    break;
+                }
+            }
+            if (!$found) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private function buildAllOfError(string $fullName, array $expectedTypes, ?string $actualType, int $line)
+    {
+        return RuleErrorBuilder::message(
+            sprintf(
+                self::ERROR_MESSAGE_ALL_OF_MISMATCH,
+                $fullName,
+                implode(', ', $expectedTypes),
+                $actualType ?? 'no return type'
+            )
+        )
+        ->identifier(self::IDENTIFIER)
+        ->line($line)
+        ->build();
+    }
+
+    private function parseUnionType(?string $type): array
+    {
+        if ($type === null) {
+            return [];
+        }
+
+        // Simple union type parsing - split by '|' and trim
+        return array_map('trim', explode('|', $type));
+    }
+
+    private function isTypeMatch(?string $actual, string $expected): bool
+    {
+        if ($actual === null) {
+            return false;
+        }
+
+        // Direct match
+        if ($actual === $expected) {
+            return true;
+        }
+
+        // Handle nullable types
+        if ($this->isNullableMatch($actual, $expected)) {
+            return true;
+        }
+
+        // Handle union types
+        $actualTypes = $this->parseUnionType($actual);
+        foreach ($actualTypes as $actualType) {
+            if ($actualType === $expected || $this->isNullableMatch($actualType, $expected)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private function shouldErrorOnObjectTypePattern(array $patternConfig, string $objectType): bool
     {
         return $patternConfig['objectTypePattern'] !== null &&
@@ -203,7 +342,7 @@ private function buildObjectTypeError(string $fullName, int $line)
 
     private function shouldErrorOnTypeMismatch(?string $returnType, string $expectedType): bool
     {
-        return $returnType !== $expectedType && !$this->isNullableMatch($returnType, $expectedType);
+        return !$this->isTypeMatch($returnType, $expectedType);
     }
 
     private function buildTypeMismatchError(string $fullName, string $expectedType, ?string $actualType, int $line)
@@ -213,7 +352,7 @@ private function buildTypeMismatchError(string $fullName, string $expectedType,
                 self::ERROR_MESSAGE_TYPE_MISMATCH,
                 $fullName,
                 $expectedType,
-                $actualType
+                $actualType ?? 'no return type'
             )
         )
         ->identifier(self::IDENTIFIER)