Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

check "never" return type more strictly #8624

Merged
merged 1 commit into from Dec 1, 2022
Merged

check "never" return type more strictly #8624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 56 additions & 15 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 11 additions & 6 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeCollector.php
View file
Open in desktop
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions src/Psalm/Internal/Analyzer/ScopeAnalyzer.php
View file
Open in desktop
Expand Up @@ -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;
}
}
3 changes: 1 addition & 2 deletions src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Provider/ReturnTypeProvider/TriggerErrorReturnTypeProvider.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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') {
Expand Down
13 changes: 11 additions & 2 deletions src/Psalm/Internal/Type/TypeCombiner.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Internal/Type/TypeExpander.php
View file
Open in desktop
Expand Up @@ -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;

Expand Down
5 changes: 4 additions & 1 deletion src/Psalm/Type.php
View file
Open in desktop
Expand Up @@ -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]);
}

Expand Down Expand Up @@ -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;
}
Expand Down
15 changes: 15 additions & 0 deletions src/Psalm/Type/MutableUnion.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 9 additions & 0 deletions src/Psalm/Type/Union.php
View file
Open in desktop
Expand Up @@ -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,
Expand Down Expand Up @@ -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
*
Expand Down
3 changes: 3 additions & 0 deletions src/Psalm/Type/UnionTrait.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down