From 357ad1aa827d3b9197cf3d94efaed9b4839f5d57 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Tue, 6 Mar 2018 11:20:54 -0500 Subject: [PATCH] Add config flags to allow stricter class invocation checks --- config.xsd | 2 + .../Statements/Expression/Call/NewChecker.php | 30 ++++++++ .../Expression/Call/StaticCallChecker.php | 47 +++++++----- src/Psalm/Checker/TypeChecker.php | 5 +- src/Psalm/Config.php | 20 +++++ src/Psalm/Type/Union.php | 2 +- tests/AnnotationTest.php | 74 +++++++++++++++++++ tests/MethodCallTest.php | 12 +++ 8 files changed, 172 insertions(+), 20 deletions(-) diff --git a/config.xsd b/config.xsd index 74a41b589..65fa59df7 100644 --- a/config.xsd +++ b/config.xsd @@ -29,6 +29,8 @@ + + diff --git a/src/Psalm/Checker/Statements/Expression/Call/NewChecker.php b/src/Psalm/Checker/Statements/Expression/Call/NewChecker.php index 2fbd33ca0..1313f8318 100644 --- a/src/Psalm/Checker/Statements/Expression/Call/NewChecker.php +++ b/src/Psalm/Checker/Statements/Expression/Call/NewChecker.php @@ -12,6 +12,7 @@ use Psalm\Context; use Psalm\Issue\AbstractInstantiation; use Psalm\Issue\DeprecatedClass; use Psalm\Issue\InterfaceInstantiation; +use Psalm\Issue\InvalidClass; use Psalm\Issue\TooManyArguments; use Psalm\IssueBuffer; use Psalm\Type; @@ -111,6 +112,35 @@ class NewChecker extends \Psalm\Checker\Statements\Expression\CallChecker return false; } + if (isset($stmt->class->inferredType)) { + foreach ($stmt->class->inferredType->getTypes() as $lhs_type_part) { + // this is always OK + if ($lhs_type_part instanceof Type\Atomic\TClassString) { + continue; + } + + if ($lhs_type_part instanceof Type\Atomic\TString) { + if ($config->allow_string_standin_for_class + && !$lhs_type_part instanceof Type\Atomic\TNumericString + ) { + continue; + } + } elseif ($lhs_type_part instanceof Type\Atomic\TMixed) { + continue; + } + + if (IssueBuffer::accepts( + new InvalidClass( + 'Type ' . $lhs_type_part . ' cannot be called as a class', + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + // fall through + } + } + } + $stmt->inferredType = Type::getObject(); return null; diff --git a/src/Psalm/Checker/Statements/Expression/Call/StaticCallChecker.php b/src/Psalm/Checker/Statements/Expression/Call/StaticCallChecker.php index d77c9ff29..2a21cfa95 100644 --- a/src/Psalm/Checker/Statements/Expression/Call/StaticCallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/Call/StaticCallChecker.php @@ -11,6 +11,7 @@ use Psalm\Config; use Psalm\Context; use Psalm\FileManipulation\FileManipulationBuffer; use Psalm\Issue\DeprecatedClass; +use Psalm\Issue\InvalidClass; use Psalm\Issue\ParentNotFound; use Psalm\IssueBuffer; use Psalm\Type; @@ -41,6 +42,8 @@ class StaticCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker $stmt->inferredType = null; + $config = $project_checker->config; + if ($stmt->class instanceof PhpParser\Node\Name) { $fq_class_name = null; @@ -179,22 +182,8 @@ class StaticCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker } else { ExpressionChecker::analyze($statements_checker, $stmt->class, $context); - /** @var Type\Union */ + /** @var Type\Union|null */ $lhs_type = $stmt->class->inferredType; - - if (!isset($lhs_type) || $lhs_type->hasString()) { - if (self::checkFunctionArguments( - $statements_checker, - $stmt->args, - null, - null, - $context - ) === false) { - return false; - } - - return null; - } } if (!$context->check_methods || !$lhs_type) { @@ -203,11 +192,33 @@ class StaticCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker $has_mock = false; - $config = Config::getInstance(); - foreach ($lhs_type->getTypes() as $lhs_type_part) { if (!$lhs_type_part instanceof TNamedObject) { - // @todo deal with it + // this is always OK + if ($lhs_type_part instanceof Type\Atomic\TClassString) { + continue; + } + + if ($lhs_type_part instanceof Type\Atomic\TString) { + if ($config->allow_string_standin_for_class + && !$lhs_type_part instanceof Type\Atomic\TNumericString + ) { + continue; + } + } elseif ($lhs_type_part instanceof Type\Atomic\TMixed) { + continue; + } + + if (IssueBuffer::accepts( + new InvalidClass( + 'Type ' . $lhs_type_part . ' cannot be called as a class', + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + // fall through + } + continue; } diff --git a/src/Psalm/Checker/TypeChecker.php b/src/Psalm/Checker/TypeChecker.php index 829f1be63..ba56481c3 100644 --- a/src/Psalm/Checker/TypeChecker.php +++ b/src/Psalm/Checker/TypeChecker.php @@ -493,7 +493,10 @@ class TypeChecker } if ($container_type_part instanceof TClassString && $input_type_part instanceof TString) { - $type_coerced = true; + if (\Psalm\Config::getInstance()->allow_coercion_from_string_to_class_const) { + $type_coerced = true; + } + return false; } diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 03e82afe1..3d2999cd4 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -188,6 +188,16 @@ class Config */ public $allow_phpstorm_generics = false; + /** + * @var bool + */ + public $allow_coercion_from_string_to_class_const = true; + + /** + * @var bool + */ + public $allow_string_standin_for_class = true; + /** * @var string[] */ @@ -441,6 +451,16 @@ class Config $config->allow_phpstorm_generics = $attribute_text === 'true' || $attribute_text === '1'; } + if (isset($config_xml['allowCoercionFromStringToClassConst'])) { + $attribute_text = (string) $config_xml['allowCoercionFromStringToClassConst']; + $config->allow_coercion_from_string_to_class_const = $attribute_text === 'true' || $attribute_text === '1'; + } + + if (isset($config_xml['allowStringToStandInForClass'])) { + $attribute_text = (string) $config_xml['allowCoercionFromStringToClassConst']; + $config->allow_string_standin_for_class = $attribute_text === 'true' || $attribute_text === '1'; + } + if (isset($config_xml->projectFiles)) { $config->project_files = ProjectFileFilter::loadFromXMLElement($config_xml->projectFiles, $base_dir, true); } diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 41622e3fb..b069fc934 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -357,7 +357,7 @@ class Union */ public function hasString() { - return isset($this->types['string']); + return isset($this->types['string']) || isset($this->types['class-string']); } /** diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php index 23aac3efe..1959e66e4 100644 --- a/tests/AnnotationTest.php +++ b/tests/AnnotationTest.php @@ -105,6 +105,80 @@ class AnnotationTest extends TestCase $this->analyzeFile('somefile.php', new Context()); } + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InvalidArgument + * + * @return void + */ + public function testDontAllowStringConstCoercion() + { + Config::getInstance()->allow_coercion_from_string_to_class_const = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InvalidClass + * + * @return void + */ + public function testDontAllowStringStandInForNewClass() + { + Config::getInstance()->allow_string_standin_for_class = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InvalidClass + * + * @return void + */ + public function testDontAllowStringStandInForStaticMethodCall() + { + Config::getInstance()->allow_string_standin_for_class = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + /** * @return array */ diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index 3280d66bd..56c724434 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -267,6 +267,18 @@ class MethodCallTest extends TestCase $arr->$b();', 'error_message' => 'InvalidMethodCall', ], + 'intVarStaticCall' => [ + ' 'InvalidClass', + ], + 'intVarNewCall' => [ + ' 'InvalidClass', + ], ]; } }