Skip to content

Commit

Permalink
Fix #1546 - catch impossible assertions to true
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Apr 12, 2019
1 parent ea20a2b commit 39af691
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 58 deletions.
116 changes: 58 additions & 58 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Expand Up @@ -480,33 +480,38 @@ private static function scrapeEqualityAssertions(
throw new \UnexpectedValueException('Unrecognised position'); throw new \UnexpectedValueException('Unrecognised position');
} }


$var_type = isset($base_conditional->inferredType) ? $base_conditional->inferredType : null;

if ($base_conditional instanceof PhpParser\Node\Expr\FuncCall) { if ($base_conditional instanceof PhpParser\Node\Expr\FuncCall) {
$conditional->assertions = self::processFunctionCall( $if_types = self::processFunctionCall(
$base_conditional, $base_conditional,
$this_class_name, $this_class_name,
$source, $source,
false false
); );
return; } else {
} $var_name = ExpressionAnalyzer::getArrayVarId(

$base_conditional,
$var_name = ExpressionAnalyzer::getArrayVarId( $this_class_name,
$base_conditional, $source
$this_class_name, );
$source
);

$var_type = isset($base_conditional->inferredType) ? $base_conditional->inferredType : null;


if ($var_name) { if ($var_name) {
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) { if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\Identical) {
$if_types[$var_name] = [['true']]; $if_types[$var_name] = [['true']];
} else {
$if_types[$var_name] = [['!falsy']];
}
} else { } else {
$if_types[$var_name] = [['!falsy']]; self::scrapeAssertions(
$base_conditional,
$this_class_name,
$source,
$codebase,
$inside_negation
);
$if_types = $base_conditional->assertions;
} }
} else {
self::scrapeAssertions($base_conditional, $this_class_name, $source, $codebase, $inside_negation);
$if_types = $base_conditional->assertions;
} }


if ($codebase && $var_type) { if ($codebase && $var_type) {
Expand Down Expand Up @@ -1133,58 +1138,53 @@ private static function scrapeInequalityAssertions(


if ($true_position) { if ($true_position) {
if ($true_position === self::ASSIGNMENT_TO_RIGHT) { if ($true_position === self::ASSIGNMENT_TO_RIGHT) {
if ($conditional->left instanceof PhpParser\Node\Expr\FuncCall) {
$conditional->assertions = self::processFunctionCall(
$conditional->left,
$this_class_name,
$source,
true
);
return;
}

$base_conditional = $conditional->left; $base_conditional = $conditional->left;
} elseif ($true_position === self::ASSIGNMENT_TO_LEFT) { } elseif ($true_position === self::ASSIGNMENT_TO_LEFT) {
if ($conditional->right instanceof PhpParser\Node\Expr\FuncCall) {
$conditional->assertions = self::processFunctionCall(
$conditional->right,
$this_class_name,
$source,
true
);
return;
}

$base_conditional = $conditional->right; $base_conditional = $conditional->right;
} else { } else {
throw new \UnexpectedValueException('Bad null variable position'); throw new \UnexpectedValueException('Bad null variable position');
} }


$var_name = ExpressionAnalyzer::getArrayVarId(
$base_conditional,
$this_class_name,
$source
);

$var_type = isset($base_conditional->inferredType) ? $base_conditional->inferredType : null; $var_type = isset($base_conditional->inferredType) ? $base_conditional->inferredType : null;


if ($var_name) { if ($base_conditional instanceof PhpParser\Node\Expr\FuncCall) {
if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical) { $if_types = self::processFunctionCall(
$if_types[$var_name] = [['!true']]; $base_conditional,
} else { $this_class_name,
$if_types[$var_name] = [['falsy']]; $source,
} true
} elseif ($var_type) { );
self::scrapeAssertions($base_conditional, $this_class_name, $source, $codebase, $inside_negation); } else {
$var_name = ExpressionAnalyzer::getArrayVarId(
$base_conditional,
$this_class_name,
$source
);


if (!isset($base_conditional->assertions)) { if ($var_name) {
throw new \UnexpectedValueException('Assertions should be set'); if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical) {
} $if_types[$var_name] = [['!true']];
} else {
$if_types[$var_name] = [['falsy']];
}
} elseif ($var_type) {
self::scrapeAssertions(
$base_conditional,
$this_class_name,
$source,
$codebase,
$inside_negation
);


$notif_types = $base_conditional->assertions; if (!isset($base_conditional->assertions)) {
throw new \UnexpectedValueException('Assertions should be set');
}


if (count($notif_types) === 1) { $notif_types = $base_conditional->assertions;
$if_types = \Psalm\Type\Algebra::negateTypes($notif_types);
if (count($notif_types) === 1) {
$if_types = \Psalm\Type\Algebra::negateTypes($notif_types);
}
} }
} }


Expand Down
20 changes: 20 additions & 0 deletions tests/TypeReconciliationTest.php
View file
Expand Up @@ -1559,6 +1559,26 @@ function getA() : A {
if ($a instanceof A) {}', if ($a instanceof A) {}',
'error_message' => 'RedundantCondition', 'error_message' => 'RedundantCondition',
], ],
'preventImpossibleComparisonToTrue' => [
'<?php
/** @return false|string */
function firstChar(string $s) {
return empty($s) ? false : $s[0];
}
if (true === firstChar("sdf")) {}',
'error_message' => 'DocblockTypeContradiction',
],
'preventAlwaysPossibleComparisonToTrue' => [
'<?php
/** @return false|string */
function firstChar(string $s) {
return empty($s) ? false : $s[0];
}
if (true !== firstChar("sdf")) {}',
'error_message' => 'RedundantConditionGivenDocblockType',
],
]; ];
} }
} }

0 comments on commit 39af691

Please sign in to comment.