php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78818 Current Type variance changes breaks the Liskov substitution principle
Submitted: 2019-11-15 13:03 UTC Modified: 2019-11-15 13:51 UTC
From: me at paveljanda dot com Assigned:
Status: Not a bug Package: *General Issues
PHP Version: 7.4.0RC6 OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: me at paveljanda dot com
New email:
PHP Version: OS:

 

 [2019-11-15 13:03 UTC] me at paveljanda dot com
Description:
------------
1, Liskov substitution principle: 

interface FuelType
{
}
class Gas implements FuelType
{
	public function burn(int $litres) {}
};
class Battery implements FuelType
{
	public function discharge(int $kWh) {}
};
class Vehicle
{
	public function drive(Gas $fuel)
	{
		$battery->burn(10);
	}
}
class ElectricVehicle extends Vehicle
{
	public function drive(FuelType $battery)
	{
		// do something
	}
}


(new ElectricVehicle)->drive(new Battery);

-> This code works fine in 7.4 RC6. I don't think it should. That code would enable ElectricVehicle::drive() to accept "smaller" argument with less functionality that the "bigger" Gas class that is required by parent (!!) class method.

2. In the example above, when I change the ElectricVehicle into following form:

class ElectricVehicle extends Vehicle
{
	public function drive(Battery $battery)
	{
		$battery->discharge(10);
	}
}

, PHP tells me different result (Warning: Declaration of ElectricVehicle::drive(Battery $battery) should be compatible with Vehicle::drive(Gas $fuel)) event though Battery is a type of FuelType which worked fine in the previous example.

It makes perfect sense to use Type variance in the opposite direction - classes that extend from Vehicle should be able to accept a class that extends Gas type. It should be OK to extends the successor behaviour. But it's not OK do make it both directions.

Thank you all for spending some time with that report!

Pavel Janda

Test script:
---------------
https://gist.github.com/paveljanda/cba6d31be920217c620289e54fed7c1c


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-11-15 13:13 UTC] girgias@php.net
-Status: Open +Status: Not a bug
 [2019-11-15 13:13 UTC] girgias@php.net
I'm sorry but this is the correct behaviour as per LSP.
Your issue here is that your abstraction (interface/classes) is wrong and bending the rules of correct type safety just to make a wrong abstraction work is not correct.

It seems you are confusing co and contravariance.
Covariance is only safe in return types and contravariuance is only safe in argument types.

Moreover you are not doing any of that you are doing flat out variance which is inherently incompatible with LSP.
 [2019-11-15 13:40 UTC] me at paveljanda dot com
Thanks for and answer.

I totally agree with you that this code's abstraction is wrong. But it should be for this demonstration.

What about the number 2? How is it that the language is behaving different way event both arguments are of the same type "FuelType"?
 [2019-11-15 13:51 UTC] requinix@php.net
-Package: PHP Language Specification +Package: *General Issues
 [2019-11-15 13:51 UTC] requinix@php.net
Contravariance says a subclass must accept everything the parent does and optionally more. If the parent requires Gas then the child must accept Gas or one of its parents  That includes interfaces, so FuelType is fine.

That does not apply to siblings, which is what Gas and Battery are.
 [2019-11-15 14:15 UTC] me at paveljanda dot com
Aha, I see.

Well I understand now perfectly that is was an intention and not a bug. Thanks for your answers.

I personally think that it can potentially lead to bad software designs. No EV car will now be able to call anything from the Vehicle class...

We'll have to pay more attention to the software designs now. :P

Cheers,

Pavel
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 20 02:01:29 2024 UTC