ForbiddenParamTypeRemovalRule - should ignore __construct

[MartinMystikJonas]

Rule ForbiddenParamTypeRemovalRule reports change of parameters even for constructors, where forcing compatibility with parent class constructor is not required and often constructor signature changes (for example child sets some parent constructor params to fixed value)

Do you agree? I can prepare PR

[TomasVotruba]

Hi, do you have some exact example? Parent types should be respected in all methods.

[MartinMystikJonas]

Constructor is usually not considered as overwritten parent method but always a class specific method. Because you always use constructor of specific class (unless you use some dynamic class name magic) and never call it on parent class type. LSP does not apply to constructors.

[MartinMystikJonas]

I found a good article with longer explanation https://www.sitepoint.com/constructors-and-the-myth-of-breaking-the-lsp/

[MartinMystikJonas]

TL;DR Subtypes often require dirrerent/more/fewer dependencies than parent. Subtypes may not require some dependency (because it uses specific hardwired version), and may require new dependencies (because it extends parent functionality). Therefore child and parent constructors needs to differ to reflect this.

LSP is about instances of subtype can be used evwrywhere where parent type can be used. But it does not say anything about creation of these instances. Therefore it does not apply to constructors.

[TomasVotruba]

Could you share the PHP code? It would be easier to understand for me

[MartinMystikJonas]

Example based on our code

 abstract class DataSource {
  public function __construct(DataMapper $mapper, $data) {
    // ...
  }
}

class RawDataSource extends DataSource {
  public function __construct($data) { // <-- there it reports removed parameter type because parameter $mapper was removed
    parent::__construct(new NoMapper(), $data) {
    // ...
  }
}

[TomasVotruba]

I see. This should be skipped, as parameter is removed completelly and passed via new :+1:

[MartinMystikJonas]

Yes. Theoretically constructor can have completely different parameters.

So either skip constrictor completely. Or on case of co stri tor maybe check only if parameter name also match?

[TomasVotruba]

Or on case of co stri tor maybe check only if parameter name also match?

The removed parameter should skip completely. Otherwise the similar names should be matched, yes