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

Fix #216 - scan assert* functions for assertions, then apply to current context

This commit is contained in:
Matt Brown 2018-02-23 15:39:33 -05:00
parent cb1fd308f6
commit 441506ad6f
16 changed files with 353 additions and 95 deletions

View File

@ -5,24 +5,24 @@ use PhpParser;
use Psalm\Checker\Statements\Expression\AssertionFinder;
use Psalm\Clause;
use Psalm\CodeLocation;
use Psalm\FileSource;
use Psalm\Issue\ParadoxicalCondition;
use Psalm\Issue\RedundantCondition;
use Psalm\IssueBuffer;
use Psalm\StatementsSource;
class AlgebraChecker
{
/**
* @param PhpParser\Node\Expr $conditional
* @param string|null $this_class_name
* @param StatementsSource $source
* @param FileSource $source
*
* @return array<int, Clause>
*/
public static function getFormula(
PhpParser\Node\Expr $conditional,
$this_class_name,
StatementsSource $source
FileSource $source
) {
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\BooleanAnd ||
$conditional instanceof PhpParser\Node\Expr\BinaryOp\LogicalAnd

View File

@ -9,6 +9,7 @@ class ScopeChecker
const ACTION_BREAK = 'BREAK';
const ACTION_CONTINUE = 'CONTINUE';
const ACTION_NONE = 'NONE';
const ACTION_RETURN = 'RETURN';
/**
* @param array<PhpParser\Node> $stmts
@ -51,11 +52,15 @@ class ScopeChecker
/**
* @param array<PhpParser\Node> $stmts
* @param bool $continue_is_break when checking inside a switch statement, continue is an alias of break
* @param bool $return_is_exit Exit and Throw statements are treated differently from return if this is false
*
* @return string[] one or more of 'LEAVE', 'CONTINUE', 'BREAK' (or empty if no single action is found)
*/
public static function getFinalControlActions(array $stmts, $continue_is_break = false)
{
public static function getFinalControlActions(
array $stmts,
$continue_is_break = false,
$return_is_exit = true
) {
if (empty($stmts)) {
return [self::ACTION_NONE];
}
@ -69,6 +74,10 @@ class ScopeChecker
$stmt instanceof PhpParser\Node\Stmt\Throw_ ||
$stmt instanceof PhpParser\Node\Expr\Exit_
) {
if (!$return_is_exit && $stmt instanceof PhpParser\Node\Stmt\Return_) {
return [self::ACTION_RETURN];
}
return [self::ACTION_END];
}
@ -123,7 +132,7 @@ class ScopeChecker
}
if ($stmt instanceof PhpParser\Node\Stmt\Switch_) {
$has_returned = false;
$has_ended = false;
$has_non_breaking_default = false;
$has_default_terminator = false;
@ -144,17 +153,17 @@ class ScopeChecker
$has_non_breaking_default = true;
}
$case_does_return = $case_actions == [self::ACTION_END];
$case_does_end = $case_actions == [self::ACTION_END];
if ($case_does_return) {
$has_returned = true;
if ($case_does_end) {
$has_ended = true;
}
if (!$case_does_return && !$has_returned) {
if (!$case_does_end && !$has_ended) {
continue 2;
}
if ($has_non_breaking_default && $case_does_return) {
if ($has_non_breaking_default && $case_does_end) {
$has_default_terminator = true;
}
}

View File

@ -6,6 +6,7 @@ use Psalm\Checker\ClassLikeChecker;
use Psalm\Checker\Statements\ExpressionChecker;
use Psalm\Checker\TypeChecker;
use Psalm\CodeLocation;
use Psalm\FileSource;
use Psalm\Issue\TypeDoesNotContainNull;
use Psalm\Issue\TypeDoesNotContainType;
use Psalm\Issue\UnevaluatedCode;
@ -23,18 +24,20 @@ class AssertionFinder
*
* @param PhpParser\Node\Expr $conditional
* @param string|null $this_class_name
* @param StatementsSource $source
* @param FileSource $source
*
* @return array<string, string>
*/
public static function getAssertions(
PhpParser\Node\Expr $conditional,
$this_class_name,
StatementsSource $source
FileSource $source
) {
$if_types = [];
$project_checker = $source->getFileChecker()->project_checker;
$project_checker = $source instanceof StatementsSource
? $source->getFileChecker()->project_checker
: null;
if ($conditional instanceof PhpParser\Node\Expr\Instanceof_) {
$instanceof_type = self::getInstanceOfTypes($conditional, $this_class_name, $source);
@ -121,7 +124,11 @@ class AssertionFinder
} else {
$if_types[$var_name] = 'falsy';
}
} elseif ($var_type && $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) {
} elseif ($var_type
&& $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical
&& $source instanceof StatementsSource
&& $project_checker
) {
$null_type = Type::getNull();
if (!TypeChecker::isContainedBy(
@ -214,7 +221,10 @@ class AssertionFinder
$if_types[$var_name] = 'falsy';
}
} elseif ($var_type) {
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) {
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical
&& $source instanceof StatementsSource
&& $project_checker
) {
$false_type = Type::getFalse();
if (!TypeChecker::isContainedBy(
@ -269,13 +279,13 @@ class AssertionFinder
/** @var PhpParser\Node\Scalar\String_ $string_expr */
$var_type = $string_expr->value;
$file_checker = $source->getFileChecker();
if (!isset(ClassLikeChecker::$GETTYPE_TYPES[$var_type])) {
if (!isset(ClassLikeChecker::$GETTYPE_TYPES[$var_type])
&& $source instanceof StatementsSource
) {
if (IssueBuffer::accepts(
new UnevaluatedCode(
'gettype cannot return this value',
new CodeLocation($file_checker, $string_expr)
new CodeLocation($source, $string_expr)
)
)) {
// fall through
@ -320,12 +330,11 @@ class AssertionFinder
throw new \UnexpectedValueException('Shouldnt get here');
}
$file_checker = $source->getFileChecker();
if (ClassLikeChecker::checkFullyQualifiedClassLikeName(
if ($source instanceof StatementsSource
&& ClassLikeChecker::checkFullyQualifiedClassLikeName(
$source,
$var_type,
new CodeLocation($file_checker, $whichclass_expr),
new CodeLocation($source, $whichclass_expr),
$source->getSuppressedIssues(),
false
) === false
@ -368,7 +377,11 @@ class AssertionFinder
if ($var_type) {
if ($var_name) {
$if_types[$var_name] = '^' . $var_type;
} elseif ($other_type && $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) {
} elseif ($other_type
&& $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical
&& $source instanceof StatementsSource
&& $project_checker
) {
if (!TypeChecker::isContainedBy(
$project_checker->codebase,
$var_type,
@ -399,7 +412,12 @@ class AssertionFinder
$var_type = isset($conditional->left->inferredType) ? $conditional->left->inferredType : null;
$other_type = isset($conditional->right->inferredType) ? $conditional->right->inferredType : null;
if ($var_type && $other_type && $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) {
if ($var_type
&& $other_type
&& $conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical
&& $source instanceof StatementsSource
&& $project_checker
) {
if (!TypeChecker::canBeIdenticalTo($project_checker->codebase, $var_type, $other_type)) {
if (IssueBuffer::accepts(
new TypeDoesNotContainType(
@ -534,13 +552,11 @@ class AssertionFinder
throw new \UnexpectedValueException('Shouldnt get here');
}
$file_checker = $source->getFileChecker();
if (!isset(ClassLikeChecker::$GETTYPE_TYPES[$var_type])) {
if (IssueBuffer::accepts(
new UnevaluatedCode(
'gettype cannot return this value',
new CodeLocation($file_checker, $whichclass_expr)
new CodeLocation($source, $whichclass_expr)
)
)) {
// fall through
@ -585,12 +601,12 @@ class AssertionFinder
throw new \UnexpectedValueException('Shouldnt get here');
}
$file_checker = $source->getFileChecker();
if (ClassLikeChecker::checkFullyQualifiedClassLikeName(
if ($source instanceof StatementsSource
&& $project_checker
&& ClassLikeChecker::checkFullyQualifiedClassLikeName(
$source,
$var_type,
new CodeLocation($file_checker, $whichclass_expr),
new CodeLocation($source, $whichclass_expr),
$source->getSuppressedIssues(),
false
) === false
@ -767,15 +783,15 @@ class AssertionFinder
/**
* @param PhpParser\Node\Expr\FuncCall $expr
* @param string|null $this_class_name
* @param StatementsSource $source
* @param bool $negate
* @param FileSource $source
* @param bool $negate
*
* @return array<string, string>
*/
protected static function processFunctionCall(
PhpParser\Node\Expr\FuncCall $expr,
$this_class_name,
StatementsSource $source,
FileSource $source,
$negate = false
) {
$prefix = $negate ? '!' : '';
@ -867,14 +883,14 @@ class AssertionFinder
/**
* @param PhpParser\Node\Expr\Instanceof_ $stmt
* @param string|null $this_class_name
* @param StatementsSource $source
* @param FileSource $source
*
* @return string|null
*/
protected static function getInstanceOfTypes(
PhpParser\Node\Expr\Instanceof_ $stmt,
$this_class_name,
StatementsSource $source
FileSource $source
) {
if ($stmt->class instanceof PhpParser\Node\Name) {
if (!in_array(strtolower($stmt->class->parts[0]), ['self', 'static', 'parent'], true)) {

View File

@ -428,6 +428,18 @@ class FunctionCallChecker extends \Psalm\Checker\Statements\Expression\CallCheck
}
}
if ($function_storage
&& strpos($function_storage->cased_name, 'assert') === 0
&& $function_storage->assertions
) {
self::applyAssertionsToContext(
$function_storage->assertions,
$stmt->args,
$context,
$statements_checker
);
}
return null;
}
}

View File

@ -451,6 +451,19 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
$returns_by_ref
|| $codebase->methods->getMethodReturnsByRef($method_id);
}
if (strpos($stmt->name, 'assert') === 0) {
$assertions = $codebase->methods->getMethodAssertions($method_id);
if ($assertions) {
self::applyAssertionsToContext(
$assertions,
$stmt->args,
$context,
$statements_checker
);
}
}
}
if ($config->after_method_checks) {

View File

@ -1441,4 +1441,60 @@ class CallChecker
return $function_id;
}
/**
* @param \Psalm\Storage\Assertion[] $assertions
* @param array<int, PhpParser\Node\Arg> $args
* @param Context $context
* @param StatementsChecker $statements_checker
*
* @return void
*/
protected static function applyAssertionsToContext(
array $assertions,
array $args,
Context $context,
StatementsChecker $statements_checker
) {
$type_assertions = [];
foreach ($assertions as $assertion) {
if (is_int($assertion->var_id)) {
if (!isset($args[$assertion->var_id])) {
continue;
}
$arg_value = $args[$assertion->var_id]->value;
$arg_var_id = ExpressionChecker::getArrayVarId($arg_value, null, $statements_checker);
if ($arg_var_id) {
$type_assertions[$arg_var_id] = $assertion->rule;
}
} else {
$type_assertions[$assertion->var_id] = $assertion->rule;
}
}
$changed_vars = [];
// while in an and, we allow scope to boil over to support
// statements of the form if ($x && $x->foo())
$op_vars_in_scope = \Psalm\Type\Reconciler::reconcileKeyedTypes(
$type_assertions,
$context->vars_in_scope,
$changed_vars,
[],
$statements_checker,
null
);
foreach ($changed_vars as $changed_var) {
if (isset($op_vars_in_scope[$changed_var])) {
$op_vars_in_scope[$changed_var]->from_docblock = true;
}
}
$context->vars_in_scope = $op_vars_in_scope;
}
}

View File

@ -27,6 +27,7 @@ use Psalm\Config;
use Psalm\Context;
use Psalm\Exception\DocblockParseException;
use Psalm\FileManipulation\FileManipulationBuffer;
use Psalm\FileSource;
use Psalm\Issue\ForbiddenCode;
use Psalm\Issue\InvalidCast;
use Psalm\Issue\InvalidClone;
@ -565,7 +566,7 @@ class ExpressionChecker
/**
* @param PhpParser\Node\Expr $stmt
* @param string|null $this_class_name
* @param StatementsSource|null $source
* @param FileSource|null $source
* @param int|null &$nesting
*
* @return string|null
@ -573,7 +574,7 @@ class ExpressionChecker
public static function getVarId(
PhpParser\Node\Expr $stmt,
$this_class_name,
StatementsSource $source = null,
FileSource $source = null,
&$nesting = null
) {
if ($stmt instanceof PhpParser\Node\Expr\Variable && is_string($stmt->name)) {
@ -626,14 +627,14 @@ class ExpressionChecker
/**
* @param PhpParser\Node\Expr $stmt
* @param string|null $this_class_name
* @param StatementsSource|null $source
* @param FileSource|null $source
*
* @return string|null
*/
public static function getRootVarId(
PhpParser\Node\Expr $stmt,
$this_class_name,
StatementsSource $source = null
FileSource $source = null
) {
if ($stmt instanceof PhpParser\Node\Expr\Variable
|| $stmt instanceof PhpParser\Node\Expr\StaticPropertyFetch
@ -659,14 +660,14 @@ class ExpressionChecker
/**
* @param PhpParser\Node\Expr $stmt
* @param string|null $this_class_name
* @param StatementsSource|null $source
* @param FileSource|null $source
*
* @return string|null
*/
public static function getArrayVarId(
PhpParser\Node\Expr $stmt,
$this_class_name,
StatementsSource $source = null
FileSource $source = null
) {
if ($stmt instanceof PhpParser\Node\Expr\Assign) {
return self::getArrayVarId($stmt->var, $this_class_name, $source);

View File

@ -286,6 +286,32 @@ class Methods
return $storage->return_type_location;
}
/**
* @param string $method_id
*
* @return array<int, \Psalm\Storage\Assertion>
*/
public function getMethodAssertions($method_id)
{
$method_id = $this->getDeclaringMethodId($method_id);
if (!$method_id) {
return [];
}
list($fq_class_name) = explode('::', $method_id);
$fq_class_storage = $this->classlike_storage_provider->get($fq_class_name);
if (!$fq_class_storage->user_defined && CallMap::inCallMap($method_id)) {
return [];
}
$storage = $this->getStorage($method_id);
return $storage->assertions;
}
/**
* @param string $method_id
* @param string $declaring_method_id

View File

@ -22,4 +22,9 @@ interface FileSource
* @return string
*/
public function getCheckedFilePath();
/**
* @return Aliases
*/
public function getAliases();
}

View File

@ -108,4 +108,12 @@ class FileScanner implements FileSource
{
return $this->file_name;
}
/**
* @return \Psalm\Aliases
*/
public function getAliases()
{
return new \Psalm\Aliases();
}
}

View File

@ -10,11 +10,6 @@ interface StatementsSource extends FileSource
*/
public function getNamespace();
/**
* @return Aliases
*/
public function getAliases();
/**
* @return array<string, string>
*/

View File

@ -0,0 +1,26 @@
<?php
namespace Psalm\Storage;
class Assertion
{
/**
* @var string the rule being asserted
*/
public $rule;
/**
* @var int|string the id of the property/variable, or
* the parameter offset of the affected arg
*/
public $var_id;
/**
* @param string|int $var_id
* @param string $rule
*/
public function __construct($var_id, $rule)
{
$this->rule = $rule;
$this->var_id = $var_id;
}
}

View File

@ -102,6 +102,6 @@ class FunctionLikeStorage
*/
public $referencing_locations;
/** @var array<int, string|int> */
/** @var array<int, Assertion> */
public $assertions = [];
}

View File

@ -38,7 +38,7 @@ class Reconciler
* @param array<string> $changed_var_ids
* @param array<string, bool> $referenced_var_ids
* @param StatementsChecker $statements_checker
* @param CodeLocation $code_location
* @param CodeLocation|null $code_location
* @param array<string> $suppressed_issues
*
* @return array<string, Type\Union>
@ -49,7 +49,7 @@ class Reconciler
array &$changed_var_ids,
array $referenced_var_ids,
StatementsChecker $statements_checker,
CodeLocation $code_location,
CodeLocation $code_location = null,
array $suppressed_issues = []
) {
$keys = [];
@ -103,7 +103,7 @@ class Reconciler
$result_type,
$key,
$statements_checker,
isset($referenced_var_ids[$key]) ? $code_location : null,
$code_location && isset($referenced_var_ids[$key]) ? $code_location : null,
$suppressed_issues,
$failed_reconciliation
);

View File

@ -17,6 +17,7 @@ use Psalm\Exception\DocblockParseException;
use Psalm\Exception\FileIncludeException;
use Psalm\Exception\IncorrectDocblockException;
use Psalm\Exception\TypeParseTreeException;
use Psalm\FileSource;
use Psalm\FunctionLikeParameter;
use Psalm\Issue\DuplicateParam;
use Psalm\Issue\InvalidDocblock;
@ -31,7 +32,7 @@ use Psalm\Storage\MethodStorage;
use Psalm\Storage\PropertyStorage;
use Psalm\Type;
class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements PhpParser\NodeVisitor
class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements PhpParser\NodeVisitor, FileSource
{
/** @var Aliases */
private $aliases;
@ -637,7 +638,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
foreach ($stmt->getParams() as $param) {
$param_array = $this->getTranslatedFunctionParam($param);
if (isset($existing_params[$param_array->name])) {
if (isset($existing_params['$' . $param_array->name])) {
if (IssueBuffer::accepts(
new DuplicateParam(
'Duplicate param $' . $param->name . ' in docblock for ' . $cased_function_id,
@ -648,7 +649,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
}
}
$existing_params[$param_array->name] = $i;
$existing_params['$' . $param_array->name] = $i;
$storage->param_types[$param_array->name] = $param_array->type;
$storage->params[] = $param_array;
@ -683,32 +684,42 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
$var_assertions = [];
foreach ($stmt->stmts as $function_stmt) {
if ($function_stmt instanceof PhpParser\Node\Stmt\If_
&& $function_stmt->stmts[0] instanceof PhpParser\Node\Stmt\Throw_
) {
$conditional = $function_stmt->cond;
if ($function_stmt instanceof PhpParser\Node\Stmt\If_) {
$final_actions = \Psalm\Checker\ScopeChecker::getFinalControlActions(
$function_stmt->stmts,
false,
false
);
if ($conditional instanceof PhpParser\Node\Expr\BooleanNot
&& $conditional->expr instanceof PhpParser\Node\Expr\Instanceof_
&& $conditional->expr->expr instanceof PhpParser\Node\Expr\Variable
&& is_string($conditional->expr->expr->name)
&& isset($existing_params[$conditional->expr->expr->name])
) {
$param_offset = $existing_params[$conditional->expr->expr->name];
if ($final_actions !== [\Psalm\Checker\ScopeChecker::ACTION_END]) {
continue;
}
if ($conditional->expr->class instanceof PhpParser\Node\Expr\Variable
&& is_string($conditional->expr->class->name)
&& isset($existing_params[$conditional->expr->class->name])
) {
$var_assertions[$param_offset]
= $existing_params[$conditional->expr->class->name];
} elseif ($conditional->expr->class instanceof PhpParser\Node\Name) {
$instanceof_class = ClassLikeChecker::getFQCLNFromNameObject(
$conditional->expr->class,
$this->aliases
$if_clauses = \Psalm\Checker\AlgebraChecker::getFormula(
$function_stmt->cond,
$this->fq_classlike_names
? $this->fq_classlike_names[count($this->fq_classlike_names) - 1]
: null,
$this->file_scanner
);
$negated_formula = \Psalm\Checker\AlgebraChecker::negateFormula($if_clauses);
$rules = \Psalm\Checker\AlgebraChecker::getTruthsFromFormula($negated_formula);
foreach ($rules as $var_id => $rule) {
if (isset($existing_params[$var_id])) {
$param_offset = $existing_params[$var_id];
$var_assertions[] = new \Psalm\Storage\Assertion(
$param_offset,
$rule
);
} elseif (strpos($var_id, '$this->') === 0) {
$var_assertions[] = new \Psalm\Storage\Assertion(
$var_id,
$rule
);
$var_assertions[$param_offset] = $instanceof_class;
}
}
}
@ -1386,4 +1397,44 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
return null;
}
/**
* @return string
*/
public function getFilePath()
{
return $this->file_path;
}
/**
* @return string
*/
public function getFileName()
{
return $this->file_scanner->getFileName();
}
/**
* @return string
*/
public function getCheckedFilePath()
{
return $this->file_scanner->getCheckedFilePath();
}
/**
* @return string
*/
public function getCheckedFileName()
{
return $this->file_scanner->getCheckedFileName();
}
/**
* @return Aliases
*/
public function getAliases()
{
return $this->aliases;
}
}

View File

@ -3,7 +3,6 @@ namespace Psalm\Tests;
class AssertTest extends TestCase
{
use Traits\FileCheckerInvalidCodeParseTestTrait;
use Traits\FileCheckerValidCodeParseTestTrait;
/**
@ -12,7 +11,7 @@ class AssertTest extends TestCase
public function providerFileCheckerValidCodeParse()
{
return [
'SKIPPED-assertInstanceOfB' => [
'assertInstanceOfB' => [
'<?php
class A {}
class B extends A {
@ -25,6 +24,60 @@ class AssertTest extends TestCase
}
}
function takesA(A $a): void {
assertInstanceOfB($a);
$a->foo();
}',
],
'assertInstanceOfBInClassMethod' => [
'<?php
class A {}
class B extends A {
public function foo(): void {}
}
class C {
private function assertInstanceOfB(A $var): void {
if (!$var instanceof B) {
throw new \Exception();
}
}
private function takesA(A $a): void {
$this->assertInstanceOfB($a);
$a->foo();
}
}',
],
'assertPropertyNotNull' => [
'<?php
class A {
public function foo(): void {}
}
class B {
/** @var A|null */
public $a;
private function assertNotNullProperty(): void {
if (!$this->a) {
throw new \Exception();
}
}
public function takesA(A $a): void {
$this->assertNotNullProperty();
$a->foo();
}
}',
],
'SKIPPED-assertInstanceOfClass' => [
'<?php
class A {}
class B extends A {
public function foo(): void {}
}
function assertInstanceOfClass(A $var, string $class): void {
if (!$var instanceof $class) {
throw new \Exception();
@ -32,23 +85,10 @@ class AssertTest extends TestCase
}
function takesA(A $a): void {
assertInstanceOfB($a);
$a->foo();
}
function takesA(A $a): void {
assertInstanceOfB($a);
assertInstanceOfClass($a, B::class);
$a->foo();
}',
],
];
}
/**
* @return array
*/
public function providerFileCheckerInvalidCodeParse()
{
return [];
}
}