Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add @psalm-require-usage annotation and report UnusedFunctionCall for @psalm-taint-escape calls #10742

Open
wants to merge 11 commits into
base: 5.x
Choose a base branch
from
Open

Add @psalm-require-usage annotation and report UnusedFunctionCall for @psalm-taint-escape calls #10742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ba36703
add @psalm-require-usage annotation for UnusedFunctionCall checks
kkmuffme Feb 23, 2024
b73ec24
Fix unused call inconsistencies/false negatives
kkmuffme Feb 23, 2024
1b70fae
update supressions
kkmuffme Feb 23, 2024
5b4276c
add error for void/never return functions that require-usage + tests
kkmuffme Feb 29, 2024
4c53579
remove unused suppressions
kkmuffme Feb 29, 2024
ae59afd
allow pure for "never" due to inconsistent behavior
kkmuffme Feb 29, 2024
6ff2f5d
fix tests
kkmuffme Feb 29, 2024
8803135
fix more tests
kkmuffme Feb 29, 2024
8717a85
pure function with assertions may return void
kkmuffme Feb 29, 2024
5c526d0
fix failing test unusedMethodCallForExternalMutationFreeClass by rest…
kkmuffme Feb 29, 2024
ecdc9a4
unrelated add missing if_false_assertions
kkmuffme Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/annotating_code/supported_annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,25 @@ echo $counter->count; // outputs 1
$counter->count = 5; // This will fail, as it's mutating a property directly
```

### `@psalm-require-usage`

Used to annotate an impure function or method the unused result of which Psalm should report.

```php
<?php
/**
* @psalm-require-usage
*
* @return int
*/
function foo() {
return rand();
}

foo(); // reports UnusedFunctionCall
echo foo(); // no error
```

### `@psalm-trace`

You can use this annotation to trace inferred type (applied to the *next* statement).
Expand Down
4 changes: 2 additions & 2 deletions docs/running_psalm/issues/ImpureFunctionCall.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function impure(array $a) : array {
}

/** @psalm-pure */
function filterOdd(array $a) : void {
impure($a);
function filterOdd(array $a) : array {
return impure($a);
}
```
1 change: 1 addition & 0 deletions src/Psalm/DocComment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

