Skip to content

Commit

Permalink
Consider uninitialized typed properties as possibly undefined
Browse files Browse the repository at this point in the history
`isset()` & friends can be used to check if a typed property has been
initialized. Phan does not consider the property as possibly undefined,
so we need to do that explicitly to avoid emitting an issue.

Bug: T378286
Change-Id: I6f9463550fd3eba63f25e8b4273bf4fe08fbab0e
  • Loading branch information
Daimona committed Oct 27, 2024
1 parent 1beb4dd commit 772ffe1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/Plugin/RedundantExistenceChecksVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ private function exprIsPossiblyUndefined( Node $expr ): bool {
// stdClass or another class with dynamic properties. These are always possibly undefined.
return true;
}

if ( !$property->getUnionType()->getRealUnionType()->isEmpty() ) {
// T378286: If it's a typed property without default value, do not emit an issue. `isset()` can be used
// to check if the property has been initialized. See also https://github.com/phan/phan/issues/4720
$defaultType = $property->getDefaultType();
if ( $defaultType && $defaultType->getRealUnionType()->isNull() ) {
return true;
}
}
}

$prevIgnoreUndefFlag = $expr->flags & PhanAnnotationAdder::FLAG_IGNORE_UNDEF;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
plugins/RedundantExistenceChecksPlugin/isset-typedprop/test.php:13 MediaWikiNoIssetIfDefined Found usage of isset() on expression $this->strWithDefault that appears to be always set. isset() should only be used to suppress errors. Check whether the expression is null instead. See https://w.wiki/98zs
plugins/RedundantExistenceChecksPlugin/isset-typedprop/test.php:14 MediaWikiNoIssetIfDefined Found usage of isset() on expression $this->untypedWithDoc that appears to be always set. isset() should only be used to suppress errors. Check whether the expression is null instead. See https://w.wiki/98zs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

class TestTypedProperty {
private stdClass $obj1;
private string $str;
private string $strWithDefault = 'foo';
/** @var string */
private $untypedWithDoc;

function doTest() {
print_r( isset( $this->obj1 ) ); // No issue (to avoid false positives)
print_r( isset( $this->str ) ); // No issue (to avoid false positives)
print_r( isset( $this->strWithDefault ) ); // Redundant isset
print_r( isset( $this->untypedWithDoc ) ); // Redundant isset

$this->str = 'XcQ';
print_r( isset( $this->str ) ); // No issue (due to upstream limitation)
}
}

0 comments on commit 772ffe1

Please sign in to comment.