From c850ef644d6a70315e648945651b2b6eee5dbe38 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 23 Oct 2017 11:47:00 -0400 Subject: [PATCH] Add PossiblyFalseReference and PossiblyFalseArgument issues Useful for catching error cases --- src/Psalm/Checker/FunctionLikeChecker.php | 1 + .../Statements/Expression/CallChecker.php | 34 ++++++++++++++++++- .../Checker/Statements/ExpressionChecker.php | 3 ++ src/Psalm/Checker/TypeChecker.php | 7 ++++ src/Psalm/Issue/PossiblyFalseArgument.php | 6 ++++ src/Psalm/Issue/PossiblyFalseReference.php | 6 ++++ src/Psalm/Type/Union.php | 16 +++++++++ tests/FunctionCallTest.php | 8 +++++ tests/ScopeTest.php | 1 + 9 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 src/Psalm/Issue/PossiblyFalseArgument.php create mode 100644 src/Psalm/Issue/PossiblyFalseReference.php diff --git a/src/Psalm/Checker/FunctionLikeChecker.php b/src/Psalm/Checker/FunctionLikeChecker.php index 9bf9a74b6..abba5461d 100644 --- a/src/Psalm/Checker/FunctionLikeChecker.php +++ b/src/Psalm/Checker/FunctionLikeChecker.php @@ -991,6 +991,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $inferred_return_type, $declared_return_type, $ignore_nullable_issues, + false, $has_scalar_match, $type_coerced )) { diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index 52277f7a2..923657bf1 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -32,6 +32,8 @@ use Psalm\Issue\NullArgument; use Psalm\Issue\NullFunctionCall; use Psalm\Issue\NullReference; use Psalm\Issue\ParentNotFound; +use Psalm\Issue\PossiblyFalseArgument; +use Psalm\Issue\PossiblyFalseReference; use Psalm\Issue\PossiblyInvalidArgument; use Psalm\Issue\PossiblyNullArgument; use Psalm\Issue\PossiblyNullFunctionCall; @@ -740,6 +742,21 @@ class CallChecker } } + if ($class_type && + is_string($stmt->name) && + $class_type->isFalsable() + ) { + if (IssueBuffer::accepts( + new PossiblyFalseReference( + 'Cannot call method ' . $stmt->name . ' on possibly false variable ' . $var_id, + new CodeLocation($statements_checker->getSource(), $stmt->var) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + } + $config = Config::getInstance(); $project_checker = $statements_checker->getFileChecker()->project_checker; @@ -750,12 +767,12 @@ class CallChecker if (!$class_type_part instanceof TNamedObject) { switch (get_class($class_type_part)) { case 'Psalm\\Type\\Atomic\\TNull': + case 'Psalm\\Type\\Atomic\\TFalse': // handled above break; case 'Psalm\\Type\\Atomic\\TInt': case 'Psalm\\Type\\Atomic\\TBool': - case 'Psalm\\Type\\Atomic\\TFalse': case 'Psalm\\Type\\Atomic\\TArray': case 'Psalm\\Type\\Atomic\\TString': case 'Psalm\\Type\\Atomic\\TNumericString': @@ -2014,6 +2031,7 @@ class CallChecker $input_type, $closure_param_type, false, + false, $scalar_type_match_found, $coerced_type ); @@ -2163,6 +2181,19 @@ class CallChecker } } + if ($input_type->isFalsable() && !$param_type->hasBool()) { + if (IssueBuffer::accepts( + new PossiblyFalseArgument( + 'Argument ' . ($argument_offset + 1) . $method_identifier . ' cannot be false, possibly ' . + 'false value provided', + $code_location + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + } + $param_type = TypeChecker::simplifyUnionType( $project_checker, $param_type @@ -2173,6 +2204,7 @@ class CallChecker $input_type, $param_type, true, + true, $scalar_type_match_found, $coerced_type, $to_string_cast diff --git a/src/Psalm/Checker/Statements/ExpressionChecker.php b/src/Psalm/Checker/Statements/ExpressionChecker.php index 0e20c3f20..6c671656e 100644 --- a/src/Psalm/Checker/Statements/ExpressionChecker.php +++ b/src/Psalm/Checker/Statements/ExpressionChecker.php @@ -322,6 +322,7 @@ class ExpressionChecker $stmt->expr->inferredType, $container_type, true, + false, $has_scalar_match ) && !$has_scalar_match @@ -1544,6 +1545,7 @@ class ExpressionChecker $left_type, Type::getString(), true, + false, $left_has_scalar_match ); @@ -1552,6 +1554,7 @@ class ExpressionChecker $right_type, Type::getString(), true, + false, $right_has_scalar_match ); diff --git a/src/Psalm/Checker/TypeChecker.php b/src/Psalm/Checker/TypeChecker.php index 381939fdb..af8494d88 100644 --- a/src/Psalm/Checker/TypeChecker.php +++ b/src/Psalm/Checker/TypeChecker.php @@ -475,6 +475,7 @@ class TypeChecker * @param Type\Union $container_type * @param ProjectChecker $project_checker * @param bool $ignore_null + * @param bool $ignore_false * @param bool &$has_scalar_match * @param bool &$type_coerced whether or not there was type coercion involved * @param bool &$to_string_cast @@ -486,6 +487,7 @@ class TypeChecker Type\Union $input_type, Type\Union $container_type, $ignore_null = false, + $ignore_false = false, &$has_scalar_match = null, &$type_coerced = null, &$to_string_cast = null @@ -503,6 +505,10 @@ class TypeChecker continue; } + if ($input_type_part instanceof TFalse && $ignore_false) { + continue; + } + $type_match_found = false; $scalar_type_match_found = false; $all_to_string_cast = true; @@ -682,6 +688,7 @@ class TypeChecker $input_param, $container_param, false, + false, $has_scalar_match, $type_coerced ) diff --git a/src/Psalm/Issue/PossiblyFalseArgument.php b/src/Psalm/Issue/PossiblyFalseArgument.php new file mode 100644 index 000000000..8c2ddf9b4 --- /dev/null +++ b/src/Psalm/Issue/PossiblyFalseArgument.php @@ -0,0 +1,6 @@ +types['null']); } + /** + * @return bool + */ + public function isFalsable() + { + return isset($this->types['false']); + } + + /** + * @return bool + */ + public function hasBool() + { + return isset($this->types['bool']) || isset($this->types['false']); + } + /** * @return bool */ diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 27239362a..7affbc8d8 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -398,6 +398,14 @@ class FunctionCallTest extends TestCase 'MixedArrayAccess', ], ], + 'undefinedFunctionInArrayMap' => [ + ' 'UndefinedFunction', + ], ]; } } diff --git a/tests/ScopeTest.php b/tests/ScopeTest.php index a30772f50..cbb924580 100644 --- a/tests/ScopeTest.php +++ b/tests/ScopeTest.php @@ -181,6 +181,7 @@ class ScopeTest extends TestCase 'assertions' => [ '$a' => 'string|null', ], + 'error_levels' => ['PossiblyFalseArgument'], ], 'refineOredType' => [ '