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

Use objects, not strings, for assertions #7410

Merged
merged 5 commits into from Jan 20, 2022
Merged

Use objects, not strings, for assertions #7410

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52aeeb2
Use objects, not strings, for assertions
muglug Jan 20, 2022
e4f7c8c
Remove unnecessary param
muglug Jan 20, 2022
a72ab5c
Remove some unnecessary checks
muglug Jan 20, 2022
485e511
Fix bad find/replace
muglug Jan 20, 2022
e5b0c6f
Add note about assertions no longer stored as strings in UPGRADING.md
muglug Jan 20, 2022
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
4 changes: 4 additions & 0 deletions phpcs.xml
View file
Open in desktop
Expand Up @@ -110,6 +110,10 @@
</properties>
</rule>

<rule ref="Squiz.Classes.ValidClassName">
<exclude-pattern>src/Psalm/Storage/Assertion/Empty_.php</exclude-pattern>
</rule>

<!--
Handle imports from namespaces.
https://github.com/slevomat/coding-standard#slevomatcodingstandardnamespacesreferenceusednamesonly-
Expand Down
6 changes: 6 additions & 0 deletions psalm.xml.dist
View file
Open in desktop
Expand Up @@ -102,6 +102,12 @@
</errorLevel>
</PossiblyUndefinedIntArrayOffset>

<ImpureMethodCall>
<errorLevel type="suppress">
<directory name="src/Psalm/Storage/Assertion"/>
</errorLevel>
</ImpureMethodCall>

<MissingThrowsDocblock errorLevel="info"/>

