Skip to content

Commit

Permalink
Fix NoEmptyIfDefined behaviour with dynamic properties
Browse files Browse the repository at this point in the history
Use hasDynamicProperties(), which already checks for stdClass, and also
for the AllowDynamicProperties attribute as of phan 5.4.2.

Also mention the addition of the NoBaseException plugin in the release
notes.

Bug: T349432
Change-Id: I61acc0f6924e14b76fb6a2199dd67d5041c4e650
  • Loading branch information
Daimona committed Oct 22, 2023
1 parent 34590f1 commit 679ac25
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 3 deletions.
4 changes: 4 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# MediaWiki-Phan-Config release history #

## UNRELEASED
* Add plugin to disallow use of `new Exception` (Daimona Eaytoy)
* Do not emit MediaWikiNoEmptyIfDefined for properties of classes with the AllowDynamicProperties attribute (Daimona Eaytoy)

## 0.13.0 / 2023-09-08
* Add plugin to forbid `empty()` on defined variables and properties (Daimona Eaytoy)
* Bump phan to 5.4.2 and taint-check to 5.0.0 (Michael Große)
Expand Down
5 changes: 2 additions & 3 deletions src/Plugin/NoEmptyIfDefinedVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use Phan\Exception\IssueException;
use Phan\Exception\NodeException;
use Phan\Exception\UnanalyzableException;
use Phan\Language\FQSEN\FullyQualifiedClassName;
use Phan\PluginV3\PluginAwarePostAnalysisVisitor;
use const ast\AST_PROP;
use const ast\AST_STATIC_PROP;
Expand Down Expand Up @@ -54,8 +53,8 @@ public function visitEmpty( Node $node ): void {
// be empty, but not possibly undefined, yet we shouldn't emit an issue.
return;
}
if ( $property->getClassFQSEN() === FullyQualifiedClassName::getStdClassFQSEN() ) {
// Properties of stdClass are always possibly undefined.
if ( $property->getClass( $this->code_base )->hasDynamicProperties( $this->code_base ) ) {
// stdClass or another class with dynamic properties. These are always possibly undefined.
return;
}
}
Expand Down
Empty file.
53 changes: 53 additions & 0 deletions tests/plugins/NoEmptyIfDefinedPlugin/dynamicprop/test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

// @phan-file-suppress PhanUndeclaredProperty,PhanNoopEmpty
// @phan-file-suppress PhanUndeclaredClassAttribute,UnusedPluginSuppression,UnusedPluginFileSuppression
// Different suppressions are needed in different PHP versions (7.4 does not "see" the attribute at all, 8.0 and 8.1
// see it but can't find the class, 8.2 sees it and knows what it is).

class ClassForDynamicPropWithoutAttribute {
}

#[AllowDynamicProperties]
class ClassForDynamicPropWithAttribute {
}

function testWithoutAttrib( ClassForDynamicPropWithoutAttribute $param ) {
$withoutAttrib = new ClassForDynamicPropWithoutAttribute();
empty( $withoutAttrib->dynamicProp );
$withoutAttrib->dynamicProp2 = 'Hello';
empty( $withoutAttrib->dynamicProp2 );

empty( $param->dynamicProp );
empty( $param->dynamicProp2 );
empty( $param->dynamicProp3 );
}

function testWithAttrib( ClassForDynamicPropWithAttribute $param ) {
$withAttrib = new ClassForDynamicPropWithAttribute();
empty( $withAttrib->dynamicProp );
$withAttrib->dynamicProp2 = 'Hello';
empty( $withAttrib->dynamicProp2 );

empty( $param->dynamicProp );
empty( $param->dynamicProp2 );
empty( $param->dynamicProp3 );
}

#[AllowDynamicProperties]
class TestAttributeInheritanceBase {}

class TestAttributeInheritanceChild extends TestAttributeInheritanceBase {}

class ExtendsStdClass extends stdClass {}

function testInheritance() {
$base = new TestAttributeInheritanceBase();
empty( $base->x );

$child = new TestAttributeInheritanceChild();
empty( $child->y );

$extendsStdClass = new ExtendsStdClass();
empty( $extendsStdClass->z );
}

0 comments on commit 679ac25

Please sign in to comment.