1
0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00

Add detection for unused function calls

This commit is contained in:
Brown 2019-08-13 13:15:23 -04:00
parent 89416c6f4f
commit b5614d03f8
19 changed files with 240 additions and 6 deletions

View File

@ -340,6 +340,7 @@
<xs:element name="UnresolvableInclude" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnevaluatedCode" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedFunctionCall" type="FunctionIssueHandlerType" minOccurs="0" />
<xs:element name="UnusedVariable" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedParam" type="IssueHandlerType" minOccurs="0" />
<xs:element name="UnusedProperty" type="PropertyIssueHandlerType" minOccurs="0" />

View File

@ -2389,6 +2389,16 @@ function foo(callable $c) : int {
}
```
### UnusedFunctionCall
Emitted when `--find-dead-code` is turned on and Psalm finds a function call that is not used anywhere
```php
$a = strlen("hello");
strlen("goodbye"); // unused
echo $a;
```
### UnusedParam
Emitted when `--find-dead-code` is turned on and Psalm cannot find any uses of a particular parameter in a private method or function

View File

@ -77,6 +77,13 @@ class Context
*/
public $inside_call = false;
/**
* Whether or not we're inside an assignment
*
* @var bool
*/
public $inside_assignment = false;
/**
* @var null|CodeLocation
*/

View File

@ -171,7 +171,9 @@ class DoAnalyzer
)
);
$inner_loop_context->inside_conditional = true;
ExpressionAnalyzer::analyze($statements_analyzer, $stmt->cond, $inner_loop_context);
$inner_loop_context->inside_conditional = false;
if ($negated_while_types) {
$changed_var_ids = [];

View File

@ -146,9 +146,11 @@ class ForeachAnalyzer
}
}
$context->inside_assignment = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->expr, $context) === false) {
return false;
}
$context->inside_assignment = false;
$key_type = null;
$value_type = null;

View File

@ -44,9 +44,11 @@ class SwitchAnalyzer
) {
$codebase = $statements_analyzer->getCodebase();
$context->inside_conditional = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->cond, $context) === false) {
return false;
}
$context->inside_conditional = false;
$switch_var_id = ExpressionAnalyzer::getArrayVarId(
$stmt->cond,
@ -255,6 +257,8 @@ class SwitchAnalyzer
$case_equality_expr = null;
if ($case->cond) {
$case_context->inside_conditional = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $case->cond, $case_context) === false) {
/** @psalm-suppress PossiblyNullPropertyAssignmentValue */
$case_scope->parent_context = null;
@ -264,6 +268,8 @@ class SwitchAnalyzer
return false;
}
$case_context->inside_conditional = false;
$switch_condition = clone $stmt->cond;
if ($switch_condition instanceof PhpParser\Node\Expr\Variable

View File

@ -159,7 +159,9 @@ class WhileAnalyzer
$statements_analyzer->addSuppressedIssues(['TypeDoesNotContainType']);
}
$while_context->inside_conditional = true;
ExpressionAnalyzer::analyze($statements_analyzer, $stmt->cond, $while_context);
$while_context->inside_conditional = false;
if (!in_array('RedundantCondition', $suppressed_issues, true)) {
$statements_analyzer->removeSuppressedIssues(['RedundantCondition']);

View File

@ -67,6 +67,10 @@ class AssignmentAnalyzer
$var_comments = [];
$comment_type = null;
$was_in_assignment = $context->inside_assignment;
$context->inside_assignment = true;
$codebase = $statements_analyzer->getCodebase();
if ($doc_comment) {
@ -658,6 +662,10 @@ class AssignmentAnalyzer
$context->vars_in_scope[$var_id] = Type::getNull();
if (!$was_in_assignment) {
$context->inside_assignment = false;
}
return $context->vars_in_scope[$var_id];
}
@ -674,10 +682,18 @@ class AssignmentAnalyzer
$context->vars_in_scope[$var_id] = Type::getEmpty();
if (!$was_in_assignment) {
$context->inside_assignment = false;
}
return $context->vars_in_scope[$var_id];
}
}
if (!$was_in_assignment) {
$context->inside_assignment = false;
}
return $assign_value_type;
}
@ -693,6 +709,10 @@ class AssignmentAnalyzer
PhpParser\Node\Expr\AssignOp $stmt,
Context $context
) {
$was_in_assignment = $context->inside_assignment;
$context->inside_assignment = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->var, $context) === false) {
return false;
}
@ -701,6 +721,10 @@ class AssignmentAnalyzer
return false;
}
if (!$was_in_assignment) {
$context->inside_assignment = false;
}
$array_var_id = ExpressionAnalyzer::getArrayVarId(
$stmt->var,
$statements_analyzer->getFQCLN(),

View File

@ -18,6 +18,7 @@ use Psalm\Issue\ImpureFunctionCall;
use Psalm\Issue\NullFunctionCall;
use Psalm\Issue\PossiblyInvalidFunctionCall;
use Psalm\Issue\PossiblyNullFunctionCall;
use Psalm\Issue\UnusedFunctionCall;
use Psalm\IssueBuffer;
use Psalm\Storage\Assertion;
use Psalm\Type;
@ -607,12 +608,39 @@ class FunctionCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expressio
);
}
if ($context->pure) {
if ($function_storage && !$function_storage->pure) {
if ($context->pure || $codebase->find_unused_variables) {
$callmap_function_pure = $function_id && $in_call_map
? $codebase->functions->isCallMapFunctionPure($codebase, $function_id, $stmt->args)
: null;
if (($function_storage
&& !$function_storage->pure)
|| ($callmap_function_pure === false)
) {
if ($context->pure) {
if (IssueBuffer::accepts(
new ImpureFunctionCall(
'Cannot call an impure function from a pure context',
new CodeLocation($statements_analyzer, $stmt->name)
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
}
} elseif ($function_id
&& (($function_storage && $function_storage->pure) || $callmap_function_pure === true)
&& $codebase->find_unused_variables
&& !$context->inside_assignment
&& !$context->inside_conditional
&& !$context->inside_call
&& !$context->inside_unset
) {
if (IssueBuffer::accepts(
new ImpureFunctionCall(
'Cannot call an impure function from a pure context',
new CodeLocation($statements_analyzer, $stmt->name)
new UnusedFunctionCall(
'The call to ' . $function_id . ' is not used',
new CodeLocation($statements_analyzer, $stmt->name),
$function_id
),
$statements_analyzer->getSuppressedIssues()
)) {
@ -758,7 +786,9 @@ class FunctionCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expressio
if ($fq_const_name !== null) {
$second_arg = $stmt->args[1];
$context->inside_call = true;
ExpressionAnalyzer::analyze($statements_analyzer, $second_arg->value, $context);
$context->inside_call = false;
$statements_analyzer->setConstType(
$fq_const_name,

View File

@ -592,9 +592,16 @@ class CallAnalyzer
|| $arg->value instanceof PhpParser\Node\Expr\Array_
|| $arg->value instanceof PhpParser\Node\Expr\BinaryOp
) {
$was_inside_call = $context->inside_call;
$context->inside_call = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $arg->value, $context) === false) {
return false;
}
if (!$was_inside_call) {
$context->inside_call = false;
}
}
if ($arg->value instanceof PhpParser\Node\Expr\PropertyFetch
@ -851,6 +858,7 @@ class CallAnalyzer
array $args,
Context $context
) {
$context->inside_call = true;
$array_arg = $args[0]->value;
if (ExpressionAnalyzer::analyze(
@ -899,6 +907,8 @@ class CallAnalyzer
return false;
}
$context->inside_call = false;
if (isset($replacement_arg->inferredType)
&& !$replacement_arg->inferredType->hasArray()
&& $replacement_arg->inferredType->hasString()

View File

@ -55,10 +55,18 @@ class IncludeAnalyzer
);
}
$was_inside_call = $context->inside_call;
$context->inside_call = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->expr, $context) === false) {
return false;
}
if (!$was_inside_call) {
$context->inside_call = false;
}
if ($stmt->expr instanceof PhpParser\Node\Scalar\String_
|| (isset($stmt->expr->inferredType) && $stmt->expr->inferredType->isSingleStringLiteral())
) {

View File

@ -41,6 +41,8 @@ class TernaryAnalyzer
) {
$first_if_cond_expr = IfAnalyzer::getDefinitelyEvaluatedExpression($stmt->cond);
$was_inside_conditional = $context->inside_conditional;
$context->inside_conditional = true;
$pre_condition_vars_in_scope = $context->vars_in_scope;
@ -70,7 +72,9 @@ class TernaryAnalyzer
$first_cond_referenced_var_ids
);
$context->inside_conditional = false;
if (!$was_inside_conditional) {
$context->inside_conditional = false;
}
$t_if_context = clone $context;

View File

@ -630,9 +630,11 @@ class ExpressionAnalyzer
$stmt->inferredType = Type::getBool();
} elseif ($stmt instanceof PhpParser\Node\Expr\Exit_) {
if ($stmt->expr) {
$context->inside_call = true;
if (self::analyze($statements_analyzer, $stmt->expr, $context) === false) {
return false;
}
$context->inside_call = false;
}
} elseif ($stmt instanceof PhpParser\Node\Expr\Include_) {
IncludeAnalyzer::analyze($statements_analyzer, $stmt, $context, $global_context);
@ -1454,15 +1456,19 @@ class ExpressionAnalyzer
}
if ($stmt->key) {
$context->inside_call = true;
if (self::analyze($statements_analyzer, $stmt->key, $context) === false) {
return false;
}
$context->inside_call = false;
}
if ($stmt->value) {
$context->inside_call = true;
if (self::analyze($statements_analyzer, $stmt->value, $context) === false) {
return false;
}
$context->inside_call = false;
if ($var_comment_type) {
$stmt->inferredType = $var_comment_type;

View File

@ -113,6 +113,7 @@ class ReturnAnalyzer
}
if ($stmt->expr) {
$context->inside_call = true;
if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->expr, $context) === false) {
return false;
}

View File

@ -512,7 +512,9 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource
| Type\Union::TAINTED_SYSTEM_SECRET;
foreach ($stmt->exprs as $i => $expr) {
$context->inside_call = true;
ExpressionAnalyzer::analyze($this, $expr, $context);
$context->inside_call = false;
if (isset($expr->inferredType)) {
if (CallAnalyzer::checkFunctionArgumentType(

View File

@ -261,6 +261,95 @@ class Functions
return isset($file_storage->functions[$function_id]) && $file_storage->functions[$function_id]->variadic;
}
/**
* @param array<int, \PhpParser\Node\Arg> $args
*/
public function isCallMapFunctionPure(Codebase $codebase, string $function_id, array $args) : bool
{
$impure_functions = [
// file io
'chdir', 'chgrp', 'chmod', 'chown', 'chroot', 'closedir', 'copy', 'file_put_contents',
'fopen', 'fread', 'fwrite', 'fclose', 'touch', 'fpassthru', 'fputs', 'fscanf', 'fseek',
'ftruncate', 'fprintf', 'symlink', 'mkdir', 'unlink', 'rename', 'rmdir', 'popen', 'pclose',
'fputcsv',
// stream/socket io
'stream_context_set_option', 'socket_write', 'stream_set_blocking', 'socket_close',
'socket_set_option', 'stream_set_write_buffer',
// meta calls
'call_user_func', 'call_user_func_array', 'define', 'create_function',
// http
'header', 'header_remove', 'http_response_code', 'setcookie',
// output buffer
'ob_start', 'ob_end_clean', 'readfile', 'var_dump', 'printf', 'print_r', 'phpinfo',
// internal optimisation
'opcache_compile_file', 'clearstatcache',
// process-related
'pcntl_signal', 'posix_kill', 'cli_set_process_title', 'pcntl_async_signals', 'proc_close',
// curl
'curl_setopt', 'curl_close', 'curl_multi_add_handle', 'curl_multi_remove_handle',
'curl_multi_select', 'curl_multi_close', 'curl_setopt_array',
// apc
'apc_store', 'apc_delete', 'apcu_store', 'apcu_delete', 'apc_clear_cache',
// newrelic
'newrelic_start_transaction', 'newrelic_name_transaction', 'newrelic_add_custom_parameter',
'newrelic_add_custom_tracer', 'newrelic_background_job', 'newrelic_end_transaction',
'newrelic_set_appname',
// well-known functions
'libxml_use_internal_errors', 'array_map', 'curl_exec', 'shell_exec',
'mt_srand', 'openssl_pkcs7_sign', 'mysqli_select_db',
// php environment
'ini_set', 'sleep', 'usleep', 'register_shutdown_function',
'error_reporting', 'register_tick_function', 'unregister_tick_function',
'set_error_handler', 'user_error', 'trigger_error', 'restore_error_handler',
'date_default_timezone_set', 'assert', 'assert_options', 'setlocale',
'set_exception_handler', 'set_time_limit', 'putenv', 'spl_autoload_register',
// logging
'openlog', 'syslog', 'error_log', 'define_syslog_variables',
];
if (\in_array(strtolower($function_id), $impure_functions, true)) {
return false;
}
if (strpos($function_id, 'image') === 0) {
return false;
}
if ($function_id === 'var_export' && !isset($args[1])) {
return false;
}
$function_callable = \Psalm\Internal\Codebase\CallMap::getCallableFromCallMapById(
$codebase,
$function_id,
$args
);
if (!$function_callable->params) {
return false;
}
foreach ($function_callable->params as $param) {
if ($param->by_ref) {
return false;
}
}
return true;
}
public static function clearCache() : void
{
self::$stubbed_functions = [];

View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class UnusedFunctionCall extends FunctionIssue
{
}

View File

@ -1125,6 +1125,30 @@ class FunctionTemplateTest extends TestCase
}',
'error_message' => 'InvalidReturnStatement',
],
'templateReturnTypeOfCallableWithIncompatibleType' => [
'<?php
class A {}
class B {
public static function returnsObjectOrNull() : ?A {
return random_int(0, 1) ? new A() : null;
}
}
/**
* @psalm-template T as object
* @psalm-param callable() : T $callback
* @psalm-return T
*/
function makeResultSet(callable $callback)
{
return $callback();
}
makeResultSet([A::class, "returnsObjectOrNull"]);',
'error_message' => 'InvalidArgument',
],
];
}
}