final class DocComment
{
public const PSALM_ANNOTATIONS = [

Check failure on line 21 in src/Psalm/DocComment.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility 

Value of constant Psalm\DocComment::PSALM_ANNOTATIONS changed from array (
  0 => 'return',
  1 => 'param',
  2 => 'template',
  3 => 'var',
  4 => 'type',
  5 => 'template-covariant',
  6 => 'property',
  7 => 'property-read',
  8 => 'property-write',
  9 => 'method',
  10 => 'assert',
  11 => 'assert-if-true',
  12 => 'assert-if-false',
  13 => 'suppress',
  14 => 'ignore-nullable-return',
  15 => 'override-property-visibility',
  16 => 'override-method-visibility',
  17 => 'seal-properties',
  18 => 'seal-methods',
  19 => 'no-seal-properties',
  20 => 'no-seal-methods',
  21 => 'ignore-falsable-return',
  22 => 'variadic',
  23 => 'pure',
  24 => 'ignore-variable-method',
  25 => 'ignore-variable-property',
  26 => 'internal',
  27 => 'taint-sink',
  28 => 'taint-source',
  29 => 'assert-untainted',
  30 => 'scope-this',
  31 => 'mutation-free',
  32 => 'external-mutation-free',
  33 => 'immutable',
  34 => 'readonly',
  35 => 'allow-private-mutation',
  36 => 'readonly-allow-private-mutation',
  37 => 'yield',
  38 => 'trace',
  39 => 'import-type',
  40 => 'flow',
  41 => 'taint-specialize',
  42 => 'taint-escape',
  43 => 'taint-unescape',
  44 => 'self-out',
  45 => 'consistent-constructor',
  46 => 'stub-override',
  47 => 'require-extends',
  48 => 'require-implements',
  49 => 'param-out',
  50 => 'ignore-var',
  51 => 'consistent-templates',
  52 => 'if-this-is',
  53 => 'this-out',
  54 => 'check-type',
  55 => 'check-type-exact',
  56 => 'api',
  57 => 'inheritors',
) to array (
  0 => 'return',
  1 => 'param',
  2 => 'template',
  3 => 'var',
  4 => 'type',
  5 => 'template-covariant',
  6 => 'property',
  7 => 'property-read',
  8 => 'property-write',
  9 => 'method',
  10 => 'assert',
  11 => 'assert-if-true',
  12 => 'assert-if-false',
  13 => 'suppress',
  14 => 'ignore-nullable-return',
  15 => 'override-property-visibility',
  16 => 'override-method-visibility',
  17 => 'seal-properties',
  18 => 'seal-methods',
  19 => 'no-seal-properties',
  20 => 'no-seal-methods',
  21 => 'ignore-falsable-return',
  22 => 'variadic',
  23 => 'pure',
  24 => 'ignore-variable-method',
  25 => 'ignore-variable-property',
  26 => 'internal',
  27 => 'taint-sink',
  28 => 'taint-source',
  29 => 'assert-untainted',
  30 => 'scope-this',
  31 => 'mutation-free',
  32 => 'external-mutation-free',
  33 => 'immutable',
  34 => 'readonly',
  35 => 'allow-private-mutation',
  36 => 'readonly-allow-private-mutation',
  37 => 'yield',
  38 => 'trace',
  39 => 'import-type',
  40 => 'flow',
  41 => 'taint-specialize',
  42 => 'taint-escape',
  43 => 'taint-unescape',
  44 => 'self-out',
  45 => 'consistent-constructor',
  46 => 'stub-override',
  47 => 'require-extends',
  48 => 'require-implements',
  49 => 'param-out',
  50 => 'ignore-var',
  51 => 'consistent-templates',
  52 => 'if-this-is',
  53 => 'this-out',
  54 => 'check-type',
  55 => 'check-type-exact',
  56 => 'api',
  57 => 'inheritors',
  58 => 'require-usage',
)
'return', 'param', 'template', 'var', 'type',
'template-covariant', 'property', 'property-read', 'property-write', 'method',
'assert', 'assert-if-true', 'assert-if-false', 'suppress',
Expand All @@ -35,6 +35,7 @@
'require-extends', 'require-implements', 'param-out', 'ignore-var',
'consistent-templates', 'if-this-is', 'this-out', 'check-type', 'check-type-exact',
'api', 'inheritors',
'require-usage',
];

/**
Expand Down
25 changes: 25 additions & 0 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Psalm\Internal\Type\TemplateStandinTypeReplacer;
use Psalm\Internal\Type\TypeExpander;
use Psalm\Issue\ImplicitToStringCast;
use Psalm\Issue\InvalidDocblock;
use Psalm\Issue\InvalidFalsableReturnType;
use Psalm\Issue\InvalidNullableReturnType;
use Psalm\Issue\InvalidParent;
Expand Down Expand Up @@ -839,6 +840,30 @@ public static function checkReturnType(
$parent_class = $classlike_storage->parent_class;
}

if ($storage->return_type->isVoid() || $storage->return_type->isNever()) {
$incompatible_annotation_text = false;
if ($storage->require_usage) {
$incompatible_annotation_text = '@psalm-require-usage';
} elseif ($storage->pure && !$storage->assertions && !$storage->return_type->isNever()) {
// currently not for "never", since exit is pure atm
// see https://github.com/vimeo/psalm/issues/10762
$incompatible_annotation_text = '@psalm-pure';
} elseif ($storage->removed_taints || $storage->conditionally_removed_taints) {
$incompatible_annotation_text = '@psalm-taint-escape';
}

if ($incompatible_annotation_text !== false) {
$return_type_text = $storage->return_type->isVoid() ? 'void' : 'never';
IssueBuffer::maybeAdd(
new InvalidDocblock(
'Return type "' . $return_type_text . '" is incompatible with '
. $incompatible_annotation_text,
$storage->return_type_location,
),
);
}
}

if (!$storage->signature_return_type || $storage->signature_return_type === $storage->return_type) {
foreach ($storage->return_type->getAtomicTypes() as $type) {
if ($type instanceof TNamedObject
Expand Down
43 changes: 43 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,14 @@ public static function analyze(
$context,
);

self::checkFunctionUnused(
$statements_analyzer,
$codebase,
$function_name,
$function_call_info,
$context,
);

if ($function_call_info->function_storage) {
if ($function_call_info->function_storage->assertions && $function_name instanceof PhpParser\Node\Name) {
self::applyAssertionsToContext(
Expand Down Expand Up @@ -1116,6 +1124,41 @@ private static function checkFunctionCallPurity(
}
}

private static function checkFunctionUnused(
StatementsAnalyzer $statements_analyzer,
Codebase $codebase,
PhpParser\Node $function_name,
FunctionCallInfo $function_call_info,
Context $context
): void {
if (!$context->collect_initializations
&& !$context->collect_mutations
&& $function_call_info->function_id !== null
&& $function_call_info->function_storage
&& !$function_call_info->function_storage->assertions
&& $codebase->find_unused_variables
&& !$context->inside_unset
&& !$context->insideUse()
&& ($function_call_info->function_storage->require_usage
|| $function_call_info->function_storage->removed_taints
|| $function_call_info->function_storage->conditionally_removed_taints)
&& !(
$function_call_info->function_storage->return_type &&
($function_call_info->function_storage->return_type->isVoid()
|| $function_call_info->function_storage->return_type->isNever())
)
) {
IssueBuffer::maybeAdd(
new UnusedFunctionCall(
'The call to ' . $function_call_info->function_id . ' is not used',
new CodeLocation($statements_analyzer, $function_name),
$function_call_info->function_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
}

private static function callUsesByReferenceArguments(
FunctionCallInfo $function_call_info,
PhpParser\Node\Expr\FuncCall $stmt
Expand Down
35 changes: 32 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallPurityAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public static function analyze(
if ($context->inside_conditional
&& !$method_storage->assertions
&& !$method_storage->if_true_assertions
&& !$method_storage->if_false_assertions
) {
$stmt->setAttribute('memoizable', true);

Expand All @@ -113,9 +114,13 @@ public static function analyze(
&& !$context->inside_call
&& !$context->inside_return
&& !$method_storage->assertions
&& !$method_storage->if_true_assertions
&& !$method_storage->if_false_assertions
&& !$method_storage->throws
&& (!$method_storage->throws
|| !$context->inside_try)
&& (($stmt->var instanceof PhpParser\Node\Expr\New_
&& $stmt->var->getAttribute('external_mutation_free') === true)
|| !($method_storage->return_type
&& ($method_storage->return_type->isVoid() || $method_storage->return_type->isNever()))
)
) {
IssueBuffer::maybeAdd(
new UnusedMethodCall(
Expand All @@ -131,6 +136,30 @@ public static function analyze(
}
}

if ($codebase->find_unused_variables
&& !$context->inside_unset
&& !$context->insideUse()
&& !$method_storage->assertions
&& (!$method_storage->throws
|| !$context->inside_try)
&& !(
$method_storage->return_type &&
($method_storage->return_type->isVoid() || $method_storage->return_type->isNever())
)
&& ($method_storage->require_usage
|| $method_storage->removed_taints
|| $method_storage->conditionally_removed_taints)
) {
IssueBuffer::maybeAdd(
new UnusedMethodCall(
'The call to ' . $cased_method_id . ' is not used',
new CodeLocation($statements_analyzer, $stmt->name),
(string) $method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}

if ($statements_analyzer->getSource() instanceof FunctionLikeAnalyzer
&& $statements_analyzer->getSource()->track_mutations
&& !$method_storage->mutation_free
Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/Codebase/Reflection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,13 @@ public function extractReflectionMethodInfo(ReflectionMethod $method): void

foreach ($callables[0]->params as $param) {
if ($param->type) {
/** @psalm-suppress UnusedMethodCall */
$param->type->queueClassLikesForScanning($this->codebase);
}
}

