1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00

Detect redundant cast (#4695)

* detect redundant cast

* fix redundant cast issues

* fix redundant cast in tests
This commit is contained in:
orklah 2020-11-25 18:04:48 +01:00 committed by Daniil Gentili
parent 51fed99c5d
commit 005373bbc2
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
20 changed files with 168 additions and 37 deletions

View File

@ -105,7 +105,7 @@ class DocComment
$special[$type] = []; $special[$type] = [];
} }
$line_number = $line_map && isset($line_map[$full_match]) ? $line_map[$full_match] : (int)$m; $line_number = $line_map && isset($line_map[$full_match]) ? $line_map[$full_match] : $m;
$special[$type][$line_number] = rtrim($data); $special[$type][$line_number] = rtrim($data);
} }
@ -123,7 +123,7 @@ class DocComment
$special[$type] = []; $special[$type] = [];
} }
$line_number = $line_map && isset($line_map[$_]) ? $line_map[$_] : (int)$m; $line_number = $line_map && isset($line_map[$_]) ? $line_map[$_] : $m;
$special[$type][$line_number] = $data; $special[$type][$line_number] = $data;
} }

View File

@ -1502,7 +1502,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
$parent_fqcln = $this->getParentFQCLN(); $parent_fqcln = $this->getParentFQCLN();
if ($resolved_name === 'self' && $context->self) { if ($resolved_name === 'self' && $context->self) {
$resolved_name = (string) $context->self; $resolved_name = $context->self;
} elseif ($resolved_name === 'parent' && $parent_fqcln) { } elseif ($resolved_name === 'parent' && $parent_fqcln) {
$resolved_name = $parent_fqcln; $resolved_name = $parent_fqcln;
} }
@ -1536,7 +1536,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
$parent_fqcln = $this->getParentFQCLN(); $parent_fqcln = $this->getParentFQCLN();
if ($resolved_name === 'self' && $context->self) { if ($resolved_name === 'self' && $context->self) {
$resolved_name = (string) $context->self; $resolved_name = $context->self;
} elseif ($resolved_name === 'parent' && $parent_fqcln) { } elseif ($resolved_name === 'parent' && $parent_fqcln) {
$resolved_name = $parent_fqcln; $resolved_name = $parent_fqcln;
} }

View File

@ -745,7 +745,7 @@ class ArrayAssignmentAnalyzer
$object_like = new TKeyedArray( $object_like = new TKeyedArray(
[$key_value->value => clone $current_type], [$key_value->value => clone $current_type],
$key_value instanceof Type\Atomic\TLiteralClassString $key_value instanceof Type\Atomic\TLiteralClassString
? [(string) $key_value->value => true] ? [$key_value->value => true]
: null : null
); );

View File

