Skip to content

Commit 0090471

Browse files
muglugclaude
muglug
and
claude
authored
Clean up redundant assignment-conditions and repeated getArgs() calls (#11888)
* Clean up redundant assignment-conditions and repeated getArgs() calls These tidy-ups were surfaced by running pzoom over the Psalm codebase; each is behaviour-preserving and removes a real (if minor) code smell. Redundant assignment-in-condition (always-truthy test): several call providers test the result of a @psalm-mutation-free, non-falsable getter inside a condition, e.g. `... && ($x = $type->getArray()) && ($x instanceof TArray || ...)`. Since getArray()/getSingleAtomic() can never return a falsy value, the `&& ($x = ...)` operand is always true. Fold the assignment into the adjacent instanceof test so the binding stays but the dead truthiness check is gone (ArgumentsAnalyzer, ArrayFilter/Pop/Rand/Reverse/Splice, FilterInput, FilterUtils). For in_array the getAtomicTypes() result is unconditional, so hoist it to a plain statement. Repeated memoized getArgs() -> local: IncludeAnalyzer (dirname) and FunctionCallReturnTypeFetcher (preg_replace) guard on `getArgs()`/`count(getArgs())` and then index `getArgs()[n]`, calling the memoized method several times. Bind `$args = $stmt->getArgs()` once and reuse it so the non-empty / length narrowing flows to the indexed accesses. GitInfoCollector::collectBranch: after `str_starts_with($result, '* ')`, `explode('* ', $result, 2)[1]` always equals `substr($result, 2)` but reads as a possibly-undefined offset (baselined). Use substr() and drop the stale baseline entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014CUuwJRfj6cG8rqz8a34d2 * Satisfy PHPCS: null coalesce in in_array provider, dedupe options access in FilterUtils - InArrayReturnTypeProvider: use `$types['array'] ?? null` instead of the isset()-ternary (ShortTernary / null-coalesce sniff). - FilterUtils: capture `$atomic_type->properties['options']` once as `$options_type` (used 5x in the block), which also brings the folded elseif condition back under the 120-char line limit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014CUuwJRfj6cG8rqz8a34d2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent be7afcf commit 0090471

14 files changed

Lines changed: 28 additions & 37 deletions

‎psalm-baseline.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1676,7 +1676,6 @@
16761676
</file>
16771677
<file src="src/Psalm/Internal/ExecutionEnvironment/GitInfoCollector.php">
16781678
<PossiblyUndefinedArrayOffset>
1679-
<code><![CDATA[$exploded[1]]]></code>
16801679
<code><![CDATA[$url]]></code>
16811680
</PossiblyUndefinedArrayOffset>
16821681
</file>

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,8 +1644,7 @@ private static function checkArgCount(
16441644
}
16451645

16461646
if ($arg_value_type->isSingle()
1647-
&& ($atomic_arg_type = $arg_value_type->getSingleAtomic())
1648-
&& $atomic_arg_type instanceof TKeyedArray
1647+
&& ($atomic_arg_type = $arg_value_type->getSingleAtomic()) instanceof TKeyedArray
16491648
&& !$atomic_arg_type->is_list
16501649
) {
16511650
//if we have a single shape, we'll check param names

‎src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,10 @@ private static function taintReturnType(
626626
if ($function_storage->return_source_params) {
627627
$removed_taints = $function_storage->removed_taints;
628628

629-
if ($function_id === 'preg_replace' && count($stmt->getArgs()) > 2) {
630-
$first_stmt_type = $statements_analyzer->node_data->getType($stmt->getArgs()[0]->value);
631-
$second_stmt_type = $statements_analyzer->node_data->getType($stmt->getArgs()[1]->value);
629+
$args = $stmt->getArgs();
630+
if ($function_id === 'preg_replace' && count($args) > 2) {
631+
$first_stmt_type = $statements_analyzer->node_data->getType($args[0]->value);
632+
$second_stmt_type = $statements_analyzer->node_data->getType($args[1]->value);
632633

633634
if ($first_stmt_type
634635
&& $second_stmt_type

‎src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,15 +353,16 @@ public static function getPathTo(
353353
$stmt->name instanceof PhpParser\Node\Name &&
354354
$stmt->name->getParts() === ['dirname']
355355
) {
356-
if ($stmt->getArgs()) {
356+
$args = $stmt->getArgs();
357+
if ($args) {
357358
$dir_level = 1;
358359

359-
if (isset($stmt->getArgs()[1])) {
360-
if ($stmt->getArgs()[1]->value instanceof PhpParser\Node\Scalar\Int_) {
361-
$dir_level = $stmt->getArgs()[1]->value->value;
360+
if (isset($args[1])) {
361+
if ($args[1]->value instanceof PhpParser\Node\Scalar\Int_) {
362+
$dir_level = $args[1]->value->value;
362363
} else {
363364
if ($statements_analyzer) {
364-
$t = $statements_analyzer->node_data->getType($stmt->getArgs()[1]->value);
365+
$t = $statements_analyzer->node_data->getType($args[1]->value);
365366
if ($t && $t->isSingleIntLiteral()) {
366367
$dir_level = $t->getSingleIntLiteral()->value;
367368
} else {
@@ -374,7 +375,7 @@ public static function getPathTo(
374375
}
375376

376377
$evaled_path = self::getPathTo(
377-
$stmt->getArgs()[0]->value,
378+
$args[0]->value,
378379
$type_provider,
379380
$statements_analyzer,
380381
$file_name,

‎src/Psalm/Internal/ExecutionEnvironment/GitInfoCollector.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use function range;
1717
use function str_contains;
1818
use function str_starts_with;
19+
use function substr;
1920
use function trim;
2021

2122
/**
@@ -64,9 +65,7 @@ private function collectBranch(): string
6465

6566
foreach ($branchesResult as $result) {
6667
if (str_starts_with($result, '* ')) {
67-
$exploded = explode('* ', $result, 2);
68-
69-
return $exploded[1];
68+
return substr($result, 2);
7069
}
7170
}
7271

‎src/Psalm/Internal/Provider/ParamsProvider/ArrayFilterParamsProvider.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ public static function getFunctionParams(FunctionParamsProviderEvent $event): ?a
122122
$first_arg_array = $fallback;
123123
} else {
124124
$first_arg_array = $first_arg_type->hasType('array')
125-
&& ($array_atomic_type = $first_arg_type->getArray())
126-
&& ($array_atomic_type instanceof TArray
125+
&& (($array_atomic_type = $first_arg_type->getArray()) instanceof TArray
127126
|| $array_atomic_type instanceof TKeyedArray)
128127
? $array_atomic_type
129128
: $fallback;

‎src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
7373
$first_arg_array = $fallback;
7474
} else {
7575
$first_arg_array = $first_arg_type->hasType('array')
76-
&& ($array_atomic_type = $first_arg_type->getArray())
77-
&& ($array_atomic_type instanceof TArray
76+
&& (($array_atomic_type = $first_arg_type->getArray()) instanceof TArray
7877
|| $array_atomic_type instanceof TKeyedArray)
7978
? $array_atomic_type
8079
: $fallback;

‎src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayPopReturnTypeProvider.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
4545
&& ($first_arg_type = $statements_source->node_data->getType($first_arg))
4646
&& $first_arg_type->hasType('array')
4747
&& !$first_arg_type->hasMixed()
48-
&& ($array_atomic_type = $first_arg_type->getArray())
49-
&& ($array_atomic_type instanceof TArray
48+
&& (($array_atomic_type = $first_arg_type->getArray()) instanceof TArray
5049
|| $array_atomic_type instanceof TKeyedArray)
5150
? $array_atomic_type
5251
: null;

‎src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayRandReturnTypeProvider.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
4242
$first_arg_array = $first_arg
4343
&& ($first_arg_type = $statements_source->node_data->getType($first_arg))
4444
&& $first_arg_type->hasType('array')
45-
&& ($array_atomic_type = $first_arg_type->getArray())
46-
&& ($array_atomic_type instanceof TArray
45+
&& (($array_atomic_type = $first_arg_type->getArray()) instanceof TArray
4746
|| $array_atomic_type instanceof TKeyedArray)
4847
? $array_atomic_type
4948
: null;

‎src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayReverseReturnTypeProvider.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev
4848
&& ($first_arg_type = $statements_source->node_data->getType($first_arg))
4949
&& $first_arg_type->hasType('array')
5050
&& $first_arg_type->isArray()
51-
&& ($array_atomic_type = $first_arg_type->getArray())
52-
&& ($array_atomic_type instanceof TArray
51+
&& (($array_atomic_type = $first_arg_type->getArray()) instanceof TArray
5352
|| $array_atomic_type instanceof TKeyedArray)
5453
? $array_atomic_type
5554
: null;

0 commit comments

Comments
 (0)