rectorphp / rector

Instant Upgrades and Automated Refactoring of any PHP 5.3+ code
https://getrector.com
MIT License
8.7k stars 685 forks source link

Bad behavior of DirNameFileConstantToDirConstantRector #8502

Closed interduo closed 8 months ago

interduo commented 8 months ago

Bug Report

Unnecessary code changes.

Minimal PHP Code Causing Issue

Applied rules:

Raw origin file is located here: https://raw.githubusercontent.com/chilek/lms/072d10e5b85b3e58b34f36fbfcfe71f54a096d77/lib/xajax/xajax_core/xajaxControl.inc.php

This is my diff after rector run.

10) lib/xajax/xajax_core/xajaxControl.inc.php:349

    ---------- begin diff ----------
@@ @@
 //EndSkipDebug

         $sClass = $this->getClass();
-        
+
         if ('%inline' != $sClass) {
             // this odd syntax is necessary to detect request for no formatting
             if (false === (false === $sIndent)) {
@@ @@
                 echo $sIndent;
             }
         }
-            
+
         echo '<';
         echo $this->sTag;
         echo ' ';
         $this->_printAttributes();
         $this->_printEvents();
-        
+
         if ('forbidden' == $this->sEndTag) {
             if ('HTML' == XAJAX_HTML_CONTROL_DOCTYPE_FORMAT) {
                 echo '>';
@@ @@
             } else if ('XHTML' == XAJAX_HTML_CONTROL_DOCTYPE_FORMAT) {
                 echo '/>';
             }
-            
+
             if ('%inline' != $sClass) {
                 // this odd syntax is necessary to detect request for no formatting
                 if (false === (false === $sIndent)) {
@@ @@
             }
         } else if ('optional' == $this->sEndTag) {
             echo '/>';
-            
+
             if ('%inline' == $sClass) {
                 // this odd syntax is necessary to detect request for no formatting
                 if (false === (false === $sIndent)) {
@@ @@
         if (0 == count($this->aChildren)) {
             if ('optional' == $this->sEndTag) {
                 echo '/>';
-                
+
                 if ('%inline' != $sClass) {
                     // this odd syntax is necessary to detect request for no formatting
                     if (false === (false === $sIndent)) {
@@ @@
                         echo "\n";
                     }
                 }
-                    
+
                 return;
             }
 //SkipDebug
    ----------- end diff -----------

My rector.php is:

<?php
declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector;

return RectorConfig::configure()
    ->withPaths([
        __DIR__ . '/bin',
        __DIR__ . '/lib',
        __DIR__ . '/modules',
        __DIR__ . '/userpanel',
        __DIR__ . '/index.php',
    ])
    ->withRules([
        Rector\Php53\Rector\FuncCall\DirNameFileConstantToDirConstantRector::class,
    ]);

In that Rule I suggest to ommit all of the lines without: __DIR__

samsonasik commented 8 months ago

It seems php-parser print mix html + php cause various line ending + space force change. This is known drawback, see https://github.com/rectorphp/rector?tab=readme-ov-file#known-drawbacks,

I suggest to use coding standard tool to standardized mixed spacing

interduo commented 8 months ago

ok, thanks for the info.

interduo commented 8 months ago

But wait look there was "no occurances of rule in that file" so why it was changed in anyway?

samsonasik commented 8 months ago

File can be changed by rector rule or post rector (auto import, remove use stmts, etc) so can't depend only on that rule applied.