Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

Commit

Permalink
[PHPStanRules] Remove parent/previous from NoMultiArrayAssignRule (#4134
Browse files Browse the repository at this point in the history
)
  • Loading branch information
TomasVotruba committed Jun 9, 2022
1 parent 1fa07a4 commit 6383101
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 33 deletions.
73 changes: 44 additions & 29 deletions packages/phpstan-rules/src/Rules/NoMultiArrayAssignRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,28 @@

namespace Symplify\PHPStanRules\Rules;

use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\If_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use Symplify\Astral\ValueObject\AttributeKey;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use Symplify\PHPStanRules\Printer\NodeComparator;
use Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Symplify\PHPStanRules\Tests\Rules\NoMultiArrayAssignRule\NoMultiArrayAssignRuleTest
* @implements Rule<Node>
*/
final class NoMultiArrayAssignRule implements Rule, DocumentedRuleInterface
{
Expand All @@ -32,35 +39,49 @@ public function __construct(
) {
}

/**
* @return class-string<Node>
*/
public function getNodeType(): string
{
return Assign::class;
return Node::class;

This comment has been minimized.

Copy link
@staabm

staabm Jun 9, 2022

Contributor

Since we talked about performance… won‘t a rule with such a node-type be invoked over and over… do you think this can/could be improved?

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jun 9, 2022

Author Member

Yes, few lines bellow I link better solution ↓

nikic/PHP-Parser#836

}

/**
* @param Assign $node
* @return string[]
* @return RuleError[]
*/
public function processNode(Node $node, Scope $scope): array
{
if (! $node->var instanceof ArrayDimFetch) {
// check all of stmts aware nodes, see https://github.com/nikic/PHP-Parser/pull/836
if (! $node instanceof ClassMethod && ! $node instanceof Function_ && ! $node instanceof Closure && ! $node instanceof If_ && ! $node instanceof Else_) {
return [];
}

// is previous array dim assign too? - print the exprt conteont
$previousArrayDimFetch = $this->matchParentArrayDimFetch($node);
if (! $previousArrayDimFetch instanceof ArrayDimFetch) {
return [];
}
foreach ((array) $node->stmts as $key => $stmt) {
$firstArrayDimFetch = $this->matchAssignToArrayDimFetch($stmt);
if (! $firstArrayDimFetch instanceof ArrayDimFetch) {
continue;
}

if (! $this->haveSameArrayDimFetchNonEmptyRoot($node->var, $previousArrayDimFetch)) {
return [];
$nextStmt = $node->stmts[$key + 1] ?? null;
if (! $nextStmt instanceof Stmt) {
return [];
}

$secondArrayDimFetch = $this->matchAssignToArrayDimFetch($nextStmt);
if (! $secondArrayDimFetch instanceof ArrayDimFetch) {
continue;
}

if (! $this->haveSameArrayDimFetchNonEmptyRoot($firstArrayDimFetch, $secondArrayDimFetch)) {
continue;
}

$ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
->line($nextStmt->getLine())
->build();

return [$ruleError];
}

return [self::ERROR_MESSAGE];
return [];
}

public function getRuleDefinition(): RuleDefinition
Expand Down Expand Up @@ -105,27 +126,21 @@ private function resolveSingleNestedArrayDimFetch(ArrayDimFetch $arrayDimFetch):
return $arrayDimFetch;
}

private function matchParentArrayDimFetch(Assign $assign): ?Expr
private function matchAssignToArrayDimFetch(Stmt $stmt): ?ArrayDimFetch
{
$parent = $assign->getAttribute(AttributeKey::PARENT);
if (! $parent instanceof Expression) {
return null;
}

$previous = $parent->getAttribute(AttributeKey::PREVIOUS);
if (! $previous instanceof Expression) {
if (! $stmt instanceof Expression) {
return null;
}

if (! $previous->expr instanceof Assign) {
if (! $stmt->expr instanceof Assign) {
return null;
}

$previousAssign = $previous->expr;
if (! $previousAssign->var instanceof ArrayDimFetch) {
$assign = $stmt->expr;
if (! $assign->var instanceof ArrayDimFetch) {
return null;
}

return $previousAssign->var;
return $assign->var;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
includes:
- ../../../config/included_services.neon

services:
-
class: Symplify\PHPStanRules\Rules\NoMultiArrayAssignRule
tags: [phpstan.rules.rule]
rules:
- Symplify\PHPStanRules\Rules\NoMultiArrayAssignRule
4 changes: 4 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -512,3 +512,7 @@ parameters:
-
message: '#Cannot call method toString\(\) on PhpParser\\Node\\Identifier\|null#'
path: packages/phpstan-rules/src/Rules/RequireConstantInAttributeArgumentRule.php

-
message: '#Cognitive complexity for "Symplify\\PHPStanRules\\Rules\\NoMultiArrayAssignRule\:\:processNode\(\)" is 11, keep it under 8#'
path: packages/phpstan-rules/src/Rules/NoMultiArrayAssignRule.php

0 comments on commit 6383101

Please sign in to comment.