1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00

Fix #806 - allow specification of functions and static methods that exit

This commit is contained in:
Matthew Brown 2018-07-12 23:26:08 -04:00
parent 5ea8b86b7c
commit d41a9a8dcc
10 changed files with 180 additions and 24 deletions

View File

@ -14,6 +14,7 @@
<xs:element name="mockClasses" type="MockClassesType" minOccurs="0" maxOccurs="1" /> <xs:element name="mockClasses" type="MockClassesType" minOccurs="0" maxOccurs="1" />
<xs:element name="stubs" type="StubsType" minOccurs="0" maxOccurs="1" /> <xs:element name="stubs" type="StubsType" minOccurs="0" maxOccurs="1" />
<xs:element name="plugins" type="PluginsType" minOccurs="0" maxOccurs="1" /> <xs:element name="plugins" type="PluginsType" minOccurs="0" maxOccurs="1" />
<xs:element name="exitFunctions" type="ExitFunctionsType" minOccurs="0" maxOccurs="1" />
<xs:element name="issueHandlers" type="IssueHandlersType" minOccurs="0" maxOccurs="1" /> <xs:element name="issueHandlers" type="IssueHandlersType" minOccurs="0" maxOccurs="1" />
<xs:element name="ignoreExceptions" type="ExceptionsType" minOccurs="0" maxOccurs="1" /> <xs:element name="ignoreExceptions" type="ExceptionsType" minOccurs="0" maxOccurs="1" />
</xs:choice> </xs:choice>
@ -92,6 +93,12 @@
</xs:sequence> </xs:sequence>
</xs:complexType> </xs:complexType>
<xs:complexType name="ExitFunctionsType">
<xs:sequence>
<xs:element name="function" maxOccurs="unbounded" type="NameAttributeType" />
</xs:sequence>
</xs:complexType>
<xs:complexType name="PluginsType"> <xs:complexType name="PluginsType">
<xs:sequence> <xs:sequence>
<xs:element name="plugin" maxOccurs="unbounded"> <xs:element name="plugin" maxOccurs="unbounded">

View File

@ -100,7 +100,10 @@ class ReturnTypeChecker
); );
if ((!$return_type || $return_type->from_docblock) if ((!$return_type || $return_type->from_docblock)
&& ScopeChecker::getFinalControlActions($function_stmts) !== [ScopeChecker::ACTION_END] && ScopeChecker::getFinalControlActions(
$function_stmts,
$project_checker->config->exit_functions
) !== [ScopeChecker::ACTION_END]
&& !$inferred_yield_types && !$inferred_yield_types
&& count($inferred_return_type_parts) && count($inferred_return_type_parts)
) { ) {
@ -119,7 +122,10 @@ class ReturnTypeChecker
&& !$return_type->from_docblock && !$return_type->from_docblock
&& !$return_type->isVoid() && !$return_type->isVoid()
&& !$inferred_yield_types && !$inferred_yield_types
&& ScopeChecker::getFinalControlActions($function_stmts) !== [ScopeChecker::ACTION_END] && ScopeChecker::getFinalControlActions(
$function_stmts,
$project_checker->config->exit_functions
) !== [ScopeChecker::ACTION_END]
) { ) {
if (IssueBuffer::accepts( if (IssueBuffer::accepts(
new InvalidReturnType( new InvalidReturnType(

View File

@ -59,6 +59,7 @@ class ScopeChecker
*/ */
public static function getFinalControlActions( public static function getFinalControlActions(
array $stmts, array $stmts,
array $exit_functions,
$in_switch = false, $in_switch = false,
$return_is_exit = true $return_is_exit = true
) { ) {
@ -82,6 +83,36 @@ class ScopeChecker
return [self::ACTION_END]; return [self::ACTION_END];
} }
if ($stmt instanceof PhpParser\Node\Stmt\Expression) {
if ($exit_functions) {
if ($stmt->expr instanceof PhpParser\Node\Expr\FuncCall
|| $stmt->expr instanceof PhpParser\Node\Expr\StaticCall
) {
if ($stmt->expr instanceof PhpParser\Node\Expr\FuncCall) {
/** @var string|null */
$resolved_name = $stmt->expr->name->getAttribute('resolvedName');
if ($resolved_name && isset($exit_functions[strtolower($resolved_name)])) {
return [self::ACTION_END];
}
} elseif ($stmt->expr->class instanceof PhpParser\Node\Name
&& $stmt->expr->name instanceof PhpParser\Node\Identifier
) {
/** @var string|null */
$resolved_class_name = $stmt->expr->class->getAttribute('resolvedName');
if ($resolved_class_name
&& isset($exit_functions[strtolower($resolved_class_name . '::' . $stmt->expr->name)])
) {
return [self::ACTION_END];
}
}
}
}
continue;
}
if ($stmt instanceof PhpParser\Node\Stmt\Continue_) { if ($stmt instanceof PhpParser\Node\Stmt\Continue_) {
if ($in_switch if ($in_switch
&& (!$stmt->num || !$stmt->num instanceof PhpParser\Node\Scalar\LNumber || $stmt->num->value < 2) && (!$stmt->num || !$stmt->num instanceof PhpParser\Node\Scalar\LNumber || $stmt->num->value < 2)
@ -103,9 +134,9 @@ class ScopeChecker
} }
if ($stmt instanceof PhpParser\Node\Stmt\If_) { if ($stmt instanceof PhpParser\Node\Stmt\If_) {
$if_statement_actions = self::getFinalControlActions($stmt->stmts, $in_switch); $if_statement_actions = self::getFinalControlActions($stmt->stmts, $exit_functions, $in_switch);
$else_statement_actions = $stmt->else $else_statement_actions = $stmt->else
? self::getFinalControlActions($stmt->else->stmts, $in_switch) ? self::getFinalControlActions($stmt->else->stmts, $exit_functions, $in_switch)
: []; : [];
$all_same = count($if_statement_actions) === 1 $all_same = count($if_statement_actions) === 1
@ -116,7 +147,11 @@ class ScopeChecker
if ($stmt->elseifs) { if ($stmt->elseifs) {
foreach ($stmt->elseifs as $elseif) { foreach ($stmt->elseifs as $elseif) {
$elseif_control_actions = self::getFinalControlActions($elseif->stmts, $in_switch); $elseif_control_actions = self::getFinalControlActions(
$elseif->stmts,
$exit_functions,
$in_switch
);
$all_same = $all_same && $elseif_control_actions == $if_statement_actions; $all_same = $all_same && $elseif_control_actions == $if_statement_actions;
@ -147,7 +182,7 @@ class ScopeChecker
for ($d = count($stmt->cases) - 1; $d >= 0; --$d) { for ($d = count($stmt->cases) - 1; $d >= 0; --$d) {
$case = $stmt->cases[$d]; $case = $stmt->cases[$d];
$case_actions = self::getFinalControlActions($case->stmts, true); $case_actions = self::getFinalControlActions($case->stmts, $exit_functions, true);
if (array_intersect([ if (array_intersect([
self::ACTION_LEAVE_SWITCH, self::ACTION_LEAVE_SWITCH,
@ -183,11 +218,14 @@ class ScopeChecker
} }
if ($stmt instanceof PhpParser\Node\Stmt\While_) { if ($stmt instanceof PhpParser\Node\Stmt\While_) {
$control_actions = array_merge(self::getFinalControlActions($stmt->stmts), $control_actions); $control_actions = array_merge(
self::getFinalControlActions($stmt->stmts, $exit_functions),
$control_actions
);
} }
if ($stmt instanceof PhpParser\Node\Stmt\Do_) { if ($stmt instanceof PhpParser\Node\Stmt\Do_) {
$do_actions = self::getFinalControlActions($stmt->stmts); $do_actions = self::getFinalControlActions($stmt->stmts, $exit_functions);
if (count($do_actions) && !in_array(self::ACTION_NONE, $do_actions, true)) { if (count($do_actions) && !in_array(self::ACTION_NONE, $do_actions, true)) {
return $do_actions; return $do_actions;
@ -197,13 +235,13 @@ class ScopeChecker
} }
if ($stmt instanceof PhpParser\Node\Stmt\TryCatch) { if ($stmt instanceof PhpParser\Node\Stmt\TryCatch) {
$try_statement_actions = self::getFinalControlActions($stmt->stmts, $in_switch); $try_statement_actions = self::getFinalControlActions($stmt->stmts, $exit_functions, $in_switch);
if ($stmt->catches) { if ($stmt->catches) {
$all_same = count($try_statement_actions) === 1; $all_same = count($try_statement_actions) === 1;
foreach ($stmt->catches as $catch) { foreach ($stmt->catches as $catch) {
$catch_actions = self::getFinalControlActions($catch->stmts, $in_switch); $catch_actions = self::getFinalControlActions($catch->stmts, $exit_functions, $in_switch);
$all_same = $all_same && $try_statement_actions == $catch_actions; $all_same = $all_same && $try_statement_actions == $catch_actions;
@ -221,6 +259,7 @@ class ScopeChecker
if ($stmt->finally->stmts) { if ($stmt->finally->stmts) {
$finally_statement_actions = self::getFinalControlActions( $finally_statement_actions = self::getFinalControlActions(
$stmt->finally->stmts, $stmt->finally->stmts,
$exit_functions,
$in_switch $in_switch
); );

View File

@ -536,7 +536,13 @@ class IfChecker
Context $outer_context, Context $outer_context,
array $pre_assignment_else_redefined_vars array $pre_assignment_else_redefined_vars
) { ) {
$final_actions = ScopeChecker::getFinalControlActions($stmt->stmts, $outer_context->inside_case); $project_checker = $statements_checker->getFileChecker()->project_checker;
$final_actions = ScopeChecker::getFinalControlActions(
$stmt->stmts,
$project_checker->config->exit_functions,
$outer_context->inside_case
);
$has_ending_statements = $final_actions === [ScopeChecker::ACTION_END]; $has_ending_statements = $final_actions === [ScopeChecker::ACTION_END];
@ -553,8 +559,6 @@ class IfChecker
$if_scope->final_actions = $final_actions; $if_scope->final_actions = $final_actions;
$project_checker = $statements_checker->getFileChecker()->project_checker;
$assigned_var_ids = $if_context->assigned_var_ids; $assigned_var_ids = $if_context->assigned_var_ids;
$possibly_assigned_var_ids = $if_context->possibly_assigned_var_ids; $possibly_assigned_var_ids = $if_context->possibly_assigned_var_ids;
$if_context->assigned_var_ids = []; $if_context->assigned_var_ids = [];
@ -1051,7 +1055,11 @@ class IfChecker
} }
} }
$final_actions = ScopeChecker::getFinalControlActions($elseif->stmts, $outer_context->inside_case); $final_actions = ScopeChecker::getFinalControlActions(
$elseif->stmts,
$project_checker->config->exit_functions,
$outer_context->inside_case
);
// has a return/throw at end // has a return/throw at end
$has_ending_statements = $final_actions === [ScopeChecker::ACTION_END]; $has_ending_statements = $final_actions === [ScopeChecker::ACTION_END];
$has_leaving_statements = $has_ending_statements $has_leaving_statements = $has_ending_statements
@ -1419,7 +1427,11 @@ class IfChecker
} }
$final_actions = $else $final_actions = $else
? ScopeChecker::getFinalControlActions($else->stmts, $outer_context->inside_case) ? ScopeChecker::getFinalControlActions(
$else->stmts,
$project_checker->config->exit_functions,
$outer_context->inside_case
)
: [ScopeChecker::ACTION_NONE]; : [ScopeChecker::ACTION_NONE];
// has a return/throw at end // has a return/throw at end
$has_ending_statements = $final_actions === [ScopeChecker::ACTION_END]; $has_ending_statements = $final_actions === [ScopeChecker::ACTION_END];

View File

@ -7,6 +7,7 @@ use Psalm\Checker\Statements\ExpressionChecker;
use Psalm\Checker\StatementsChecker; use Psalm\Checker\StatementsChecker;
use Psalm\Clause; use Psalm\Clause;
use Psalm\CodeLocation; use Psalm\CodeLocation;
use Psalm\Config;
use Psalm\Context; use Psalm\Context;
use Psalm\IssueBuffer; use Psalm\IssueBuffer;
use Psalm\Scope\LoopScope; use Psalm\Scope\LoopScope;
@ -72,7 +73,7 @@ class LoopChecker
); );
} }
$final_actions = ScopeChecker::getFinalControlActions($stmts); $final_actions = ScopeChecker::getFinalControlActions($stmts, Config::getInstance()->exit_functions);
$has_break_statement = $final_actions === [ScopeChecker::ACTION_BREAK]; $has_break_statement = $final_actions === [ScopeChecker::ACTION_BREAK];
if ($assignment_map) { if ($assignment_map) {

View File

@ -58,11 +58,17 @@ class SwitchChecker
$case_action_map = []; $case_action_map = [];
$config = \Psalm\Config::getInstance();
// create a map of case statement -> ultimate exit type // create a map of case statement -> ultimate exit type
for ($i = count($stmt->cases) - 1; $i >= 0; --$i) { for ($i = count($stmt->cases) - 1; $i >= 0; --$i) {
$case = $stmt->cases[$i]; $case = $stmt->cases[$i];
$case_actions = $case_action_map[$i] = ScopeChecker::getFinalControlActions($case->stmts, true); $case_actions = $case_action_map[$i] = ScopeChecker::getFinalControlActions(
$case->stmts,
$config->exit_functions,
true
);
if (!in_array(ScopeChecker::ACTION_NONE, $case_actions, true)) { if (!in_array(ScopeChecker::ACTION_NONE, $case_actions, true)) {
if ($case_actions === [ScopeChecker::ACTION_END]) { if ($case_actions === [ScopeChecker::ACTION_END]) {

View File

@ -31,15 +31,15 @@ class TryChecker
$catch_actions = []; $catch_actions = [];
$all_catches_leave = true; $all_catches_leave = true;
/** @var int $i */
foreach ($stmt->catches as $i => $catch) {
$catch_actions[$i] = ScopeChecker::getFinalControlActions($catch->stmts);
$all_catches_leave = $all_catches_leave && !in_array(ScopeChecker::ACTION_NONE, $catch_actions[$i], true);
}
$project_checker = $statements_checker->getFileChecker()->project_checker; $project_checker = $statements_checker->getFileChecker()->project_checker;
$codebase = $project_checker->codebase; $codebase = $project_checker->codebase;
/** @var int $i */
foreach ($stmt->catches as $i => $catch) {
$catch_actions[$i] = ScopeChecker::getFinalControlActions($catch->stmts, $codebase->config->exit_functions);
$all_catches_leave = $all_catches_leave && !in_array(ScopeChecker::ACTION_NONE, $catch_actions[$i], true);
}
$existing_thrown_exceptions = $context->possibly_thrown_exceptions; $existing_thrown_exceptions = $context->possibly_thrown_exceptions;
/** /**

View File

@ -277,6 +277,13 @@ class Config
/** @var ClassLoader|null */ /** @var ClassLoader|null */
private $composer_class_loader; private $composer_class_loader;
/**
* Custom functions that always exit
*
* @var array<string, bool>
*/
public $exit_functions = [];
protected function __construct() protected function __construct()
{ {
self::$instance = $this; self::$instance = $this;
@ -564,7 +571,14 @@ class Config
if (isset($config_xml->ignoreExceptions) && isset($config_xml->ignoreExceptions->class)) { if (isset($config_xml->ignoreExceptions) && isset($config_xml->ignoreExceptions->class)) {
/** @var \SimpleXMLElement $exception_class */ /** @var \SimpleXMLElement $exception_class */
foreach ($config_xml->ignoreExceptions->class as $exception_class) { foreach ($config_xml->ignoreExceptions->class as $exception_class) {
$config->ignored_exceptions[(string)$exception_class ['name']] = true; $config->ignored_exceptions[(string) $exception_class['name']] = true;
}
}
if (isset($config_xml->exitFunctions) && isset($config_xml->exitFunctions->function)) {
/** @var \SimpleXMLElement $exit_function */
foreach ($config_xml->exitFunctions->function as $exit_function) {
$config->exit_functions[strtolower((string) $exit_function['name'])] = true;
} }
} }

View File

@ -891,6 +891,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
if ($function_stmt instanceof PhpParser\Node\Stmt\If_) { if ($function_stmt instanceof PhpParser\Node\Stmt\If_) {
$final_actions = \Psalm\Checker\ScopeChecker::getFinalControlActions( $final_actions = \Psalm\Checker\ScopeChecker::getFinalControlActions(
$function_stmt->stmts, $function_stmt->stmts,
$this->config->exit_functions,
false, false,
false false
); );

View File

@ -495,6 +495,76 @@ class ConfigTest extends TestCase
$this->analyzeFile($file_path, new Context()); $this->analyzeFile($file_path, new Context());
} }
/**
* @return void
*/
public function testExitFunctions()
{
$this->project_checker = $this->getProjectCheckerWithConfig(
TestConfig::loadFromXML(
dirname(__DIR__),
'<?xml version="1.0"?>
<psalm>
<exitFunctions>
<function name="leave" />
<function name="Foo\namespacedLeave" />
<function name="Foo\Bar::staticLeave" />
</exitFunctions>
</psalm>'
)
);
$file_path = getcwd() . '/src/somefile.php';
$this->addFile(
$file_path,
'<?php
namespace {
function leave() : void {
exit();
}
function mightLeave() : string {
if (rand(0, 1)) {
leave();
} else {
return "here";
}
}
function mightLeaveWithNamespacedFunction() : string {
if (rand(0, 1)) {
\Foo\namespacedLeave();
} else {
return "here";
}
}
function mightLeaveWithStaticMethod() : string {
if (rand(0, 1)) {
Foo\Bar::staticLeave();
} else {
return "here";
}
}
}
namespace Foo {
function namespacedLeave() : void {
exit();
}
class Bar {
public static function staticLeave() : void {
exit();
}
}
}'
);
$this->analyzeFile($file_path, new Context());
}
/** /**
* @return void * @return void
*/ */