# inheritance check too strict?

- Thread: https://externals.io/message/49447
- Original archive: https://news-web.php.net/php.internals/49447
- Started by Stas Malyshev on 2010-08-19T07:51:15+00:00
- Messages: 22
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #49447 — Stas Malyshev — 2010-08-19T07:51:15+00:00

URL: https://externals.io/message/49447#49447

Hi!

I recently noticed this code:

<?php
error_reporting(E_ALL | E_STRICT);

class ObjParent {
	public function set($param2 = ''){ }
}

class ObjChild extends ObjParent {
         public function set(){ }
}

produces a `E_STRICT` warning. I don't really see a point in it - there's 
no problem whatsoever with child function ignoring some arguments from 
parent call. Anybody could explain why this check is there?
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49448 — Dave Ingram — 2010-08-19T08:06:28+00:00

URL: https://externals.io/message/49447#49448 · In reply to: #49447

> Hi!
>
> I recently noticed this code:
>
> <?php
> error_reporting(E_ALL | E_STRICT);
>
> class ObjParent {
>     public function set($param2 = ''){ }
> }
>
> class ObjChild extends ObjParent {
>         public function set(){ }
> }
>
> produces a `E_STRICT` warning. I don't really see a point in it -
> there's no problem whatsoever with child function ignoring some
> arguments from parent call. Anybody could explain why this check is
> there?
I was under the impression that, in order for inheritance to provide
proper polymorphism, overridden methods should share the parent's method
signature, although they can have additional optional arguments.
Otherwise, if you're passed an ObjChild when you are treating it as an
ObjParent, you could get unexpected behaviour if it ignores $param2 when
you decide to specify it. PHP doesn't complain about a programmer
passing too many arguments to the function as far as I recall, so this
sort of bug can be easy to overlook.

I hope that makes sense :-/


Dave

---

## #49449 — Stas Malyshev — 2010-08-19T08:13:07+00:00

URL: https://externals.io/message/49447#49449 · In reply to: #49448

Hi!

> I was under the impression that, in order for inheritance to provide
> proper polymorphism, overridden methods should share the parent's method
> signature, although they can have additional optional arguments.

Your impression is wrong. Overriden method should have _compatible_ 
signature - i.e. accept any argument set that parent object accepted. 
Nothing requires it to have the same signature.

> Otherwise, if you're passed an ObjChild when you are treating it as an
> ObjParent, you could get unexpected behaviour if it ignores $param2 when
> you decide to specify it. PHP doesn't complain about a programmer

Unexpected by whom? Writer of ObjChild->set() knew how to implement it 
without that parameter, so why behavior would be unexpected?

> passing too many arguments to the function as far as I recall, so this
> sort of bug can be easy to overlook.

How this is a "bug"?
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49450 — Dave Ingram — 2010-08-19T08:31:32+00:00

URL: https://externals.io/message/49447#49450 · In reply to: #49449

> Hi!
>
> > […]
>
> Your impression is wrong. Overriden method should have _compatible_
> signature - i.e. accept any argument set that parent object accepted.
> Nothing requires it to have the same signature.
Maybe I didn't express myself clearly: I meant that the arguments in the
child's signature must be a superset of the arguments in the parent
signature, in the same order. Any required arguments in the parent may
be made optional in the child, but not vice versa. The child is free to
accept additional optional arguments, but not exclude any from the parent.

This provides consistency in the same way that (although I haven't
tested in PHP) you should be able to increase the visibility of an
overridden method in a subclass (e.g. protected -> public) but not
decrease it (e.g. protected -> private).

> > […]
>
> Unexpected by whom? Writer of ObjChild->set() knew how to implement it
> without that parameter, so why behavior would be unexpected?
I'm assuming that the parameter would cause variations in behaviour
(which is generally the reason for having parameters), so by definition
ignoring it would cause different behaviour. I presume that this would
be unexpected.

> > […]
>
> How this is a "bug"?
Arguments being silently ignored sounds like a potential bug to me. I
know it's something I've seen in code I've maintained.

I am prepared to be shown to be wrong on any of these points, though
this is the way I understand things to work from both common sense and
half-remembered OO principles in CompSci.


D

---

