Skip to content

Commit

Permalink
Merge pull request #7335 from orklah/errors_on_int_reconciliation
Browse files Browse the repository at this point in the history 
Errors on superior/inferior reconciliation
  • Loading branch information
@orklah
orklah committed Jan 7, 2022
2 parents 9bd28a2 + cc529e8 commit 0d9480a
Show file tree
Hide file tree
Showing 8 changed files with 379 additions and 54 deletions.
9 changes: 8 additions & 1 deletion psalm-baseline.xml
View file
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="dev-master">
<files psalm-version="4.9999999.9999999.9999999-dev">
<file src="examples/TemplateChecker.php">
<PossiblyUndefinedIntArrayOffset occurrences="2">
<code>$comment_block-&gt;tags['variablesfrom'][0]</code>
Expand Down Expand Up @@ -421,6 +421,13 @@
<code>$stmt-&gt;props[0]</code>
</PossiblyUndefinedIntArrayOffset>
</file>
<file src="src/Psalm/Internal/LanguageServer/LanguageClient.php">
<DocblockTypeContradiction occurrences="3">
<code>$type &lt; 1</code>
<code>$type &lt; 1 || $type &gt; 4</code>
<code>$type &gt; 4</code>
</DocblockTypeContradiction>
</file>
<file src="src/Psalm/Internal/LanguageServer/Message.php">
<PossiblyUndefinedIntArrayOffset occurrences="1">
<code>$pair[1]</code>
Expand Down
181 changes: 154 additions & 27 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Expand Up @@ -35,6 +35,7 @@
use Psalm\Type\Atomic\TLiteralString;
use Psalm\Type\Atomic\TMixed;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Atomic\TNever;
use Psalm\Type\Atomic\TNonEmptyArray;
use Psalm\Type\Atomic\TNonEmptyList;
use Psalm\Type\Atomic\TNonEmptyLowercaseString;
Expand Down Expand Up @@ -91,6 +92,8 @@ public static function reconcile(
return $existing_var_type;
}

$old_var_type_string = $existing_var_type->getId();

if ($assertion === 'isset') {
return self::reconcileIsset(
$existing_var_type,
Expand Down Expand Up @@ -133,16 +136,26 @@ public static function reconcile(
if ($assertion[0] === '>') {
return self::reconcileSuperiorTo(
$existing_var_type,
substr($assertion, 1),
$inside_loop
$assertion,
$inside_loop,
$old_var_type_string,
$key,
$negated,
$code_location,
$suppressed_issues
);
}

if ($assertion[0] === '<') {
return self::reconcileInferiorTo(
$existing_var_type,
substr($assertion, 1),
$inside_loop
$assertion,
$inside_loop,
$old_var_type_string,
$key,
$negated,
$code_location,
$suppressed_issues
);
}

Expand Down Expand Up @@ -1616,30 +1629,52 @@ private static function reconcileHasArrayKey(
return $existing_var_type;
}

/**
* @param string[] $suppressed_issues
*/
private static function reconcileSuperiorTo(
Union $existing_var_type,
string $assertion,
bool $inside_loop
Union $existing_var_type,
string $assertion,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
): Union {
$assertion_value = (int)$assertion;
$assertion_value = (int)substr($assertion, 1);

$did_remove_type = false;

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
}

if ($atomic_type instanceof TIntRange) {
$existing_var_type->removeType($atomic_type->getKey());
if ($atomic_type->min_bound === null) {
$atomic_type->min_bound = $assertion_value;
} else {
$atomic_type->min_bound = TIntRange::getNewHighestBound(
$assertion_value,
$atomic_type->min_bound
);
if ($atomic_type->contains($assertion_value)) {
// if the range contains the assertion, the range must be adapted
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
if ($atomic_type->min_bound === null) {
$atomic_type->min_bound = $assertion_value;
} else {
$atomic_type->min_bound = TIntRange::getNewHighestBound(
$assertion_value,
$atomic_type->min_bound
);
}
$existing_var_type->addType($atomic_type);
} elseif ($atomic_type->isLesserThan($assertion_value)) {
// if the range is lesser than the assertion, the type must be removed
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
} elseif ($atomic_type->isGreaterThan($assertion_value)) {
// if the range is greater than the assertion, the check is redundant
}
$existing_var_type->addType($atomic_type);
} elseif ($atomic_type instanceof TLiteralInt) {
if ($atomic_type->value < $assertion_value) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
} /*elseif ($inside_loop) {
//when inside a loop, allow the range to extends the type
Expand All @@ -1652,39 +1687,96 @@ private static function reconcileSuperiorTo(
}*/
} elseif ($atomic_type instanceof TPositiveInt) {
if ($assertion_value > 1) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
$existing_var_type->addType(new TIntRange($assertion_value, null));
}
} elseif ($atomic_type instanceof TInt) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
$existing_var_type->addType(new TIntRange($assertion_value, null));
} else {
// we assume that other types may have been removed (empty strings? numeric strings?)
//It may be worth refining to improve reconciliation while keeping in mind we're on loose comparison
$did_remove_type = true;
}
}