$storage->setParams($callables[0]->params);

$storage->return_type = $callables[0]->return_type;
/** @psalm-suppress UnusedMethodCall */
$storage->return_type->queueClassLikesForScanning($this->codebase);
} else {
$params = $method->getParameters();
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Internal/Codebase/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ public function queueClassLikeForScanning(

foreach ($public_mapped_properties as $public_mapped_property) {
$property_type = Type::parseString($public_mapped_property);
/** @psalm-suppress UnusedMethodCall */
$property_type->queueClassLikesForScanning(
$this->codebase,
null,
Expand Down
11 changes: 2 additions & 9 deletions src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
$this->type_aliases,
true,
);
/** @psalm-suppress UnusedMethodCall */

$yield_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -590,7 +590,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
$this->type_aliases,
true,
);
/** @psalm-suppress UnusedMethodCall */

$pseudo_property_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -689,7 +689,6 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool
true,
);

/** @psalm-suppress UnusedMethodCall */
$mixin_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -844,7 +843,6 @@ public function finish(PhpParser\Node\Stmt\ClassLike $node): ClassLikeStorage
foreach ($mapped_properties as $property_name => $public_mapped_property) {
$property_type = Type::parseString($public_mapped_property);

/** @psalm-suppress UnusedMethodCall */
$property_type->queueClassLikesForScanning($this->codebase, $this->file_storage);

if (!isset($classlike_storage->properties[$property_name])) {
Expand Down Expand Up @@ -1015,7 +1013,6 @@ private function extendTemplatedType(
);
}

/** @psalm-suppress UnusedMethodCall */
$extended_union_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -1101,7 +1098,6 @@ private function implementTemplatedType(
return;
}

/** @psalm-suppress UnusedMethodCall */
$implemented_union_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -1187,7 +1183,6 @@ private function useTemplatedType(
return;
}

/** @psalm-suppress UnusedMethodCall */
$used_union_type->queueClassLikesForScanning(
$this->codebase,
$this->file_storage,
Expand Down Expand Up @@ -1628,7 +1623,6 @@ private function visitPropertyDeclaration(
$doc_var_group_type = $var_comment->type ?? null;

if ($doc_var_group_type) {
/** @psalm-suppress UnusedMethodCall */
$doc_var_group_type->queueClassLikesForScanning($this->codebase, $this->file_storage);
}

Expand Down Expand Up @@ -1731,7 +1725,6 @@ private function visitPropertyDeclaration(
}
}

/** @psalm-suppress UnusedMethodCall */
$property_storage->type->queueClassLikesForScanning($this->codebase, $this->file_storage);
}

Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/PhpVisitor/Reflector/ExpressionScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ private static function registerClassMapFunctionCall(

foreach ($callable->params as $function_param) {
if ($function_param->type) {
/** @psalm-suppress UnusedMethodCall */
$function_param->type->queueClassLikesForScanning(
$codebase,
$file_storage,
Expand All @@ -125,7 +124,6 @@ private static function registerClassMapFunctionCall(
}

if ($callable->return_type && !$callable->return_type->hasMixed()) {
/** @psalm-suppress UnusedMethodCall */
$callable->return_type->queueClassLikesForScanning($codebase, $file_storage);
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,8 @@ public static function parse(
$info->pure = isset($parsed_docblock->tags['psalm-pure'])
|| isset($parsed_docblock->tags['pure']);

$info->require_usage = isset($parsed_docblock->tags['psalm-require-usage']);

if (isset($parsed_docblock->tags['psalm-mutation-free'])) {
$info->mutation_free = true;
}
Expand Down
9 changes: 4 additions & 5 deletions src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ public static function addDocblockInfo(
}
}

if ($docblock_info->require_usage) {
$storage->require_usage = true;
}

if ($docblock_info->specialize_call) {
$storage->specialize_call = true;
}
Expand Down Expand Up @@ -649,7 +653,6 @@ private static function getAssertionParts(
return null;
}

/** @psalm-suppress UnusedMethodCall */
$namespaced_type->queueClassLikesForScanning(
$codebase,
$file_storage,
Expand Down Expand Up @@ -818,7 +821,6 @@ private static function improveParamsFromDocblock(

$storage_param->has_docblock_type = true;

/** @psalm-suppress UnusedMethodCall */
$new_param_type->queueClassLikesForScanning(
$codebase,
$file_storage,
Expand Down Expand Up @@ -1043,7 +1045,6 @@ private static function handleReturn(
}
}

/** @psalm-suppress UnusedMethodCall */
$storage->return_type->queueClassLikesForScanning($codebase, $file_storage);
} catch (TypeParseTreeException $e) {
$storage->docblock_issues[] = new InvalidDocblock(
Expand Down Expand Up @@ -1193,7 +1194,6 @@ private static function handleRemovedTaint(
$type_aliases,
);

/** @psalm-suppress UnusedMethodCall */
$removed_taint->queueClassLikesForScanning($codebase, $file_storage);

$removed_taint_single = $removed_taint->getSingleAtomic();
Expand Down Expand Up @@ -1413,7 +1413,6 @@ private static function handleParamOut(
return;
}

/** @psalm-suppress UnusedMethodCall */
$out_type->queueClassLikesForScanning(
$codebase,
$file_storage,
Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ public function enterNode(PhpParser\Node $node): ?int

if (isset($this->codebase->config->globals[$var_id])) {
$var_type = Type::parseString($this->codebase->config->globals[$var_id]);
/** @psalm-suppress UnusedMethodCall */
$var_type->queueClassLikesForScanning($this->codebase, $this->file_storage);
}
}
Expand Down Expand Up @@ -359,7 +358,6 @@ public function enterNode(PhpParser\Node $node): ?int
}

$var_type = $var_comment->type;
/** @psalm-suppress UnusedMethodCall */
$var_type->queueClassLikesForScanning($this->codebase, $this->file_storage);
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/Internal/Scanner/FunctionDocblockComment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ final class FunctionDocblockComment
*/
public bool $pure = false;

/**
* Whether or not the function return type must not be unused if the function is impure
*/
public bool $require_usage = false;

/**
* Whether or not to specialize a given call (useful for taint analysis)
*/
Expand Down