Skip to content
/ rector-src Public

[DeadCode] Skip Array Callable dynamic method using __CLASS__ with constructor (no default args) on RemoveUnusedPrivateMethodRector#5774

Merged
samsonasik merged 2 commits intorectorphp:mainfrom
hogejiro:fix_array_callable_using_CLASS_with_constructor_args
Mar 28, 2024
Merged

[DeadCode] Skip Array Callable dynamic method using __CLASS__ with constructor (no default args) on RemoveUnusedPrivateMethodRector#5774
samsonasik merged 2 commits intorectorphp:mainfrom
hogejiro:fix_array_callable_using_CLASS_with_constructor_args

Conversation

@hogejiro
Copy link
Copy Markdown
Contributor

@hogejiro hogejiro commented Mar 26, 2024

@hogejiro
[DeadCode] Skip Array Callable dynamic method using __CLASS__ with co…
f2fc3a1 
…nstructor (no default args) on RemoveUnusedPrivateMethodRector
hogejiro
hogejiro commented Mar 26, 2024
View reviewed changes
src/NodeCollector/NodeAnalyzer/ArrayCallableMethodMatcher.php
$callerType = $this->resolveClassConstFetchType($expr, $scope, $classMethodName);
if ($expr instanceof ClassConstFetch || $expr instanceof Class_) {
// class context means self|static ::class or __CLASS__
$callerType = $this->resolveClassContextType($expr, $scope, $classMethodName);
Copy link
Copy Markdown
Contributor Author

@hogejiro hogejiro Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note)
I wanted to avoid using the abstract term 'context'.
But I couldn't think of a better way to express it...

hogejiro
hogejiro commented Mar 26, 2024
View reviewed changes
...usedPrivateMethodRector/Fixture/skip_array_callables_magic_constant_with_constructor.php.inc
@@ -0,0 +1,27 @@
<?php
Copy link
Copy Markdown
Contributor Author

@hogejiro hogejiro Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note)
This code case showed me unexpected result.

rectorphp/rector skip

rectorphp/rector-src do not skip

  • Remove the used method. ( State of commit ff0705b )
PHPUnit result 
$ vendor/bin/phpunit -d --enable-pretty-print rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php
PHPUnit 10.5.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.3
Configuration: /path/to/rector-src/phpunit.xml

....................F..................                           39 / 39 (100%)

Time: 00:01.575, Memory: 90.00 MB

There was 1 failure:

1) Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector\RemoveUnusedPrivateMethodRectorTest::test with data set #20 ('...')
Failed on fixture file "skip_array_callables_magic_constant_with_constructor.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@

         return $array;
     }
-
-    private function sort($a, $b)
-    {
-        return $a <=> $b;
-    }
 }

/path/to/rector-src/src/Testing/PHPUnit/AbstractRectorTestCase.php:248
/path/to/rector-src/src/Testing/PHPUnit/AbstractRectorTestCase.php:164
/path/to/rector-src/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php:16

samsonasik
samsonasik reviewed Mar 28, 2024
View reviewed changes
samsonasik
samsonasik approved these changes Mar 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@samsonasik samsonasik merged commit eb525ca into rectorphp:main Mar 28, 2024
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Mar 28, 2024

Thank you @hogejiro

@hogejiro hogejiro deleted the fix_array_callable_using_CLASS_with_constructor_args branch March 28, 2024 08:20
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Mar 28, 2024

@hogejiro it seems cause running rector on rules got error:

➜  rector-src git:(main) bin/rector process rules
 638/638 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
                                                                                                                        
 [ERROR] Could not process                                                                                              
         "/Users/samsonasik/www/rector-src/rules/CodingStyle/Rector/String_/UseClassKeywordForClassNameResolutionRector.
         php" file, due to:                                                                                             
         "System error:                                                                                                 
         "Rector\NodeCollector\NodeAnalyzer\ArrayCallableMethodMatcher::isRequiredClassReflectionResolution(): Argument 
         #1 ($classConstantReference) must be of type string, int given, called in                                      
         src/NodeCollector/NodeAnalyzer/ArrayCallableMethodMatcher.php147"                                              
         Run Rector with "--debug" option and post the report here: https://github.com/rectorphp/rector/issues/new". On line: 206

I will take a look :)

@samsonasik samsonasik mentioned this pull request Mar 28, 2024
Merged
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Mar 28, 2024

#5780

@hogejiro
Copy link
Copy Markdown
Contributor Author

hogejiro commented Mar 28, 2024 •
edited
Loading

@samsonasik
Oh sorry. Thank you for fixing it 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsonasik samsonasik samsonasik approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect behavior of RemoveUnusedPrivateMethodRector

2 participants

@hogejiro @samsonasik