From e3a9cb98c3906ffc86d7c1500b37a524a46f46a3 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Mon, 16 Jan 2017 12:59:09 -0500 Subject: [PATCH] Add extra issue for invalid clone and fix issue reporting; --- config.xsd | 1 + src/Psalm/Checker/ClassLikeChecker.php | 2 +- src/Psalm/Checker/FileChecker.php | 2 +- .../Statements/Expression/CallChecker.php | 8 ++-- .../Checker/Statements/ExpressionChecker.php | 45 ++++++++++++++++--- src/Psalm/Checker/StatementsChecker.php | 2 +- src/Psalm/Config.php | 19 ++++---- src/Psalm/Config/ErrorLevelFileFilter.php | 1 + src/Psalm/Config/IssueHandler.php | 6 +-- src/Psalm/Issue/InvalidClone.php | 6 +++ src/Psalm/IssueBuffer.php | 2 +- 11 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 src/Psalm/Issue/InvalidClone.php diff --git a/config.xsd b/config.xsd index 8a77b1224..c3fb3183d 100644 --- a/config.xsd +++ b/config.xsd @@ -85,6 +85,7 @@ + diff --git a/src/Psalm/Checker/ClassLikeChecker.php b/src/Psalm/Checker/ClassLikeChecker.php index 94011580c..9bfbfaeeb 100644 --- a/src/Psalm/Checker/ClassLikeChecker.php +++ b/src/Psalm/Checker/ClassLikeChecker.php @@ -472,7 +472,7 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc $global_context ? clone $global_context : null ); - if (!$config->excludeIssueInFile('InvalidReturnType', $source->getFileName())) { + if (!$config->excludeIssueInFile('InvalidReturnType', $source->getFilePath())) { $return_type_location = null; $secondary_return_type_location = null; diff --git a/src/Psalm/Checker/FileChecker.php b/src/Psalm/Checker/FileChecker.php index 2b897592c..536b56850 100644 --- a/src/Psalm/Checker/FileChecker.php +++ b/src/Psalm/Checker/FileChecker.php @@ -307,7 +307,7 @@ class FileChecker extends SourceChecker implements StatementsSource $function_context = new Context($this->file_name, $this->context->self); $function_checker->analyze($function_context, $this->context); - if (!$config->excludeIssueInFile('InvalidReturnType', $this->file_name)) { + if (!$config->excludeIssueInFile('InvalidReturnType', $this->file_path)) { /** @var string */ $method_id = $function_checker->getMethodId(); diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index ceafafa19..80d8d68e6 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -955,11 +955,11 @@ class CallChecker if ($function_params !== null) { $by_ref = $argument_offset < count($function_params) ? $function_params[$argument_offset]->by_ref - : $last_param->is_variadic && $last_param->by_ref; + : $last_param && $last_param->is_variadic && $last_param->by_ref; $by_ref_type = null; - if ($by_ref) { + if ($by_ref && $last_param) { $by_ref_type = $argument_offset < count($function_params) ? clone $function_params[$argument_offset]->type : clone $last_param->type; @@ -992,11 +992,11 @@ class CallChecker if ($function_params !== null) { $by_ref = $argument_offset < count($function_params) ? $function_params[$argument_offset]->by_ref - : $last_param->is_variadic && $last_param->by_ref; + : $last_param && $last_param->is_variadic && $last_param->by_ref; $by_ref_type = null; - if ($by_ref) { + if ($by_ref && $last_param) { $by_ref_type = $argument_offset < count($function_params) ? clone $function_params[$argument_offset]->type : clone $last_param->type; diff --git a/src/Psalm/Checker/Statements/ExpressionChecker.php b/src/Psalm/Checker/Statements/ExpressionChecker.php index 3e4826474..54aa2afca 100644 --- a/src/Psalm/Checker/Statements/ExpressionChecker.php +++ b/src/Psalm/Checker/Statements/ExpressionChecker.php @@ -17,6 +17,7 @@ use Psalm\CodeLocation; use Psalm\Config; use Psalm\Context; use Psalm\Issue\ForbiddenCode; +use Psalm\Issue\InvalidClone; use Psalm\Issue\InvalidOperand; use Psalm\Issue\InvalidScope; use Psalm\Issue\InvalidStaticVariable; @@ -305,13 +306,7 @@ class ExpressionChecker $stmt->inferredType = Type::getNull(); } elseif ($stmt instanceof PhpParser\Node\Expr\Clone_) { - if (self::analyze($statements_checker, $stmt->expr, $context) === false) { - return false; - } - - if (property_exists($stmt->expr, 'inferredType')) { - $stmt->inferredType = $stmt->expr->inferredType; - } + self::analyzeClone($statements_checker, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Expr\Instanceof_) { if (self::analyze($statements_checker, $stmt->expr, $context) === false) { return false; @@ -1686,6 +1681,42 @@ class ExpressionChecker } } + /** + * @param StatementsChecker $statements_checker + * @param PhpParser\Node\Expr\Clone_ $stmt + * @param Context $context + * @return false|null + */ + protected static function analyzeClone( + StatementsChecker $statements_checker, + PhpParser\Node\Expr\Clone_ $stmt, + Context $context + ) { + if (self::analyze($statements_checker, $stmt->expr, $context) === false) { + return false; + } + + if (isset($stmt->expr->inferredType)) { + foreach ($stmt->expr->inferredType->types as $clone_type_part) { + if (!$clone_type_part instanceof TNamedObject && !$clone_type_part instanceof TObject) { + if (IssueBuffer::accepts( + new InvalidClone( + 'Cannot clone ' . $clone_type_part, + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + + return; + } + } + + $stmt->inferredType = $stmt->expr->inferredType; + } + } + /** * @param string $fq_class_name * @return boolean diff --git a/src/Psalm/Checker/StatementsChecker.php b/src/Psalm/Checker/StatementsChecker.php index d8da2ecb9..957743be5 100644 --- a/src/Psalm/Checker/StatementsChecker.php +++ b/src/Psalm/Checker/StatementsChecker.php @@ -186,7 +186,7 @@ class StatementsChecker extends SourceChecker implements StatementsSource $config = Config::getInstance(); - if (!$config->excludeIssueInFile('InvalidReturnType', $this->getFileName())) { + if (!$config->excludeIssueInFile('InvalidReturnType', $this->getFilePath())) { /** @var string */ $method_id = $function_checkers[$stmt->name]->getMethodId(); diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 1a1b5647b..5003d63d8 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -422,24 +422,22 @@ class Config /** * @param string $issue_type - * @param string $file_name + * @param string $file_path * @return bool */ - public function excludeIssueInFile($issue_type, $file_name) + public function excludeIssueInFile($issue_type, $file_path) { if (!$this->totally_typed && in_array($issue_type, self::$MIXED_ISSUES)) { return true; } - $file_name = $this->shortenFileName($file_name); - if ($this->project_files && $this->hide_external_errors) { - if (!$this->isInProjectDirs($file_name)) { + if (!$this->isInProjectDirs($file_path)) { return true; } } - if ($this->getReportingLevelForFile($issue_type, $file_name) === self::REPORT_SUPPRESS) { + if ($this->getReportingLevelForFile($issue_type, $file_path) === self::REPORT_SUPPRESS) { return true; } @@ -452,18 +450,21 @@ class Config */ public function isInProjectDirs($file_path) { + if ($file_path[0] !== '/') { + throw new \UnexpectedValueException('eesh'); + } return $this->project_files && $this->project_files->allows($file_path); } /** * @param string $issue_type - * @param string $file_name + * @param string $file_path * @return string */ - public function getReportingLevelForFile($issue_type, $file_name) + public function getReportingLevelForFile($issue_type, $file_path) { if (isset($this->issue_handlers[$issue_type])) { - return $this->issue_handlers[$issue_type]->getReportingLevelForFile($file_name); + return $this->issue_handlers[$issue_type]->getReportingLevelForFile($file_path); } return self::REPORT_ERROR; diff --git a/src/Psalm/Config/ErrorLevelFileFilter.php b/src/Psalm/Config/ErrorLevelFileFilter.php index a57ae740e..d50d9b88a 100644 --- a/src/Psalm/Config/ErrorLevelFileFilter.php +++ b/src/Psalm/Config/ErrorLevelFileFilter.php @@ -13,6 +13,7 @@ class ErrorLevelFileFilter extends FileFilter /** * @param SimpleXMLElement $e + * @param Config $config * @param bool $inclusive * @return self */ diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php index fbf366f68..dabf03f83 100644 --- a/src/Psalm/Config/IssueHandler.php +++ b/src/Psalm/Config/IssueHandler.php @@ -55,13 +55,13 @@ class IssueHandler } /** - * @param string $file_name + * @param string $file_path * @return string */ - public function getReportingLevelForFile($file_name) + public function getReportingLevelForFile($file_path) { foreach ($this->custom_levels as $custom_level) { - if ($custom_level->allows($file_name)) { + if ($custom_level->allows($file_path)) { return $custom_level->getErrorLevel(); } } diff --git a/src/Psalm/Issue/InvalidClone.php b/src/Psalm/Issue/InvalidClone.php new file mode 100644 index 000000000..c8aebf3a7 --- /dev/null +++ b/src/Psalm/Issue/InvalidClone.php @@ -0,0 +1,6 @@ +excludeIssueInFile($issue_type, $e->getFileName())) { + if ($config->excludeIssueInFile($issue_type, $e->getFilePath())) { return false; }