## #49451 — johannes@schlueters.de — 2010-08-19T08:32:15+00:00

URL: https://externals.io/message/49447#49451 · In reply to: #49449

> Hi!
> 
> > […]
> 
> Your impression is wrong. Overriden method should have _compatible_ 
> signature - i.e. accept any argument set that parent object accepted. 
> Nothing requires it to have the same signature.

Let|s take a look at making it one step more complex:

class A {
    public function foo(Foo $a = null) {}
}

class B extends A {
    public function foo() {}
}

class C extends B {
    public function foo(Bar $a = null) {}
}

Here B::foo() is compatible with A:foo() and as the parameter is
optional C::foo() is compatible with B::foo(). But C::foo() is no more
compatible with A::foo().

So I consider the message good and correct.

johannes

---

## #49452 — Nathan Rixham — 2010-08-19T09:25:44+00:00

URL: https://externals.io/message/49447#49452 · In reply to: #49451

Johannes Schlüter wrote:
> 
> > […]
> 
> Let|s take a look at making it one step more complex:
> 
> class A {
>     public function foo(Foo $a = null) {}
> }
> 
> class B extends A {
>     public function foo() {}
> }
> 
> class C extends B {
>     public function foo(Bar $a = null) {}
> }
> 
> Here B::foo() is compatible with A:foo() and as the parameter is
> optional C::foo() is compatible with B::foo(). But C::foo() is no more
> compatible with A::foo().
> 
> So I consider the message good and correct.

what if Bar implements Foo, or Bar extends Foo - surely that should be 
compatible given the inheritance chain.

---

## #49453 — Tjerk Anne Meesters — 2010-08-19T09:42:26+00:00

URL: https://externals.io/message/49447#49453 · In reply to: #49451

2010/8/19 Johannes Schlüter <johannes@schlueters.de>:
> 
> > […]
>
> Let|s take a look at making it one step more complex:
>
> class A {
>    public function foo(Foo $a = null) {}
> }
>
> class B extends A {
>    public function foo() {}
> }
>
> class C extends B {
>    public function foo(Bar $a = null) {}
> }
>
> Here B::foo() is compatible with A:foo() and as the parameter is
> optional C::foo() is compatible with B::foo(). But C::foo() is no more
> compatible with A::foo().
>
> So I consider the message good and correct.

I suppose, when it comes to optional parameters, one can't rely on the
transitive nature of class extension validity. You would need to
iterate the definition chain of this method until it hits the
signature that actually defines the "type" of the parameter(s) before
throwing any messages. In my opinion as a regular developer, B::foo()
would make a valid signature.

>
> johannes
>
>
> --
> 
> 
>
>



-- 
--
Tjerk

---

## #49454 — Ryan Panning — 2010-08-19T13:16:40+00:00

URL: https://externals.io/message/49447#49454 · In reply to: #49452

Nathan Rixham wrote:
> what if Bar implements Foo, or Bar extends Foo - surely that should be 
> compatible given the inheritance chain.

I ran into this exact issue and thought it was strange. Is there a 
reason this shouldn't be allowed?

---

## #49455 — Ionut G. Stan — 2010-08-19T14:04:26+00:00

URL: https://externals.io/message/49447#49455 · In reply to: #49454

> Nathan Rixham wrote:
> > […]
>
> I ran into this exact issue and thought it was strange. Is there a
> reason this shouldn't be allowed?

It still breaks the interface:

class Parent
{
     public function foo(Foo $foo)
     {}
}

class Child
{
     public function foo(Bar $bar)
     {}
}

class Foo
{}

class Bar extends Foo
{}



All fine until here, but what if...



class Taz extends Foo.
{}


I can't call Child::foo() with an instance of Taz, but I can call 
Parent::foo() with such an instance. So, I can't use instances of Child 
wherever instances of Parent would be accepted.

-- 
Ionuț G. Stan  |  http://igstan.ro

---

## #49456 — Zeev Suraski — 2010-08-19T15:10:11+00:00

URL: https://externals.io/message/49447#49456 · In reply to: #49447

