check "never" return type more strictly

* require explicit "never" return type when function always exits, except if it only throws * error if function does not exit, but return type explicitly contains "never" * Fix: #8175 * Fix: #8178
vimeo · Dec 1, 2022 · 694b7d8 · 694b7d8
1 parent bce4b55
commit 694b7d8
Show file tree

Hide file tree

Showing 12 changed files with 204 additions and 27 deletions.
diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
@@ -48,13 +48,10 @@
 use Psalm\Storage\MethodStorage;
 use Psalm\Type;
 use Psalm\Type\Atomic\TNamedObject;
-use Psalm\Type\Atomic\TNever;
 use Psalm\Type\Atomic\TNull;
 use Psalm\Type\Union;
 
 use function array_diff;
-use function array_filter;
-use function array_values;
 use function count;
 use function implode;
 use function in_array;
@@ -238,23 +235,12 @@ public static function verifyReturnType(
             return null;
         }
 
-        $number_of_types = count($inferred_return_type_parts);
-        // we filter TNever that have no bearing on the return type
-        if ($number_of_types > 1) {
-            $inferred_return_type_parts = array_filter(
-                $inferred_return_type_parts,
-                static fn(Union $union_type): bool => !$union_type->isNever()
-            );
-        }
-
-        $inferred_return_type_parts = array_values($inferred_return_type_parts);
-
         $inferred_return_type = $inferred_return_type_parts
             ? Type::combineUnionTypeArray($inferred_return_type_parts, $codebase)
             : Type::getVoid();
 
         if ($function_always_exits) {
-            $inferred_return_type = new Union([new TNever]);
+            $inferred_return_type = Type::getNever();
         }
 
         $inferred_yield_type = $inferred_yield_types
