RemoveEmptyClassMethodRector removes empty method in child class required by parent if parent is not in current analyze scope

[raphaelincert]

Bug Report

Subject	Details
Rector version	2.6.6

Hi,
i am using rector on a huge project with many files and without directory restrictions it keeps crashing.
If i add directory restrictions via ->withPaths() and child classes with empty methods inside of that directory have parent classes that are not inside the directory that force the child to implement these methods, the methods will be removed due to the rule RemoveEmptyClassMethodRector.

Minimal PHP Code Causing Issue

Directory Structure
Running rector withPath() on /Common/Webservice/External/Implementations/ (with already 10k+ files)

.
└── Common
    ├── Posting
    │   └── AbstractPosting.php
    └── Webservice
        └── External
            └── Implementations
                └── SAP
                    └── S4HANA
                        └── PostingBL
                            └── SAPPosting.php

AbstractPosting.php

abstract class AbstractPosting
{
    abstract protected function submitPosting(Posting $posting): void;
}

SAPPosting.php

final class SAPPosting extends AbstractPosting implements AccountingListAggregatedPostingSubmit
{
    protected function submitPosting(Posting $posting): void
    {
    }
    
    public function submitPostingsToDateLimit(?DateTime $dt = null): void
    {
         // code happens here due to extra interface implement
    }
}

Working example:
https://getrector.com/demo/02764c42-e332-441d-9c77-7051cfe3617f
Not working (commented out file that is not in the same directory)
https://getrector.com/demo/250ff764-0243-48b9-9da8-714ced7dc9de

Expected Behaviour

Do not remove the empty methods.

Is it possible to tell rector to skip certain rules if for example the parent class or interface is not in the current analyze scope?

Thank you

[TomasVotruba]

The class must exists and be autoloadable. Otherwise lot of rules will start to behave unexpectedly.
Always run Rector on full project.

There is a new feature that might come handy though

--filter, see https://github.com/rectorphp/rector/releases/tag/2.6.6

[raphaelincert]

Hi,
if it is not possible to (or should not be done) run rector only on a few files, what would be the best way to integrate it in CI/CD?
I use it on a large project where running rector on the entire project would take forever.

If you mind taking a look at the following custom workflow action for example:

name: Code Beautifier & Fixer

on:
  pull_request:
    branches: [ "stage" ]

jobs:
  beautify:
    runs-on: ubuntu-latest

... some irrelevant stuff ..

      - name: Install Dependencies
        run: composer install --no-progress --prefer-dist --optimize-autoloader --no-interaction --no-plugins --no-scripts

      - name: Run PHP-CS-Fixer & PHPCBf on changed files
        run: |
          CHANGED_FILES=$(git diff --name-only stage...HEAD | grep '\.php$' || true)
          
          if [ -z "$CHANGED_FILES" ]; then
            echo "no changed files."
            exit 0
          fi
          
          echo "Optimize files:"
          echo "$CHANGED_FILES"
          
          ./vendor/bin/php-cs-fixer fix --path-mode=intersection -- $CHANGED_FILES || true
          ./vendor/bin/phpcbf $CHANGED_FILES || true

      - name: Commit and Push changes
        run: |
          git config --global user.name "gitea-actions[bot]"
          git config --global user.email "gitea-actions[bot]@noreply.gitea.io"
          
          if [ -n "$(git status --porcelain)" ]; then
            git add .
            git commit -m "style: automated php-cs-fixer & phpcbf fixes"
            git push origin HEAD:${{ gitea.event.pull_request.head.ref }}
            echo "changes pushed"
          else
            echo "nothing changed"
          fi

how could i integrate rector if i can not run it on the full project?
On new pull requests for the stage branch (first step before getting something into production) i run various tools on the changed files.
If i would change only SAPPosting.php in a pull request and run rector on it, it would break the pr.
Thanks for your time.
Best regards

[TomasVotruba]

You can make use of cache in Github. Agents will handle it.

[TomasVotruba]

Things like CHANGED_FILES are obsolete and risky.

[raphaelincert]

Hi,
i just checked the pr you already created (rectorphp/rector-src#8433) for this issue.
Wouldn´t it be a good idea to add parent classes/interfaces/etc. to the analyze scope if they are referenced?
Especially considering that running it on the full project is the way to go as you mentioned.
Its kind of a strange behaviour in my opinion that running rector with different analyze scopes produces a different result for the same file.
Best regards

[raphaelincert]

or maybe event add an option like "--with-references"?

[TomasVotruba]

As said, that would lead to unexpected behavior. Always analyse whole project or let Rector cache handle it.

This post sums it up pretty nicely: https://phpstan.org/blog/why-you-should-always-analyse-whole-project