PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Generic.CodeAnalysis.UselessOverridingMethod false positives with type narrowing

Open aadmathijssen opened this issue 3 years ago • 0 comments

Describe the bug While investigating different ways to implement type-safe collections, I noticed that the Generic.CodeAnalysis.UselessOverridingMethod generates false positives when narrowing types of the base class.

Code sample

<?php

class StdClassCollection extends ArrayIterator
{
    public function __construct(stdClass ...$stdObjects)
    {
        parent::__construct($stdObjects);
    }

    public function key(): int
    {
        return parent::key();
    }

    public function current(): stdClass
    {
        return parent::current();
    }
}

Custom ruleset

<?xml version="1.0"?>
<ruleset>
    <rule ref="Generic.CodeAnalysis.UselessOverridingMethod"/>
</ruleset>

To reproduce Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php...
  3. See error message displayed
FILE: test.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------------
  5 | WARNING | Possible useless method overriding detected
    |         | (Generic.CodeAnalysis.UselessOverridingMethod.Found)
 10 | WARNING | Possible useless method overriding detected
    |         | (Generic.CodeAnalysis.UselessOverridingMethod.Found)
 15 | WARNING | Possible useless method overriding detected
    |         | (Generic.CodeAnalysis.UselessOverridingMethod.Found)
-----------------------------------------------------------------------------------------------------------------

Expected behavior I expected no warnings to be displayed.

Versions:

  • OS: Mac OSX, Linux
  • PHP: 7.3, 7.4, 8.0
  • PHPCS: 3.6.1 (stable)
  • Standard: see custom ruleset above

Additional context I am aware of the fact that PHP_CodeSniffer checks one file at the same, and is therefore not able to determine the method signature of any of the referenced classes (which is required to determine whether we are dealing with a false positive or not).

So I think it is better to add a comment/warning somewhere that this sniff might generate false positives.

Issues #519 and #3035 are similar to this issue: only the signature of the overridden methods is changed while the body is just a parent call. However, in these issues the default values are changed, whereas in this issue the argument/return types are changed.

aadmathijssen avatar Nov 10 '21 14:11 aadmathijssen