<PossiblyUnusedProperty>
Expand Down
12 changes: 6 additions & 6 deletions src/Psalm/Context.php
View file
Open in desktop
Expand Up @@ -564,7 +564,6 @@ public static function filterClauses(
?StatementsAnalyzer $statements_analyzer = null
): array {
$new_type_string = $new_type ? $new_type->getId() : '';

$clauses_to_keep = [];

foreach ($clauses as $clause) {
Expand All @@ -578,8 +577,9 @@ public static function filterClauses(
}
}

if (!isset($clause->possibilities[$remove_var_id]) ||
$clause->possibilities[$remove_var_id] === [$new_type_string]
if (!isset($clause->possibilities[$remove_var_id])
|| (count($clause->possibilities[$remove_var_id]) === 1
&& (string)$clause->possibilities[$remove_var_id][0] === $new_type_string)
) {
$clauses_to_keep[] = $clause;
} elseif ($statements_analyzer &&
Expand All @@ -590,15 +590,15 @@ public static function filterClauses(

// if the clause contains any possibilities that would be altered
// by the new type
foreach ($clause->possibilities[$remove_var_id] as $type) {
foreach ($clause->possibilities[$remove_var_id] as $assertion) {
// if we're negating a type, we generally don't need the clause anymore
if ($type[0] === '!' && $type !== '!falsy' && $type !== '!empty') {
if ($assertion->isNegation()) {
$type_changed = true;
break;
}

$result_type = AssertionReconciler::reconcile(
$type,
$assertion,
clone $new_type,
null,
$statements_analyzer,
Expand Down
113 changes: 52 additions & 61 deletions src/Psalm/Internal/Algebra.php
View file
Open in desktop
Expand Up @@ -3,6 +3,8 @@
namespace Psalm\Internal;

use Psalm\Exception\ComplicatedExpressionException;
use Psalm\Storage\Assertion;
use Psalm\Storage\Assertion\Falsy;
use UnexpectedValueException;

use function array_diff_key;
Expand All @@ -14,19 +16,17 @@
use function array_unique;
use function array_values;
use function count;
use function in_array;
use function mt_rand;
use function substr;

/**
* @internal
*/
class Algebra
{
/**
* @param array<string, non-empty-list<non-empty-list<string>>> $all_types
* @param array<string, non-empty-list<non-empty-list<Assertion>>> $all_types
*
* @return array<string, non-empty-list<non-empty-list<string>>>
* @return array<string, non-empty-list<non-empty-list<Assertion>>>
*
* @psalm-pure
*/
Expand All @@ -35,9 +35,9 @@ public static function negateTypes(array $all_types): array
return array_filter(
array_map(
/**
* @param non-empty-list<non-empty-list<string>> $anded_types
* @param non-empty-list<non-empty-list<Assertion>> $anded_types
*
* @return list<non-empty-list<string>>
* @return list<non-empty-list<Assertion>>
*/
function (array $anded_types): array {
if (count($anded_types) > 1) {
Expand All @@ -48,7 +48,7 @@ function (array $anded_types): array {
return [];
}

$new_anded_types[] = self::negateType($orred_types[0]);
$new_anded_types[] = $orred_types[0]->getNegation();
}

return [$new_anded_types];
Expand All @@ -57,7 +57,7 @@ function (array $anded_types): array {
$new_orred_types = [];

foreach ($anded_types[0] as $orred_type) {
$new_orred_types[] = [self::negateType($orred_type)];
$new_orred_types[] = [$orred_type->getNegation()];
}

return $new_orred_types;
Expand All @@ -67,18 +67,6 @@ function (array $anded_types): array {
);
}

/**
* @psalm-pure
*/
public static function negateType(string $type): string
{
if ($type === 'mixed') {
return $type;
}

return $type[0] === '!' ? substr($type, 1) : '!' . $type;
}

/**
* This is a very simple simplification heuristic
* for CNF formulae.
Expand Down Expand Up @@ -152,14 +140,12 @@ public static function simplifyCNF(array $clauses): array
foreach ($clause_a->possibilities as $key => $a_possibilities) {
$b_possibilities = $clause_b->possibilities[$key];

if ($a_possibilities === $b_possibilities) {
if ($clause_a->possibility_strings[$key] === $clause_b->possibility_strings[$key]) {
continue;
}

if (count($a_possibilities) === 1 && count($b_possibilities) === 1) {
if ($a_possibilities[0] === '!' . $b_possibilities[0]
|| $b_possibilities[0] === '!' . $a_possibilities[0]
) {
if ($a_possibilities[0]->isNegationOf($b_possibilities[0])) {
$opposing_keys[] = $key;
continue;
}
Expand Down Expand Up @@ -187,38 +173,45 @@ public static function simplifyCNF(array $clauses): array

$clause_var = array_keys($clause_a->possibilities)[0];
$only_type = array_pop(array_values($clause_a->possibilities)[0]);
$negated_clause_type = self::negateType($only_type);
$negated_clause_type = $only_type->getNegation();
$negated_clause_type_string = (string)$negated_clause_type;

foreach ($cloned_clauses as $clause_b_hash => $clause_b) {
if ($clause_a === $clause_b || !$clause_b->reconcilable || $clause_b->wedge) {
continue;
}

if (isset($clause_b->possibilities[$clause_var]) &&
in_array($negated_clause_type, $clause_b->possibilities[$clause_var], true)
) {
$clause_var_possibilities = array_values(
array_filter(
$clause_b->possibilities[$clause_var],
fn(string $possible_type): bool => $possible_type !== $negated_clause_type
)
);
if (isset($clause_b->possibilities[$clause_var])) {
$unmatched = [];
$matched = [];

foreach ($clause_b->possibilities[$clause_var] as $possible_type) {
if ((string)$possible_type === $negated_clause_type_string) {
$matched[] = $possible_type;
} else {
$unmatched[] = $possible_type;
}
}

unset($cloned_clauses[$clause_b_hash]);
if ($matched) {
$clause_var_possibilities = $unmatched;

if (!$clause_var_possibilities) {
$updated_clause = $clause_b->removePossibilities($clause_var);
unset($cloned_clauses[$clause_b_hash]);

if (!$clause_var_possibilities) {
$updated_clause = $clause_b->removePossibilities($clause_var);

if ($updated_clause) {
$cloned_clauses[$updated_clause->hash] = $updated_clause;
}
} else {
$updated_clause = $clause_b->addPossibilities(
$clause_var,
$clause_var_possibilities
);

if ($updated_clause) {
$cloned_clauses[$updated_clause->hash] = $updated_clause;
}
} else {
$updated_clause = $clause_b->addPossibilities(
$clause_var,
$clause_var_possibilities
);

$cloned_clauses[$updated_clause->hash] = $updated_clause;
}
}
}
Expand Down Expand Up @@ -257,9 +250,9 @@ public static function simplifyCNF(array $clauses): array
*
* @param list<Clause> $clauses
* @param array<string, bool> $cond_referenced_var_ids
* @param array<string, array<int, array<int, string>>> $active_truths
* @param array<string, array<int, array<int, Assertion>>> $active_truths
*
* @return array<string, list<array<int, string>>>
* @return array<string, list<list<Assertion>>>
*/
public static function getTruthsFromFormula(
array $clauses,
Expand Down Expand Up @@ -303,23 +296,23 @@ public static function getTruthsFromFormula(
}
} else {
// if there's only one active clause, return all the non-negation clause members ORed together
$things_that_can_be_said = array_filter(
$possible_types,
fn(string $possible_type): bool => $possible_type[0] !== '!'
);
$things_that_can_be_said = [];

if ($things_that_can_be_said && count($things_that_can_be_said) === count($possible_types)) {
$things_that_can_be_said = array_unique($things_that_can_be_said);
foreach ($possible_types as $assertion) {
if ($assertion instanceof Falsy || !$assertion->isNegation()) {
$things_that_can_be_said[(string)$assertion] = $assertion;
}
}

if ($things_that_can_be_said && count($things_that_can_be_said) === count($possible_types)) {
if ($clause->generated && count($possible_types) > 1) {
unset($cond_referenced_var_ids[$var]);
}

/** @var array<int, string> $things_that_can_be_said */
$truths[$var] = [$things_that_can_be_said];
$truths[$var] = [array_values($things_that_can_be_said)];

if ($creating_conditional_id && $creating_conditional_id === $clause->creating_conditional_id) {
$active_truths[$var] = [$things_that_can_be_said];
$active_truths[$var] = [array_values($things_that_can_be_said)];
}
}
}
Expand Down Expand Up @@ -396,7 +389,7 @@ public static function groupImpossibilities(array $clauses): array
$ith = $new_clause_possibilities[$var][$i];
$jth = $new_clause_possibilities[$var][$j];

if ($ith === '!' . $jth || $jth === '!' . $ith) {
if ($ith->isNegationOf($jth)) {
$removed_indexes[$i] = true;
$removed_indexes[$j] = true;
}
Expand Down Expand Up @@ -492,7 +485,7 @@ public static function combineOredClauses(
continue;
}

/** @var array<string, non-empty-list<string>> */
/** @var array<string, non-empty-list<Assertion>> */
$possibilities = [];

$can_reconcile = true;
Expand Down Expand Up @@ -533,9 +526,7 @@ public static function combineOredClauses(

foreach ($possibilities as $var_possibilities) {
if (count($var_possibilities) === 2) {
if ($var_possibilities[0] === '!' . $var_possibilities[1]
|| $var_possibilities[1] === '!' . $var_possibilities[0]
) {
if ($var_possibilities[0]->isNegationOf($var_possibilities[1])) {
continue 2;
}
}
Expand Down
16 changes: 4 additions & 12 deletions src/Psalm/Internal/Algebra/FormulaGenerator.php
View file
Open in desktop
Expand Up @@ -12,11 +12,11 @@
use Psalm\Node\Expr\BinaryOp\VirtualBooleanAnd;
use Psalm\Node\Expr\BinaryOp\VirtualBooleanOr;
use Psalm\Node\Expr\VirtualBooleanNot;
use Psalm\Storage\Assertion\Truthy;

use function array_merge;
use function count;
use function spl_object_id;
use function strlen;
use function substr;

/**
Expand Down Expand Up @@ -165,11 +165,7 @@ public static function getFormula(
spl_object_id($conditional->expr),
false,
true,
$orred_types[0][0] === '='
|| $orred_types[0][0] === '~'
|| (strlen($orred_types[0]) > 1
&& ($orred_types[0][1] === '='
|| $orred_types[0][1] === '~')),
$orred_types[0]->hasEquality(),
$redefined ? [$var => true] : []
);
}
Expand Down Expand Up @@ -436,11 +432,7 @@ public static function getFormula(
$creating_object_id,
false,
true,
$orred_types[0][0] === '='
|| $orred_types[0][0] === '~'
|| (strlen($orred_types[0]) > 1
&& ($orred_types[0][1] === '='
|| $orred_types[0][1] === '~')),
$orred_types[0]->hasEquality(),
$redefined ? [$var => true] : []
);
}
Expand All @@ -455,6 +447,6 @@ public static function getFormula(
$conditional_ref = '*' . $conditional->getAttribute('startFilePos')
. ':' . $conditional->getAttribute('endFilePos');

return [new Clause([$conditional_ref => ['!falsy']], $conditional_object_id, $creating_object_id)];
return [new Clause([$conditional_ref => [new Truthy()]], $conditional_object_id, $creating_object_id)];
}
}
5 changes: 3 additions & 2 deletions src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php
View file
Open in desktop
Expand Up @@ -10,11 +10,12 @@
use Psalm\Issue\ParadoxicalCondition;
use Psalm\Issue\RedundantCondition;
use Psalm\IssueBuffer;
use Psalm\Storage\Assertion\InArray;
use Psalm\Storage\Assertion\NotInArray;

use function array_intersect_key;
use function count;
use function implode;
use function preg_match;

/**
* @internal
Expand Down Expand Up @@ -100,7 +101,7 @@ public static function checkForParadox(
break;
}
foreach ($keyed_possibilities as $possibility) {
if (preg_match('@^!?in-array-@', $possibility)) {
if ($possibility instanceof InArray || $possibility instanceof NotInArray) {
$negated_clause_2_contains_1_possibilities = false;
break;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Open in desktop
Expand Up @@ -147,6 +147,7 @@ public static function analyze(
$array_type = new TList($item_value_type ?? Type::getMixed());
} else {
$array_type = new TNonEmptyList($item_value_type ?? Type::getMixed());
/** @psalm-suppress PossiblyInvalidPropertyAssignmentValue */
$array_type->count = count($array_creation_info->property_types);
}

Expand Down Expand Up @@ -234,6 +235,7 @@ public static function analyze(
$item_value_type ?? Type::getMixed(),
]);

/** @psalm-suppress PossiblyInvalidPropertyAssignmentValue */
$array_type->count = count($array_creation_info->property_types);

$stmt_type = new Union([
Expand Down