add BreakingREmovalGuard, CommentableNodeResolver, and @fixme to un-removable nodes in ini_get/ini_set

composer.json

@@ -66,6 +66,7 @@
             "Rector\\Celebrity\\": "rules/celebrity/src",
             "Rector\\ChangesReporting\\": "packages/changes-reporting/src",
             "Rector\\CodeQuality\\": "rules/code-quality/src",
+            "Rector\\NodeRemoval\\": "packages/node-removal/src",
             "Rector\\Reporting\\": "packages/reporting/src",
             "Rector\\CodingStyle\\": "rules/coding-style/src",
             "Rector\\ConsoleDiffer\\": "packages/console-differ/src",

packages/node-removal/config/config.yaml

@@ -0,0 +1,8 @@
+services:
+    _defaults:
+        public: true
+        autowire: true
+        autoconfigure: true
+
+    Rector\NodeRemoval\:
+        resource: '../src'

packages/node-removal/src/BreakingRemovalGuard.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\NodeRemoval;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\BooleanNot;
+use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\While_;
+use Rector\Core\Exception\ShouldNotHappenException;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+final class BreakingRemovalGuard
+{
+    public function ensureNodeCanBeRemove(Node $node): void
+    {
+        // validate the $parentNodenode can be removed
+        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+        if ($this->isLegalNodeRemoval($node)) {
+            return;
+        }
+
+        throw new ShouldNotHappenException(sprintf(
+            'Node "%s" is child of "%s", so it cannot be removed as it would break PHP code. Change or remove the parent node instead.',
+            get_class($node),
+            /** @var Node $parentNode */
+            get_class($parentNode)
+        ));
+    }
+
+    public function isLegalNodeRemoval(Node $node): bool
+    {
+        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+
+        if ($parentNode instanceof BooleanNot) {
+            $parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE);
+        }
+
+        return ! $parentNode instanceof Assign && ! $this->isIfCondition($node) && ! $this->isWhileCondition($node);
+    }
+
+    private function isIfCondition(Node $node): bool
+    {
+        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+        if (! $parentNode instanceof If_) {
+            return false;
+        }
+
+        return $parentNode->cond === $node;
+    }
+
+    private function isWhileCondition(Node $node): bool
+    {
+        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+        if (! $parentNode instanceof While_) {
+            return false;
+        }
+
+        return $parentNode->cond === $node;
+    }
+}

packages/post-rector/src/Collector/NodesToRemoveCollector.php

@@ -10,6 +10,7 @@ use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use Rector\ChangesReporting\Collector\AffectedFilesCollector;
 use Rector\Core\Exception\ShouldNotHappenException;
+use Rector\NodeRemoval\BreakingRemovalGuard;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PostRector\Contract\Collector\NodeCollectorInterface;
 use Symplify\SmartFileSystem\SmartFileInfo;
@@ -26,9 +27,17 @@ final class NodesToRemoveCollector implements NodeCollectorInterface
      */
     private $affectedFilesCollector;
 
-    public function __construct(AffectedFilesCollector $affectedFilesCollector)
-    {
+    /**
+     * @var BreakingRemovalGuard
+     */
+    private $breakingRemovalGuard;
+
+    public function __construct(
+        AffectedFilesCollector $affectedFilesCollector,
+        BreakingRemovalGuard $breakingRemovalGuard
+    ) {
         $this->affectedFilesCollector = $affectedFilesCollector;
+        $this->breakingRemovalGuard = $breakingRemovalGuard;
     }
 
     public function addNodeToRemove(Node $node): void
@@ -40,6 +49,8 @@ final class NodesToRemoveCollector implements NodeCollectorInterface
         if (! $node instanceof Expression && $parentNode instanceof Expression) {
             // only expressions can be removed
             $node = $parentNode;
+        } else {
+            $this->breakingRemovalGuard->ensureNodeCanBeRemove($node);
         }
 
         /** @var SmartFileInfo|null $fileInfo */

phpstan.neon

@@ -314,3 +314,4 @@ parameters:
         - '#Parameter \#1 \$type of class PhpParser\\Node\\NullableType constructor expects PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|string, PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\|PhpParser\\Node\\UnionType given#'
         - '#Parameter \#1 \$oldMethod of class Rector\\PHPOffice\\ValueObject\\ConditionalSetValue constructor expects string, bool\|int\|string given#'
         - '#Parameter \#2 \$scope of method PHPStan\\Analyser\\NodeScopeResolver\:\:processNodes\(\) expects PHPStan\\Analyser\\MutatingScope, PHPStan\\Analyser\\Scope given#'
+        - '#Parameter \#1 \$object of function get_class expects object, PhpParser\\Node\|null given#'

rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php

@@ -84,19 +84,7 @@ PHP
         $hasChanged = false;
 
         foreach ((array) $node->stmts as $key => $stmt) {
-            $currentStmtVariableName = null;
-
-            $stmt = $this->unwrapExpression($stmt);
-
-            if ($stmt instanceof Assign || $stmt instanceof MethodCall) {
-                if ($this->shouldSkipLeftVariable($stmt)) {
-                    continue;
-                }
-
-                if (! $stmt->var instanceof MethodCall && ! $stmt->var instanceof StaticCall) {
-                    $currentStmtVariableName = $this->getName($stmt->var);
-                }
-            }
+            $currentStmtVariableName = $this->resolveCurrentStmtVariableName($stmt);
 
             if ($this->shouldAddEmptyLine($currentStmtVariableName, $node, $key)) {
                 $hasChanged = true;
@@ -134,6 +122,7 @@ PHP
         if (! $this->isNewVariableThanBefore($currentStmtVariableName)) {
             return false;
         }
+
         // this is already empty line before
         return ! $this->isPreceededByEmptyLine($node, $key);
     }
@@ -182,4 +171,21 @@ PHP
 
         return $node;
     }
+
+    private function resolveCurrentStmtVariableName(Node $node): ?string
+    {
+        $node = $this->unwrapExpression($node);
+
+        if ($node instanceof Assign || $node instanceof MethodCall) {
+            if ($this->shouldSkipLeftVariable($node)) {
+                return null;
+            }
+
+            if (! $node->var instanceof MethodCall && ! $node->var instanceof StaticCall) {
+                return $this->getName($node->var);
+            }
+        }
+
+        return null;
+    }
 }

