This parameter type widening RFC, I didn't know about, but I have a remark.
The feature, as implemented, will allow accidental omission of type-hints,
will it not?
Previously, the implements keyword guaranteed a correctly type-hinted
implementation - it's now possible to (purposefully, obviously, in certain
rare cases) omit type-hints by accident, making the implements keyword much
less (or not any) of a guarantee that the interface is implemented
correctly.
The addition of an explicit "mixed" or "any" pseudo-type would have made
this possible, without losing the guarantee that the implements keyword
used to provide - that is, it would have been possible to have this feature
for the few cases where it's useful, without affecting safety in the
majority of other cases where it's not.
I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.
Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.
The RFC page doesn't link to any discussion, and the Github thread was shut
down after some negative remarks.
I didn't see a discussion or a vote here on internals - did I miss
something? Where or how did this get discussed and passed? Are these
discussions happening somewhere else besides internals now?
This parameter type widening RFC, I didn't know about, but I have a remark.
The feature, as implemented, will allow accidental omission of type-hints,
will it not?Previously, the implements keyword guaranteed a correctly type-hinted
implementation - it's now possible to (purposefully, obviously, in certain
rare cases) omit type-hints by accident, making the implements keyword much
less (or not any) of a guarantee that the interface is implemented
correctly.The addition of an explicit "mixed" or "any" pseudo-type would have made
this possible, without losing the guarantee that the implements keyword
used to provide - that is, it would have been possible to have this feature
for the few cases where it's useful, without affecting safety in the
majority of other cases where it's not.I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.The RFC page doesn't link to any discussion, and the Github thread was shut
down after some negative remarks.I didn't see a discussion or a vote here on internals - did I miss
something? Where or how did this get discussed and passed? Are these
discussions happening somewhere else besides internals now?
Hey Rasmus!
- Discussion: https://marc.info/?t=147972136200001&r=1&w=2
I also had a look at the GitHub discussion, and I think that the things
that were written there have nothing to do with your concern. The people
commenting there simply did not understand LSP.
That being said, your remark is actually more than legit and I have to
agree that this is indeed just crying out for static analysis tools to
add more warnings to code.
I would very much love to see "any", but introducing yet another keyword
is imho not a good idea. We have already an extremely mixed up
terminology, hence, "mixed" would do and match the existing grammar of
PHP nicely.
There is time left for PHP 7.2 (20. July I think), maybe this is fixable
before that? I guess this will require an RFC too though.
--
Richard "Fleshgrinder" Fussenegger
Hi,
I also had a look at the GitHub discussion, and I think that the things
that were written there have nothing to do with your concern. The people
commenting there simply did not understand LSP.
Well, here's one who doesn't claim to fully understand LSP -> Me
I'm trying, but fail to find a source that says replacing stdClass
with mixed/any/etc is ok. Every (re)definition and example talks about
substituting objects with their subtypes, and one particular source*
even explicitly defines what is considered a subtype.
The RFC is very much lacking in details and I can only assume, by the
above logic, that it considers "mixed" a subtype of ... everything?
What am I missing?
Cheers,
Andrey.
I'm trying, but fail to find a source that says replacing stdClass
with mixed/any/etc is ok.
It is OK for a subtype to handle a wider range of types than its contract,
it is not ok for the subtype to handle a smaller range of types than its
contract.
A contract says what you "must" be able to do to satisfy it, so the
invariants hold.
Marco Pivetta
Hi Marco,
I'm trying, but fail to find a source that says replacing stdClass
with mixed/any/etc is ok.It is OK for a subtype to handle a wider range of types than its contract,
it is not ok for the subtype to handle a smaller range of types than its
contract.A contract says what you "must" be able to do to satisfy it, so the
invariants hold.
Sorry, but by "source" I didn't mean somebody state it here. :)
When I said I don't claim to fully understand LSP, I didn't mean "tell
me what it is" - I'm familiar with it. What I don't understand is how
do we get from this:
Wikipedia:
an object of type T may be substituted with any object of a subtype S
To this:
A single type may be substituted by any other type
Cheers,
Andrey.
Sorry, but by "source" I didn't mean somebody state it here. :)
When I said I don't claim to fully understand LSP, I didn't mean "tell
me what it is" - I'm familiar with it. What I don't understand is how
do we get from this:Wikipedia:
an object of type T may be substituted with any object of a subtype S
To this:
A single type may be substituted by any other type
Cheers,
Andrey.
Hey Andrey!
Do not mix the LSP with type variance on arguments or return types. The
LSP is targeting subtyping directly. The quoted sentence refers to the
fact that S is not allowed to behave differently than T. I think you get
a better understanding if you read the "A typical violation" part of the
Wikipedia article.
The problem we are dealing with is variance of method arguments (and
return values). The corresponding Wikipedia article is here:
https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science)
Specifically contravariant arguments:
Hope this helps to clarify things. :)
--
Richard "Fleshgrinder" Fussenegger
Andrey Andreev narf@devilix.net wrote:
When I said I don't claim to fully understand LSP
Hi Andrey,
The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.
(disclaimer - I helped Niklas draft the RFC, but I still muddle up the
terms occassionaly also.)
What I don't understand is how do we get from this...To this:
It's not difficult, just subtle. First, some code, to define some classes:
interface A {
function foo(Bar $bar);
}
class B implements A {
function foo($bar) {
// this function is capable of handling either objects
// of type Bar, or strings
}
}
There are two separate things here.
First - method B::foo is contravariant to method A::foo, which just
means that anything that can be successfully passed as the parameter
to A::foo can also be passed to B::foo.
Second - because of the contravariance, and because PHP allows classes
to implement interfaces, the Liskov substition principle kicks in, and
we can substitute an object of type B anywhere where an object of type
A is expected.
e.g. If we have a function that takes A as a parameter:
function doSomething(A $a) {
$a->foo(new Bar());
}
Both of these are guaranteed* to be work equivalently due to LSP:
doSomething(new A());
doSomething(new B());
by the above logic, that it considers "mixed" a subtype of ... everything?
No, I think you've been confused by people saying LSP where they meant
contravariance. Although class B can accept either an object of type
Bar, or a string that doesn't meant that a string is a sub-type of
Bar.
Instead because B obeys the contravariance rules for the
implementation of the method, an object of type B can be substituted
where A is expected.
Incidentally, this would be a lot nicer if we had passed the union
type RFC, and so were able to write:
class B implements A {
function foo(Bar|string $bar) {
// hurrah for static code inspection.
}
}
Or for avoiding 'clashing' interfaces:
interface C {
function foo(Bar $bar);
}
interface D {
function foo(string $bar);
}
class E implements C, D {
function foo(Bar|string $bar) {
//This method satifies the contracts for both interfaces.
}
}
Every (re)definition and example talks about
substituting objects with their subtypes
It is a little unfortunate that a lot of information about type
systems is presented in object-oriented languages, as the same thing
applies to things that aren't objects, e.g. arrays, generators =>
iterable and also callback functions e.g. the callable passed to
set_exception_handler() must accept at least \Exception in PHP 5.6 and
\Throwable in PHP 7, but it's fine for it to accept a wider set of
parameters.
cheers
Dan
*guarantees about program behaviour with LSP are kind of...bogus. In
fact it's probably better to concentrate on how it usually makes your
program be easier to write, rather than relying too heavily on it for
an application to actually 'behave' correctly in all scenarios.
Hi,
The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.
Perhaps this was the issue ... I was under the impression that LSP was
used as (part of) the motivation for the RFC.
The rest is pretty clear, though thanks for the lengthy explanation. :)
Cheers,
Andrey.
Hi,
On Thu, May 25, 2017 at 11:02 PM, Dan Ackroyd danack@basereality.com
wrote:The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.Perhaps this was the issue ... I was under the impression that LSP was
used as (part of) the motivation for the RFC.
The RFC respects the variance rules, the variance rules are a direct
consequence of LSP. That's why we're talking about LSP in this context. The
implication chain is LSP => variance => this RFC.
Nikita
2017-05-26 10:35 GMT+02:00 Andrey Andreev narf@devilix.net:
Hi,
On Thu, May 25, 2017 at 11:02 PM, Dan Ackroyd danack@basereality.com
wrote:The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.Perhaps this was the issue ... I was under the impression that LSP was
used as (part of) the motivation for the RFC.The rest is pretty clear, though thanks for the lengthy explanation. :)
Sorry for the delay and confusion. As Nikita already mentioned, it's a
consequence of LSP, not directly what LSP is about.
Regards, Niklas
2017-05-26 10:35 GMT+02:00 Andrey Andreev narf@devilix.net:
On Thu, May 25, 2017 at 11:02 PM, Dan Ackroyd danack@basereality.com
wrote:The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.Perhaps this was the issue ... I was under the impression that LSP was
used as (part of) the motivation for the RFC.The rest is pretty clear, though thanks for the lengthy explanation. :)
Sorry for the delay and confusion. As Nikita already mentioned, it's a
consequence of LSP, not directly what LSP is about.
Indeed, it is not about LSP, so I suggest to fix the note in UPGRADING[1].
[1]
https://github.com/php/php-src/blob/e5c273dc0ea4cb0e0583af9b4c5889a8c431264f/UPGRADING#L100-L103
--
Christoph M. Becker
Am 26.05.2017 um 14:16 schrieb Christoph M. Becker:
2017-05-26 10:35 GMT+02:00 Andrey Andreev narf@devilix.net:
On Thu, May 25, 2017 at 11:02 PM, Dan Ackroyd danack@basereality.com
wrote:The RFC specifically didn't mention LSP....because that is separate
from co/contravariance. It's unfortunate for other people to be
throwing the two around at you with a lack of precision.Perhaps this was the issue ... I was under the impression that LSP was
used as (part of) the motivation for the RFC.The rest is pretty clear, though thanks for the lengthy explanation. :)
Sorry for the delay and confusion. As Nikita already mentioned, it's a
consequence of LSP, not directly what LSP is about.Indeed, it is not about LSP, so I suggest to fix the note in UPGRADING[1]
does that also fix the issue https://bugs.php.net/bug.php?id=74394
does that also fix the issue https://bugs.php.net/bug.php?id=74394
Dear Anonymous,
That "issue" is actually 3 issues.
case 1
class A {
public function test($a) {}
}
class B extends A {
public function test(string $a) { }
}
This breaks LSP - because B::test doesn't accept all the things that
A::test can accept and so is unlikely to ever be supported.
case 2
class A {
public function test($a) {}
}
class B extends A {
public function test($a): string {}
}
This works since return types were introduced.
case 3
class A {
public function test($a): string { }
}
class B extends A {
public function test($a) {}
}
This breaks LSP - anything that is calling A::test would only expect a
string to be returned, but B::test can return anything so is unlikely
to ever be supported.
btw I assume that lists@rhsoft.net and spam2 at rhsoft dot net are the
same anonymous person. Quoting some of your words:
spam2 at rhsoft dot net
maybe you guys also don't see the possibility of shared-libraries used by hundrets of
applications by just use "include_path" when you talk about "deplyoment" and "2017"....that is not possible with a stupid fatal error
lists@rhsoft.net wrote:
frankly get out of my sight with composer
i maintain the whole webstacke for many years at my own (rpm packages) and so the Fedora > repos have excluded anything relevant to PHP - trying to build composer by just download > the src.rpm ends in a ton of build dependencies
we maintain a large codebase (250000 LOC) over 15 years
Your tone of words is quite rude and dismissive of other people both
on this list and in the bug reports. That's really not a good way to
persuade people, but particularly so when you seem to not understand
subtle computer science principles but instead want PHP to be
customised for your own way of working.
If you're installing Composer through an RPM and it's requiring you to
install lots of dependencies, something has gone horribly wrong.
Composer is a standalone phar executable which just needs a normal PHP
environment.
I'd strongly recommend having another go at using it, and in general
opening yourself up to new ways of doing things rather than being rude
to people who don't share your way of working.
cheers
Dan
Am 26.05.2017 um 16:26 schrieb Dan Ackroyd:
does that also fix the issue https://bugs.php.net/bug.php?id=74394
Dear Anonymous,
That "issue" is actually 3 issues.
case 1
class A {
public function test($a) {}
}
class B extends A {
public function test(string $a) { }
}This breaks LSP - because B::test doesn't accept all the things that
A::test can accept and so is unlikely to ever be supported.
and how is that a problem?
A accepts anything
B limits it's inut to a subset of "anything"
case 2
class A {
public function test($a) {}
}class B extends A {
public function test($a): string {}
}This works since return types were introduced.
but is not helpful when you introduce typehints and have to change all
consumers before and at least it should be consistent while param
typehints trigger warnings and return-types fatal errors
case 3
class A {
public function test($a): string { }
}class B extends A {
public function test($a) {}
}This breaks LSP - anything that is calling A::test would only expect a
string to be returned, but B::test can return anything so is unlikely
to ever be supported.
and how is that a practical problem?
if i add a return-type to A it is again a subset of "anything" and can
be no problem for the extending class and as a cosumer of B i do not
need anything to know about the parent method
Your tone of words is quite rude and dismissive of other people both
on this list and in the bug reports. That's really not a good way to
persuade people, but particularly so when you seem to not understand
subtle computer science principles but instead want PHP to be
customised for your own way of working.
i understand more than you imagine
If you're installing Composer through an RPM and it's requiring you to
install lots of dependencies, something has gone horribly wrong.
Composer is a standalone phar executable which just needs a normal PHP
environment.
you don't understand what i talk about - when i maintain the complete
webstack with own packages no distribution package ever will work with
php subpackage dependencies
I'd strongly recommend having another go at using it, and in general
opening yourself up to new ways of doing things rather than being rude
to people who don't share your way of working
i have no need for composer and i just pointed out that people thinking
"hey there is a package manager named composer and that's the new way
everyting works" are just plain wrong
PHP is a programming language and the same way as i never compromise
my environment with perl CPAN stuff or "pip" i won't for PHP
people these days seem to think having a ton of package managers
parallel and going back to days with static linking by using
flatpak/snap is a long term solution - it is not - but that same people
also don't maintain servers without reinstall over decades or maintain a
whole application stack for more the a decade without throwing it all
alway and start from scratch every few years
Am 26.05.2017 um 16:26 schrieb Dan Ackroyd:
does that also fix the issue https://bugs.php.net/bug.php?id=74394
Dear Anonymous,
That "issue" is actually 3 issues.
case 1
class A {
public function test($a) {}
}
class B extends A {
public function test(string $a) { }
}This breaks LSP - because B::test doesn't accept all the things that
A::test can accept and so is unlikely to ever be supported.and how is that a problem?
A accepts anything
B limits it's inut to a subset of "anything"Because, if I have a function foo(A $a) { $a->test(new Bar()) } that I
cannot pass B into this function, even though it is a subclass of A. You
should be able to pass any subclass where its parent is expected, allowing
parameter narrowing (covariance) would break this.
Am 26.05.2017 um 18:10 schrieb Ryan Pallas:
On Fri, May 26, 2017 at 9:01 AM, lists@rhsoft.net
mailto:lists@rhsoft.net <lists@rhsoft.net mailto:lists@rhsoft.net>
wrote:Am 26.05.2017 um 16:26 schrieb Dan Ackroyd: On 26 May 2017 at 13:23, lists@rhsoft.net <mailto:lists@rhsoft.net> <lists@rhsoft.net <mailto:lists@rhsoft.net>> wrote: does that also fix the issue https://bugs.php.net/bug.php?id=74394 <https://bugs.php.net/bug.php?id=74394> Dear Anonymous, That "issue" is actually 3 issues. case 1 class A { public function test($a) {} } class B extends A { public function test(string $a) { } } This breaks LSP - because B::test doesn't accept all the things that A::test can accept and so is unlikely to ever be supported. and how is that a problem? A accepts anything B limits it's inut to a *subset* of "anything"Because, if I have a function foo(A $a) { $a->test(new Bar()) } that I
cannot pass B into this function, even though it is a subclass of A. You
should be able to pass any subclass where its parent is expected,
allowing parameter narrowing (covariance) would break this.
well, that case is at least a warning
but add a return-type to the parent is a fatal error until you update
the child class and that makes introducing return-types harder than it
should be
something like "covariance class {}" would be xtremely helpful for all
that cases where you don't pass objects all day around but have them as
a library in a singletone - in such cases there is no technical
difference when you add a retrun-type to the parent class then before
except that what was over years just a "@return integer" is now instead
of a comment a language construct
This parameter type widening RFC, I didn't know about, but I have a remark.
The feature, as implemented, will allow accidental omission of type-hints,
will it not?Previously, the implements keyword guaranteed a correctly type-hinted
implementation - it's now possible to (purposefully, obviously, in certain
rare cases) omit type-hints by accident, making the implements keyword much
less (or not any) of a guarantee that the interface is implemented
correctly.The addition of an explicit "mixed" or "any" pseudo-type would have made
this possible, without losing the guarantee that the implements keyword
used to provide - that is, it would have been possible to have this feature
for the few cases where it's useful, without affecting safety in the
majority of other cases where it's not.I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.
One of the primary motivations behind this RFC is to allow the addition of
typehints (for example in library code) without introducing a backwards
compatibility break for any code implementing or extending your
interface/class. Requiring an explicit "mixed" or "any" type would not
allow this. I think this is 95% of the reason for this RFC, so without it,
the change itself is probably not worthwhile.
The RFC page doesn't link to any discussion, and the Github thread was shut
down after some negative remarks.
I didn't see a discussion or a vote here on internals - did I miss
something? Where or how did this get discussed and passed? Are these
discussions happening somewhere else besides internals now?
The RFC was discussed in http://markmail.org/message/2oydyyl45v4korau. You
participated in that discussion.
The vote started in http://markmail.org/message/jgn4hwcgeezzk22w.
Both of those were found by searching for the literal RFC name.
Nikita
One of the primary motivations behind this RFC is to allow the addition of
typehints (for example in library code) without introducing a backwards
compatibility break for any code implementing or extending your
interface/class. Requiring an explicit "mixed" or "any" type would not
allow this. I think this is 95% of the reason for this RFC, so without it,
the change itself is probably not worthwhile.
Yeah, I remember that and it is actually one of the very first sentences
in the RFC:
Implementing this RFC would allow libraries to be upgraded to use
type declarations in their method signatures. Currently adding types
to a method of a class in a library would break any code that extends
that class.
This was/is a great feature for library developers. We should probably
add Rasmus' concern to the list of things that can be added in PHP 8.
Another possible approach here could be to enforce an explicit "mixed"
if "strict_types" are active for the declaring class.
--
Richard "Fleshgrinder" Fussenegger
Hi Rasmus,
This parameter type widening RFC, I didn't know about, but I have a remark.
The feature, as implemented, will allow accidental omission of type-hints,
will it not?
Yes, correct.
Previously, the implements keyword guaranteed a correctly type-hinted
implementation - it's now possible to (purposefully, obviously, in certain
rare cases) omit type-hints by accident, making the implements keyword much
less (or not any) of a guarantee that the interface is implemented
correctly.
The interface will just work correctly, what's the actual problem you're
seeing?
The addition of an explicit "mixed" or "any" pseudo-type would have made
this possible, without losing the guarantee that the implements keyword
used to provide - that is, it would have been possible to have this feature
for the few cases where it's useful, without affecting safety in the
majority of other cases where it's not.I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.
Unfortunately, that doesn't work for the second (maybe even primary) use
case of the RFC: Adding types to a class isn't a BC break anymore with the
RFC, as child classes will just widen the type and need explicit checks as
before. Without the RFC or with a version with "mixed" or "any", method
signatures will be incompatible again and thus be a BC break when adding
types to the base class.
"mixed" has been mentioned in the original discussion, but there was not a
lot of interest and mostly explicit disinterest.
The RFC page doesn't link to any discussion, and the Github thread was shut
down after some negative remarks.I didn't see a discussion or a vote here on internals - did I miss
something? Where or how did this get discussed and passed? Are these
discussions happening somewhere else besides internals now?
- Original discussion: https://externals.io/thread/505
- Vote announcement: https://externals.io/thread/613
- Vote resumption after internals having issues:
https://externals.io/thread/619
Regards, Niklas
The feature, as implemented, will allow accidental omission of
type-hints,
will it not?Yes, correct.
So this breaks the expected behavior of interfaces.
This will be harmful to everyone, but especially harmful to new
programmers, who may never even learn how to correctly implement an
interface, since now they don't have to.
Promising to address this in a future version isn't helpful, as that would
be a BC break, which rarely anyone condones, even for major releases -
changing it now isn't a BC break, so it's easy, but fixing it will be a BC
break, which will be very hard to push through.
Addressing this by allowing developers to opt-in to proper interface
validation in strict mode isn't helpful either, and especially not to new
programmers who are likely much farther from learning about strict mode
than they are from learning about basic things like properly implementing
an interface - and much, much further from learning about parameter
type-hint widening.
IMO, this is likely to become another one of those historically bad design
decisions that were made in spite of everything we know about other
programming languages :-(
Hi Rasmus,
This parameter type widening RFC, I didn't know about, but I have a
remark.The feature, as implemented, will allow accidental omission of type-hints,
will it not?Yes, correct.
Previously, the implements keyword guaranteed a correctly type-hinted
implementation - it's now possible to (purposefully, obviously, in certain
rare cases) omit type-hints by accident, making the implements keyword
much
less (or not any) of a guarantee that the interface is implemented
correctly.The interface will just work correctly, what's the actual problem you're
seeing?The addition of an explicit "mixed" or "any" pseudo-type would have made
this possible, without losing the guarantee that the implements keyword
used to provide - that is, it would have been possible to have this
feature
for the few cases where it's useful, without affecting safety in the
majority of other cases where it's not.I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting
a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.Unfortunately, that doesn't work for the second (maybe even primary) use
case of the RFC: Adding types to a class isn't a BC break anymore with the
RFC, as child classes will just widen the type and need explicit checks as
before. Without the RFC or with a version with "mixed" or "any", method
signatures will be incompatible again and thus be a BC break when adding
types to the base class."mixed" has been mentioned in the original discussion, but there was not a
lot of interest and mostly explicit disinterest.The RFC page doesn't link to any discussion, and the Github thread was
shut
down after some negative remarks.I didn't see a discussion or a vote here on internals - did I miss
something? Where or how did this get discussed and passed? Are these
discussions happening somewhere else besides internals now?
- Original discussion: https://externals.io/thread/505
- Vote announcement: https://externals.io/thread/613
- Vote resumption after internals having issues: https://externals.io/
thread/619Regards, Niklas
So this breaks the expected behavior of interfaces.
No it doesn't. It allows interfaces to use contravariance within PHP's
type system, which is different from before, but it doesn't break
anything.
Niklas Keller wrote:
The interface will just work correctly, what's the actual problem you're
seeing?
Rasmus, not answering people's questions and instead saying emotional
stuff suggesting other people have made historically bad decisions
isn't a productive way to conduct a conversation.
cheers
Dan
So this breaks the expected behavior of interfaces.
No it doesn't. It allows interfaces to use contravariance within PHP's
type system, which is different from before, but it doesn't break
anything.
The RFC explicitly states that this is not contravariance.
And if it's not clear, what I mean by "breaks the expected behavior", is,
as far as I understand, what this RFC actually does is it removes
expected behavior.
Correct me if I'm wrong, but really all this RFC does, is remove the
requirement that and implementation of an interface be type-hinted?
To me, that's "broken".
You're favorizing a very, very small number of isolated, valid use-cases -
by removing the normal, natural, expected constraint that an implementation
be type-hinted as specified by the interface. It's part of the reason
interfaces even exist - and this simply ignores that and removes these
checks.
In my opinion, this will very likely lead to a large number of accidental
type-hint omissions - and a very, very small number of actually useful
solutions to real problems.
In my opinion, we can do much better than that, by adding a mixed
type-hint, which will ensure people aren't just accidentally omitting
type-hints and instead explicitly opting out of it to achieve parameter
widening. Solving this from the start also ensures we can have a clean
implementation of real contravariance in the future, without that being a
break change.
Rasmus, not answering people's questions and instead saying emotional
stuff suggesting other people have made historically bad decisions
isn't a productive way to conduct a conversation.
Okay, I'm emotional, sorry about that.
But I think I'm pointing out a real problem and a real possible solution
that doesn't make the trade-offs of the current solution.
That is, I'm fine with introducing this simplified type of contravariance,
but I think it needs to be made explicit - simply omitting a type-hint does
not indicate the desired behavior; it doesn't indicate anything, and
thereby creates a new problem case of accidental type-hint omission.
Is there a very good reason we shouldn't address that, if it's at all
possible?
On Thu, May 25, 2017 at 12:32 PM, Dan Ackroyd danack@basereality.com
wrote:
So this breaks the expected behavior of interfaces.
No it doesn't. It allows interfaces to use contravariance within PHP's
type system, which is different from before, but it doesn't break
anything.Niklas Keller wrote:
The interface will just work correctly, what's the actual problem you're
seeing?Rasmus, not answering people's questions and instead saying emotional
stuff suggesting other people have made historically bad decisions
isn't a productive way to conduct a conversation.cheers
Dan
2017-05-28 12:30 GMT+02:00 Rasmus Schultz rasmus@mindplay.dk:
So this breaks the expected behavior of interfaces.
No it doesn't. It allows interfaces to use contravariance within PHP's
type system, which is different from before, but it doesn't break
anything.The RFC explicitly states that this is not contravariance.
Where does it state that? (The "true" in the future scope section should
probably be replaced with "full".)
And if it's not clear, what I mean by "breaks the expected behavior", is,
as far as I understand, what this RFC actually does is it removes
expected behavior.
How does this remove expected behavior? Could you show a concrete example
of code that breaks?
Correct me if I'm wrong, but really all this RFC does, is remove the
requirement that and implementation of an interface be type-hinted?
Correct, but not just for interfaces.
To me, that's "broken".
As said, please show a concrete example that breaks.
You're favorizing a very, very small number of isolated, valid use-cases -
by removing the normal, natural, expected constraint that an implementation
be type-hinted as specified by the interface. It's part of the reason
interfaces even exist - and this simply ignores that and removes these
checks.
No, interfaces exist to guarantee the caller can always pass a type XY
there. With type constraints removed in a subclass, this guarantee does not
break.
In my opinion, this will very likely lead to a large number of accidental
type-hint omissions - and a very, very small number of actually useful
solutions to real problems.In my opinion, we can do much better than that, by adding a mixed
type-hint, which will ensure people aren't just accidentally omitting
type-hints and instead explicitly opting out of it to achieve parameter
widening. Solving this from the start also ensures we can have a clean
implementation of real contravariance in the future, without that being a
break change.
You're, again, ignoring one of the major benefits of the RFC: Being able to
move from explicit type checks to type declarations without breaking
subclasses.
Rasmus, not answering people's questions and instead saying emotional
stuff suggesting other people have made historically bad decisions
isn't a productive way to conduct a conversation.Okay, I'm emotional, sorry about that.
But I think I'm pointing out a real problem and a real possible solution
that doesn't make the trade-offs of the current solution.
You haven't pointed out a real problem, yet.
Regards, Niklas
Alright, here's an example of code where a type-hint was accidentally left
out.
Before this change:
And after this change:
In other words, accidental omission of type-hints now means that this
faulty implementation is valid code.
And hopefully something will error, deep inside a call-stack, maybe in a
database-component that performs a type-check or something, but there is no
guarantee that it will error at all - if you passed an object that does
have a getID() method, because there's actually no type-check for the
Identity interface when calling the implementation, worst case, it actually
executes and quietly saves something to the database.
No, interfaces exist to guarantee the caller can always pass a type XY
there.
With type constraints removed in a subclass, this guarantee does not
break.
No, but you no longer get a type-check.
So yes, for correct code, there is no problem - but if you didn't write
the code correctly the first time, with this change, you may never even
know you've done something wrong, or you may have to drill through a much
larger call-stack and try to derive the source of the problem.
Before this change, a faulty implementation would simply get rejected.
To me, that's a setback that leads to harder-to-debug errors, and increases
learning-curve.
With an explicit mixed type-hint, you can still achieve what you want in
those rare cases where this simplified kind of contravariance is actually
useful, without also creating new problem cases for everyday coding.
2017-05-28 12:30 GMT+02:00 Rasmus Schultz rasmus@mindplay.dk:
So this breaks the expected behavior of interfaces.
No it doesn't. It allows interfaces to use contravariance within PHP's
type system, which is different from before, but it doesn't break
anything.The RFC explicitly states that this is not contravariance.
Where does it state that? (The "true" in the future scope section should
probably be replaced with "full".)And if it's not clear, what I mean by "breaks the expected behavior", is,
as far as I understand, what this RFC actually does is it removes
expected behavior.How does this remove expected behavior? Could you show a concrete
example of code that breaks?Correct me if I'm wrong, but really all this RFC does, is remove the
requirement that and implementation of an interface be type-hinted?Correct, but not just for interfaces.
To me, that's "broken".
As said, please show a concrete example that breaks.
You're favorizing a very, very small number of isolated, valid use-cases -
by removing the normal, natural, expected constraint that an
implementation
be type-hinted as specified by the interface. It's part of the reason
interfaces even exist - and this simply ignores that and removes these
checks.No, interfaces exist to guarantee the caller can always pass a type XY
there. With type constraints removed in a subclass, this guarantee does not
break.In my opinion, this will very likely lead to a large number of accidental
type-hint omissions - and a very, very small number of actually useful
solutions to real problems.In my opinion, we can do much better than that, by adding a mixed
type-hint, which will ensure people aren't just accidentally omitting
type-hints and instead explicitly opting out of it to achieve parameter
widening. Solving this from the start also ensures we can have a clean
implementation of real contravariance in the future, without that being a
break change.You're, again, ignoring one of the major benefits of the RFC: Being able
to move from explicit type checks to type declarations without breaking
subclasses.Rasmus, not answering people's questions and instead saying emotional
stuff suggesting other people have made historically bad decisions
isn't a productive way to conduct a conversation.Okay, I'm emotional, sorry about that.
But I think I'm pointing out a real problem and a real possible solution
that doesn't make the trade-offs of the current solution.You haven't pointed out a real problem, yet.
Regards, Niklas
With an explicit mixed type-hint, you can still achieve what you want
You are making a valid point in general, but you clearly haven't read or
understood the explanations you've been given why an explicit mixed
type-hint will NOT achieve what the authors of this RFC wanted.
If an existing class implements a library's interface, or extends a
library's base class, with no type hint, then the library currently has
to break both backward and forward compatibility to add type hints: the
user of the library will have to add the type hints to their
implementation, and once they do so, they will have to require that
minimum version of the library.
If the user of the library has to add "mixed" to all their method
signatures to use the now-type-hinted library, the library still has to
declare a breaking change, so that feature would not help this use case,
even if it would be better in others.
Regards,
--
Rowan Collins
[IMSoP]
So, if I understand everything here correctly
interface Foo
{
public function bar(Boo $Boo);
}
and
interface Foo
{
/*
* @param $Boo, pretty please accept type Boo here
*/
public function bar($Boo);
}
will be equivalent in 7.2? :(
Regards,
Aidan
With an explicit mixed type-hint, you can still achieve what you want
You are making a valid point in general, but you clearly haven't read or
understood the explanations you've been given why an explicit mixed
type-hint will NOT achieve what the authors of this RFC wanted.If an existing class implements a library's interface, or extends a
library's base class, with no type hint, then the library currently has to
break both backward and forward compatibility to add type hints: the user
of the library will have to add the type hints to their implementation,
and once they do so, they will have to require that minimum version of
the library.If the user of the library has to add "mixed" to all their method
signatures to use the now-type-hinted library, the library still has to
declare a breaking change, so that feature would not help this use case,
even if it would be better in others.Regards,
--
Rowan Collins
[IMSoP]
So, if I understand everything here correctly
...
will be equivalent in 7.2? :(
Not equivalent. Adding the Boo param type to an implementation, when
it is only a comment in the parent gives an error.
interface Foo
{
/*
- @param $Boo, pretty please accept type Boo here
*/
public function bar( $Boo);
}
class Zot implements Foo {
public function bar(Boo $Boo) {}
}
Fatal error: Declaration of Zot::bar(Boo $Boo) must be compatible with
Foo::bar($Boo) in %s on line %d
cheers
Dan
Am 28.05.2017 um 23:28 schrieb Dan Ackroyd:
So, if I understand everything here correctly
...
will be equivalent in 7.2? :(Not equivalent. Adding the Boo param type to an implementation, when
it is only a comment in the parent gives an error.interface Foo
{
/*
* @param $Boo, pretty please accept type Boo here
*/
public function bar( $Boo);
}class Zot implements Foo {
public function bar(Boo $Boo) {}
}Fatal error: Declaration of Zot::bar(Boo $Boo) must be compatible with
Foo::bar($Boo) in %s on line %d
all these things should be just warnings instead fatal errors no matter
if we talk about params or return-types simply because any sane code
over years had types in the doc-block comments and now with throw fatal
errors you just make it only harder move that comment hints to code
So, if I understand everything here correctly
interface Foo { public function bar(Boo $Boo); }and
interface Foo { /* * @param $Boo, pretty please accept type Boo here */ public function bar($Boo); }will be equivalent in 7.2? :(
No, what's being proposed is that this will be allowed:
interface Foo
{
public function bar(Boo $Boo);
}
class Bar implements Foo
{
public function bar($Boo) { ... }
}
Whereas currently, the class would have to look like this:
class Bar implements Foo
{
public function bar(Boo $Boo) { ... }
}
An interface's job is to provide guarantees to calling code about where
and how an object can be used. It doesn't place any limits on what
else the class can do - class Bar might have extra methods, public
properties, be written in a completely different style, etc. Accepting
extra types as well as the one specified in the interface is in the
same category: the class is doing more than the interface, and that's
none of the interface's business.
I think some of the examples here are approaching interfaces as a
broader contract, that allows a library author to say "you must
implement the library this way". It's similar to the debate about
whether strict type hints should be the choice of the caller or the
callee. If you treat each module of the program as a black box, then all
that matters is the guarantees of what's acceptable, which type hints
and interfaces provide; but if you want more control over how somebody
meets those guarantees, then I can see that this RFC, and the
caller-selected type hint modes, would feel like a step in the wrong
direction.
Regards,
--
Rowan Collins
[IMSoP]
To me, that's a setback that leads to harder-to-debug errors, and increases
learning-curve.
Everything is a trade-off. The type widening RFC makes it easier to
move to parameter types for existing libraries.
While I can see what you mean about how it could lead to more
accidental errors, for me at least, it would be actually quite hard to
not notice those errors in the code.
First, when people are programming against the interface rather than
the implementation, that type of mistake is much easier to spot.
function foo(Repository $repo) {
$repo->save(new stdClass());
// The above line is marked as an error in any decent IDE
// or code analysis tool.
}
Second, accidentally dropping the type from the parameter is also an
easy mistake to spot.
class UserRepository implements Repository {
public function save($item) {
// When inside this function the programmer has no idea
// what type item is. Any call to a method or property will
// be marked as a potential error either by an IDE, static
// analysis tool, or just the programmer realising
// "that parameter has no type".
}
}
So, to me (and possibly others who voted for the RFC) the ability to
add types to interfaces without breaking implementing library is a
good trade-off.
And yes, the type system in PHP could definitely be improved. Perhaps
you could propose an RFC? I suspect it might need to target PHP 8. I'm
personally hoping that some form of the union type RFC could return,
though possibly in a more general form.
cheers
Dan
Ack
The type widening RFC makes it easier to move to parameter types for
existing libraries.
I'm curious to see how you'll set the version constraint in your
composer.json file.
After adding a scalar type-hint to an interface, which is a breaking change
in 7.0 and 7.1, but a non-breaking change in 7.2, is the new version of
your package a major or minor release?
If you release it as minor, you'll need a PHP version constraint like
"^5.5, ^7.2", but excluding 7.0-7.1 In that way is itself a breaking
change, so likely your only practical move is to release (and commit to
maintain!) both major and minor versions, which doesn't sound easy at all.
To me, that's a setback that leads to harder-to-debug errors, and
increases
learning-curve.Everything is a trade-off. The type widening RFC makes it easier to
move to parameter types for existing libraries.While I can see what you mean about how it could lead to more
accidental errors, for me at least, it would be actually quite hard to
not notice those errors in the code.First, when people are programming against the interface rather than
the implementation, that type of mistake is much easier to spot.function foo(Repository $repo) {
$repo->save(new stdClass());
// The above line is marked as an error in any decent IDE
// or code analysis tool.
}Second, accidentally dropping the type from the parameter is also an
easy mistake to spot.class UserRepository implements Repository {
public function save($item) {
// When inside this function the programmer has no idea
// what type item is. Any call to a method or property will
// be marked as a potential error either by an IDE, static
// analysis tool, or just the programmer realising
// "that parameter has no type".
}
}So, to me (and possibly others who voted for the RFC) the ability to
add types to interfaces without breaking implementing library is a
good trade-off.And yes, the type system in PHP could definitely be improved. Perhaps
you could propose an RFC? I suspect it might need to target PHP 8. I'm
personally hoping that some form of the union type RFC could return,
though possibly in a more general form.cheers
Dan
Ack
The type widening RFC makes it easier to move to parameter types for
existing libraries.I'm curious to see how you'll set the version constraint in your
composer.json file.After adding a scalar type-hint to an interface, which is a breaking change
in 7.0 and 7.1, but a non-breaking change in 7.2, is the new version of
your package a major or minor release?If you release it as minor, you'll need a PHP version constraint like
"^5.5, ^7.2", but excluding 7.0-7.1 In that way is itself a breaking
change, so likely your only practical move is to release (and commit to
maintain!) both major and minor versions, which doesn't sound easy at all.
That's a good point, and reduces the immediate value of the change
slightly, but there is perhaps still value in making upgrading easier
on users.
For instance, you might want to use some other feature of PHP 7.2
anyway, so be willing to set that as your minimum version, but still not
want users to have to go through checking all their code for the correct
type hints. If they're already using 7.2, they might be able to migrate
to your new version without changing a single line of code.
Regards,
--
Rowan Collins
[IMSoP]
Rowan Collins rowan.collins@gmail.com schrieb am Di., 30. Mai 2017, 19:39:
The type widening RFC makes it easier to move to parameter types for
existing libraries.I'm curious to see how you'll set the version constraint in your
composer.json file.
=min-php-version.
After adding a scalar type-hint to an interface, which is a breaking
changein 7.0 and 7.1, but a non-breaking change in 7.2, is the new version of
your package a major or minor release?
Depends on the new minimum version. If it is 7.0 or 7.1, then it's a
breaking change so new major, otherwise new minor.
If you release it as minor, you'll need a PHP version constraint like
"^5.5, ^7.2", but excluding 7.0-7.1 In that way is itself a breaking
As scalar types do not work on PHP 5, why would I use that constraint?
change, so likely your only practical move is to release (and commit to
maintain!) both major and minor versions, which doesn't sound easy at
all.
I don't see a reason to release a new major when increasing the minimum PHP
version requirement. The old major.minor can still get bug fixes but won't
receive new features.
That's a good point, and reduces the immediate value of the change
slightly, but there is perhaps still value in making upgrading easier
on users.For instance, you might want to use some other feature of PHP 7.2
anyway, so be willing to set that as your minimum version, but still not
want users to have to go through checking all their code for the correct
type hints. If they're already using 7.2, they might be able to migrate
to your new version without changing a single line of code.
Regards, Niklas
In my opinion, this will very likely lead to a large number of accidental
type-hint omissions - and a very, very small number of actually useful
solutions to real problems.In my opinion, we can do much better than that, by adding a mixed
type-hint, which will ensure people aren't just accidentally omitting
type-hints and instead explicitly opting out of it to achieve parameter
widening. Solving this from the start also ensures we can have a clean
implementation of real contravariance in the future, without that being a
break change.
Something like that has already been suggested in the original RFC
discussion[1] but has been rejected, so I don't think it makes much
sense to repeat this discussion again.
[1] http://news.php.net/php.internals/97106
--
Christoph M. Becker
I feel like this feature takes a pretty dangerous shortcut by simply
removing a constraint check that we used to have - in favor of supporting a
few rare cases, we removed a guarantee that interfaces and the
implements-keyword has always provided.Those rare cases could have been supported in a safe manner by introducing
a "mixed" or "any" type, which would have made the use of this feature
explicit - which would have removed the risk of accidental omission of
type-hints in the majority of cases.
This was my opinion at voting time which is why i voted against the RFC. It
passed 24 to 5 so it's just something we'll have to live with for now.
The feature, as implemented, will allow accidental omission of type-hints
Hi, stated the purpose of declare(strict_types=1) and that this is
used by developers and libraries that actually crave for strictness,
would it be votable an RFC that reintroduce the Warning only with this
declare active?
Filippo Tessarotto