diff --git a/src/Psalm/Checker/ClassLikeChecker.php b/src/Psalm/Checker/ClassLikeChecker.php index cdfa59fe8..264469a9e 100644 --- a/src/Psalm/Checker/ClassLikeChecker.php +++ b/src/Psalm/Checker/ClassLikeChecker.php @@ -431,24 +431,42 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc } } - foreach ($this->method_checkers as $method_id => $method_checker) { - $method_checker->check(clone $class_context, null); + foreach ($this->class->stmts as $stmt) { + if ($stmt instanceof PhpParser\Node\Stmt\ClassMethod) { + $method_checker = new MethodChecker($stmt, $this); - if (!$config->excludeIssueInFile('InvalidReturnType', $this->file_name)) { - $secondary_return_type_location = null; + $method_checker->check(clone $class_context, null); - $return_type_location = MethodChecker::getMethodReturnTypeLocation( - $method_id, - $secondary_return_type_location - ); + $method_id = (string)$method_checker->getMethodId(); - $method_checker->checkReturnTypes( - $update_docblocks, - MethodChecker::getMethodReturnType($method_id), - $this->fq_class_name, - $return_type_location, - $secondary_return_type_location - ); + if (!$config->excludeIssueInFile('InvalidReturnType', $this->file_name)) { + $secondary_return_type_location = null; + + $return_type_location = MethodChecker::getMethodReturnTypeLocation( + $method_id, + $secondary_return_type_location + ); + + $method_checker->checkReturnTypes( + $update_docblocks, + MethodChecker::getMethodReturnType($method_id), + $class_context->self, + $return_type_location, + $secondary_return_type_location + ); + } + } elseif ($stmt instanceof PhpParser\Node\Stmt\TraitUse) { + foreach ($stmt->traits as $trait) { + $trait_name = self::getFQCLNFromNameObject( + $trait, + $this->namespace, + $this->aliased_classes + ); + + $trait_checker = self::$trait_checkers[$trait_name]; + + $trait_checker->checkMethods($class_context, $global_context, $update_docblocks); + } } } } @@ -510,10 +528,13 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc PhpParser\Node\Stmt\ClassMethod $stmt, Context $class_context ) { - $method_id = $this->fq_class_name . '::' . strtolower($stmt->name); - $storage = self::$storage[$class_context->self]; + $method_name = strtolower($stmt->name); + $method_id = $this->fq_class_name . '::' . $method_name; + $storage = self::$storage[$this->fq_class_name]; - $this->method_checkers[$method_id] = new MethodChecker($stmt, $this); + if (!isset($storage->methods[$method_name])) { + FunctionLikeChecker::register($stmt, $this); + } if (!$stmt->isAbstract() && $class_context->self) { $implemented_method_id = @@ -589,8 +610,6 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc $trait_checker->check($class_context); - $this->method_checkers = array_merge($trait_checker->method_checkers, $this->method_checkers); - ClassLikeChecker::registerTraitUse($this->fq_class_name, $trait_name); FileChecker::addFileInheritanceToClass( @@ -858,11 +877,19 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc $namespace, array $aliased_classes ) { + if ($class_name->parts == ['self']) { + return 'self'; + } + if ($class_name instanceof PhpParser\Node\Name\FullyQualified) { return implode('\\', $class_name->parts); } - return self::getFQCLNFromString(implode('\\', $class_name->parts), $namespace, $aliased_classes); + return self::getFQCLNFromString( + implode('\\', $class_name->parts), + $namespace, + $aliased_classes + ); } /** diff --git a/src/Psalm/Checker/FileChecker.php b/src/Psalm/Checker/FileChecker.php index a3c6b0c92..343b3312b 100644 --- a/src/Psalm/Checker/FileChecker.php +++ b/src/Psalm/Checker/FileChecker.php @@ -236,7 +236,7 @@ class FileChecker extends SourceChecker implements StatementsSource // hoist functions to the top foreach ($function_stmts as $stmt) { - $function_checkers[$stmt->name] = new FunctionChecker($stmt, $this, $this->file_path); + $function_checkers[$stmt->name] = new FunctionChecker($stmt, $this); $function_id = $function_checkers[$stmt->name]->getMethodId(); $this->function_checkers[$function_id] = $function_checkers[$stmt->name]; } diff --git a/src/Psalm/Checker/FunctionChecker.php b/src/Psalm/Checker/FunctionChecker.php index 72a28e62c..ec1e25886 100644 --- a/src/Psalm/Checker/FunctionChecker.php +++ b/src/Psalm/Checker/FunctionChecker.php @@ -35,9 +35,8 @@ class FunctionChecker extends FunctionLikeChecker /** * @param mixed $function * @param StatementsSource $source - * @param string $base_file_path */ - public function __construct($function, StatementsSource $source, $base_file_path) + public function __construct($function, StatementsSource $source) { if (!$function instanceof PhpParser\Node\Stmt\Function_) { throw new \InvalidArgumentException('Bad'); @@ -45,7 +44,7 @@ class FunctionChecker extends FunctionLikeChecker parent::__construct($function, $source); - $this->registerFunction($function, $base_file_path); + self::register($function, $this); } /** @@ -117,7 +116,8 @@ class FunctionChecker extends FunctionLikeChecker /** @var \ReflectionParameter $param */ foreach ($reflection_params as $param) { - $storage->params[] = self::getReflectionParamArray($param); + $param_obj = self::getReflectionParamData($param); + $storage->params[] = $param_obj; } $storage->cased_name = $reflection_function->getName(); @@ -180,186 +180,6 @@ class FunctionChecker extends FunctionLikeChecker return $file_storage->functions[$function_id]->cased_name; } - /** - * @param PhpParser\Node\Stmt\Function_ $function - * @param string $file_path - * @return null|false - */ - protected function registerFunction(PhpParser\Node\Stmt\Function_ $function, $file_path) - { - $cased_function_id = ($this->namespace ? $this->namespace . '\\' : '') . $function->name; - $function_id = strtolower($cased_function_id); - - $file_storage = FileChecker::$storage[$file_path]; - - if (isset($file_storage->functions[$function_id])) { - return null; - } - - $storage = $file_storage->functions[$function_id] = new FunctionLikeStorage(); - - $storage->namespace = $this->namespace; - $storage->cased_name = $cased_function_id; - - $function_param_names = []; - - /** @var PhpParser\Node\Param $param */ - foreach ($function->getParams() as $param) { - $param_array = self::getTranslatedParam( - $param, - $this - ); - - $storage->params[] = $param_array; - - if (isset($function_param_names[$param->name])) { - if (IssueBuffer::accepts( - new DuplicateParam( - 'Duplicate param $' . $param->name . ' in docblock for ' . $this->function->name, - new CodeLocation($this, $param, true) - ), - $this->suppressed_issues - )) { - return false; - } - } - - $function_param_names[$param->name] = $param_array->type; - } - - $config = Config::getInstance(); - $return_type = null; - - $return_type_location = null; - - $docblock_info = null; - - $this->suppressed_issues = []; - - $doc_comment = $function->getDocComment(); - - if ($function->returnType) { - $parser_return_type = $function->returnType; - - $suffix = ''; - - if ($parser_return_type instanceof PhpParser\Node\NullableType) { - $suffix = '|null'; - $parser_return_type = $parser_return_type->type; - } - - if (is_string($parser_return_type)) { - $return_type_string = $parser_return_type . $suffix; - } else { - $fq_class_name = ClassLikeChecker::getFQCLNFromNameObject( - $parser_return_type, - $this->namespace, - $this->getAliasedClasses() - ); - - ClassLikeChecker::checkFullyQualifiedClassLikeName( - $fq_class_name, - $this->getFileChecker(), - new CodeLocation($this, $parser_return_type), - $this->suppressed_issues - ); - - $return_type_string = $fq_class_name . $suffix; - } - - $return_type = Type::parseString($return_type_string); - - $return_type_location = new CodeLocation($this->getSource(), $function, false, self::RETURN_TYPE_REGEX); - } - - $storage->return_type = $return_type; - $storage->return_type_location = $return_type_location; - - if ($doc_comment) { - try { - $docblock_info = CommentChecker::extractDocblockInfo( - (string)$doc_comment, - $doc_comment->getLine() - ); - } catch (DocblockParseException $e) { - if (IssueBuffer::accepts( - new InvalidDocblock( - $e->getMessage() . ' in docblock for ' . $this->function->name, - new CodeLocation($this, $function, true) - ) - )) { - // fall through - } - } - - if ($docblock_info) { - if ($docblock_info->deprecated) { - $storage->deprecated = true; - } - - if ($docblock_info->variadic) { - $storage->variadic = true; - } - - $this->suppressed_issues = $docblock_info->suppress; - - if ($config->use_docblock_types) { - if ($docblock_info->return_type) { - $docblock_return_type = - Type::parseString( - self::fixUpLocalType( - (string)$docblock_info->return_type, - null, - $this->namespace, - $this->getAliasedClasses() - ) - ); - - if (!$return_type_location) { - $return_type_location = new CodeLocation($this->getSource(), $function, true); - } - - if ($return_type && - !TypeChecker::isContainedBy( - $docblock_return_type, - $return_type, - $this->getFileChecker() - ) - ) { - if (IssueBuffer::accepts( - new InvalidDocblock( - 'Docblock return type does not match function return type for ' . - $this->getMethodId(), - new CodeLocation($this, $function, true) - ) - )) { - return false; - } - } else { - $return_type = $docblock_return_type; - } - - $return_type_location->setCommentLine($docblock_info->return_type_line_number); - - $storage->return_type = $return_type; - $storage->return_type_location = $return_type_location; - } - - if ($docblock_info->params) { - $this->improveParamsFromDocblock( - $docblock_info->params, - $function_param_names, - $storage->params, - new CodeLocation($this, $function, false) - ); - } - } - } - } - - return null; - } - /** * @param string $function_id * @return array|null diff --git a/src/Psalm/Checker/FunctionLikeChecker.php b/src/Psalm/Checker/FunctionLikeChecker.php index c73ad5fa1..638e349fe 100644 --- a/src/Psalm/Checker/FunctionLikeChecker.php +++ b/src/Psalm/Checker/FunctionLikeChecker.php @@ -8,6 +8,7 @@ use PhpParser; use Psalm\CodeLocation; use Psalm\Checker\Statements\ExpressionChecker; use Psalm\Checker\TypeChecker; +use Psalm\Config; use Psalm\Context; use Psalm\EffectsAnalyser; use Psalm\Exception\DocblockParseException; @@ -22,6 +23,8 @@ use Psalm\Issue\MissingReturnType; use Psalm\Issue\MixedInferredReturnType; use Psalm\IssueBuffer; use Psalm\StatementsSource; +use Psalm\Storage\FunctionLikeStorage; +use Psalm\Storage\MethodStorage; use Psalm\Type; abstract class FunctionLikeChecker extends SourceChecker implements StatementsSource @@ -113,16 +116,14 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo */ public function check(Context $context, Context $global_context = null) { + /** @var array */ $function_stmts = $this->function->getStmts() ?: []; - $statements_checker = new StatementsChecker($this); - $hash = null; - $closure_return_type = null; - $closure_return_type_location = null; + $cased_method_id = null; - if ($this instanceof MethodChecker) { + if ($this->function instanceof ClassMethod) { if (ClassLikeChecker::getThisClass()) { $hash = $this->getMethodId() . json_encode([ $context->vars_in_scope, @@ -142,14 +143,14 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $context->vars_in_scope['$this'] = new Type\Union([new Type\Atomic($context->self)]); } - $function_params = MethodChecker::getMethodParams((string)$this->getMethodId()); - - if ($function_params === null) { - throw new \InvalidArgumentException('Cannot get params for own method'); - } - $method_id = (string)$this->getMethodId($context->self); + $fq_class_name = (string)$context->self; + + $storage = MethodChecker::getStorage(MethodChecker::getDeclaringMethodId($method_id)); + + $cased_method_id = $fq_class_name . '::' . $storage->cased_name; + $implemented_method_ids = MethodChecker::getOverriddenMethodIds($method_id); if ($implemented_method_ids) { @@ -171,8 +172,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } foreach ($implemented_params as $i => $implemented_param) { - if (!isset($function_params[$i])) { - $cased_method_id = MethodChecker::getCasedMethodId((string)$this->getMethodId()); + if (!isset($storage->params[$i])) { $parent_method_id = MethodChecker::getCasedMethodId($implemented_method_id); if (IssueBuffer::accepts( @@ -189,7 +189,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo break; } - if ((string)$function_params[$i]->signature_type !== + if ((string)$storage->params[$i]->signature_type !== (string)$implemented_param->signature_type ) { $cased_method_id = MethodChecker::getCasedMethodId((string)$this->getMethodId()); @@ -198,7 +198,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo if (IssueBuffer::accepts( new MethodSignatureMismatch( 'Argument ' . ($i + 1) . ' of ' . $cased_method_id .' has wrong type \'' . - $function_params[$i]->signature_type . '\', expecting \'' . + $storage->params[$i]->signature_type . '\', expecting \'' . $implemented_param->signature_type . '\' as defined by ' . $parent_method_id, new CodeLocation($this, $this->function) @@ -213,163 +213,32 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } } } - } elseif ($this instanceof FunctionChecker) { - $function_params = FunctionChecker::getParams(strtolower((string)$this->getMethodId()), $this->file_path); + } elseif ($this->function instanceof Function_) { + $storage = FileChecker::$storage[$this->file_path]->functions[(string)$this->getMethodId()]; + + $cased_method_id = $this->function->name; } else { // Closure - $function_params = []; - $function_param_names = []; - - foreach ($this->function->getParams() as $param) { - $param_array = self::getTranslatedParam( - $param, - $this - ); - - $function_params[] = $param_array; - - if (isset($function_param_names[$param->name])) { - if (IssueBuffer::accepts( - new DuplicateParam( - 'Duplicate param $' . $param->name . ' in closure docblock', - new CodeLocation($this, $param, true) - ), - $this->suppressed_issues - )) { - return false; - } - } - - $function_param_names[$param->name] = $param_array->type; - } - - $doc_comment = $this->function->getDocComment(); - - if ($this->function->returnType) { - $parser_return_type = $this->function->returnType; - - $suffix = ''; - - if ($parser_return_type instanceof PhpParser\Node\NullableType) { - $suffix = '|null'; - $parser_return_type = $parser_return_type->type; - } - - if (is_string($parser_return_type)) { - $return_type_string = $parser_return_type . $suffix; - } else { - $fq_class_name = ClassLikeChecker::getFQCLNFromNameObject( - $parser_return_type, - $this->namespace, - $this->getAliasedClasses() - ); - - ClassLikeChecker::checkFullyQualifiedClassLikeName( - $fq_class_name, - $this->getFileChecker(), - new CodeLocation($this, $parser_return_type), - $this->suppressed_issues - ); - - $return_type_string = $fq_class_name . $suffix; - } - - $closure_return_type = Type::parseString($return_type_string); - - $closure_return_type_location = new CodeLocation( - $this->getSource(), - $this->function, - false, - self::RETURN_TYPE_REGEX - ); - } - - if ($doc_comment) { - try { - $docblock_info = CommentChecker::extractDocblockInfo( - (string)$doc_comment, - $doc_comment->getLine() - ); - } catch (DocblockParseException $e) { - if (IssueBuffer::accepts( - new InvalidDocblock( - 'Invalid type passed in docblock for ' . $this->getMethodId(), - new CodeLocation($this, $this->function, true) - ) - )) { - return false; - } - } - - if ($docblock_info) { - $this->suppressed_issues = $docblock_info->suppress; - - $config = \Psalm\Config::getInstance(); - - if ($config->use_docblock_types) { - if ($docblock_info->return_type) { - $closure_docblock_return_type = - Type::parseString( - self::fixUpLocalType( - (string)$docblock_info->return_type, - null, - $this->namespace, - $this->getAliasedClasses() - ) - ); - - if (!$closure_return_type_location) { - $closure_return_type_location = new CodeLocation( - $this->getSource(), - $this->function, - true - ); - } - - if ($closure_return_type && - !TypeChecker::isContainedBy( - $closure_return_type, - $closure_docblock_return_type, - $statements_checker->getFileChecker() - ) - ) { - if (IssueBuffer::accepts( - new InvalidDocblock( - 'Docblock return type does not match closure return type ' . $this->getMethodId(), - new CodeLocation($this, $this->function, true) - ) - )) { - return false; - } - } else { - $closure_return_type = $closure_docblock_return_type; - } - - $closure_return_type_location->setCommentLine($docblock_info->return_type_line_number); - } - - if ($docblock_info->params) { - $this->improveParamsFromDocblock( - $docblock_info->params, - $function_param_names, - $function_params, - new CodeLocation($this, $this->function, false) - ); - } - } - } - } + $storage = self::register($this->function, $this->getSource()); /** @var PhpParser\Node\Expr\Closure $this->function */ $this->function->inferredType = new Type\Union([ new Type\Fn( 'Closure', - $function_params, - $closure_return_type ?: Type::getMixed() + $storage->params, + $storage->return_type ?: Type::getMixed() ) ]); } - foreach ($function_params as $offset => $function_param) { + $this->suppressed_issues = $storage->suppressed_issues; + + if ($storage instanceof MethodStorage && $storage->is_static) { + $this->is_static = true; + } + + $statements_checker = new StatementsChecker($this); + + foreach ($storage->params as $offset => $function_param) { $param_type = ExpressionChecker::fleshOutTypes( clone $function_param->type, [], @@ -394,15 +263,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo ) ) { $method_id = $this->getMethodId(); - $cased_method_id = $method_id; - - if ($cased_method_id) { - if ($this instanceof MethodChecker) { - $cased_method_id = MethodChecker::getCasedMethodId($cased_method_id); - } elseif ($this->function instanceof PhpParser\Node\Stmt\Function_) { - $cased_method_id = $this->function->name; - } - } if (IssueBuffer::accepts( new InvalidParamDefault( @@ -446,12 +306,12 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $this->checkReturnTypes( false, - $closure_return_type, + $storage->return_type, $this->fq_class_name, - $closure_return_type_location + $storage->return_type_location ); - if (!$closure_return_type || $closure_return_type->isMixed()) { + if (!$storage->return_type || $storage->return_type->isMixed()) { $closure_yield_types = []; $closure_return_types = EffectsAnalyser::getReturnTypes( $this->function->stmts, @@ -504,6 +364,225 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo return null; } + /** + * @param Closure|Function_|ClassMethod $function + * @param StatementsSource $source + * @return FunctionLikeStorage + */ + public static function register( + $function, + StatementsSource $source + ) { + $namespace = $source->getNamespace(); + + if ($function instanceof PhpParser\Node\Stmt\Function_) { + $cased_function_id = ($namespace ? $namespace . '\\' : '') . $function->name; + $function_id = strtolower($cased_function_id); + + $file_storage = FileChecker::$storage[$source->getFilePath()]; + + if (isset($file_storage->functions[$function_id])) { + throw new \InvalidArgumentException('Cannot re-register ' . $function_id); + } + + $storage = $file_storage->functions[$function_id] = new FunctionLikeStorage(); + } elseif ($function instanceof PhpParser\Node\Stmt\ClassMethod) { + $fq_class_name = $source->getFQCLN(); + + $function_id = $fq_class_name . '::' . strtolower($function->name); + $cased_function_id = $fq_class_name . '::' . $function->name; + + if (!isset(ClassLikeChecker::$storage[$fq_class_name])) { + throw new \UnexpectedValueException('$class_storage cannot be empty for ' . $function_id); + } + + $class_storage = ClassLikeChecker::$storage[$fq_class_name]; + + if (isset($class_storage->methods[strtolower($function->name)])) { + throw new \InvalidArgumentException('Cannot re-register ' . $function_id); + } + + $storage = $class_storage->methods[strtolower($function->name)] = new MethodStorage(); + + $class_name_parts = explode('\\', $fq_class_name); + $class_name = array_pop($class_name_parts); + + if (strtolower((string)$function->name) === strtolower($class_name)) { + MethodChecker::setDeclaringMethodId($fq_class_name . '::__construct', $function_id); + MethodChecker::setAppearingMethodId($fq_class_name . '::__construct', $function_id); + } + + $class_storage->declaring_method_ids[strtolower($function->name)] = $function_id; + $class_storage->appearing_method_ids[strtolower($function->name)] = $function_id; + + if (!isset($class_storage->overridden_method_ids[strtolower($function->name)])) { + $class_storage->overridden_method_ids[strtolower($function->name)] = []; + } + + /** @var bool */ + $storage->is_static = $function->isStatic(); + + if ($function->isPrivate()) { + $storage->visibility = ClassLikeChecker::VISIBILITY_PRIVATE; + } elseif ($function->isProtected()) { + $storage->visibility = ClassLikeChecker::VISIBILITY_PROTECTED; + } else { + $storage->visibility = ClassLikeChecker::VISIBILITY_PUBLIC; + } + } else { + $function_id = $cased_function_id = 'closure'; + + $storage = new FunctionLikeStorage(); + } + + if ($function instanceof ClassMethod || $function instanceof Function_) { + $storage->cased_name = $function->name; + } + + $storage->file_name = $source->getFileName(); + $storage->namespace = $source->getNamespace(); + + /** @var PhpParser\Node\Param $param */ + foreach ($function->getParams() as $param) { + $param_array = self::getTranslatedParam($param, $source); + + if (isset($storage->param_types[$param_array->name])) { + if (IssueBuffer::accepts( + new DuplicateParam( + 'Duplicate param $' . $param->name . ' in docblock for ' . $cased_function_id, + new CodeLocation($source, $param, true) + ), + $source->getSuppressedIssues() + )) { + // fall through + } + } + + $storage->param_types[$param_array->name] = $param_array->type; + $storage->params[] = $param_array; + } + + $config = Config::getInstance(); + + $docblock_info = null; + $doc_comment = $function->getDocComment(); + + if ($parser_return_type = $function->getReturnType()) { + $suffix = ''; + + if ($parser_return_type instanceof PhpParser\Node\NullableType) { + $suffix = '|null'; + $parser_return_type = $parser_return_type->type; + } + + if (is_string($parser_return_type)) { + $return_type_string = $parser_return_type . $suffix; + } else { + $return_type_fq_class_name = ClassLikeChecker::getFQCLNFromNameObject( + $parser_return_type, + $namespace, + $source->getAliasedClasses() + ); + + $return_type_string = $return_type_fq_class_name . $suffix; + } + + $storage->return_type = Type::parseString($return_type_string); + $storage->return_type_location = new CodeLocation( + $source, + $function, + false, + self::RETURN_TYPE_REGEX + ); + } + + if (!$doc_comment) { + return $storage; + } + + + $docblock_info = null; + + try { + $docblock_info = CommentChecker::extractDocblockInfo((string)$doc_comment, $doc_comment->getLine()); + } catch (DocblockParseException $e) { + if (IssueBuffer::accepts( + new InvalidDocblock( + $e->getMessage() . ' in docblock for ' . $cased_function_id, + new CodeLocation($source, $function, true) + ) + )) { + // fall through + } + } + + if (!$docblock_info) { + return $storage; + } + + if ($docblock_info->deprecated) { + $storage->deprecated = true; + } + + if ($docblock_info->variadic) { + $storage->variadic = true; + } + + $storage->suppressed_issues = $docblock_info->suppress; + + if (!$config->use_docblock_types) { + return $storage; + } + + if ($docblock_info->return_type) { + $docblock_return_type = Type::parseString( + self::fixUpLocalType( + (string)$docblock_info->return_type, + $source->getFQCLN(), + $source->getNamespace(), + $source->getAliasedClasses() + ) + ); + + if (!$storage->return_type_location) { + $storage->return_type_location = new CodeLocation($source, $function, true); + } + + if ($storage->return_type && + !TypeChecker::hasIdenticalTypes( + $storage->return_type, + $docblock_return_type, + $source->getFileChecker() + ) + ) { + if (IssueBuffer::accepts( + new InvalidDocblock( + 'Docblock return type does not match method return type for ' . $cased_function_id, + new CodeLocation($source, $function, true) + ) + )) { + // fall through + } + } else { + $storage->return_type = $docblock_return_type; + } + + $storage->return_type_location->setCommentLine($docblock_info->return_type_line_number); + } + + if ($docblock_info->params) { + self::improveParamsFromDocblock( + $storage, + $docblock_info->params, + $source, + $source->getFQCLN(), + new CodeLocation($source, $function, true) + ); + } + + return $storage; + } + /** * Adds return types for the given function * @@ -541,10 +620,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo if ($this->function instanceof ClassMethod) { $function_name = (string)$this->function->name; - if (strtolower($function_name) === strtolower((string)$this->class_name)) { - $function_name = '__construct'; - } - return ($context_self ?: $this->fq_class_name) . '::' . strtolower($function_name); } @@ -840,32 +915,31 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo /** * @param array $docblock_params - * @param array $function_param_names - * @param array<\Psalm\FunctionLikeParameter> &$function_signature - * @param CodeLocation $code_location + * @param FunctionLikeStorage $storage + * @param StatementsSource $source + * @param string $fq_class_name + * @param CodeLocation $code_location * @return false|null */ - protected function improveParamsFromDocblock( + protected static function improveParamsFromDocblock( + FunctionLikeStorage $storage, array $docblock_params, - array $function_param_names, - array &$function_signature, + StatementsSource $source, + $fq_class_name, CodeLocation $code_location ) { $docblock_param_vars = []; - $method_id = $cased_method_id = $this->getMethodId(); + $base = $fq_class_name ? $fq_class_name . '::' : ''; - if ($method_id) { - $cased_method_id = $this instanceof MethodChecker - ? MethodChecker::getCasedMethodId($method_id) - : FunctionChecker::getCasedFunctionId($method_id, $this->file_path); - } + $method_id = $base . strtolower($storage->cased_name); + $cased_method_id = $base . $storage->cased_name; foreach ($docblock_params as $docblock_param) { $param_name = $docblock_param['name']; $line_number = $docblock_param['line_number']; - if (!array_key_exists($param_name, $function_param_names)) { + if (!isset($storage->param_types[$param_name])) { $code_location->setCommentLine($line_number); if (IssueBuffer::accepts( new InvalidDocblock( @@ -886,23 +960,23 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo self::fixUpLocalType( $docblock_param['type'], null, - $this->namespace, - $this->getAliasedClasses() + $source->getNamespace(), + $source->getAliasedClasses() ) ); - if ($function_param_names[$param_name] && !$function_param_names[$param_name]->isMixed()) { + if (!$storage->param_types[$param_name]->isMixed()) { if (!TypeChecker::isContainedBy( $new_param_type, - $function_param_names[$param_name], - $this->getFileChecker() + $storage->param_types[$param_name], + $source->getFileChecker() ) ) { $code_location->setCommentLine($line_number); if (IssueBuffer::accepts( new InvalidDocblock( 'Parameter $' . $param_name .' has wrong type \'' . $new_param_type . '\', should be \'' . - $function_param_names[$param_name] . '\'', + $storage->param_types[$param_name] . '\'', $code_location ) )) { @@ -913,7 +987,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } } - foreach ($function_signature as &$function_signature_param) { + foreach ($storage->params as $function_signature_param) { if ($function_signature_param->name === $param_name) { $existing_param_type_nullable = $function_signature_param->is_nullable; @@ -928,8 +1002,10 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } } - foreach ($function_signature as &$function_signature_param) { - if (!isset($docblock_param_vars[$function_signature_param->name]) && $function_signature_param->code_location) { + foreach ($storage->params as $function_signature_param) { + if (!isset($docblock_param_vars[$function_signature_param->name]) && + $function_signature_param->code_location + ) { if (IssueBuffer::accepts( new InvalidDocblock( 'Parameter $' . $function_signature_param->name .' does not appear in the docbock for ' . @@ -1032,7 +1108,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo * @param \ReflectionParameter $param * @return FunctionLikeParameter */ - protected static function getReflectionParamArray(\ReflectionParameter $param) + protected static function getReflectionParamData(\ReflectionParameter $param) { $param_type_string = null; @@ -1204,8 +1280,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo array $args, FileChecker $file_checker ) { - $function_params = null; - if (count($function_param_options) === 1) { return $function_param_options[0]; } diff --git a/src/Psalm/Checker/MethodChecker.php b/src/Psalm/Checker/MethodChecker.php index 17d406872..163b8b592 100644 --- a/src/Psalm/Checker/MethodChecker.php +++ b/src/Psalm/Checker/MethodChecker.php @@ -32,9 +32,6 @@ class MethodChecker extends FunctionLikeChecker } parent::__construct($function, $source); - - $this->registerMethod($function); - $this->is_static = $function->isStatic(); } /** @@ -148,6 +145,13 @@ class MethodChecker extends FunctionLikeChecker $method_name = strtolower($method->getName()); $class_storage = ClassLikeChecker::$storage[$method->class]; + + if (isset($class_storage->methods[strtolower($method_name)])) { + return; + } + + $method_id = $method->class . '::' . $method_name; + $storage = $class_storage->methods[strtolower($method_name)] = new MethodStorage(); $storage->cased_name = $method->name; @@ -157,15 +161,9 @@ class MethodChecker extends FunctionLikeChecker self::setAppearingMethodId($method->class . '::__construct', $method->class . '::' . $method_name); } - if ($storage->reflected) { - return null; - } - /** @var \ReflectionClass */ $declaring_class = $method->getDeclaringClass(); - $storage->reflected = true; - $storage->is_static = $method->isStatic(); $storage->file_name = $method->getFileName(); $storage->namespace = $declaring_class->getNamespaceName(); @@ -188,19 +186,10 @@ class MethodChecker extends FunctionLikeChecker /** @var \ReflectionParameter $param */ foreach ($params as $param) { - $param_array = self::getReflectionParamArray($param); + $param_array = self::getReflectionParamData($param); $storage->params[] = $param_array; - $method_param_names[$param->name] = true; - $method_param_types[$param->name] = $param_array->type; + $storage->param_types[$param->name] = $param_array->type; } - - $return_types = null; - - $config = Config::getInstance(); - - $return_type = null; - - $storage->return_type = $return_type; return null; } @@ -255,206 +244,6 @@ class MethodChecker extends FunctionLikeChecker return true; } - /** - * @param PhpParser\Node\Stmt\ClassMethod $method - * @return null|false - * @psalm-suppress MixedAssignment - */ - protected function registerMethod(PhpParser\Node\Stmt\ClassMethod $method) - { - $method_id = $this->fq_class_name . '::' . strtolower($method->name); - - $class_storage = ClassLikeChecker::$storage[$this->fq_class_name]; - $storage = $class_storage->methods[strtolower($method->name)] = new MethodStorage(); - - $cased_method_id = $storage->cased_name = $method->name; - - if (strtolower((string)$method->name) === strtolower((string)$this->class_name)) { - self::setDeclaringMethodId($this->fq_class_name . '::__construct', $method_id); - self::setAppearingMethodId($this->fq_class_name . '::__construct', $method_id); - } - - if ($storage->reflected || $storage->registered) { - $this->suppressed_issues = $storage->suppressed_issues; - - return null; - } - - if (!$this->source instanceof TraitChecker) { - $storage->registered = true; - } - - $class_storage->declaring_method_ids[strtolower($method->name)] = $method_id; - $class_storage->appearing_method_ids[strtolower($method->name)] = $method_id; - - if (!isset($class_storage->overridden_method_ids[strtolower($method->name)])) { - $class_storage->overridden_method_ids[strtolower($method->name)] = []; - } - - $storage->is_static = $method->isStatic(); - - $storage->namespace = $this->namespace; - $storage->file_name = $this->file_name; - - if ($method->isPrivate()) { - $storage->visibility = ClassLikeChecker::VISIBILITY_PRIVATE; - } elseif ($method->isProtected()) { - $storage->visibility = ClassLikeChecker::VISIBILITY_PROTECTED; - } else { - $storage->visibility = ClassLikeChecker::VISIBILITY_PUBLIC; - } - - $method_param_names = []; - - foreach ($method->getParams() as $param) { - $param_array = $this->getTranslatedParam( - $param, - $this - ); - - $storage->params[] = $param_array; - - if (isset($function_param_names[$param->name])) { - if (IssueBuffer::accepts( - new DuplicateParam( - 'Duplicate param $' . $param->name . ' in docblock for ' . $cased_method_id, - new CodeLocation($this, $param, true) - ), - $this->suppressed_issues - )) { - return false; - } - } - - $method_param_names[$param->name] = $param_array->type; - } - - $config = Config::getInstance(); - $return_type = null; - $return_type_location = null; - - $doc_comment = $method->getDocComment(); - - $storage->suppressed_issues = []; - - if (isset($method->returnType)) { - $parser_return_type = $method->returnType; - - $suffix = ''; - - if ($parser_return_type instanceof PhpParser\Node\NullableType) { - $suffix = '|null'; - $parser_return_type = $parser_return_type->type; - } - - if (is_string($parser_return_type)) { - $return_type_string = $parser_return_type . $suffix; - } else { - $fq_class_name = ClassLikeChecker::getFQCLNFromNameObject( - $parser_return_type, - $this->namespace, - $this->getAliasedClasses() - ); - - if ($fq_class_name !== 'self') { - ClassLikeChecker::checkFullyQualifiedClassLikeName( - $fq_class_name, - $this->getFileChecker(), - new CodeLocation($this, $parser_return_type), - $this->suppressed_issues - ); - } - - $return_type_string = $fq_class_name . $suffix; - } - - $return_type = Type::parseString($return_type_string); - - $return_type_location = new CodeLocation($this->getSource(), $method, false, self::RETURN_TYPE_REGEX); - } - - if ($doc_comment) { - $docblock_info = null; - - try { - $docblock_info = CommentChecker::extractDocblockInfo((string)$doc_comment, $doc_comment->getLine()); - } catch (DocblockParseException $e) { - if (IssueBuffer::accepts( - new InvalidDocblock( - 'Invalid type passed in docblock for ' . $cased_method_id, - new CodeLocation($this, $method) - ) - )) { - return false; - } - } - - if ($docblock_info) { - if ($docblock_info->deprecated) { - $storage->deprecated = true; - } - - if ($docblock_info->variadic) { - $storage->variadic = true; - } - - $this->suppressed_issues = $docblock_info->suppress; - $storage->suppressed_issues = $this->suppressed_issues; - - if ($config->use_docblock_types) { - if ($docblock_info->return_type) { - $docblock_return_type = Type::parseString( - $this->fixUpLocalType( - (string)$docblock_info->return_type, - $this->fq_class_name, - $this->namespace, - $this->getAliasedClasses() - ) - ); - - if (!$return_type_location) { - $return_type_location = new CodeLocation($this->getSource(), $method, true); - } - - if ($return_type && - !TypeChecker::hasIdenticalTypes( - $return_type, - $docblock_return_type, - $this->getFileChecker() - ) - ) { - if (IssueBuffer::accepts( - new InvalidDocblock( - 'Docblock return type does not match method return type for ' . $this->getMethodId(), - new CodeLocation($this, $method, true) - ) - )) { - return false; - } - } else { - $return_type = $docblock_return_type; - } - - $return_type_location->setCommentLine($docblock_info->return_type_line_number); - } - - if ($docblock_info->params) { - $this->improveParamsFromDocblock( - $docblock_info->params, - $method_param_names, - $storage->params, - new CodeLocation($this, $method, true) - ); - } - } - } - } - - $storage->return_type_location = $return_type_location; - $storage->return_type = $return_type; - return null; - } - /** * @param string $return_type * @param string $method_id @@ -575,7 +364,7 @@ class MethodChecker extends FunctionLikeChecker /** * @param string $method_id - * @return MethodStorage|null + * @return MethodStorage */ public static function getStorage($method_id) { @@ -583,11 +372,11 @@ class MethodChecker extends FunctionLikeChecker $class_storage = ClassLikeChecker::$storage[$fq_class_name]; - if (isset($class_storage->methods[strtolower($method_name)])) { - return $class_storage->methods[strtolower($method_name)]; + if (!isset($class_storage->methods[strtolower($method_name)])) { + throw new \UnexpectedValueException('$storage should not be null for ' . $method_id); } - return null; + return $class_storage->methods[strtolower($method_name)]; } /** @@ -601,10 +390,6 @@ class MethodChecker extends FunctionLikeChecker $method_id = (string) self::getDeclaringMethodId($method_id); $storage = self::getStorage($method_id); - if (!$storage) { - throw new \UnexpectedValueException('$storage should not be null'); - } - if ($storage->deprecated) { if (IssueBuffer::accepts( new DeprecatedMethod( @@ -751,6 +536,10 @@ class MethodChecker extends FunctionLikeChecker { list($fq_class_name, $method_name) = explode('::', $method_id); + if (!isset(ClassLikeChecker::$storage[$fq_class_name])) { + throw new \UnexpectedValueException('$storage should not be null for ' . $fq_class_name); + } + if (isset(ClassLikeChecker::$storage[$fq_class_name]->declaring_method_ids[$method_name])) { return ClassLikeChecker::$storage[$fq_class_name]->declaring_method_ids[$method_name]; } @@ -766,6 +555,10 @@ class MethodChecker extends FunctionLikeChecker { list($fq_class_name, $method_name) = explode('::', $method_id); + if (!isset(ClassLikeChecker::$storage[$fq_class_name])) { + throw new \UnexpectedValueException('$storage should not be null for ' . $fq_class_name); + } + if (isset(ClassLikeChecker::$storage[$fq_class_name]->appearing_method_ids[$method_name])) { return ClassLikeChecker::$storage[$fq_class_name]->appearing_method_ids[$method_name]; } diff --git a/src/Psalm/Checker/SourceChecker.php b/src/Psalm/Checker/SourceChecker.php index 967655661..3821e9510 100644 --- a/src/Psalm/Checker/SourceChecker.php +++ b/src/Psalm/Checker/SourceChecker.php @@ -287,6 +287,9 @@ abstract class SourceChecker implements StatementsSource return $this->suppressed_issues; } + /** + * @return string + */ public function getNamespace() { return ''; diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index 605d623fc..a0ecbec51 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -111,7 +111,7 @@ class CallChecker if (isset($stmt->name->inferredType)) { foreach ($stmt->name->inferredType->types as $var_type_part) { if ($var_type_part instanceof Type\Fn) { - $function_params = $var_type_part->parameters; + $function_params = $var_type_part->params; if ($var_type_part->return_type) { if (isset($stmt->inferredType)) { @@ -1147,7 +1147,7 @@ class CallChecker continue; } - if (count($closure_type->parameters) > $expected_closure_param_count) { + if (count($closure_type->params) > $expected_closure_param_count) { if (IssueBuffer::accepts( new TooManyArguments( 'Too many arguments in closure for ' . $method_id, @@ -1157,7 +1157,7 @@ class CallChecker )) { return false; } - } elseif (count($closure_type->parameters) < $expected_closure_param_count) { + } elseif (count($closure_type->params) < $expected_closure_param_count) { if (IssueBuffer::accepts( new TooFewArguments( 'You must supply a param in the closure for ' . $method_id, @@ -1170,11 +1170,14 @@ class CallChecker } - $closure_params = $closure_type->parameters; + $closure_params = $closure_type->params; $closure_return_type = $closure_type->return_type; - foreach ($closure_params as $i => $closure_param) { + $i = 0; + + foreach ($closure_params as $param_name => $closure_param) { if (!$array_arg_types[$i]) { + $i++; continue; } @@ -1184,6 +1187,7 @@ class CallChecker $input_type = $array_arg_type->type_params[1]; if ($input_type->isMixed()) { + $i++; continue; } @@ -1234,6 +1238,8 @@ class CallChecker return false; } } + + $i++; } } } diff --git a/src/Psalm/Checker/StatementsChecker.php b/src/Psalm/Checker/StatementsChecker.php index 98144a095..ee6dc515b 100644 --- a/src/Psalm/Checker/StatementsChecker.php +++ b/src/Psalm/Checker/StatementsChecker.php @@ -171,7 +171,7 @@ class StatementsChecker // hoist functions to the top foreach ($stmts as $stmt) { if ($stmt instanceof PhpParser\Node\Stmt\Function_) { - $function_checker = new FunctionChecker($stmt, $this->source, $this->file_path); + $function_checker = new FunctionChecker($stmt, $this->source); $function_checkers[$stmt->name] = $function_checker; } } diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index 31a943bef..3faa3f36e 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -179,7 +179,7 @@ class IssueBuffer if ($start_time) { echo('Checks took ' . ((float)microtime(true) - self::$start_time)); - echo(' and used ' . memory_get_peak_usage() . PHP_EOL); + echo(' and used ' . number_format(memory_get_peak_usage() / (1024 * 1024), 3) . 'MB' . PHP_EOL); } if (count(self::$emitted)) { diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index 77772ced8..57d6a0dcd 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -17,6 +17,11 @@ class FunctionLikeStorage */ public $params = []; + /** + * @var array + */ + public $param_types = []; + /** * @var string */ diff --git a/src/Psalm/Storage/MethodStorage.php b/src/Psalm/Storage/MethodStorage.php index 84da71dc2..f069dd7fb 100644 --- a/src/Psalm/Storage/MethodStorage.php +++ b/src/Psalm/Storage/MethodStorage.php @@ -5,6 +5,11 @@ use Psalm\Type; class MethodStorage extends FunctionLikeStorage { + /** + * @var string + */ + public $fq_class_name; + /** * @var bool */ diff --git a/src/Psalm/Type/Fn.php b/src/Psalm/Type/Fn.php index ed5a2471c..2fc37123c 100644 --- a/src/Psalm/Type/Fn.php +++ b/src/Psalm/Type/Fn.php @@ -13,7 +13,7 @@ class Fn extends Atomic /** * @var array */ - public $parameters = []; + public $params = []; /** * @var Union @@ -24,12 +24,12 @@ class Fn extends Atomic * Constructs a new instance of a generic type * * @param string $value - * @param array $parameters + * @param array $params * @param Union $return_type */ - public function __construct($value, array $parameters, Union $return_type) + public function __construct($value, array $params, Union $return_type) { - $this->parameters = $parameters; + $this->params = $params; $this->return_type = $return_type; } } diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php index 9204fe7eb..757921226 100644 --- a/tests/AnnotationTest.php +++ b/tests/AnnotationTest.php @@ -181,9 +181,9 @@ class AnnotationTest extends PHPUnit_Framework_TestCase } /** - * @return array> + * @return array */ - function foo2() : array { + function foo3() : array { return ["hello"]; } '); diff --git a/tests/IssueSuppressionTest.php b/tests/IssueSuppressionTest.php index b5773005d..ef9b7646c 100644 --- a/tests/IssueSuppressionTest.php +++ b/tests/IssueSuppressionTest.php @@ -38,7 +38,7 @@ class IssueSuppressionTest extends PHPUnit_Framework_TestCase * @psalm-suppress MixedMethodCall * @psalm-suppress MissingReturnType */ - public function a() { + public function b() { B::fooFoo()->barBar()->bat()->baz()->bam()->bas()->bee()->bet()->bes()->bis(); } }