At 10:51 19/08/2010, Stas Malyshev wrote:
>Hi!
>
>I recently noticed this code:
>
><?php
>error_reporting(E_ALL | E_STRICT);
>
>class ObjParent {
>         public function set($param2 = ''){ }
>}
>
>class ObjChild extends ObjParent {
>         public function set(){ }
>}
>
>produces a `E_STRICT` warning. I don't really see a point in it - 
>there's no problem whatsoever with child function ignoring some 
>arguments from parent call. Anybody could explain why this check is there?

As others noted I also think that this warning is correct.  Code that 
deals with ObjParent objects is allowed to call to set() with a 
parameter.  It's also supposed to be able to treat ObjChild objects 
transparently, because they're specialized of ObjParent.  If this 
function signature was allowed - it can end up calling 
ObjChild::set() with an argument - which ObjChild() doesn't support.

The other way around - making ObjChild::set() more support more 
signatures than the signature it's 'overriding' - makes perfect sense 
and is allowed.

Regarding the other issue that was raised here, I didn't recall we 
made `is_a()` checks on class type hints during signature validation, 
but if we do - it should be done using the same rules - a child 
class's signature must be at least as lax as it's parent - or more.

Zeev

---

## #49457 — Zeev Suraski — 2010-08-19T15:14:35+00:00

URL: https://externals.io/message/49447#49457 · In reply to: #49455

At 17:04 19/08/2010, Ionut G. Stan wrote:
class Parent
>{
>     public function foo(Foo $foo)
>     {}
>}
>
>class Child
>{
>     public function foo(Bar $bar)
>     {}
>}
>
>class Foo
>{}
>
>class Bar extends Foo
>{}
>
>
>
>All fine until here, but what if...
>
>
>
>class Taz extends Foo.
>{}
>
>
>I can't call Child::foo() with an instance of Taz, but I can call 
>Parent::foo() with such an instance. So, I can't use instances of 
>Child wherever instances of Parent would be accepted.