if (!$inside_loop && !$did_remove_type && $var_id && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$old_var_type_string,
$var_id,
$assertion,
true,
$negated,
$code_location,
$suppressed_issues
);
}

if ($existing_var_type->isUnionEmpty()) {
if ($var_id && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$old_var_type_string,
$var_id,
$assertion,
false,
$negated,
$code_location,
$suppressed_issues
);
}
$existing_var_type->addType(new TNever());
}

return $existing_var_type;
}

/**
* @param string[] $suppressed_issues
*/
private static function reconcileInferiorTo(
Union $existing_var_type,
string $assertion,
bool $inside_loop
Union $existing_var_type,
string $assertion,
bool $inside_loop,
string $old_var_type_string,
?string $var_id,
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues
): Union {
$assertion_value = (int)$assertion;
$assertion_value = (int)substr($assertion, 1);

$did_remove_type = false;

foreach ($existing_var_type->getAtomicTypes() as $atomic_type) {
if ($inside_loop) {
continue;
}

if ($atomic_type instanceof TIntRange) {
$existing_var_type->removeType($atomic_type->getKey());
if ($atomic_type->max_bound === null) {
$atomic_type->max_bound = $assertion_value;
} else {
$atomic_type->max_bound = min($atomic_type->max_bound, $assertion_value);
if ($atomic_type->contains($assertion_value)) {
// if the range contains the assertion, the range must be adapted
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
if ($atomic_type->max_bound === null) {
$atomic_type->max_bound = $assertion_value;
} else {
$atomic_type->max_bound = min($atomic_type->max_bound, $assertion_value);
}
$existing_var_type->addType($atomic_type);
} elseif ($atomic_type->isLesserThan($assertion_value)) {
// if the range is lesser than the assertion, the check is redundant
} elseif ($atomic_type->isGreaterThan($assertion_value)) {
// if the range is greater than the assertion, the type must be removed
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
}
$existing_var_type->addType($atomic_type);
} elseif ($atomic_type instanceof TLiteralInt) {
if ($atomic_type->value > $assertion_value) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
} /* elseif ($inside_loop) {
//when inside a loop, allow the range to extends the type
Expand All @@ -1696,14 +1788,49 @@ private static function reconcileInferiorTo(
}
}*/
} elseif ($atomic_type instanceof TPositiveInt) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
if ($assertion_value >= 1) {
$existing_var_type->addType(new TIntRange(1, $assertion_value));
}
} elseif ($atomic_type instanceof TInt) {
$did_remove_type = true;
$existing_var_type->removeType($atomic_type->getKey());
$existing_var_type->addType(new TIntRange(null, $assertion_value));
} else {
// we assume that other types may have been removed (empty strings? numeric strings?)
//It may be worth refining to improve reconciliation while keeping in mind we're on loose comparison
$did_remove_type = true;
}
}

if (!$inside_loop && !$did_remove_type && $var_id && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$old_var_type_string,
$var_id,
$assertion,
true,
$negated,
$code_location,
$suppressed_issues
);
}

if ($existing_var_type->isUnionEmpty()) {
if ($var_id && $code_location) {
self::triggerIssueForImpossible(
$existing_var_type,
$old_var_type_string,
$var_id,
$assertion,
false,
$negated,
$code_location,
$suppressed_issues
);
}
$existing_var_type->addType(new TNever());
}

return $existing_var_type;
Expand Down

0 comments on commit 0d9480a

Please sign in to comment.