@@ -507,6 +493,61 @@ public static function verifyReturnType(
 
             $union_comparison_results = new TypeComparisonResult();
 
+            if ($declared_return_type->explicit_never === true && $inferred_return_type->explicit_never === false) {
+                if (IssueBuffer::accepts(
+                    new MoreSpecificReturnType(
+                        'The declared return type \'' . $declared_return_type->getId() . '|never\' for '
+                        . $cased_method_id . ' is more specific than the inferred return type '
+                        . '\'' . $inferred_return_type->getId() . '\'',
+                        $return_type_location
+                    ),
+                    $suppressed_issues
+                )) {
+                    return false;
+                }
+            }
+
+            if (!$declared_return_type->isNever()
+                && $function_always_exits
+                // never return type only available from PHP 8.1 in non-docblock
+                && ($declared_return_type->from_docblock || $codebase->analysis_php_version_id >= 8_10_00)
+                // no error for single throw, as extending a class might not work without errors
+                // https://3v4l.org/vCSF4#v8.1.12
+                && !ScopeAnalyzer::onlyThrows($function_stmts)
+            ) {
+                if ($codebase->alter_code
+                    && isset($project_analyzer->getIssuesToFix()['InvalidReturnType'])
+                    && !in_array('InvalidReturnType', $suppressed_issues)
+                ) {
+                    self::addOrUpdateReturnType(
+                        $function,
+                        $project_analyzer,
+                        Type::getNever(),
+                        $source,
+                        ($project_analyzer->only_replace_php_types_with_non_docblock_types
+                         || $unsafe_return_type)
+                        && $inferred_return_type->from_docblock,
+                        $function_like_storage
+                    );
+
+                    return null;
+                }
+
+                if (IssueBuffer::accepts(
+                    new InvalidReturnType(
+                        'The declared return type \''
+                        . $declared_return_type->getId()
+                        . '\' for ' . $cased_method_id
+                        . ' is incorrect, got \'never\'',
+                        $return_type_location
+                    ),
+                    $suppressed_issues,
+                    true
+                )) {
+                    return false;
+                }
+            }
+
             if (!UnionTypeComparator::isContainedBy(
                 $codebase,
                 $inferred_return_type,

diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeCollector.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeCollector.php
@@ -79,22 +79,27 @@ public static function getReturnTypes(
             }
 
             if ($stmt instanceof PhpParser\Node\Stmt\Throw_) {
-                if ($collapse_types) {
-                    $return_types[] = Type::getNever();
-                }
+                $return_types[] = Type::getNever();
 
                 break;
             }
 
             if ($stmt instanceof PhpParser\Node\Stmt\Expression) {
                 if ($stmt->expr instanceof PhpParser\Node\Expr\Exit_) {
-                    if ($collapse_types) {
-                        $return_types[] = Type::getNever();
-                    }
+                    $return_types[] = Type::getNever();
 
                     break;
                 }
 
+                if ($stmt->expr instanceof PhpParser\Node\Expr\FuncCall) {
+                    $stmt_type = $nodes->getType($stmt->expr);
+                    if ($stmt_type && ($stmt_type->isNever() || $stmt_type->explicit_never)) {
+                        $return_types[] = Type::getNever();
+
+                        break;
+                    }
+                }
+
                 if ($stmt->expr instanceof PhpParser\Node\Expr\Assign) {
                     $return_types = [
                         ...$return_types,

diff --git a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php
@@ -411,4 +411,24 @@ public static function onlyThrowsOrExits(NodeTypeProvider $type_provider, array
 
         return false;
     }
+
+    /**
+     * @param   array<PhpParser\Node> $stmts
+     *
+     */
+    public static function onlyThrows(array $stmts): bool
+    {
+        $stmts_count = count($stmts);
+        if ($stmts_count !== 1) {
+            return false;
+        }
+
+        foreach ($stmts as $stmt) {
+            if ($stmt instanceof PhpParser\Node\Stmt\Throw_) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php
@@ -45,7 +45,6 @@
 use Psalm\Storage\PropertyStorage;
 use Psalm\Type;
 use Psalm\Type\Atomic\TArray;
-use Psalm\Type\Atomic\TNever;
 use Psalm\Type\Atomic\TNull;
 use Psalm\Type\Union;
 use ReflectionFunction;
@@ -742,7 +741,7 @@ public function start(PhpParser\Node\FunctionLike $stmt, bool $fake_method = fal
                 }
 
                 if ($attribute->fq_class_name === 'JetBrains\\PhpStorm\\NoReturn') {
-                    $storage->return_type = new Union([new TNever()]);
+                    $storage->return_type = Type::getNever();
                 }
 
                 $storage->attributes[] = $attribute;

diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/TriggerErrorReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/TriggerErrorReturnTypeProvider.php
@@ -7,6 +7,7 @@
 use Psalm\Internal\Type\TypeCombiner;
 use Psalm\Plugin\EventHandler\Event\FunctionReturnTypeProviderEvent;
 use Psalm\Plugin\EventHandler\FunctionReturnTypeProviderInterface;
+use Psalm\Type;
 use Psalm\Type\Atomic\TBool;
 use Psalm\Type\Atomic\TFalse;
 use Psalm\Type\Atomic\TLiteralInt;
@@ -39,7 +40,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
         $codebase = $event->getStatementsSource()->getCodebase();
         $config = $codebase->config;
         if ($config->trigger_error_exits === 'always') {
-            return new Union([new TNever()]);
+            return Type::getNever();
         }
 
         if ($config->trigger_error_exits === 'never') {

diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php
@@ -356,13 +356,22 @@ public static function combine(
         if (!$new_types && !$has_never) {
             throw new UnexpectedValueException('There should be types here');
         } elseif (!$new_types && $has_never) {
-            $union_type = Type::getNever();
+            $union_type = Type::getNever($from_docblock);
         } else {
             $union_type = new Union($new_types);
         }
 
+        $union_properties = [];
         if ($from_docblock) {
-            return $union_type->setProperties(['from_docblock' => true]);
+            $union_properties['from_docblock'] = true;
+        }
+
+        if ($has_never) {
+            $union_properties['explicit_never'] = true;
+        }
+
+        if ($union_properties !== []) {
+            return $union_type->setProperties($union_properties);
         }
 
         return $union_type;

diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php
@@ -119,6 +119,7 @@ public static function expandUnion(
         $fleshed_out_type->initialized = $return_type->initialized;
         $fleshed_out_type->from_property = $return_type->from_property;
         $fleshed_out_type->from_static_property = $return_type->from_static_property;
+        $fleshed_out_type->explicit_never = $return_type->explicit_never;
         $fleshed_out_type->had_template = $return_type->had_template;
         $fleshed_out_type->parent_nodes = $return_type->parent_nodes;
 

diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php
@@ -349,7 +349,6 @@ public static function getScalar(bool $from_docblock = false): Union
     public static function getNever(bool $from_docblock = false): Union
     {
         $type = new TNever($from_docblock);
-
         return new Union([$type]);
     }
 
@@ -609,6 +608,10 @@ public static function combineUnionTypes(
                 $combined_type->ignore_falsable_issues = true;
             }
 
+            if ($type_1->explicit_never && $type_2->explicit_never) {
+                $combined_type->explicit_never = true;
+            }
+
             if ($type_1->had_template && $type_2->had_template) {
                 $combined_type->had_template = true;
             }

diff --git a/src/Psalm/Type/MutableUnion.php b/src/Psalm/Type/MutableUnion.php
@@ -18,6 +18,7 @@
 use Psalm\Type\Atomic\TLiteralString;
 use Psalm\Type\Atomic\TMixed;
 use Psalm\Type\Atomic\TNamedObject;
+use Psalm\Type\Atomic\TNever;
 use Psalm\Type\Atomic\TString;
 use Psalm\Type\Atomic\TTemplateParamClass;
 use Psalm\Type\Atomic\TTrue;
@@ -131,6 +132,16 @@ final class MutableUnion implements TypeNode, Stringable
      */
     public $possibly_undefined_from_try = false;
 
+    /**
+     * whether this type had never set explicitly
+     * since it's the bottom type, it's combined into everything else and lost
+     *
+     * @psalm-suppress PossiblyUnusedProperty used in setTypes and addType
+     *
+     * @var bool
+     */
+    public $explicit_never = false;
+
     /**
      * Whether or not this union had a template, since replaced
      *
@@ -244,6 +255,8 @@ public function setTypes(array $types): self
                 && ($type->as_type || $type instanceof TTemplateParamClass)
             ) {
                 $this->typed_class_strings[$key] = $type;
+            } elseif ($type instanceof TNever) {
+                $this->explicit_never = true;
             }
 
             $from_docblock = $from_docblock || $type->from_docblock;
@@ -291,6 +304,8 @@ public function addType(Atomic $type): self
             foreach ($this->literal_float_types as $key => $_) {
                 unset($this->literal_float_types[$key], $this->types[$key]);
             }
+        } elseif ($type instanceof TNever) {
+            $this->explicit_never = true;
         }
 
         $this->bustCache();

diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php
@@ -29,6 +29,7 @@
  *      ignore_isset?: bool,
  *      possibly_undefined?: bool,
  *      possibly_undefined_from_try?: bool,
+ *      explicit_never?: bool,
  *      had_template?: bool,
  *      from_template_default?: bool,
  *      by_ref?: bool,
@@ -144,6 +145,14 @@ final class Union implements TypeNode, Stringable
      */
     public $possibly_undefined_from_try = false;
 
+    /**
+     * whether this type had never set explicitly
+     * since it's the bottom type, it's combined into everything else and lost
+     *
+     * @var bool
+     */
+    public $explicit_never = false;
+
     /**
      * Whether or not this union had a template, since replaced
      *

diff --git a/src/Psalm/Type/UnionTrait.php b/src/Psalm/Type/UnionTrait.php
@@ -31,6 +31,7 @@
 use Psalm\Type\Atomic\TLowercaseString;
 use Psalm\Type\Atomic\TMixed;
 use Psalm\Type\Atomic\TNamedObject;
+use Psalm\Type\Atomic\TNever;
 use Psalm\Type\Atomic\TNonEmptyLowercaseString;
 use Psalm\Type\Atomic\TNonspecificLiteralInt;
 use Psalm\Type\Atomic\TNonspecificLiteralString;
@@ -94,6 +95,8 @@ public function __construct(array $types, array $properties = [])
                 && ($type->as_type || $type instanceof TTemplateParamClass)
             ) {
                 $this->typed_class_strings[$key] = $type;
+            } elseif ($type instanceof TNever) {
+                $this->explicit_never = true;
             }
 
             $from_docblock = $from_docblock || $type->from_docblock;