Phpstan: Constructor has unused parameter - not a issue when in interface

Created on 11 Nov 2017  路  3Comments  路  Source: phpstan/phpstan

If there is an interface with requested constructor and the descendant doesn reuse all it's parameter, this error doesn't make sense:

interface Foo {
    function __construct(string $bar);
}

class Bas implements Foo {
    function __construct(string $bar) {}    
}

https://phpstan.org/r/73c58e9ef06566269b50c17a27211eed

Most helpful comment

Constructors don't belong to interfaces 馃槉 So your design is questionable at best. When instantiating an object, you always know what specific class you are creating. So there's no point to enforce a constructor. Also, you're limiting your implementations, because the beauty of interfaces is that you don't have to care about implementation details - and constructor is an implementation detail.

And this issue is even weirder, because you're enforcing a constructor parameter that you don't use, making the code quite useless.

I don't plan to address this with a fix, sorry.

All 3 comments

Constructors don't belong to interfaces 馃槉 So your design is questionable at best. When instantiating an object, you always know what specific class you are creating. So there's no point to enforce a constructor. Also, you're limiting your implementations, because the beauty of interfaces is that you don't have to care about implementation details - and constructor is an implementation detail.

And this issue is even weirder, because you're enforcing a constructor parameter that you don't use, making the code quite useless.

I don't plan to address this with a fix, sorry.

I fully identify with this reasoning about proper architecture, though, few comments:

  • "When instantiating an object, you always know what specific class you are creating."

    This is not true. That's the main point why I used this solution. Many other arguments follow this premise, which is not always true.

  • I expect PhpStan will catch type errors, calls to undefined methods, etc. An unsused argument doesn't produce any notice, neither other error. No compiler ever fails because of unsued parametr and if so, the language provides a way to silence it (_) and do not force you to change a type signature. I may understand that this is your partial "shortcut" before proper implementation of #305 will be implemented.
  • My usecase may be understood as a static invocation, just a configurable one. The passing arguments through constructor just makes the code more readable. There is no way a user could require other dependency, provide directly other constructor argument. That's it.

So I'm not satisfied with the response. I do not expect static analyzer to do architecture validation, that's a job for other tooling.

PHPStan aims to catch common errors in PHP applications - looking for unused constructor arguments and unused closure uses proved to be useful many times - when a variable is passed to constructor and not used, that means that it does not have to be passed there, or that the developer forgot to use it. (PHPStan does not do that yet for other methods, because influencing their signatures may be out of the question because of interface/parent class requirements.)

Interface constructor does not make sense (and should be deprecated in PHP, for example Java does not allow it which tells you something) because you're interested in an interface only when it's typehinted as a method argument or return type, and at that point the object is already created and all you can do is to call methods on it. When calling new keyword, you always have a concrete class available (maybe in a variable, but still), because you can't do new FooInterface. (But a factory with general create method would still be better.)

Even if I accept that you want to have an interface constructor, I consider it valuable feedback for you that you forgot to use the parameter in Bas class. That's what the rule is for. Of course, PHPStan provides ignoreErrors configuration key in case this code issue is deliberate.

Was this page helpful?
0 / 5 - 0 ratings