Skip to content

Bug: false positive in RemoveUnusedPrivatePropertyRector for usage in closure #1646

@Levivb

Description

@Levivb
Subject Details
PHP version PHP 7.2
Full Command vendor/bin/rector --config ci/config/rector.yml process Tests/Unit/Exceptions/HandlerTest.php -n
parameters:
  php_version_features: '7.2'

services:
  Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector: ~

Current Behaviour

Rector v0.5.5
Config file: ci/config/rector.yml

 3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

1 file with changes
===================

1) Tests/Unit/Exceptions/HandlerTest.php

    ---------- begin diff ----------
--- Original
+++ New
@@ -52,9 +52,6 @@
     /** @var LoggerInterface */
     private $logger;

-    /** @var Message */
-    private $message;
-
     /** @var Redirector */
     private $redirector;

@@ -72,7 +69,6 @@
         parent::setUp();

         $this->request = Mockery::mock(Request::class);
-        $this->message = $this->createMock(Message::class);
         $this->sentry = $this->createMock(Raven_Client::class);
         $this->container = $this->createMock(Container::class);
         $this->mailer = $this->createMock(Mailer::class);
    ----------- end diff -----------

Applied rectors:

 * Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector

 [OK] Rector is done! 1 changed files

Minimal PHP Code Causing Issue

<?php
declare(strict_types=1);

namespace Tests\Unit\Exceptions;

use App\Exceptions\Handler;
use Closure;
use Exception;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Mail\Mailer;
use Illuminate\Contracts\Translation\Translator;
use Illuminate\Contracts\View\Factory;
use Illuminate\Mail\Message;
use Modules\Core\Tests\LaravelTestCase;
use Raven_Client;

class HandlerTest extends LaravelTestCase
{
    /** @var Handler */
    private $handler;

    /** @var Mailer */
    private $mailer;

    /** @var Message */
    private $message;

    public function setUp()
    {
        parent::setUp();

        $this->message = $this->createMock(Message::class);
        $this->mailer = $this->createMock(Mailer::class);

        $this->handler = new Handler(
            $this->createMock(Container::class),
            $this->createMock(Raven_Client::class),
            $this->mailer,
            $this->createMock(Factory::class),
            $this->createMock(Translator::class),
            'some app',
            false
        );
    }

    public function testReportWillMailIfReportToSentryFails(): void
    {
        $this->mailer
            ->method('raw')
            ->with($this->identicalTo('Sentry has crashed!!!'), $this->callback(function (Closure $closure): bool {
                $this->message
                    ->expects($this->once())
                    ->method('to')
                    ->with($this->identicalTo('developers@maximum.nl'), $this->identicalTo('Developers'));

                $closure->call($this->handler, $this->message);

                return true;
            }));

        $this->handler->report(new Exception('Nice exception message!'));
    }
}

Expected Behaviour

Rector reports the $this->message property as unused, but it's used in an anonymous function. Behaviour of report is pretty weird because if any of the following diffs are applied, rector doesn't report any dead code, even though there isn't really changed anything significant with regards to the usage of $this->message:

$this->mailer
            ->method('raw')
-          ->with($this->identicalTo('Sentry has crashed!!!'), $this->callback(function (Closure $closure): bool {
+          ->with($this->callback(function (Closure $closure): bool {
                $this->message
-                   ->expects($this->once())
                    ->method('to')
                    ->with($this->identicalTo('developers@maximum.nl'), $this->identicalTo('Developers'));

               $closure->call($this->handler, $this->message);
                $this->message
                   ->expects($this->once())
-                  ->method('to')
                    ->with($this->identicalTo('developers@maximum.nl'), $this->identicalTo('Developers'));

               $closure->call($this->handler, $this->message);
                $this->message
                   ->expects($this->once())
                  ->method('to')
-                ->with($this->identicalTo('developers@maximum.nl'), $this->identicalTo('Developers'));

               $closure->call($this->handler, $this->message);
                $this->message
                   ->expects($this->once())
                  ->method('to')
                  ->with($this->identicalTo('developers@maximum.nl'), $this->identicalTo('Developers'));

-               $closure->call($this->handler, $this->message);

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions