danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 20:34:47 +01:00
Code Issues Projects Releases Wiki Activity

Add check for paradoxical statements

Browse Source
This commit is contained in:
Matthew Brown 2017-04-02 15:26:10 -04:00
parent 83edf8c4db
commit dcedd65215
12 changed files with 278 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config.xsd
View File

@ -141,6 +141,7 @@
<xs:element name="NullPropertyFetch" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullReference" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenMethodAccess" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParadoxicalCondition" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParentNotFound" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyUndefinedVariable" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyNullArgument" type="IssueHandlerType" minOccurs="0" />

85
src/Psalm/Checker/AlgebraChecker.php
View File

@ -4,6 +4,9 @@ namespace Psalm\Checker;
use PhpParser;
use Psalm\Checker\Statements\Expression\AssertionFinder;
use Psalm\Clause;
use Psalm\CodeLocation;
use Psalm\IssueBuffer;
use Psalm\Issue\ParadoxicalCondition;
use Psalm\StatementsSource;
class AlgebraChecker
@ -156,25 +159,73 @@ class AlgebraChecker
return;
}
$clause->impossibilities = array_map(
$impossibilities = [];
foreach ($clause->possibilities as $var_id => $possiblity) {
$impossibility = [];
foreach ($possiblity as $type) {
if ($type[0] !== '^') {
$impossibility[] = TypeChecker::negateType($type);
}
}
if ($impossibility) {
$impossibilities[$var_id] = $impossibility;
}
}
$clause->impossibilities = $impossibilities;
}
/**
* @param array<string> $types
* @return array<string>
* This looks to see if there are any clauses in one formula that contradict
* clauses in another formula
*
* e.g.
* if ($a) { }
* elseif ($a) { }
*
* @param array<int, Clause> $formula1
* @param array<int, Clause> $formula2
* @param StatementsChecker $statements_checker,
* @param PhpParser\Node $stmt
* @return void
*/
function (array $types) {
return array_map(
/**
* @param string $type
* @return string
*/
function ($type) {
return TypeChecker::negateType($type);
},
$types
);
},
$clause->possibilities
);
public static function checkForParadox(
array $formula1,
array $formula2,
StatementsChecker $statements_checker,
PhpParser\Node $stmt
) {
// remove impossible types
foreach ($formula1 as $clause_a) {
if (!$clause_a->reconcilable || $clause_a->wedge) {
continue;
}
self::calculateNegation($clause_a);
foreach ($formula2 as $clause_b) {
if ($clause_a === $clause_b || !$clause_b->reconcilable || $clause_b->wedge) {
continue;
}
if ($clause_a->impossibilities == $clause_b->possibilities) {
if (IssueBuffer::accepts(
new ParadoxicalCondition(
'Encountered a paradox when evaluating the conditional',
new CodeLocation($statements_checker, $stmt)
),
$statements_checker->getSuppressedIssues()
)) {
// fall through
}
return;
}
}
}
}
/**

2
src/Psalm/Checker/Statements/Block/ForChecker.php
View File

@ -63,7 +63,7 @@ class ForChecker
$for_context->vars_in_scope[$var]
);
$context->removeVarFromClauses($var);
$context->removeVarFromConflictingClauses($var);
}
}

2
src/Psalm/Checker/Statements/Block/ForeachChecker.php
View File

@ -228,7 +228,7 @@ class ForeachChecker
$foreach_context->vars_in_scope[$var]
);
$context->removeVarFromClauses($var);
$context->removeVarFromConflictingClauses($var);
}
}

13
src/Psalm/Checker/Statements/Block/IfChecker.php
View File

@ -98,6 +98,9 @@ class IfChecker
$statements_checker
);
// this will see whether any of the clauses in set A conflict with the clauses in set B
AlgebraChecker::checkForParadox($context->clauses, $if_clauses, $statements_checker, $stmt->cond);
$if_context->clauses = AlgebraChecker::simplifyCNF(array_merge($context->clauses, $if_clauses));
$if_scope->negated_clauses = AlgebraChecker::negateFormula($if_clauses);
@ -351,7 +354,7 @@ class IfChecker
);
foreach ($changed_vars as $changed_var) {
$outer_context->removeVarFromClauses($changed_var);
$outer_context->removeVarFromConflictingClauses($changed_var);
}
if ($outer_context_vars_reconciled === false) {
@ -460,10 +463,14 @@ class IfChecker
$statements_checker
);
$entry_clauses = array_merge($original_context->clauses, $if_scope->negated_clauses);
// this will see whether any of the clauses in set A conflict with the clauses in set B
AlgebraChecker::checkForParadox($entry_clauses, $elseif_clauses, $statements_checker, $elseif->cond);
$elseif_context->clauses = AlgebraChecker::simplifyCNF(
array_merge(
$original_context->clauses,
$if_scope->negated_clauses,
$entry_clauses,
$elseif_clauses
)
);

4
src/Psalm/Checker/Statements/Block/WhileChecker.php
View File

@ -39,6 +39,8 @@ class WhileChecker
$statements_checker
);
$while_context->parent_context = $context;
$while_context->clauses = AlgebraChecker::simplifyCNF(array_merge($context->clauses, $while_clauses));
$reconcilable_while_types = AlgebraChecker::getTruthsFromFormula($while_context->clauses);
@ -113,7 +115,7 @@ class WhileChecker
$context->vars_in_scope = $vars_in_scope_reconciled;
foreach ($changed_vars as $changed_var) {
$context->removeVarFromClauses($changed_var);
$context->removeVarFromConflictingClauses($changed_var);
}
}
}

11
src/Psalm/Checker/Statements/Expression/AssignmentChecker.php
View File

@ -101,7 +101,7 @@ class AssignmentChecker
if ($assign_value && ExpressionChecker::analyze($statements_checker, $assign_value, $context) === false) {
if ($var_id) {
if ($array_var_id) {
$context->removeDescendents($array_var_id);
$context->removeDescendents($array_var_id, null, $assign_value_type);
}
// if we're not exiting immediately, make everything mixed
@ -123,10 +123,13 @@ class AssignmentChecker
}
if ($array_var_id && isset($context->vars_in_scope[$array_var_id])) {
if ((string)$context->vars_in_scope[$array_var_id] !== (string)$assign_value_type) {
// removes dependennt vars from $context
$context->removeDescendents($array_var_id);
}
$context->removeDescendents(
$array_var_id,
$context->vars_in_scope[$array_var_id],
$assign_value_type,
$statements_checker->getFileChecker()
);
}
if ($assign_value_type->isMixed()) {

4
src/Psalm/Checker/TypeChecker.php
View File

@ -1070,9 +1070,7 @@ class TypeChecker
*/
public static function simplifyUnionType(Type\Union $union, FileChecker $file_checker)
{
$union_type_count = count($union->types);
if ($union_type_count === 1 || ($union_type_count === 2 && $union->isNullable())) {
if (count($union->types) === 1) {
return $union;
}

72
src/Psalm/Context.php
View File

@ -1,6 +1,9 @@
<?php
namespace Psalm;
use Psalm\Type\Union;
use Psalm\Checker\FileChecker;
class Context
{
/**
@ -242,52 +245,93 @@ class Context
unset($this->vars_possibly_in_scope[$remove_var_id]);
if (isset($this->vars_in_scope[$remove_var_id])) {
$type = $this->vars_in_scope[$remove_var_id];
$existing_type = $this->vars_in_scope[$remove_var_id];
unset($this->vars_in_scope[$remove_var_id]);
$this->removeDescendents($remove_var_id, $type);
$this->removeDescendents($remove_var_id, $existing_type);
}
}
/**
* @param string $remove_var_id
* @param Union|null $new_type
* @param FileChecker|null $file_checker
* @return void
*/
public function removeVarFromClauses($remove_var_id)
{
public function removeVarFromConflictingClauses(
$remove_var_id,
Union $new_type = null,
FileChecker $file_checker = null
) {
$clauses_to_keep = [];
$new_type_string = (string)$new_type;
foreach ($this->clauses as $clause) {
if (!isset($clause->possibilities[$remove_var_id])) {
\Psalm\Checker\AlgebraChecker::calculateNegation($clause);
if (!isset($clause->possibilities[$remove_var_id]) ||
$clause->possibilities[$remove_var_id] === [$new_type_string]
) {
$clauses_to_keep[] = $clause;
} elseif ($file_checker && $new_type && !in_array('empty', $clause->possibilities[$remove_var_id])) {
$type_changed = false;
// if the clause contains any possibilities that would be altered
foreach ($clause->possibilities[$remove_var_id] as $type) {
$result_type = \Psalm\Checker\TypeChecker::reconcileTypes(
$type,
clone $new_type,
null,
$file_checker,
null,
[],
$failed_reconciliation
);
if ((string)$result_type !== $new_type_string) {
$type_changed = true;
break;
}
}
if (!$type_changed) {
$clauses_to_keep[] = $clause;
}
}
}
$this->clauses = $clauses_to_keep;
if ($this->parent_context) {
$this->parent_context->removeVarFromClauses($remove_var_id);
$this->parent_context->removeVarFromConflictingClauses($remove_var_id);
}
}
/**
* @param string $remove_var_id
* @param \Psalm\Type\Union|null $type
* @param \Psalm\Type\Union|null $existing_type
* @param \Psalm\Type\Union|null $new_type
* @param FileChecker|null $file_checker
* @return void
*/
public function removeDescendents($remove_var_id, \Psalm\Type\Union $type = null)
{
if (!$type && isset($this->vars_in_scope[$remove_var_id])) {
$type = $this->vars_in_scope[$remove_var_id];
public function removeDescendents(
$remove_var_id,
Union $existing_type = null,
Union $new_type = null,
FileChecker $file_checker = null
) {
if (!$existing_type && isset($this->vars_in_scope[$remove_var_id])) {
$existing_type = $this->vars_in_scope[$remove_var_id];
}
if (!$type) {
if (!$existing_type) {
return;
}
$this->removeVarFromClauses($remove_var_id);
$this->removeVarFromConflictingClauses($remove_var_id, $new_type, $file_checker);
if ($type->hasArray() || $type->isMixed()) {
if ($existing_type->hasArray() || $existing_type->isMixed()) {
$vars_to_remove = [];
foreach ($this->vars_in_scope as $var_id => $_) {

6
src/Psalm/Issue/ParadoxicalCondition.php Normal file
View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class ParadoxicalCondition extends CodeError
{
}

4
src/Psalm/Type.php
View File

@ -672,10 +672,6 @@ abstract class Type
$value_types[$type_key] = [];
}
if ($type instanceof TArray) {
throw new \InvalidArgumentException('Cannot have a non-generic array');
}
$value_types[$type_key][(string) $type] = null;
}
}

118
tests/TypeAlgebraTest.php
View File

@ -489,4 +489,122 @@ class TypeAlgebraTest extends PHPUnit_Framework_TestCase
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
/**
* @expectedException \Psalm\Exception\CodeException
* @expectedExceptionMessage ParadoxicalCondition
* @return void
*/
public function testRepeatedIfStatements()
{
$stmts = self::$parser->parse('<?php
/** @return string|null */
function foo(?string $a) {
if ($a) {
return $a;
}
if ($a) {
}
}
');
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
/**
* @expectedException \Psalm\Exception\CodeException
* @expectedExceptionMessage ParadoxicalCondition
* @return void
*/
public function testRepeatedConditionals()
{
$stmts = self::$parser->parse('<?php
function foo(?string $a) : void {
if ($a) {
// do something
} elseif ($a) {
// can never get here
}
}
');
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
/**
* This shouldn't throw an error
*
* @return void
*/
public function testDifferentValueChecks()
{
$stmts = self::$parser->parse('<?php
function foo(string $a) : void {
if ($a === "foo") {
// do something
} elseif ($a === "bar") {
// can never get here
}
}
');
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
/**
* This shouldn't throw an error
*
* @return void
*/
public function testRepeatedSet()
{
$stmts = self::$parser->parse('<?php
function foo() : void {
if ($a = rand(0, 1) ? "" : null) {
return;
}
if (rand(0, 1)) {
$a = rand(0, 1) ? "hello" : null;
if ($a) {
}
}
}
');
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
/**
* @return void
*/
public function testRepeatedSetInsideWhile()
{
$stmts = self::$parser->parse('<?php
function foo() : void {
if ($a = rand(0, 1) ? "" : null) {
return;
} else {
while (rand(0, 1)) {
$a = rand(0, 1) ? "hello" : null;
}
if ($a) {
}
}
}
');
$file_checker = new FileChecker('somefile.php', $this->project_checker, $stmts);
$file_checker->visitAndAnalyzeMethods();
}
}