Skip to content

Commit

Permalink
Fixed false positives introduced in 0.12.6
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 20, 2020
1 parent 7c0fdfe commit 4b18031
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ private function processStmtNode(
new StatementExitPoint($stmt, $scope),
]);
} elseif ($stmt instanceof If_) {
$conditionType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($stmt->cond)->toBoolean();
$conditionType = $scope->getType($stmt->cond)->toBoolean();
$ifAlwaysTrue = $conditionType instanceof ConstantBooleanType && $conditionType->getValue();
$condResult = $this->processExprNode($stmt->cond, $scope, $nodeCallback, ExpressionContext::createDeep());
$exitPoints = [];
Expand All @@ -545,7 +545,7 @@ private function processStmtNode(
$condScope = $scope;
foreach ($stmt->elseifs as $elseif) {
$nodeCallback($elseif, $scope);
$elseIfConditionType = $condScope->doNotTreatPhpDocTypesAsCertain()->getType($elseif->cond)->toBoolean();
$elseIfConditionType = $condScope->getType($elseif->cond)->toBoolean();
$condResult = $this->processExprNode($elseif->cond, $condScope, $nodeCallback, ExpressionContext::createDeep());
$condScope = $condResult->getScope();
$branchScopeStatementResult = $this->processStmtNodes($elseif, $elseif->stmts, $condResult->getTruthyScope(), $nodeCallback);
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/native-types.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public function doIfElse(\DateTimeInterface $date): void
}

assertType(\DateTimeImmutable::class, $date);
assertNativeType(\DateTimeInterface::class, $date);
assertNativeType(\DateTimeImmutable::class, $date); // could be DateTimeInterface

if ($date instanceof \DateTime) {

Expand Down
8 changes: 4 additions & 4 deletions tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ public function testInstanceof(): void
[
'Instanceof between ImpossibleInstanceOf\Bar&ImpossibleInstanceOf\Foo and ImpossibleInstanceOf\Foo will always evaluate to true.',
238,
$tipText,
//$tipText,
],
[
'Instanceof between *NEVER* and ImpossibleInstanceOf\Bar will always evaluate to false.',
240,
$tipText,
//$tipText,
],
[
'Instanceof between object and Exception will always evaluate to false.',
Expand Down Expand Up @@ -212,7 +212,7 @@ public function testInstanceofWithoutAlwaysTrue(): void
[
'Instanceof between *NEVER* and ImpossibleInstanceOf\Bar will always evaluate to false.',
240,
$tipText,
//$tipText,
],
[
'Instanceof between object and Exception will always evaluate to false.',
Expand Down Expand Up @@ -295,7 +295,7 @@ public function testReportTypesFromPhpDocs(): void
[
'Instanceof between DateTimeImmutable and DateTime will always evaluate to false.',
36,
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
//'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
],
]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ public function testRule(): void
[
'Result of && is always true.',
64,
$tipText,
//$tipText,
],
[
'Result of && is always false.',
66,
$tipText,
//$tipText,
],
[
'Result of && is always false.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ public function testReportPhpDoc(): void
[
'Else branch is unreachable because previous condition is always true.',
54,
$tipText,
//$tipText,
],
[
'Elseif branch is unreachable because previous condition is always true.',
64,
$tipText,
//$tipText,
],
]);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,9 @@ public function testRuleTopLevel(): void
]);
}

public function testBugWithoutGitHubIssue1(): void
{
$this->analyse([__DIR__ . '/data/bug-without-issue-1.php'], []);
}

}
117 changes: 117 additions & 0 deletions tests/PHPStan/Rules/DeadCode/data/bug-without-issue-1.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<?php

namespace BugWithoutIssue1;

interface User {
function getId() : int;
}

interface Message {
/** @return object|null */
public function getScheduleRequest();
public function getFromUser() : User;
}

interface MessageThread {
/** @return object|null */
public function getLastScheduleRequest();
/** @return User[] */
public function getParticipants() : array;
/** @return Message[] */
public function getMessages() : array;
}

class X
{
public function checkAndSendThreadNotRepliedNotification(MessageThread $thread) : bool
{
$threadSR = $thread->getLastScheduleRequest();

$p = [];
foreach($thread->getParticipants() as $user)
$p[$user->getId()] = $user;

$reminderMsg = null;
$reachedSR = !$threadSR;
foreach($thread->getMessages() as $msg)
{
$msgSR = $msg->getScheduleRequest();
if(!$reachedSR && ($threadSR && $msgSR != $threadSR))
continue;

$reachedSR = true;

if(!$reminderMsg)
$reminderMsg = $msg;

unset($p[$msg->getFromUser()->getId()]);

if(!$p)
return false;
}

if(!$reminderMsg)
throw new \UnexpectedValueException('Expected a reminderMsg but got null for thread');

return true;
}
}

class Foo
{
/** @var int */
protected $index = 0;

/** @var string[][] */
protected $data = [
0 => ['type' => 'id', 'value' => 'foo'],
1 => ['type' => 'special', 'value' => '.'],
2 => ['type' => 'id', 'value' => 'bar'],
3 => ['type' => 'special', 'value' => ';'],
];

protected function next(): void
{
$this->index = $this->index + 1;
}

protected function check(string $type, ?string $value = null): bool {
return ($this->type() === $type) && (($value === null) || ($this->value() === $value));
}

protected function type(): string
{
return $this->data[$this->index]['type'];
}

protected function value(): string
{
return $this->data[$this->index]['value'];
}

public function separatedName(): string
{
$name = '';
$previousType = null;
$separator = '.';

$currentValue = $this->value();

while ((($this->check('special', $separator)) || ($this->check('id'))) &&
(($previousType === null) || ($this->type() !== $previousType)) &&
(($previousType !== null) || ($currentValue !== $separator))
) {
$name .= $currentValue;
$previousType = $this->type();

$this->next();
$currentValue = $this->value();
}

if (($previousType === null) || ($previousType !== 'id')) {
throw new \RuntimeException();
}

return $name;
}
}
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,10 @@ public function testCheckMissingReturnWithTemplateMixedType(): void
]);
}

public function testBug2875(): void
{
$this->checkExplicitMixedMissingReturn = true;
$this->analyse([__DIR__ . '/data/bug-2875.php'], []);
}

}
18 changes: 18 additions & 0 deletions tests/PHPStan/Rules/Missing/data/bug-2875.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace Bug2875MissingReturn;

class A {}
class B {}

class HelloWorld
{
/** @param A|B|null $obj */
function one($obj): int
{
if ($obj === null) return 1;
else if ($obj instanceof A) return 2;
else if ($obj instanceof B) return 3;
}

}

0 comments on commit 4b18031

Please sign in to comment.