rules/coding-style/tests/Rector/ClassMethod/NewlineBeforeNewAssignSetRector/Fixture/skip_method_call_property_call.php.inc

@@ -0,0 +1,20 @@
+<?php
+
+namespace Rector\CodingStyle\Tests\Rector\ClassMethod\NewlineBeforeNewAssignSetRector\Fixture;
+
+use Nette\Loaders\RobotLoader;
+
+final class SkipMethodCallPropertyCall
+{
+    public function run($directories, $name)
+    {
+        $robotLoader = new RobotLoader();
+        foreach ($directories as $directory) {
+            $robotLoader->addDirectory($directory);
+        }
+
+        $robotLoader->setTempDirectory(sys_get_temp_dir() . '/_rector_finder');
+        $robotLoader->acceptFiles = [$name];
+        $robotLoader->rebuild();
+    }
+}

src/Comments/CommentableNodeResolver.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Comments;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+/**
+ * Resolve nearest node, where we can add comment
+ */
+final class CommentableNodeResolver
+{
+    public function resolve(Node $node): Node
+    {
+        $currentNode = $node;
+        $previousNode = $node;
+
+        while (! $currentNode instanceof Stmt) {
+            $currentNode = $currentNode->getAttribute(AttributeKey::PARENT_NODE);
+            if ($currentNode === null) {
+                return $previousNode;
+            }
+
+            $previousNode = $currentNode;
+        }
+
+        return $currentNode;
+    }
+}

src/Rector/FuncCall/RemoveIniGetSetFuncCallRector.php

@@ -4,11 +4,15 @@ declare(strict_types=1);
 
 namespace Rector\Core\Rector\FuncCall;
 
+use PhpParser\Comment;
 use PhpParser\Node;
 use PhpParser\Node\Expr\FuncCall;
+use Rector\Core\Comments\CommentableNodeResolver;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\RectorDefinition\ConfiguredCodeSample;
 use Rector\Core\RectorDefinition\RectorDefinition;
+use Rector\NodeRemoval\BreakingRemovalGuard;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 
 /**
  * @sponsor Thanks https://twitter.com/afilina & Zenika (CAN) for sponsoring this rule - visit them on https://zenika.ca/en/en
@@ -22,12 +26,27 @@ final class RemoveIniGetSetFuncCallRector extends AbstractRector
      */
     private $keysToRemove = [];
 
+    /**
+     * @var BreakingRemovalGuard
+     */
+    private $breakingRemovalGuard;
+
+    /**
+     * @var CommentableNodeResolver
+     */
+    private $commentableNodeResolver;
+
     /**
      * @param string[] $keysToRemove
      */
-    public function __construct(array $keysToRemove = [])
-    {
+    public function __construct(
+        BreakingRemovalGuard $breakingRemovalGuard,
+        CommentableNodeResolver $commentableNodeResolver,
+        array $keysToRemove = []
+    ) {
         $this->keysToRemove = $keysToRemove;
+        $this->breakingRemovalGuard = $breakingRemovalGuard;
+        $this->commentableNodeResolver = $commentableNodeResolver;
     }
 
     public function getDefinition(): RectorDefinition
@@ -73,7 +92,12 @@ CODE_SAMPLE
             return null;
         }
 
-        $this->removeNode($node);
+        if ($this->breakingRemovalGuard->isLegalNodeRemoval($node)) {
+            $this->removeNode($node);
+        } else {
+            $commentableNode = $this->commentableNodeResolver->resolve($node);
+            $commentableNode->setAttribute(AttributeKey::COMMENTS, [new Comment('// @fixme')]);
+        }
 
         return null;
     }

tests/Rector/FuncCall/RemoveIniGetSetFuncCallRector/Fixture/get_assign.php.inc

@@ -22,9 +22,11 @@ class GetAssign
 {
     public function run()
     {
+        // @fixme
         $getAssign = ini_get('safe_mode');
 
-        $notGetAssign = !ini_get('safe_mode');
+        // @fixme
+        $notGetAssign = ! ini_get('safe_mode');
     }
 }