@ -9,6 +9,8 @@ use Psalm\CodeLocation;
use Psalm\Context; use Psalm\Context;
use Psalm\Issue\InvalidCast; use Psalm\Issue\InvalidCast;
use Psalm\Issue\PossiblyInvalidCast; use Psalm\Issue\PossiblyInvalidCast;
use Psalm\Issue\RedundantCondition;
use Psalm\Issue\RedundantConditionGivenDocblockType;
use Psalm\Issue\UnrecognizedExpression; use Psalm\Issue\UnrecognizedExpression;
use Psalm\IssueBuffer; use Psalm\IssueBuffer;
use Psalm\Type; use Psalm\Type;
@ -45,6 +47,26 @@ class CastAnalyzer
$maybe_type = $statements_analyzer->node_data->getType($stmt->expr); $maybe_type = $statements_analyzer->node_data->getType($stmt->expr);
if ($maybe_type) { if ($maybe_type) {
if ($maybe_type->isInt()) {
if ($maybe_type->from_docblock) {
$issue = new RedundantConditionGivenDocblockType(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
} else {
$issue = new RedundantCondition(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
}
if (IssueBuffer::accepts($issue, $statements_analyzer->getSuppressedIssues())) {
// fall through
}
}
$maybe = $maybe_type->getAtomicTypes(); $maybe = $maybe_type->getAtomicTypes();
if (count($maybe) === 1 && current($maybe) instanceof Type\Atomic\TBool) { if (count($maybe) === 1 && current($maybe) instanceof Type\Atomic\TBool) {
@ -78,6 +100,28 @@ class CastAnalyzer
$maybe_type = $statements_analyzer->node_data->getType($stmt->expr); $maybe_type = $statements_analyzer->node_data->getType($stmt->expr);
if ($maybe_type) {
if ($maybe_type->isFloat()) {
if ($maybe_type->from_docblock) {
$issue = new RedundantConditionGivenDocblockType(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
} else {
$issue = new RedundantCondition(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
}
if (IssueBuffer::accepts($issue, $statements_analyzer->getSuppressedIssues())) {
// fall through
}
}
}
$type = Type::getFloat(); $type = Type::getFloat();
if ($statements_analyzer->data_flow_graph if ($statements_analyzer->data_flow_graph
@ -98,6 +142,28 @@ class CastAnalyzer
$maybe_type = $statements_analyzer->node_data->getType($stmt->expr); $maybe_type = $statements_analyzer->node_data->getType($stmt->expr);
if ($maybe_type) {
if ($maybe_type->isBool()) {
if ($maybe_type->from_docblock) {
$issue = new RedundantConditionGivenDocblockType(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
} else {
$issue = new RedundantCondition(
'Redundant cast to ' . $maybe_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$maybe_type->getKey()
);
}
if (IssueBuffer::accepts($issue, $statements_analyzer->getSuppressedIssues())) {
// fall through
}
}
}
$type = Type::getBool(); $type = Type::getBool();
if ($statements_analyzer->data_flow_graph if ($statements_analyzer->data_flow_graph
@ -119,6 +185,26 @@ class CastAnalyzer
$stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr); $stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr);
if ($stmt_expr_type) { if ($stmt_expr_type) {
if ($stmt_expr_type->isString()) {
if ($stmt_expr_type->from_docblock) {
$issue = new RedundantConditionGivenDocblockType(
'Redundant cast to ' . $stmt_expr_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$stmt_expr_type->getKey()
);
} else {
$issue = new RedundantCondition(
'Redundant cast to ' . $stmt_expr_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$stmt_expr_type->getKey()
);
}
if (IssueBuffer::accepts($issue, $statements_analyzer->getSuppressedIssues())) {
// fall through
}
}
$stmt_type = self::castStringAttempt( $stmt_type = self::castStringAttempt(
$statements_analyzer, $statements_analyzer,
$context, $context,
@ -170,6 +256,26 @@ class CastAnalyzer
$all_permissible = false; $all_permissible = false;
if ($stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr)) { if ($stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr)) {
if ($stmt_expr_type->isArray()) {
if ($stmt_expr_type->from_docblock) {
$issue = new RedundantConditionGivenDocblockType(
'Redundant cast to ' . $stmt_expr_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$stmt_expr_type->getKey()
);
} else {
$issue = new RedundantCondition(
'Redundant cast to ' . $stmt_expr_type->getKey(),
new CodeLocation($statements_analyzer->getSource(), $stmt),
$stmt_expr_type->getKey()
);
}
if (IssueBuffer::accepts($issue, $statements_analyzer->getSuppressedIssues())) {
// fall through
}
}
$all_permissible = true; $all_permissible = true;
foreach ($stmt_expr_type->getAtomicTypes() as $type) { foreach ($stmt_expr_type->getAtomicTypes() as $type) {

View File

@ -315,13 +315,13 @@ class Reflection
private function getReflectionParamData(\ReflectionParameter $param): FunctionLikeParameter private function getReflectionParamData(\ReflectionParameter $param): FunctionLikeParameter
{ {
$param_type = self::getPsalmTypeFromReflectionType($param->getType()); $param_type = self::getPsalmTypeFromReflectionType($param->getType());
$param_name = (string)$param->getName(); $param_name = $param->getName();
$is_optional = (bool)$param->isOptional(); $is_optional = $param->isOptional();
$parameter = new FunctionLikeParameter( $parameter = new FunctionLikeParameter(
$param_name, $param_name,
(bool)$param->isPassedByReference(), $param->isPassedByReference(),
$param_type, $param_type,
null, null,
null, null,

View File

@ -506,7 +506,7 @@ class LanguageServer extends AdvancedJsonRpc\Dispatcher
throw new \InvalidArgumentException("Not a valid file URI: $uri"); throw new \InvalidArgumentException("Not a valid file URI: $uri");
} }
$filepath = urldecode((string) $fragments['path']); $filepath = urldecode($fragments['path']);
if (strpos($filepath, ':') !== false) { if (strpos($filepath, ':') !== false) {
if ($filepath[0] === '/') { if ($filepath[0] === '/') {

View File

@ -92,7 +92,7 @@ class StatementsProvider
$from_cache = false; $from_cache = false;
$version = (string) PHP_PARSER_VERSION . $this->this_modified_time; $version = PHP_PARSER_VERSION . $this->this_modified_time;
$file_contents = $this->file_provider->getContents($file_path); $file_contents = $this->file_provider->getContents($file_path);
$modified_time = $this->file_provider->getModifiedTime($file_path); $modified_time = $this->file_provider->getModifiedTime($file_path);

View File

@ -166,8 +166,8 @@ class ObjectComparator
&& $intersection_input_type instanceof TTemplateParam && $intersection_input_type instanceof TTemplateParam
) { ) {
if ($intersection_container_type->param_name !== $intersection_input_type->param_name if ($intersection_container_type->param_name !== $intersection_input_type->param_name
|| ((string)$intersection_container_type->defining_class || ($intersection_container_type->defining_class
!== (string)$intersection_input_type->defining_class !== $intersection_input_type->defining_class
&& \substr($intersection_input_type->defining_class, 0, 3) !== 'fn-' && \substr($intersection_input_type->defining_class, 0, 3) !== 'fn-'
&& \substr($intersection_container_type->defining_class, 0, 3) !== 'fn-') && \substr($intersection_container_type->defining_class, 0, 3) !== 'fn-')
) { ) {

View File

@ -569,7 +569,7 @@ class TypeCombiner
} }
if (!$allow_mixed_union) { if (!$allow_mixed_union) {
return Type::getMixed((bool) $combination->mixed_from_loop_isset); return Type::getMixed($combination->mixed_from_loop_isset);
} }
} }

View File

@ -891,7 +891,7 @@ class TypeParser
$offset_template_type = array_values($offset_template_data[''][0]->getAtomicTypes())[0]; $offset_template_type = array_values($offset_template_data[''][0]->getAtomicTypes())[0];
if ($offset_template_type instanceof Atomic\TTemplateKeyOf) { if ($offset_template_type instanceof Atomic\TTemplateKeyOf) {
$offset_defining_class = (string) $offset_template_type->defining_class; $offset_defining_class = $offset_template_type->defining_class;
} }
} }

View File

@ -1416,6 +1416,30 @@ class Union implements TypeNode
) === count($this->types); ) === count($this->types);
} }
/**
* @return bool true if this is a boolean
*/
public function isBool(): bool
{
if (!$this->isSingle()) {
return false;
}
return isset($this->types['bool']);
}
/**
* @return bool true if this is an array
*/
public function isArray(): bool
{
if (!$this->isSingle()) {
return false;
}
return isset($this->types['array']);
}
/** /**
* @return bool true if this is a string literal with only one possible value * @return bool true if this is a string literal with only one possible value
*/ */

View File

@ -129,7 +129,7 @@ function requireAutoloaders(string $current_dir, bool $has_explicit_root, string
exit(1); exit(1);
} }
define('PSALM_VERSION', (string)\PackageVersions\Versions::getVersion('vimeo/psalm')); define('PSALM_VERSION', \PackageVersions\Versions::getVersion('vimeo/psalm'));
define('PHP_PARSER_VERSION', \PackageVersions\Versions::getVersion('nikic/php-parser')); define('PHP_PARSER_VERSION', \PackageVersions\Versions::getVersion('nikic/php-parser'));
return $first_autoloader; return $first_autoloader;

View File

@ -821,7 +821,7 @@ if (!isset($options['i'])) {
$init_level = \Psalm\Config\Creator::getLevel( $init_level = \Psalm\Config\Creator::getLevel(
array_merge(...array_values($issues_by_file)), array_merge(...array_values($issues_by_file)),
(int) array_sum($mixed_counts) array_sum($mixed_counts)
); );
} }

View File

@ -1032,18 +1032,6 @@ class ArrayAssignmentTest extends TestCase
'$b' => 'array<empty, empty>', '$b' => 'array<empty, empty>',
], ],
], ],
'coerceListToArray' => [
'<?php
/**
* @param list<int> $_bar
*/
function foo(array $_bar) : void {}
/**
* @param list<int> $bar
*/
function baz(array $bar) : void { foo((array) $bar); }',
],
'getOnCoercedArray' => [ 'getOnCoercedArray' => [
'<?php '<?php
function getArray() : array { function getArray() : array {
@ -1884,6 +1872,19 @@ class ArrayAssignmentTest extends TestCase
$_a = [$a => "a"];', $_a = [$a => "a"];',
'error_message' => 'InvalidArrayOffset', 'error_message' => 'InvalidArrayOffset',
], ],
'coerceListToArray' => [
'<?php
/**
* @param list<int> $_bar
*/
function foo(array $_bar) : void {}
/**
* @param list<int> $bar
*/
function baz(array $bar) : void { foo((array) $bar); }',
'error_message' => 'RedundantCondition',
],
]; ];
} }
} }

View File

@ -107,7 +107,7 @@ class ClassLoadOrderTest extends TestCase
class B extends A { class B extends A {
/** @return void */ /** @return void */
public function foo() { public function foo() {
echo (string)(new C)->bar; echo (new C)->bar;
} }
} }

View File

@ -453,7 +453,7 @@ class MethodCallTest extends TestCase
'defineVariableCreatedInArgToMixed' => [ 'defineVariableCreatedInArgToMixed' => [
'<?php '<?php
function bar($a) : void { function bar($a) : void {
if ($a->foo($b = (int) 5)) { if ($a->foo($b = (int) "5")) {
echo $b; echo $b;
} }
}', }',

View File

@ -496,7 +496,7 @@ class MethodSignatureTest extends TestCase
{ {
[ [
$this->id, $this->id,
] = (array) \unserialize((string) $serialized); ] = (array) \unserialize($serialized);
} }
public function serialize() : string public function serialize() : string

View File

@ -355,7 +355,7 @@ class AssignmentInConditionalTest extends \Psalm\Tests\TestCase
'passedByRefInIf' => [ 'passedByRefInIf' => [
'<?php '<?php
if (preg_match("/bad/", "badger", $matches)) { if (preg_match("/bad/", "badger", $matches)) {
echo (string)$matches[0]; echo $matches[0];
}', }',
], ],
'passByRefInIfCheckAfter' => [ 'passByRefInIfCheckAfter' => [
@ -363,13 +363,13 @@ class AssignmentInConditionalTest extends \Psalm\Tests\TestCase
if (!preg_match("/bad/", "badger", $matches)) { if (!preg_match("/bad/", "badger", $matches)) {
exit(); exit();
} }
echo (string)$matches[0];', echo $matches[0];',
], ],
'passByRefInIfWithBoolean' => [ 'passByRefInIfWithBoolean' => [
'<?php '<?php
$a = (bool)rand(0, 1); $a = (bool)rand(0, 1);
if ($a && preg_match("/bad/", "badger", $matches)) { if ($a && preg_match("/bad/", "badger", $matches)) {
echo (string)$matches[0]; echo $matches[0];
}', }',
], ],
'bleedElseifAssignedVarsIntoElseScope' => [ 'bleedElseifAssignedVarsIntoElseScope' => [

View File

@ -40,7 +40,7 @@ class ScopeTest extends \Psalm\Tests\TestCase
'<?php '<?php
$a = preg_match("/bad/", "badger", $matches) > 0; $a = preg_match("/bad/", "badger", $matches) > 0;
if ($a) { if ($a) {
echo (string)$matches[1]; echo $matches[1];
}', }',
], ],
'functionExists' => [ 'functionExists' => [

View File

@ -147,10 +147,10 @@ class UnusedVariableTest extends TestCase
], ],
'varRedefinedInIfWithReference' => [ 'varRedefinedInIfWithReference' => [
'<?php '<?php
$a = (string) "fdf"; $a = (string) 5;
if (rand(0, 1)) { if (rand(0, 1)) {
$a = (string) "ard"; $a = (string) 6;
} }
echo $a;', echo $a;',