Re: [PHP-DEV] Warning for "confusable" types
On Mon, Oct 14, 2019 at 9:21 AM Matteo Beccati wrote: > Hi Stas, > > Maybe we should just change the error message to this?> > must be an > instance of the class "resource", but resource type is > given> > OTOH, since there are actually no resource type hints, and > naming your> class "resource" is an extremely bad idea, having a warning > there> wouldn't hurt too. > I was about to give my +1 on the more generic approach, but type hints > could also be interfaces and "instance of the class X" is not a good fit. > We've tried to improve these error messages a couple of times already, which is why it now say something like "an instance of resource, resource given" rather than just "resource, resource given". This does help, but there's a couple of disadvantages of trying to clarify the error message when a type error occurs, rather than throwing a warning during compilation: * The error is introduced during the function declaration, because that's where the incorrect type is used. Later errors will generally point to uses of the function, which is not where the error lies in this case. * There are many different errors that can be triggered. There's argument types, return types, property types, reference types, as well as things like default values (something I saw recently: Someone being confused about why "public double $foo = 1.5" throws an error.) All of these use different error messages and it's something of a loosing battle to try and address this issue in all of them. * Changing type errors is extremely costly, both as a one-time cost (test updates) and in terms of maintenance. The current type errors we throw for function arguments and returns are quite specialized, which makes it unnecessarily hard to incorporate future type-system extensions. For property types we went down a much simpler route of just printing the canonical type name and not specializing anything, which would get ugly pretty quickly, especially for reference errors that involve more than one type. This is why I think that this confusion needs to be addressed at the root, when the type is actually used, not at some later point. Nikita
Re: [PHP-DEV] Warning for "confusable" types
On that note, since traits aren't possible as a return type. Would it be possible to determine that the given return type is a trait? In that case a more specific error message could also be possible. I don't know much of the internals, so i don't know if that is even possible atm, but it would be a nice addition. On Mon, 14 Oct 2019 at 09:53, Stanislav Malyshev wrote: > > Hi! > > > I was about to give my +1 on the more generic approach, but type hints > > could also be interfaces and "instance of the class X" is not a good fit. > > We could use "class or interface" if that's important. > > -- > Stas Malyshev > smalys...@gmail.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warning for "confusable" types
Hi! > I was about to give my +1 on the more generic approach, but type hints > could also be interfaces and "instance of the class X" is not a good fit. We could use "class or interface" if that's important. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warning for "confusable" types
Hi Stas, > Maybe we should just change the error message to this?> > must be an instance > of the class "resource", but resource type is given> > OTOH, since there are actually no resource type hints, and naming your> class "resource" is an extremely bad idea, having a warning there> wouldn't hurt too. I was about to give my +1 on the more generic approach, but type hints could also be interfaces and "instance of the class X" is not a good fit. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warning for "confusable" types
On Fri, Oct 11, 2019, at 1:46 PM, Bishop Bettini wrote:
> On Fri, Oct 11, 2019 at 1:47 PM Larry Garfield
> wrote:
>
> > On Fri, Oct 11, 2019, at 8:54 AM, Nikita Popov wrote:
> > > Hi internals,
> > >
> > > Something I've seen play out a couple of times: Newbies try to use
> > > something like "integer" or "resource" as a type, and then get a
> > confusing
> > > error message along the lines of "must be an instance of resource,
> > resource
> > > given".
> > >
> > > I would like to throw a compiler warning in this case, that looks as one
> > of
> > > the following:
> > >
> > > > Warning: "integer" will be interpreted as a class type. Did you mean
> > > "int"? Use qualified name or "use" to suppress this warning
> > >
> > > > Warning: "resource" is not a supported builtin type and will be
> > > interpreted as a class type. Use qualified name or "use" to suppress this
> > > warning
> > >
> > > This warning only triggers if the type is lowercase (integer but not
> > > Integer), is unqualified (integer but not \integer) and is not imported
> > > (there is no "use integer"). This provides multiple ways to avoid the
> > > warning for code that does legitimately want to use lowercase "integer"
> > as
> > > a class type.
> > >
> > > Implementation: https://github.com/php/php-src/pull/4815
> > >
> > > Thoughts?
> > >
> > > Nikita
> >
> > Can you clarify where exactly "compiler warning" would be displayed? Is
> > that an E_WARNING? How would I as a user see a message when I write
> >
> > function foo(integer $a) { ... }
> >
>
> It's an E_COMPILE_WARNING, and arises when the compiler sees you're type
> hinting for a class, but no such class loaded. This checks that given class
> against a known list of "probable mistakes" and provides guidance. Eg, I
> typed "integer $a" when I mean "int $a", and don't understand the current
> compiler warning: "Argument 1 passed to foo must be an instance of integer,
> int given"
>
> See also https://stackoverflow.com/a/40638544/2908724
OK, so it's just taking an existing error message and customizing it in cases
where we have a good idea of what the user got wrong? That's a very +1 in my
book.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warning for "confusable" types
Hi! > Something I've seen play out a couple of times: Newbies try to use > something like "integer" or "resource" as a type, and then get a confusing > error message along the lines of "must be an instance of resource, resource > given". Maybe we should just change the error message to this? must be an instance of the class "resource", but resource type is given OTOH, since there are actually no resource type hints, and naming your class "resource" is an extremely bad idea, having a warning there wouldn't hurt too. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warning for "confusable" types
On Fri, Oct 11, 2019 at 1:47 PM Larry Garfield
wrote:
> On Fri, Oct 11, 2019, at 8:54 AM, Nikita Popov wrote:
> > Hi internals,
> >
> > Something I've seen play out a couple of times: Newbies try to use
> > something like "integer" or "resource" as a type, and then get a
> confusing
> > error message along the lines of "must be an instance of resource,
> resource
> > given".
> >
> > I would like to throw a compiler warning in this case, that looks as one
> of
> > the following:
> >
> > > Warning: "integer" will be interpreted as a class type. Did you mean
> > "int"? Use qualified name or "use" to suppress this warning
> >
> > > Warning: "resource" is not a supported builtin type and will be
> > interpreted as a class type. Use qualified name or "use" to suppress this
> > warning
> >
> > This warning only triggers if the type is lowercase (integer but not
> > Integer), is unqualified (integer but not \integer) and is not imported
> > (there is no "use integer"). This provides multiple ways to avoid the
> > warning for code that does legitimately want to use lowercase "integer"
> as
> > a class type.
> >
> > Implementation: https://github.com/php/php-src/pull/4815
> >
> > Thoughts?
> >
> > Nikita
>
> Can you clarify where exactly "compiler warning" would be displayed? Is
> that an E_WARNING? How would I as a user see a message when I write
>
> function foo(integer $a) { ... }
>
It's an E_COMPILE_WARNING, and arises when the compiler sees you're type
hinting for a class, but no such class loaded. This checks that given class
against a known list of "probable mistakes" and provides guidance. Eg, I
typed "integer $a" when I mean "int $a", and don't understand the current
compiler warning: "Argument 1 passed to foo must be an instance of integer,
int given"
See also https://stackoverflow.com/a/40638544/2908724
Re: [PHP-DEV] Warning for "confusable" types
On Fri, Oct 11, 2019, 6:47 PM Larry Garfield wrote:
> On Fri, Oct 11, 2019, at 8:54 AM, Nikita Popov wrote:
> > Hi internals,
> >
> > Something I've seen play out a couple of times: Newbies try to use
> > something like "integer" or "resource" as a type, and then get a
> confusing
> > error message along the lines of "must be an instance of resource,
> resource
> > given".
> >
> > I would like to throw a compiler warning in this case, that looks as one
> of
> > the following:
> >
> > > Warning: "integer" will be interpreted as a class type. Did you mean
> > "int"? Use qualified name or "use" to suppress this warning
> >
> > > Warning: "resource" is not a supported builtin type and will be
> > interpreted as a class type. Use qualified name or "use" to suppress this
> > warning
> >
> > This warning only triggers if the type is lowercase (integer but not
> > Integer), is unqualified (integer but not \integer) and is not imported
> > (there is no "use integer"). This provides multiple ways to avoid the
> > warning for code that does legitimately want to use lowercase "integer"
> as
> > a class type.
> >
> > Implementation: https://github.com/php/php-src/pull/4815
> >
> > Thoughts?
> >
> > Nikita
>
> Can you clarify where exactly "compiler warning" would be displayed? Is
> that an E_WARNING? How would I as a user see a message when I write
>
> function foo(integer $a) { ... }
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
As a warning I think, seems I've encountered those errors many times ago.
The solution Nikita propose is good, it doesn't deal with too much.
These errors are there already, only just giving it a properly meaningful
description.
+1 for the proposed description.
Re: [PHP-DEV] Warning for "confusable" types
On Fri, Oct 11, 2019, at 8:54 AM, Nikita Popov wrote:
> Hi internals,
>
> Something I've seen play out a couple of times: Newbies try to use
> something like "integer" or "resource" as a type, and then get a confusing
> error message along the lines of "must be an instance of resource, resource
> given".
>
> I would like to throw a compiler warning in this case, that looks as one of
> the following:
>
> > Warning: "integer" will be interpreted as a class type. Did you mean
> "int"? Use qualified name or "use" to suppress this warning
>
> > Warning: "resource" is not a supported builtin type and will be
> interpreted as a class type. Use qualified name or "use" to suppress this
> warning
>
> This warning only triggers if the type is lowercase (integer but not
> Integer), is unqualified (integer but not \integer) and is not imported
> (there is no "use integer"). This provides multiple ways to avoid the
> warning for code that does legitimately want to use lowercase "integer" as
> a class type.
>
> Implementation: https://github.com/php/php-src/pull/4815
>
> Thoughts?
>
> Nikita
Can you clarify where exactly "compiler warning" would be displayed? Is that
an E_WARNING? How would I as a user see a message when I write
function foo(integer $a) { ... }
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Warning for "confusable" types
Hi internals, Something I've seen play out a couple of times: Newbies try to use something like "integer" or "resource" as a type, and then get a confusing error message along the lines of "must be an instance of resource, resource given". I would like to throw a compiler warning in this case, that looks as one of the following: > Warning: "integer" will be interpreted as a class type. Did you mean "int"? Use qualified name or "use" to suppress this warning > Warning: "resource" is not a supported builtin type and will be interpreted as a class type. Use qualified name or "use" to suppress this warning This warning only triggers if the type is lowercase (integer but not Integer), is unqualified (integer but not \integer) and is not imported (there is no "use integer"). This provides multiple ways to avoid the warning for code that does legitimately want to use lowercase "integer" as a class type. Implementation: https://github.com/php/php-src/pull/4815 Thoughts? Nikita
