Skip to content

Commit

Permalink
Deduplicate loop analysis
Browse files Browse the repository at this point in the history 
Also remove some risky truthy falsy comparisons
  • Loading branch information
@theodorejb
theodorejb committed Mar 3, 2024
1 parent f13a302 commit 612f0a1
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 251 deletions.
45 changes: 1 addition & 44 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="5.x-dev@ba4e312594006059b0d9afb0c5ebeea649a59112">
<files psalm-version="5.x-dev@c13ca2697334844b34bebaf56ce1001368b6f30c">
<file src="vendor/nikic/php-parser/lib/PhpParser/Node/Expr/ArrowFunction.php">
<PossiblyUndefinedStringArrayOffset>
<code><![CDATA[$subNodes['expr']]]></code>
Expand Down Expand Up @@ -75,7 +75,6 @@
<code><![CDATA[!$config_path]]></code>
<code><![CDATA[!$file_path]]></code>
<code><![CDATA[!strpos($issue_type, 'Reference')]]></code>
<code><![CDATA[$cwd]]></code>
<code><![CDATA[$dir]]></code>
<code><![CDATA[$e->function_id]]></code>
<code><![CDATA[$igbinary_version = phpversion('igbinary')]]></code>
Expand Down Expand Up @@ -303,18 +302,10 @@
<code><![CDATA[$source_pos = strpos($source, '*')]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$do_context->branch_point]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php">
<ArgumentTypeCoercion>
<code><![CDATA[$stmt->cond]]></code>
</ArgumentTypeCoercion>
<RiskyTruthyFalsyComparison>
<code><![CDATA[$for_context->branch_point]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php">
<ConflictingReferenceConstraint>
Expand All @@ -325,7 +316,6 @@
<code><![CDATA[!$var_comment->var_id]]></code>
<code><![CDATA[!$var_comment->var_id]]></code>
<code><![CDATA[$calling_type_params]]></code>
<code><![CDATA[$foreach_context->branch_point]]></code>
<code><![CDATA[$generic_storage->template_types]]></code>
<code><![CDATA[$statements_analyzer->getTemplateTypeMap()]]></code>
<code><![CDATA[$var_comment->line_number]]></code>
Expand Down Expand Up @@ -356,7 +346,6 @@
<RiskyTruthyFalsyComparison>
<code><![CDATA[$context->branch_point]]></code>
<code><![CDATA[$else_context->branch_point]]></code>
<code><![CDATA[$else_context->branch_point]]></code>
<code><![CDATA[$if_scope->assigned_var_ids]]></code>
<code><![CDATA[$if_scope->new_vars]]></code>
<code><![CDATA[$if_scope->redefined_vars]]></code>
Expand Down Expand Up @@ -384,24 +373,13 @@
<code><![CDATA[$traverser->traverse([$switch_condition])[0]]]></code>
</PossiblyUndefinedIntArrayOffset>
<RiskyTruthyFalsyComparison>
<code><![CDATA[$case_context->branch_point]]></code>
<code><![CDATA[$nested_or_options]]></code>
<code><![CDATA[$switch_var_id]]></code>
<code><![CDATA[$switch_var_id]]></code>
<code><![CDATA[$switch_var_id]]></code>
<code><![CDATA[$type_statements]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$try_context->branch_point]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Block/WhileAnalyzer.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$while_context->branch_point]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$var_id]]></code>
Expand Down Expand Up @@ -865,8 +843,6 @@
<code><![CDATA[!$context->self]]></code>
<code><![CDATA[!$fq_class_name]]></code>
<code><![CDATA[$class_storage->mixin_declaring_fqcln]]></code>
<code><![CDATA[$class_storage->parent_class]]></code>
<code><![CDATA[$class_storage->parent_class]]></code>
<code><![CDATA[$context->calling_method_id]]></code>
<code><![CDATA[$context->calling_method_id]]></code>
<code><![CDATA[$context->self]]></code>
Expand Down Expand Up @@ -1104,7 +1080,6 @@
</file>
<file src="src/Psalm/Internal/Cli/LanguageServer.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[!$root_path]]></code>
<code><![CDATA[$options['verbose']]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand All @@ -1115,9 +1090,6 @@
<RiskyTruthyFalsyComparison>
<code><![CDATA[!$config->error_baseline]]></code>
<code><![CDATA[!$paths_to_check]]></code>
<code><![CDATA[!$root_path]]></code>
<code><![CDATA[!file_put_contents($current_dir . 'psalm.xml', $template_contents)]]></code>
<code><![CDATA[!file_put_contents($current_dir . 'psalm.xml', $template_contents)]]></code>
<code><![CDATA[$baseline_file_path]]></code>
<code><![CDATA[$cache_directory]]></code>
<code><![CDATA[$config->threads]]></code>
Expand All @@ -1128,7 +1100,6 @@
</file>
<file src="src/Psalm/Internal/Cli/Psalter.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[!$root_path]]></code>
<code><![CDATA[$paths_to_check]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand All @@ -1140,7 +1111,6 @@
<code><![CDATA[!$last_arg]]></code>
<code><![CDATA[!$last_arg]]></code>
<code><![CDATA[!$last_arg]]></code>
<code><![CDATA[!$root_path]]></code>
<code><![CDATA[strpos($last_arg_part, '::')]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand Down Expand Up @@ -1203,11 +1173,6 @@
<code><![CDATA[$migrated_source_fqcln]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Codebase/ConstantTypeResolver.php">
<RiskyTruthyFalsyComparison>
<code><![CDATA[$cond->value]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/Codebase/Functions.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$stub]]></code>
Expand Down Expand Up @@ -1441,7 +1406,6 @@
</file>
<file src="src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$doc_line_parts[1]]]></code>
<code><![CDATA[$matches[0]]]></code>
<code><![CDATA[$method_tree->children[0]]]></code>
<code><![CDATA[$method_tree->children[1]]]></code>
Expand Down Expand Up @@ -1498,9 +1462,6 @@
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php">
<PossiblyUndefinedIntArrayOffset>
<code><![CDATA[$since_parts[1]]]></code>
</PossiblyUndefinedIntArrayOffset>
<RedundantCondition>
<code><![CDATA[count($line_parts) > 0]]></code>
</RedundantCondition>
Expand Down Expand Up @@ -1929,9 +1890,6 @@
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->strings]]></code>
<code><![CDATA[$combination->value_types['string'] instanceof TNonFalsyString
? $type->value
: $type->value !== '']]></code>
<code><![CDATA[$shared_classlikes]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand Down Expand Up @@ -2278,7 +2236,6 @@
<RiskyTruthyFalsyComparison>
<code><![CDATA[!strpos($key, '$')]]></code>
<code><![CDATA[!strpos($key, '[')]]></code>
<code><![CDATA[$array_key_offset]]></code>
<code><![CDATA[$failed_reconciliation]]></code>
<code><![CDATA[strpos($base_key, '::')]]></code>
<code><![CDATA[strpos($key, '::')]]></code>
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Context.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ final class Context
public $is_global = false;

/**
* @var array<string, bool>
* @var array<string, bool|int>
*/
public $protected_var_ids = [];

Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public static function analyze(

$codebase = $statements_analyzer->getCodebase();

if ($codebase->alter_code) {
$do_context->branch_point = $do_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
if ($codebase->alter_code && $do_context->branch_point === null) {
$do_context->branch_point = (int) $stmt->getAttribute('startFilePos');
}

$loop_scope = new LoopScope($do_context, $context);
Expand Down
117 changes: 7 additions & 110 deletions src/Psalm/Internal/Analyzer/Statements/Block/ForAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,10 @@

use PhpParser;
use Psalm\Context;
use Psalm\Internal\Analyzer\ScopeAnalyzer;
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Internal\Scope\LoopScope;
use UnexpectedValueException;

use function array_merge;
use function count;
use function in_array;
use function is_string;

/**
Expand Down Expand Up @@ -56,113 +51,15 @@ public static function analyze(

$while_true = !$stmt->cond && !$stmt->init && !$stmt->loop;

$pre_context = null;

if ($while_true) {
$pre_context = clone $context;
}

$for_context = clone $context;

$for_context->inside_loop = true;
$for_context->break_types[] = 'loop';

$codebase = $statements_analyzer->getCodebase();

if ($codebase->alter_code) {
$for_context->branch_point = $for_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
}

$loop_scope = new LoopScope($for_context, $context);

$loop_scope->protected_var_ids = array_merge(
$assigned_var_ids,
$context->protected_var_ids,
);

if (LoopAnalyzer::analyze(
return LoopAnalyzer::analyzeForOrWhile(
$statements_analyzer,
$stmt->stmts,
$stmt,
$context,
$while_true,
$init_var_types,
$assigned_var_ids,
$stmt->cond,
$stmt->loop,
$loop_scope,
$inner_loop_context,
) === false) {
return false;
}

if (!$inner_loop_context) {
throw new UnexpectedValueException('There should be an inner loop context');
}

$always_enters_loop = false;

foreach ($stmt->cond as $cond) {
if ($cond_type = $statements_analyzer->node_data->getType($cond)) {
$always_enters_loop = $cond_type->isAlwaysTruthy();
}

if (count($stmt->init) === 1
&& count($stmt->cond) === 1
&& $cond instanceof PhpParser\Node\Expr\BinaryOp
&& ($cond_value = $statements_analyzer->node_data->getType($cond->right))
&& ($cond_value->isSingleIntLiteral() || $cond_value->isSingleStringLiteral())
&& $cond->left instanceof PhpParser\Node\Expr\Variable
&& is_string($cond->left->name)
&& isset($init_var_types[$cond->left->name])
&& $init_var_types[$cond->left->name]->isSingleIntLiteral()
) {
$init_value = $init_var_types[$cond->left->name]->getSingleLiteral()->value;
$cond_value = $cond_value->getSingleLiteral()->value;

if ($cond instanceof PhpParser\Node\Expr\BinaryOp\Smaller && $init_value < $cond_value) {
$always_enters_loop = true;
break;
}

if ($cond instanceof PhpParser\Node\Expr\BinaryOp\SmallerOrEqual && $init_value <= $cond_value) {
$always_enters_loop = true;
break;
}

if ($cond instanceof PhpParser\Node\Expr\BinaryOp\Greater && $init_value > $cond_value) {
$always_enters_loop = true;
break;
}

if ($cond instanceof PhpParser\Node\Expr\BinaryOp\GreaterOrEqual && $init_value >= $cond_value) {
$always_enters_loop = true;
break;
}
}
}

if ($while_true) {
$always_enters_loop = true;
}

$can_leave_loop = !$while_true
|| in_array(ScopeAnalyzer::ACTION_BREAK, $loop_scope->final_actions, true);

if ($always_enters_loop && $can_leave_loop) {
LoopAnalyzer::setLoopVars($inner_loop_context, $context, $loop_scope);
}

$for_context->loop_scope = null;

if ($can_leave_loop) {
$context->vars_possibly_in_scope = array_merge(
$context->vars_possibly_in_scope,
$for_context->vars_possibly_in_scope,
);
} elseif ($pre_context) {
$context->vars_possibly_in_scope = $pre_context->vars_possibly_in_scope;
}

if ($context->collect_exceptions) {
$context->mergeExceptions($for_context);
}

return null;
);
}
}
5 changes: 2 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,8 @@ public static function analyze(
$foreach_context->inside_loop = true;
$foreach_context->break_types[] = 'loop';

if ($codebase->alter_code) {
$foreach_context->branch_point =
$foreach_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
if ($codebase->alter_code && $foreach_context->branch_point === null) {
$foreach_context->branch_point = (int) $stmt->getAttribute('startFilePos');
}

if ($stmt->keyVar instanceof PhpParser\Node\Expr\Variable && is_string($stmt->keyVar->name)) {
Expand Down
5 changes: 2 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,8 @@ public static function analyze(
}

if ($stmt->else) {
if ($codebase->alter_code) {
$else_context->branch_point =
$else_context->branch_point ?: (int) $stmt->getAttribute('startFilePos');
if ($codebase->alter_code && $else_context->branch_point === null) {
$else_context->branch_point = (int) $stmt->getAttribute('startFilePos');
}
}

Expand Down
Loading

0 comments on commit 612f0a1

Please sign in to comment.