Skip to content

Commit

Permalink
Add more checks to DuplicateExpressionPlugin
Browse files Browse the repository at this point in the history
Fixes phan#2691
  • Loading branch information
TysonAndre committed May 1, 2019
1 parent 6358637 commit 13f3020
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 13 deletions.
96 changes: 87 additions & 9 deletions .phan/plugins/DuplicateExpressionPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,18 +282,96 @@ public function visitConditional(Node $node)
);
return;
}
if ($cond_node instanceof Node && $cond_node->kind === ast\AST_ISSET) {
if (ASTHasher::hash($cond_node->children['var']) === $true_node_hash) {
$this->emitPluginIssue(
$this->code_base,
$this->context,
'PhanPluginDuplicateConditionalNullCoalescing',
'"isset(X) ? X : Y" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was {CODE}',
[ASTReverter::toShortString($cond_node->children['var'])]
);
if (!$cond_node instanceof Node) {
return;
}
switch ($cond_node->kind) {
case ast\AST_ISSET:
if (ASTHasher::hash($cond_node->children['var']) === $true_node_hash) {
$this->warnDuplicateConditionalNullCoalescing('isset(X) ? X : Y', $node->children['true']);
}
break;
case ast\AST_BINARY_OP:
$this->checkBinaryOpOfConditional($cond_node, $true_node_hash);
break;
case ast\AST_UNARY_OP:
$this->checkUnaryOpOfConditional($cond_node, $true_node_hash);
break;
}
}

/**
* @param int|string $true_node_hash
*/
private function checkBinaryOpOfConditional(Node $cond_node, $true_node_hash)
{
if ($cond_node->flags !== ast\flags\BINARY_IS_NOT_IDENTICAL) {
return;
}
$left_node = $cond_node->children['left'];
$right_node = $cond_node->children['right'];
if (self::isNullConstantNode($left_node)) {
if (ASTHasher::hash($right_node) === $true_node_hash) {
$this->warnDuplicateConditionalNullCoalescing('null !== X ? X : Y', $right_node);
}
} elseif (self::isNullConstantNode($right_node)) {
if (ASTHasher::hash($left_node) === $true_node_hash) {
$this->warnDuplicateConditionalNullCoalescing('X !== null ? X : Y', $left_node);
}
}
}

/**
* @param int|string $true_node_hash
*/
private function checkUnaryOpOfConditional(Node $cond_node, $true_node_hash)
{
if ($cond_node->flags !== ast\flags\UNARY_BOOL_NOT) {
return;
}
$expr = $cond_node->children['expr'];
if (!$expr instanceof Node) {
return;
}
if ($expr->kind === ast\AST_CALL) {
$function = $expr->children['expr'];
if ($function->kind !== ast\AST_NAME || strcasecmp((string)($function->children['name'] ?? ''), 'is_null') !== 0) {
return;
}
$args = $expr->children['args']->children;
if (count($args) !== 1) {
return;
}
if (ASTHasher::hash($args[0]) === $true_node_hash) {
$this->warnDuplicateConditionalNullCoalescing('!is_null(X) ? X : Y', $args[0]);
}
}
}

/**
* @param Node|mixed $node
*/
private static function isNullConstantNode($node) : bool
{
if (!$node instanceof Node) {
return false;
}
return $node->kind === ast\AST_CONST && strcasecmp((string)$node->children['name']->children['name'] ?? '', 'null') === 0;
}

/**
* @param ?(Node|string|int|float) $x_node
*/
private function warnDuplicateConditionalNullCoalescing(string $expr, $x_node)
{
$this->emitPluginIssue(
$this->code_base,
$this->context,
'PhanPluginDuplicateConditionalNullCoalescing',
'"' . $expr . '" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was {CODE}',
[ASTReverter::toShortString($x_node)]
);
}
}

// Every plugin needs to return an instance of itself at the
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ New features(CLI, Configs):
New features(Analysis):
+ Emit `PhanDeprecatedClassConstant` for code using a constant marked with `@deprecated`.

Plugins:
+ Add more forms of checks such as `$x !== null ? $x : null` to `PhanPluginDuplicateConditionalNullCoalescing` (#2691)

28 Apr 2019, Phan 1.3.2
-----------------------
Expand Down
6 changes: 4 additions & 2 deletions src/Phan/CLI.php
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,8 @@ private static function isPathExcludedByRegex(
}

// Bound the percentage to [0, 1]
private static function boundPercentage(float $p) : float {
private static function boundPercentage(float $p) : float
{
return \min(\max($p, 0.0), 1.0);
}

Expand Down Expand Up @@ -1568,7 +1569,8 @@ public static function debugProgress(string $msg, float $p, $details)
/**
* @return void
*/
public static function debugOutput(string $line) {
public static function debugOutput(string $line)
{
\fwrite(STDERR, $line . "\n");
}

Expand Down
4 changes: 2 additions & 2 deletions src/Phan/Debug/Frame.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
use Phan\Language\Element\AddressableElement;
use Phan\Language\Element\UnaddressableTypedElement;
use Phan\Language\FQSEN;
use Phan\Library\None;
use Phan\Library\Some;
use Phan\Language\Type;
use Phan\Language\UnionType;
use Phan\Library\None;
use Phan\Library\Some;
use Phan\Library\StringUtil;
use function count;
use function get_class;
Expand Down
5 changes: 5 additions & 0 deletions tests/plugin_test/expected/118_more_isset_checks.php.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
src/118_more_isset_checks.php:2 PhanUnreferencedFunction Possibly zero references to function \example118()
src/118_more_isset_checks.php:3 PhanPluginDuplicateConditionalNullCoalescing "isset(X) ? X : Y" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was $arg
src/118_more_isset_checks.php:4 PhanPluginDuplicateConditionalNullCoalescing "!is_null(X) ? X : Y" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was $arg
src/118_more_isset_checks.php:5 PhanPluginDuplicateConditionalNullCoalescing "X !== null ? X : Y" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was $arg
src/118_more_isset_checks.php:6 PhanPluginDuplicateConditionalNullCoalescing "null !== X ? X : Y" can usually be simplified to "X ?? Y" in PHP 7. The duplicated expression X was $arg
7 changes: 7 additions & 0 deletions tests/plugin_test/src/118_more_isset_checks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php
function example118(?string $arg) {
echo isset($arg) ? $arg : 'default';
echo !is_null($arg) ? $arg : 'default';
echo ($arg !== null) ? $arg : 'default';
echo (null !== $arg) ? $arg : 'default';
}

0 comments on commit 13f3020

Please sign in to comment.