Skip to content
Permalink
Browse files

Change how $changed_var_ids is communicated

  • Loading branch information
muglug committed Dec 7, 2019
1 parent 6ec947b commit df395944f871ea04809743148ef4ce17b9ec2124
@@ -487,7 +487,7 @@ public function remove($remove_var_id)
}

/**
* @param string[] $changed_var_ids
* @param array<string, bool> $changed_var_ids
*
* @return void
*/
@@ -503,7 +503,7 @@ function (Clause $c) use ($changed_var_ids) {
}

foreach ($c->possibilities as $key => $_) {
if (in_array($key, $changed_var_ids, true)) {
if (isset($changed_var_ids[$key])) {
return false;
}
}
@@ -520,7 +520,7 @@ public static function analyzeIfConditional(
function (Clause $c) use ($changed_var_ids) {
return count($c->possibilities) > 1
|| $c->wedge
|| !in_array(array_keys($c->possibilities)[0], $changed_var_ids, true);
|| !isset($changed_var_ids[array_keys($c->possibilities)[0]]);
}
)
);
@@ -766,20 +766,20 @@ protected static function analyzeIfBlock(
$if_scope->assigned_var_ids = $new_assigned_var_ids;
$if_scope->possibly_assigned_var_ids = $new_possibly_assigned_var_ids;

$changed_var_ids = array_keys($new_assigned_var_ids);
$changed_var_ids = $new_assigned_var_ids;

// if the variable was only set in the conditional, it's not possibly redefined
foreach ($if_scope->possibly_redefined_vars as $var_id => $_) {
if (!isset($new_possibly_assigned_var_ids[$var_id])
&& in_array($var_id, $if_scope->if_cond_changed_var_ids, true)
&& isset($if_scope->if_cond_changed_var_ids[$var_id])
) {
unset($if_scope->possibly_redefined_vars[$var_id]);
}
}

if ($if_scope->reasonable_clauses) {
// remove all reasonable clauses that would be negated by the if stmts
foreach ($changed_var_ids as $var_id) {
foreach ($changed_var_ids as $var_id => $_) {

This comment has been minimized.

Copy link
@staabm

staabm Dec 7, 2019

Contributor

As we have similar constructions in psalm consumer codebases: Would it make sense to allow to express a type which means that a a array is just meant for looking up its key (because its fast) and the actual value only exists because pho does not allow a map without values?

So the type would make sure that no code exists which uses the values of such arrays..

This comment has been minimized.

Copy link
@weirdan

weirdan Dec 8, 2019

Collaborator

void is more or less behaving this way already: https://psalm.dev/r/0b91c9ddfc . Doesn't work for iteration though.

$if_scope->reasonable_clauses = Context::filterClauses(
$var_id,
$if_scope->reasonable_clauses,
@@ -816,18 +816,13 @@ protected static function analyzeIfBlock(
)
);

foreach ($changed_var_ids as $changed_var_id) {
foreach ($changed_var_ids as $changed_var_id => $_) {
$outer_context->removeVarFromConflictingClauses($changed_var_id);
}

$changed_var_ids = array_unique(
array_merge(
$changed_var_ids,
array_keys($new_assigned_var_ids)
)
);
$changed_var_ids += $new_assigned_var_ids;

foreach ($changed_var_ids as $var_id) {
foreach ($changed_var_ids as $var_id => $_) {
$if_scope->negated_clauses = Context::filterClauses(
$var_id,
$if_scope->negated_clauses
@@ -1222,7 +1217,7 @@ function ($carry, Clause $clause) {

foreach ($possibly_redefined_vars as $var_id => $_) {
if (!isset($new_stmts_assigned_var_ids[$var_id])
&& in_array($var_id, $changed_var_ids, true)
&& isset($changed_var_ids[$var_id])
) {
unset($possibly_redefined_vars[$var_id]);
}
@@ -447,7 +447,7 @@ public static function analyze(
new CodeLocation($statements_analyzer->getSource(), $pre_conditions[0])
);

foreach ($changed_var_ids as $var_id) {
foreach ($changed_var_ids as $var_id => $_) {
if (isset($vars_in_scope_reconciled[$var_id])
&& isset($loop_scope->loop_parent_context->vars_in_scope[$var_id])
) {
@@ -460,7 +460,7 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
) {
$var_id = ExpressionAnalyzer::getVarId($stmt->left, $context->self);

if (!$var_id || !\in_array($var_id, $changed_var_ids, true)) {
if (!$var_id || !isset($changed_var_ids[$var_id])) {
if ($naive_type->from_docblock) {
if (IssueBuffer::accepts(
new \Psalm\Issue\DocblockTypeContradiction(
@@ -593,24 +593,24 @@ public static function analyze(
$assert_type_assertions = Algebra::getTruthsFromFormula($simplified_clauses);

if ($assert_type_assertions) {
$changed_vars = [];
$changed_var_ids = [];

// while in an and, we allow scope to boil over to support
// statements of the form if ($x && $x->foo())
$op_vars_in_scope = Reconciler::reconcileKeyedTypes(
$assert_type_assertions,
$context->vars_in_scope,
$changed_vars,
$changed_var_ids,
[],
$statements_analyzer,
[],
$context->inside_loop,
new CodeLocation($statements_analyzer->getSource(), $stmt)
);

foreach ($changed_vars as $changed_var) {
if (isset($op_vars_in_scope[$changed_var])) {
$op_vars_in_scope[$changed_var]->from_docblock = true;
foreach ($changed_var_ids as $var_id => $_) {
if (isset($op_vars_in_scope[$var_id])) {
$op_vars_in_scope[$var_id]->from_docblock = true;
}
}

@@ -3474,7 +3474,7 @@ protected static function applyAssertionsToContext(
}
}

$changed_vars = [];
$changed_var_ids = [];

foreach ($type_assertions as $var_id => $_) {
$asserted_keys[$var_id] = true;
@@ -3486,19 +3486,19 @@ protected static function applyAssertionsToContext(
$op_vars_in_scope = \Psalm\Type\Reconciler::reconcileKeyedTypes(
$type_assertions,
$context->vars_in_scope,
$changed_vars,
$changed_var_ids,
$asserted_keys,
$statements_analyzer,
$template_type_map,
$context->inside_loop,
new CodeLocation($statements_analyzer->getSource(), $expr)
);

foreach ($changed_vars as $changed_var) {
if (isset($op_vars_in_scope[$changed_var])) {
$op_vars_in_scope[$changed_var]->from_docblock = true;
foreach ($changed_var_ids as $var_id => $_) {
if (isset($op_vars_in_scope[$var_id])) {
$op_vars_in_scope[$var_id]->from_docblock = true;

foreach ($op_vars_in_scope[$changed_var]->getTypes() as $changed_atomic_type) {
foreach ($op_vars_in_scope[$var_id]->getTypes() as $changed_atomic_type) {
$changed_atomic_type->from_docblock = true;

if ($changed_atomic_type instanceof Type\Atomic\TNamedObject
@@ -51,7 +51,7 @@ class IfScope
public $negated_types = [];

/**
* @var array<mixed, string>
* @var array<string, bool>
*/
public $if_cond_changed_var_ids = [];

@@ -59,7 +59,7 @@ class Reconciler
*
* @param array<string, string[][]> $new_types
* @param array<string, Type\Union> $existing_types
* @param array<string> $changed_var_ids
* @param array<string, bool> $changed_var_ids
* @param array<string, bool> $referenced_var_ids
* @param StatementsAnalyzer $statements_analyzer
* @param CodeLocation|null $code_location
@@ -271,7 +271,7 @@ public static function reconcileKeyedTypes(
$type_changed = !$before_adjustment || !$result_type->equals($before_adjustment);

if ($type_changed || $failed_reconciliation) {
$changed_var_ids[] = $key;
$changed_var_ids[$key] = true;

if (substr($key, -1) === ']' && !$has_inverted_isset && !$has_empty) {
$key_parts = self::breakUpPathIntoParts($key);
@@ -313,7 +313,7 @@ function (array $new_type_part_parts): string {
$suppressed_issues
);
} elseif (!$has_negation && !$has_falsyish && !$has_isset) {
$changed_var_ids[] = $key;
$changed_var_ids[$key] = true;
}

if ($failed_reconciliation === 2) {
@@ -696,7 +696,7 @@ protected static function triggerIssueForImpossible(
/**
* @param string[] $key_parts
* @param array<string,Type\Union> $existing_types
* @param array<string> $changed_var_ids
* @param array<string, bool> $changed_var_ids
*
* @return void
*/
@@ -768,7 +768,7 @@ private static function adjustObjectLikeType(

$new_base_type->addType($base_atomic_type);

$changed_var_ids[] = $base_key . '[' . $array_key . ']';
$changed_var_ids[$base_key . '[' . $array_key . ']'] = true;

if ($key_parts[count($key_parts) - 1] === ']') {
self::adjustObjectLikeType(
@@ -1941,6 +1941,100 @@ function foo(): void {
}
}'
],
'definedInConditionalAndCheckedInSubbranch' => [
'<?php
class A {
public function foo() : void {}
}
function getA(): ?A {
return rand(0, 1) ? new A() : null;
}
function foo(): void {
if (($a = getA()) || rand(0, 1)) {
if ($a) {
$a->foo();
}
}
}'
],
'definedInRhsOfConditionalInNegation' => [
'<?php
class A {
public function foo() : void {}
}
function getA(): ?A {
return rand(0, 1) ? new A() : null;
}
function foo(): void {
if (rand(0, 1) && ($a = getA()) !== null) {
$a->foo();
}
}'
],
'literalStringComparisonInIf' => [
'<?php
function foo(string $t, bool $b) : void {
if ($t !== "a") {
if ($t === "b" && $b) {}
}
}
function bar(string $t, bool $b) : void {
if ($t !== "a") {
if ($t === "b" || $b) {}
}
}'
],
'literalStringComparisonInElseif' => [
'<?php
function foo(string $t, bool $b) : void {
if ($t === "a") {
} elseif ($t === "b" && $b) {}
}
function bar(string $t, bool $b) : void {
if ($t === "a") {
} elseif ($t === "b" || $b) {}
}'
],
'literalStringComparisonInElse' => [
'<?php
function foo(string $t, bool $b) : void {
if ($t === "a") {
} else {
if ($t === "b" && $b) {}
}
}
function bar(string $t, bool $b) : void {
if ($t === "a") {
} else {
if ($t === "b" || $b) {}
}
}'
],
'definedInOrRHS' => [
'<?php
class A {
public function foo() : void {}
}
function getA(): ?A {
return rand(0, 1) ? new A() : null;
}
function foo(bool $b): void {
$a = null;
if (!$b || !($a = getA())) {
return;
}
$a->foo();
}'
],
'assertOnArrayThings' => [
'<?php
/** @var array<string, array<int, string>> */

0 comments on commit df39594

Please sign in to comment.
You can’t perform that action at this time.