zendframework / zend-coding-standard

Zend Framework Coding Standard
BSD 3-Clause "New" or "Revised" License
35 stars 8 forks source link

[v2] Rule: UselessOverridingMethod #12

Closed michalbundyra closed 5 years ago

michalbundyra commented 5 years ago

Another one:

FILE: zend-diactoros/src/Exception/InvalidStreamPointerPositionException.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 17 | WARNING | Possible useless method overriding detected
    |         | (Generic.CodeAnalysis.UselessOverridingMethod.Found)
-----------------------------------------------------------------------------------------------------------------

and the code is like:

class InvalidStreamPointerPositionException extends RuntimeException implements ExceptionInterface
{
    public function __construct(
        string $message = 'Invalid pointer position',
        $code = 0,
        ?Throwable $previous = null
    ) {
        parent::__construct($message, $code, $previous);
    }
}

we are changing in the constructor default value for $message. I think we should disable that sniff.

Ocramius commented 5 years ago

Seems like a bug upstream?

michalbundyra commented 5 years ago

@Ocramius Before I reported the issue in PHP_CodeSniffer I've found: https://github.com/squizlabs/PHP_CodeSniffer/issues/519

What would you say? Should we add manual exclusion or maybe we should change our exceptions to:

class InvalidStreamPointerPositionException extends RuntimeException implements ExceptionInterface
{
    public static function invalidPointerPosition() : self
    {
        return new self('Invalid pointer position');
    }
}

(I think in some other places we are doing like that already).

/cc @weierophinney

geerteltink commented 5 years ago

If you don't need to change the message it's fine to change it.

But according to squizlabs/PHP_CodeSniffer#519 it's a bug and can't be fixed because of the way phpcs works. I'd say disable that sniff anyway and add a comment explaining why. Not sure if tools like phpstan can check this properly.

geerteltink commented 5 years ago

Closed in #13

boesing commented 4 years ago

Is it possible that this rule found its way back to v2?

composer show zendframework/zend-coding-standard
name     : zendframework/zend-coding-standard
descrip. : Zend Framework Coding Standard
keywords : Coding Standard, ZendFramework, zf
versions : * 2.0.0-alpha.3
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 13 | WARNING | Possible useless method overriding detected
----------------------------------------------------------------------
final class MyDomainException extends \DomainException
{
    private function __construct($message)
    {
        parent::__construct($message);
    }

    public static function factoryMethod() : self
    {
        return new self('Foo');
    }
}