mirror of
https://github.com/danog/psalm.git
synced 2024-11-30 04:39:00 +01:00
Fix #382 - improve reserved word checks
This commit is contained in:
parent
6b68da0e4d
commit
75daea5f04
@ -16,6 +16,7 @@ use Psalm\Issue\InvalidReturnType;
|
||||
use Psalm\Issue\MissingConstructor;
|
||||
use Psalm\Issue\MissingPropertyType;
|
||||
use Psalm\Issue\PropertyNotSetInConstructor;
|
||||
use Psalm\Issue\ReservedWord;
|
||||
use Psalm\Issue\UndefinedClass;
|
||||
use Psalm\Issue\UndefinedTrait;
|
||||
use Psalm\Issue\UnimplementedAbstractMethod;
|
||||
@ -192,6 +193,32 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
|
||||
) {
|
||||
$fq_class_name = $class_context && $class_context->self ? $class_context->self : $this->fq_class_name;
|
||||
|
||||
if (preg_match(
|
||||
'/(^|\\\)(int|float|bool|string|void|null|false|true|resource|object|numeric|mixed)$/i',
|
||||
$fq_class_name
|
||||
)
|
||||
) {
|
||||
$class_name_parts = explode('\\', $fq_class_name);
|
||||
$class_name = array_pop($class_name_parts);
|
||||
|
||||
if (IssueBuffer::accepts(
|
||||
new ReservedWord(
|
||||
$class_name . ' is a reserved word',
|
||||
new CodeLocation(
|
||||
$this,
|
||||
$this->class,
|
||||
null,
|
||||
true
|
||||
)
|
||||
),
|
||||
$this->source->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
$storage = $this->storage;
|
||||
|
||||
$project_checker = $this->file_checker->project_checker;
|
||||
@ -974,6 +1001,27 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc
|
||||
return true;
|
||||
}
|
||||
|
||||
if (preg_match(
|
||||
'/(^|\\\)(int|float|bool|string|void|null|false|true|resource|object|numeric|mixed)$/i',
|
||||
$fq_class_name
|
||||
)
|
||||
) {
|
||||
$class_name_parts = explode('\\', $fq_class_name);
|
||||
$class_name = array_pop($class_name_parts);
|
||||
|
||||
if (IssueBuffer::accepts(
|
||||
new ReservedWord(
|
||||
$class_name . ' is a reserved word',
|
||||
$code_location
|
||||
),
|
||||
$suppressed_issues
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
$class_exists = ClassChecker::classExists($project_checker, $fq_class_name);
|
||||
$interface_exists = InterfaceChecker::interfaceExists($project_checker, $fq_class_name);
|
||||
|
||||
|
@ -30,6 +30,7 @@ use Psalm\Issue\MoreSpecificImplementedParamType;
|
||||
use Psalm\Issue\MoreSpecificImplementedReturnType;
|
||||
use Psalm\Issue\MoreSpecificReturnType;
|
||||
use Psalm\Issue\OverriddenMethodAccess;
|
||||
use Psalm\Issue\ReservedWord;
|
||||
use Psalm\Issue\UntypedParam;
|
||||
use Psalm\Issue\UnusedParam;
|
||||
use Psalm\Issue\UnusedVariable;
|
||||
@ -370,7 +371,25 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
false
|
||||
);
|
||||
} else {
|
||||
$param_type->check($this->source, $function_param->type_location, $this->suppressed_issues, [], false);
|
||||
if ($param_type->isVoid()) {
|
||||
if (IssueBuffer::accepts(
|
||||
new ReservedWord(
|
||||
'Parameter cannot be void',
|
||||
$function_param->type_location
|
||||
),
|
||||
$this->suppressed_issues
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
|
||||
$param_type->check(
|
||||
$this->source,
|
||||
$function_param->type_location,
|
||||
$this->suppressed_issues,
|
||||
[],
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
if ($this->getFileChecker()->project_checker->collect_references) {
|
||||
@ -405,55 +424,69 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
);
|
||||
}
|
||||
|
||||
if ($storage->return_type
|
||||
&& $storage->signature_return_type
|
||||
&& $storage->return_type_location
|
||||
&& !$this->function instanceof Closure
|
||||
) {
|
||||
$fleshed_out_return_type = ExpressionChecker::fleshOutType(
|
||||
$project_checker,
|
||||
$storage->return_type,
|
||||
$class_storage ? $class_storage->name : null
|
||||
);
|
||||
if ($storage->return_type && $storage->return_type_location && !$storage->has_template_return_type) {
|
||||
if (!$storage->signature_return_type || $storage->signature_return_type === $storage->return_type) {
|
||||
$fleshed_out_return_type = ExpressionChecker::fleshOutType(
|
||||
$project_checker,
|
||||
$storage->return_type,
|
||||
$context->self
|
||||
);
|
||||
|
||||
$fleshed_out_signature_type = ExpressionChecker::fleshOutType(
|
||||
$project_checker,
|
||||
$storage->signature_return_type,
|
||||
$class_storage ? $class_storage->name : null
|
||||
);
|
||||
|
||||
if (!TypeChecker::isContainedBy(
|
||||
$project_checker,
|
||||
$fleshed_out_return_type,
|
||||
$fleshed_out_signature_type
|
||||
)
|
||||
) {
|
||||
if ($project_checker->alter_code
|
||||
&& isset($project_checker->getIssuesToFix()['MismatchingDocblockReturnType'])
|
||||
) {
|
||||
$this->addOrUpdateReturnType($project_checker, $storage->signature_return_type);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
if (IssueBuffer::accepts(
|
||||
new MismatchingDocblockReturnType(
|
||||
'Docblock has incorrect return type \'' . $storage->return_type .
|
||||
'\', should be \'' . $storage->signature_return_type . '\'',
|
||||
$storage->return_type_location
|
||||
),
|
||||
$storage->suppressed_issues
|
||||
)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$storage->signature_return_type->check(
|
||||
$fleshed_out_return_type->check(
|
||||
$this,
|
||||
$storage->return_type_location,
|
||||
$storage->suppressed_issues,
|
||||
[],
|
||||
false
|
||||
);
|
||||
} elseif (!$this->function instanceof Closure) {
|
||||
$fleshed_out_return_type = ExpressionChecker::fleshOutType(
|
||||
$project_checker,
|
||||
$storage->return_type,
|
||||
$context->self,
|
||||
$cased_method_id
|
||||
);
|
||||
|
||||
$fleshed_out_signature_type = ExpressionChecker::fleshOutType(
|
||||
$project_checker,
|
||||
$storage->signature_return_type,
|
||||
$context->self,
|
||||
$cased_method_id
|
||||
);
|
||||
|
||||
if (!TypeChecker::isContainedBy(
|
||||
$project_checker,
|
||||
$fleshed_out_return_type,
|
||||
$fleshed_out_signature_type
|
||||
)
|
||||
) {
|
||||
if ($project_checker->alter_code
|
||||
&& isset($project_checker->getIssuesToFix()['MismatchingDocblockReturnType'])
|
||||
) {
|
||||
$this->addOrUpdateReturnType($project_checker, $storage->signature_return_type);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
if (IssueBuffer::accepts(
|
||||
new MismatchingDocblockReturnType(
|
||||
'Docblock has incorrect return type \'' . $storage->return_type .
|
||||
'\', should be \'' . $storage->signature_return_type . '\'',
|
||||
$storage->return_type_location
|
||||
),
|
||||
$storage->suppressed_issues
|
||||
)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$storage->signature_return_type->check(
|
||||
$this,
|
||||
$storage->return_type_location,
|
||||
$storage->suppressed_issues,
|
||||
[],
|
||||
false
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1313,16 +1346,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
|
||||
return null;
|
||||
}
|
||||
|
||||
if (!$declared_return_type->isMixed()) {
|
||||
$declared_return_type->check(
|
||||
$this,
|
||||
$return_type_location,
|
||||
$this->getSuppressedIssues(),
|
||||
[],
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
if (!$declared_return_type->isMixed()) {
|
||||
if ($inferred_return_type->isVoid() && $declared_return_type->isVoid()) {
|
||||
return null;
|
||||
|
@ -30,10 +30,11 @@ abstract class Type
|
||||
* Parses a string type representation
|
||||
*
|
||||
* @param string $type_string
|
||||
* @param bool $php_compatible
|
||||
*
|
||||
* @return Union
|
||||
*/
|
||||
public static function parseString($type_string)
|
||||
public static function parseString($type_string, $php_compatible = false)
|
||||
{
|
||||
// remove all unacceptable characters
|
||||
$type_string = preg_replace('/[^A-Za-z0-9_\\\\|\? \<\>\{\}:,\]\[\(\)\$]/', '', trim($type_string));
|
||||
@ -51,14 +52,14 @@ abstract class Type
|
||||
$type_tokens = self::tokenize($type_string);
|
||||
|
||||
if (count($type_tokens) === 1) {
|
||||
$type_tokens[0] = self::fixScalarTerms($type_tokens[0]);
|
||||
$type_tokens[0] = self::fixScalarTerms($type_tokens[0], $php_compatible);
|
||||
|
||||
return new Union([Atomic::create($type_tokens[0])]);
|
||||
return new Union([Atomic::create($type_tokens[0], $php_compatible)]);
|
||||
}
|
||||
|
||||
try {
|
||||
$parse_tree = ParseTree::createFromTokens($type_tokens);
|
||||
$parsed_type = self::getTypeFromTree($parse_tree);
|
||||
$parsed_type = self::getTypeFromTree($parse_tree, $php_compatible);
|
||||
} catch (TypeParseTreeException $e) {
|
||||
throw $e;
|
||||
} catch (\Exception $e) {
|
||||
@ -74,50 +75,55 @@ abstract class Type
|
||||
|
||||
/**
|
||||
* @param string $type_string
|
||||
* @param bool $php_compatible
|
||||
*
|
||||
* @return string
|
||||
*/
|
||||
public static function fixScalarTerms($type_string)
|
||||
public static function fixScalarTerms($type_string, $php_compatible = false)
|
||||
{
|
||||
if (in_array(
|
||||
strtolower($type_string),
|
||||
[
|
||||
'numeric',
|
||||
'int',
|
||||
'void',
|
||||
'float',
|
||||
'string',
|
||||
'bool',
|
||||
'true',
|
||||
'false',
|
||||
'null',
|
||||
'array',
|
||||
'object',
|
||||
'mixed',
|
||||
'resource',
|
||||
'callable',
|
||||
'iterable',
|
||||
],
|
||||
true
|
||||
)) {
|
||||
return strtolower($type_string);
|
||||
} elseif ($type_string === 'boolean') {
|
||||
return 'bool';
|
||||
} elseif ($type_string === 'integer') {
|
||||
return 'int';
|
||||
} elseif ($type_string === 'double' || $type_string === 'real') {
|
||||
return 'float';
|
||||
$type_string_lc = strtolower($type_string);
|
||||
|
||||
switch ($type_string_lc) {
|
||||
case 'int':
|
||||
case 'void':
|
||||
case 'float':
|
||||
case 'string':
|
||||
case 'bool':
|
||||
case 'callable':
|
||||
case 'iterable':
|
||||
case 'array':
|
||||
case 'object':
|
||||
case 'numeric':
|
||||
case 'true':
|
||||
case 'false':
|
||||
case 'null':
|
||||
case 'mixed':
|
||||
case 'resource':
|
||||
return $type_string_lc;
|
||||
}
|
||||
|
||||
switch ($type_string) {
|
||||
case 'boolean':
|
||||
return $php_compatible ? $type_string : 'bool';
|
||||
|
||||
case 'integer':
|
||||
return $php_compatible ? $type_string : 'int';
|
||||
|
||||
case 'double':
|
||||
case 'real':
|
||||
return $php_compatible ? $type_string : 'float';
|
||||
}
|
||||
|
||||
return $type_string;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param ParseTree $parse_tree
|
||||
* @param ParseTree $parse_tree
|
||||
* @param bool $php_compatible
|
||||
*
|
||||
* @return Atomic|TArray|TGenericObject|ObjectLike|Union
|
||||
*/
|
||||
private static function getTypeFromTree(ParseTree $parse_tree)
|
||||
private static function getTypeFromTree(ParseTree $parse_tree, $php_compatible)
|
||||
{
|
||||
if (!$parse_tree->value) {
|
||||
throw new \InvalidArgumentException('Parse tree must have a value');
|
||||
@ -131,7 +137,7 @@ abstract class Type
|
||||
* @return Union
|
||||
*/
|
||||
function (ParseTree $child_tree) {
|
||||
$tree_type = self::getTypeFromTree($child_tree);
|
||||
$tree_type = self::getTypeFromTree($child_tree, false);
|
||||
|
||||
return $tree_type instanceof Union ? $tree_type : new Union([$tree_type]);
|
||||
},
|
||||
@ -142,7 +148,7 @@ abstract class Type
|
||||
throw new \InvalidArgumentException('Generic type must have a value');
|
||||
}
|
||||
|
||||
$generic_type_value = self::fixScalarTerms($generic_type->value);
|
||||
$generic_type_value = self::fixScalarTerms($generic_type->value, false);
|
||||
|
||||
if (($generic_type_value === 'array' || $generic_type_value === 'Generator') &&
|
||||
count($generic_params) === 1
|
||||
@ -167,7 +173,7 @@ abstract class Type
|
||||
* @return Atomic
|
||||
*/
|
||||
function (ParseTree $child_tree) {
|
||||
$atomic_type = self::getTypeFromTree($child_tree);
|
||||
$atomic_type = self::getTypeFromTree($child_tree, false);
|
||||
|
||||
if (!$atomic_type instanceof Atomic) {
|
||||
throw new \UnexpectedValueException(
|
||||
@ -190,10 +196,10 @@ abstract class Type
|
||||
|
||||
foreach ($parse_tree->children as $i => $property_branch) {
|
||||
if ($property_branch->value !== ParseTree::OBJECT_PROPERTY) {
|
||||
$property_type = self::getTypeFromTree($property_branch);
|
||||
$property_type = self::getTypeFromTree($property_branch, false);
|
||||
$property_key = (string)$i;
|
||||
} elseif (count($property_branch->children) === 2) {
|
||||
$property_type = self::getTypeFromTree($property_branch->children[1]);
|
||||
$property_type = self::getTypeFromTree($property_branch->children[1], false);
|
||||
$property_key = (string)($property_branch->children[0]->value);
|
||||
} else {
|
||||
throw new \InvalidArgumentException('Unexpected number of property parts');
|
||||
@ -212,9 +218,9 @@ abstract class Type
|
||||
return new ObjectLike($properties);
|
||||
}
|
||||
|
||||
$atomic_type = self::fixScalarTerms($parse_tree->value);
|
||||
$atomic_type = self::fixScalarTerms($parse_tree->value, $php_compatible);
|
||||
|
||||
return Atomic::create($atomic_type);
|
||||
return Atomic::create($atomic_type, $php_compatible);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -49,15 +49,13 @@ abstract class Atomic
|
||||
|
||||
/**
|
||||
* @param string $value
|
||||
* @param bool $php_compatible
|
||||
*
|
||||
* @return Atomic
|
||||
*/
|
||||
public static function create($value)
|
||||
public static function create($value, $php_compatible = false)
|
||||
{
|
||||
switch ($value) {
|
||||
case 'numeric':
|
||||
return new TNumeric();
|
||||
|
||||
case 'int':
|
||||
return new TInt();
|
||||
|
||||
@ -73,35 +71,38 @@ abstract class Atomic
|
||||
case 'bool':
|
||||
return new TBool();
|
||||
|
||||
case 'true':
|
||||
return new TTrue();
|
||||
case 'object':
|
||||
return new TObject();
|
||||
|
||||
case 'false':
|
||||
return new TFalse();
|
||||
|
||||
case 'empty':
|
||||
return new TEmpty();
|
||||
|
||||
case 'scalar':
|
||||
return new TScalar();
|
||||
|
||||
case 'null':
|
||||
return new TNull();
|
||||
case 'callable':
|
||||
return new TCallable();
|
||||
|
||||
case 'array':
|
||||
return new TArray([new Union([new TMixed]), new Union([new TMixed])]);
|
||||
|
||||
case 'object':
|
||||
return new TObject();
|
||||
case 'resource':
|
||||
return $php_compatible ? new TNamedObject($value) : new TResource();
|
||||
|
||||
case 'numeric':
|
||||
return $php_compatible ? new TNamedObject($value) : new TNumeric();
|
||||
|
||||
case 'true':
|
||||
return $php_compatible ? new TNamedObject($value) : new TTrue();
|
||||
|
||||
case 'false':
|
||||
return $php_compatible ? new TNamedObject($value) : new TFalse();
|
||||
|
||||
case 'empty':
|
||||
return $php_compatible ? new TNamedObject($value) : new TEmpty();
|
||||
|
||||
case 'scalar':
|
||||
return $php_compatible ? new TNamedObject($value) : new TScalar();
|
||||
|
||||
case 'null':
|
||||
return $php_compatible ? new TNamedObject($value) : new TNull();
|
||||
|
||||
case 'mixed':
|
||||
return new TMixed();
|
||||
|
||||
case 'resource':
|
||||
return new TResource();
|
||||
|
||||
case 'callable':
|
||||
return new TCallable();
|
||||
return $php_compatible ? new TNamedObject($value) : new TMixed();
|
||||
|
||||
case 'numeric-string':
|
||||
return new TNumericString();
|
||||
|
@ -738,7 +738,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
|
||||
$return_type_string = $return_type_fq_classlike_name . $suffix;
|
||||
}
|
||||
|
||||
$storage->return_type = Type::parseString($return_type_string);
|
||||
$storage->return_type = Type::parseString($return_type_string, true);
|
||||
$storage->return_type_location = new CodeLocation(
|
||||
$this->file_checker,
|
||||
$stmt,
|
||||
@ -964,7 +964,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
|
||||
$param_type_string .= '|null';
|
||||
}
|
||||
|
||||
$param_type = Type::parseString($param_type_string);
|
||||
$param_type = Type::parseString($param_type_string, true);
|
||||
|
||||
if ($param->variadic) {
|
||||
$param_type = new Type\Union([
|
||||
|
@ -722,6 +722,13 @@ class AnnotationTest extends TestCase
|
||||
}',
|
||||
'error_message' => 'UndefinedClass',
|
||||
],
|
||||
'preventBadBoolean' => [
|
||||
'<?php
|
||||
function foo() : boolean {
|
||||
return true;
|
||||
}',
|
||||
'error_message' => 'UndefinedClass',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
@ -307,7 +307,7 @@ class InterfaceTest extends TestCase
|
||||
'<?php
|
||||
interface I {
|
||||
/**
|
||||
* @return Boolean
|
||||
* @return int
|
||||
*/
|
||||
public function check();
|
||||
}
|
||||
|
@ -579,6 +579,16 @@ class ReturnTypeTest extends TestCase
|
||||
}',
|
||||
'error_message' => 'ReservedWord',
|
||||
],
|
||||
'voidParamType' => [
|
||||
'<?php
|
||||
function f(void $p): void {}',
|
||||
'error_message' => 'ReservedWord',
|
||||
],
|
||||
'voidClass' => [
|
||||
'<?php
|
||||
class void {}',
|
||||
'error_message' => 'ReservedWord',
|
||||
],
|
||||
'disallowReturningExplicitVoid' => [
|
||||
'<?php
|
||||
function returnsVoid() : void {}
|
||||
|
Loading…
Reference in New Issue
Block a user