Child should clearly not be allowed to inherit Parent in the code 
above, since the signature of Child::foo() is more restrictive than 
the signature of Parent::foo().  The other way around could work 
(although I don't recall if we allow it):

class Foo {}
class Bar extends Foo {}

class Parent
{
         public function foo(Bar $bar){}
}

class Child extends Parent
{
         public function foo(Foo $foo){}
}


No issues here - since any Bar object is also a Foo object and would 
pass the is_a validation of Foo.  Again, I don't recall if we allow 
such signature overrides or not.

Zeev

---

## #49458 — mathieu.suen — 2010-08-19T15:21:50+00:00

URL: https://externals.io/message/49447#49458 · In reply to: #49451

I consider having type in the declaration of a method a bad idea and 
this thread just prove it.
You can also read this:
http://gbracha.blogspot.com/2009/09/systemic-overload.html


> 
>    
> > […]
> Let|s take a look at making it one step more complex:
>
> class A {
>      public function foo(Foo $a = null) {}
> }
>
> class B extends A {
>      public function foo() {}
> }
>
> class C extends B {
>      public function foo(Bar $a = null) {}
> }
>
> Here B::foo() is compatible with A:foo() and as the parameter is
> optional C::foo() is compatible with B::foo(). But C::foo() is no more
> compatible with A::foo().
>
> So I consider the message good and correct.
>
> johannes
>
>
>    


-- 
	

• *Mathieu Suen* | It Team | www.easyflirt.com
• mathieu [dot] suen [at] easyflirt [dot] com 
<mathieu%20[dot]%20suen%20[at]%20easyflirt%20[dot]%20com>
• EasyFlirt - Park Nord, Les Pléiades, 74370 - Metz-Tessy - FRANCE


• Pensez à l'environnement, n'imprimez cet e-mail qu'en cas de réelle 
nécessité

---

## #49461 — Stas Malyshev — 2010-08-19T16:42:24+00:00

URL: https://externals.io/message/49447#49461 · In reply to: #49451

Hi!

> class A {
>      public function foo(Foo $a = null) {}
> }
>
> class B extends A {
>      public function foo() {}
> }
>
> class C extends B {
>      public function foo(Bar $a = null) {}
> }
>
> Here B::foo() is compatible with A:foo() and as the parameter is
> optional C::foo() is compatible with B::foo(). But C::foo() is no more
> compatible with A::foo().

Between B and C there's of course incompatibility - while B accepts 
anything (and ignores it), C accepts only Bar or nothing. But I was 
talking between A and B!

-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49462 — Stas Malyshev — 2010-08-19T16:45:14+00:00

URL: https://externals.io/message/49447#49462 · In reply to: #49456

Hi!

> transparently, because they're specialized of ObjParent.  If this
> function signature was allowed - it can end up calling
> ObjChild::set() with an argument - which ObjChild() doesn't support.

But it does! It just silently ignores the argument - which it does not 
need. But you can add as much extra arguments as you want, they always 
were silently ignored. Also, ObjChild might use func_get_args, etc. to 
get the arguments.

> The other way around - making ObjChild::set() more support more
> signatures than the signature it's 'overriding' - makes perfect sense
> and is allowed.

But that's exactly what happens - ObjChild supports more signatures 
(inclusive!) than ObjParent.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49463 — Stas Malyshev — 2010-08-19T16:53:40+00:00

URL: https://externals.io/message/49447#49463 · In reply to: #49450

Hi!
> Maybe I didn't express myself clearly: I meant that the arguments in the
> child's signature must be a superset of the arguments in the parent
> signature, in the same order. Any required arguments in the parent may

It was clear to me you said that, however it is in no way clear why it 
must be so. LSP requires that any code that calls ObjParent could also 
call ObjChild, this is true. It does not say anything about supersets - 
it only says that preconditions should not be strengthened, and that's 
exactly what happens.

> I'm assuming that the parameter would cause variations in behaviour
> (which is generally the reason for having parameters), so by definition
> ignoring it would cause different behaviour. I presume that this would
> be unexpected.

Again, you are assuming behavior which has nothing to do with 
signatures. This way we had to disallow overriding at all - because 
somebody could write the overriding code that just ignores parameters 
and always returns 42! Behavior that is written into the code is of 
course expected, it's why it is there.

> Arguments being silently ignored sounds like a potential bug to me. I
> know it's something I've seen in code I've maintained.

No, it's not a bug - extra arguments were silently ignored in PHP since 
the dawn of time.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49464 — Ferenc Kovacs — 2010-08-19T18:05:53+00:00

URL: https://externals.io/message/49447#49464 · In reply to: #49463

>
>
>  Arguments being silently ignored sounds like a potential bug to me. I
> > […]
>
> No, it's not a bug - extra arguments were silently ignored in PHP since the
> dawn of time.


I know that you know this, but I want this to be clear:
the extra args ignored only from the class definition point of view, they
can be harvested by the `func_get_args()` function.

Tyrael

---

## #49465 — Pierre Joye — 2010-08-19T18:29:52+00:00

URL: https://externals.io/message/49447#49465 · In reply to: #49463

hi Stas,



> > […]
>
> No, it's not a bug - extra arguments were silently ignored in PHP since the
> dawn of time.

Inconsistently. Many internals functions do not accept more arguments
than the supported ones.

However, what are the benefits to be less strict in your example? I
can't think of any practical (or critical) example where you have to
remove this optional argument.

Cheers,
-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

---

## #49466 — Stas Malyshev — 2010-08-19T18:30:13+00:00

URL: https://externals.io/message/49447#49466 · In reply to: #49464

Hi!

> I know that you know this, but I want this to be clear:
> the extra args ignored only from the class definition point of view,
> they can be harvested by the `func_get_args()` function.

Right. Same can happen when inheriting, of course. There's no additional 
restriction when defining a method this way. That's entirely what LSP 
demands - no additional restriction. So why have the warning?
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49467 — Stas Malyshev — 2010-08-19T18:58:05+00:00

URL: https://externals.io/message/49447#49467 · In reply to: #49465

Hi!

> However, what are the benefits to be less strict in your example? I
> can't think of any practical (or critical) example where you have to
> remove this optional argument.

I have tons of code that does it, but the first example would be, of 
course, using `func_get_args()`.
Another example would be if the parent function has some option and the 
child want to always have this option set to particular value or maybe 
taken not from the parameter but from some other place. He can do it in 
the code anyway, but why have an argument that is never used?

The idea is that `E_STRICT` warning are useful if they definitely point to 
some wrong code - like overriding foo($a) with foo($a, $b) (can't work - 
missing arg) or with foo(Bar $a) (can't work if $a is not Bar). In the 
example we discuss it definitely can work and actually does work - only 
reason to put out warning there will be misguided notions of how the 
code should be written and it ultimetely would lead people to just 
disable those warnings altogether - who needs them if they warn about 
perfectly good code and serve no purpose but to annoy people?

These things are only useful if they help. The whole notion of "you have 
to prove this code is legit" is wrong - even though, as I showed above, 
I can prove it's legit - we should only warn about the code *we* can 
prove is *not* legit. Which is definitely not the case here.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

---

## #49468 — Nathan Rixham — 2010-08-19T19:33:19+00:00

URL: https://externals.io/message/49447#49468 · In reply to: #49457

Zeev Suraski wrote:
> At 17:04 19/08/2010, Ionut G. Stan wrote:
> > […]
> 
> Child should clearly not be allowed to inherit Parent in the code above, 
> since the signature of Child::foo() is more restrictive than the 
> signature of Parent::foo().  The other way around could work (although I 
> don't recall if we allow it):
> 
> class Foo {}
> class Bar extends Foo {}
> 
> class Parent
> {
>         public function foo(Bar $bar){}
> }
> 
> class Child extends Parent
> {
>         public function foo(Foo $foo){}
> }
> 
> 
> No issues here - since any Bar object is also a Foo object and would 
> pass the is_a validation of Foo.  Again, I don't recall if we allow such 
> signature overrides or not.
> 
> Zeev

Guys, this is going a bit nuts, let's swap all the Foo and Bar's for a 
real example - Zeev I've copied the way you specified above.

class Point2D { // Foo
   public $x;
   public $y;
}

class Point3D extends Point2D { // Bar extends Foo
   public $z;
}

class Point2DManager { // Parent
   public function distanceBetween( Point3D $p1 , Point3D $p2 ) {};
}

class Point3DManager extends Point2DManager { // Child extends Parent
   public function distanceBetween( Point2D $p1 , Point2D $p2 ) {};
}

You're saying that makes sense and is no problem? honestly?

Best,

Nathan

ps: no offence / nothing personal just want to get this cleared up

---

## #49469 — Chris Stockton — 2010-08-19T21:00:12+00:00

URL: https://externals.io/message/49447#49469 · In reply to: #49468

Hello,


> Guys, this is going a bit nuts, let's swap all the Foo and Bar's for a real
> example - Zeev I've copied the way you specified above.

I think your misunderstanding his position. To summarize my
understanding: he clearly states that in such a use case a warning is
in order, however currently the warning is overly aggressive. I think
in general this is a small fish in the pond but I see his point.

Kind Regards,

-Chris

---

## #49470 — Nathan Rixham — 2010-08-19T21:10:07+00:00

URL: https://externals.io/message/49447#49470 · In reply to: #49469

Chris Stockton wrote:
> Hello,
> 
> 
> > […]
> 
> I think your misunderstanding his position. To summarize my
> understanding: he clearly states that in such a use case a warning is
> in order, however currently the warning is overly aggressive. I think
> in general this is a small fish in the pond but I see his point.
> 

Perhaps I am, I think his example was wrong - if it was an example of 
doing it the wrong way and an example which should trigger a warning 
then yes I totally agree.

To clarify, hopefully - given:

   class Point2D {}
   class Point3D extends Point2D {}

the following is incorrect and should raise a (heavy) warning (Zeev's 
example with class names changed for clarity):

   class Point2DManager {
     public function distanceBetween( Point3D $p1 , Point3D $p2 ) {};
   }
   class Point3DManager extends Point2DManager {
     public function distanceBetween( Point2D $p1 , Point2D $p2 ) {};
   }

whilst the following is correct, and should not raise any warning:

   class Point2DManager {
     public function distanceBetween( Point2D $p1 , Point2D $p2 ) {};
   }
   class Point3DManager extends Point2DManager {
     public function distanceBetween( Point3D $p1 , Point3D $p2 ) {};
   }

If we're all saying the same thing then great and apologies for the 
confusion.

Best,

Nathan