Properties and multi-getters

[Cédric Champeau <ce...@gmail.com>]

Hi guys,

While migrating Gradle to Groovy 2.4.4, I spent several hours debugging an
integration test that mysteriously failed under 2.4.4. It took me a lot of
time to figure out that the offending line was this one:

https://github.com/gradle/gradle/blob/9d9798019a47aaf034cdb85a931eb8fefdd4060f/subprojects/launcher/src/integTest/groovy/org/gradle/launcher/daemon/testing/DaemonsState.groovy#L44

In Groovy 2.3, `wildcard` called `isWildcard`. Since we added support for
multiple setters (I think it is related, but not 100% sure), the behavior
seem to have changed: now Groovy calls `getWildcard`.

But as you can see, there's a culprit: `getWildcard` is a static method,
while `isWildcard` is an instance one. How silly!

I think this should be a compile time error (having both a static `get`
method and an instance `is` one, and the reverse).

It shouldn't be a problem to have both static or both instance methods, as
long as the behavior is deterministic. That is, if you find both:

boolean getA()
boolean isA()

and that you access the property using:

foo.a

It should *always* be the same method which is called. My preference would
go with the "is" version first, but I'm not sure we make any guarantee on
this.

WDYT?

Re: Properties and multi-getters

[Guillaume Laforge <gl...@gmail.com>]

Right, a compile time error would make such odd situations easier to debug.
Le 21 août 2015 20:11, "Cédric Champeau" <ce...@gmail.com> a
écrit :

> Just to make it clear: the compile time error that I am suggesting is when
> you mix static and non static versions of the same property.
>
> 2015-08-21 20:07 GMT+02:00 Thibault Kruse <ti...@googlemail.com>:
>
>> JavaBeans specification is to use "is" version first (for boolean
>> fields only), so +1 on Groovy adhering to that.
>> +1 to have consistent behavior.
>>
>> Compile time error seems a bit harsh. But maybe codenarc could raise
>> this as code smell in any case.
>>
>> On Fri, Aug 21, 2015 at 5:35 PM, Cédric Champeau
>> <ce...@gmail.com> wrote:
>> > Hi guys,
>> >
>> > While migrating Gradle to Groovy 2.4.4, I spent several hours debugging
>> an
>> > integration test that mysteriously failed under 2.4.4. It took me a lot
>> of
>> > time to figure out that the offending line was this one:
>> >
>> >
>> https://github.com/gradle/gradle/blob/9d9798019a47aaf034cdb85a931eb8fefdd4060f/subprojects/launcher/src/integTest/groovy/org/gradle/launcher/daemon/testing/DaemonsState.groovy#L44
>> >
>> > In Groovy 2.3, `wildcard` called `isWildcard`. Since we added support
>> for
>> > multiple setters (I think it is related, but not 100% sure), the
>> behavior
>> > seem to have changed: now Groovy calls `getWildcard`.
>> >
>> > But as you can see, there's a culprit: `getWildcard` is a static method,
>> > while `isWildcard` is an instance one. How silly!
>> >
>> > I think this should be a compile time error (having both a static `get`
>> > method and an instance `is` one, and the reverse).
>> >
>> > It shouldn't be a problem to have both static or both instance methods,
>> as
>> > long as the behavior is deterministic. That is, if you find both:
>> >
>> > boolean getA()
>> > boolean isA()
>> >
>> > and that you access the property using:
>> >
>> > foo.a
>> >
>> > It should *always* be the same method which is called. My preference
>> would
>> > go with the "is" version first, but I'm not sure we make any guarantee
>> on
>> > this.
>> >
>> > WDYT?
>>
>
>

Re: Properties and multi-getters

[Thibault Kruse <ti...@googlemail.com>]

would the compiler then refuse to compile a class having this, or
merely refuse to resolve the shorthand access "foo.wildcard" if that
is ambiguous (requiring the developer to choose by explicitly invoking
the desired method)? In the latter case, a compiler error (or runtime
exception) seems okay to me.
Compiler error just for defining a class with silly methods should IMO
not be a compiler error.

On Fri, Aug 21, 2015 at 8:11 PM, Cédric Champeau
<ce...@gmail.com> wrote:
> Just to make it clear: the compile time error that I am suggesting is when
> you mix static and non static versions of the same property.
>
> 2015-08-21 20:07 GMT+02:00 Thibault Kruse <ti...@googlemail.com>:
>>
>> JavaBeans specification is to use "is" version first (for boolean
>> fields only), so +1 on Groovy adhering to that.
>> +1 to have consistent behavior.
>>
>> Compile time error seems a bit harsh. But maybe codenarc could raise
>> this as code smell in any case.
>>
>> On Fri, Aug 21, 2015 at 5:35 PM, Cédric Champeau
>> <ce...@gmail.com> wrote:
>> > Hi guys,
>> >
>> > While migrating Gradle to Groovy 2.4.4, I spent several hours debugging
>> > an
>> > integration test that mysteriously failed under 2.4.4. It took me a lot
>> > of
>> > time to figure out that the offending line was this one:
>> >
>> >
>> > https://github.com/gradle/gradle/blob/9d9798019a47aaf034cdb85a931eb8fefdd4060f/subprojects/launcher/src/integTest/groovy/org/gradle/launcher/daemon/testing/DaemonsState.groovy#L44
>> >
>> > In Groovy 2.3, `wildcard` called `isWildcard`. Since we added support
>> > for
>> > multiple setters (I think it is related, but not 100% sure), the
>> > behavior
>> > seem to have changed: now Groovy calls `getWildcard`.
>> >
>> > But as you can see, there's a culprit: `getWildcard` is a static method,
>> > while `isWildcard` is an instance one. How silly!
>> >
>> > I think this should be a compile time error (having both a static `get`
>> > method and an instance `is` one, and the reverse).
>> >
>> > It shouldn't be a problem to have both static or both instance methods,
>> > as
>> > long as the behavior is deterministic. That is, if you find both:
>> >
>> > boolean getA()
>> > boolean isA()
>> >
>> > and that you access the property using:
>> >
>> > foo.a
>> >
>> > It should *always* be the same method which is called. My preference
>> > would
>> > go with the "is" version first, but I'm not sure we make any guarantee
>> > on
>> > this.
>> >
>> > WDYT?
>
>

Re: Properties and multi-getters

[Cédric Champeau <ce...@gmail.com>]

Just to make it clear: the compile time error that I am suggesting is when
you mix static and non static versions of the same property.

2015-08-21 20:07 GMT+02:00 Thibault Kruse <ti...@googlemail.com>:

> JavaBeans specification is to use "is" version first (for boolean
> fields only), so +1 on Groovy adhering to that.
> +1 to have consistent behavior.
>
> Compile time error seems a bit harsh. But maybe codenarc could raise
> this as code smell in any case.
>
> On Fri, Aug 21, 2015 at 5:35 PM, Cédric Champeau
> <ce...@gmail.com> wrote:
> > Hi guys,
> >
> > While migrating Gradle to Groovy 2.4.4, I spent several hours debugging
> an
> > integration test that mysteriously failed under 2.4.4. It took me a lot
> of
> > time to figure out that the offending line was this one:
> >
> >
> https://github.com/gradle/gradle/blob/9d9798019a47aaf034cdb85a931eb8fefdd4060f/subprojects/launcher/src/integTest/groovy/org/gradle/launcher/daemon/testing/DaemonsState.groovy#L44
> >
> > In Groovy 2.3, `wildcard` called `isWildcard`. Since we added support for
> > multiple setters (I think it is related, but not 100% sure), the behavior
> > seem to have changed: now Groovy calls `getWildcard`.
> >
> > But as you can see, there's a culprit: `getWildcard` is a static method,
> > while `isWildcard` is an instance one. How silly!
> >
> > I think this should be a compile time error (having both a static `get`
> > method and an instance `is` one, and the reverse).
> >
> > It shouldn't be a problem to have both static or both instance methods,
> as
> > long as the behavior is deterministic. That is, if you find both:
> >
> > boolean getA()
> > boolean isA()
> >
> > and that you access the property using:
> >
> > foo.a
> >
> > It should *always* be the same method which is called. My preference
> would
> > go with the "is" version first, but I'm not sure we make any guarantee on
> > this.
> >
> > WDYT?
>

Re: Properties and multi-getters

[Thibault Kruse <ti...@googlemail.com>]

JavaBeans specification is to use "is" version first (for boolean
fields only), so +1 on Groovy adhering to that.
+1 to have consistent behavior.

Compile time error seems a bit harsh. But maybe codenarc could raise
this as code smell in any case.

On Fri, Aug 21, 2015 at 5:35 PM, Cédric Champeau
<ce...@gmail.com> wrote:
> Hi guys,
>
> While migrating Gradle to Groovy 2.4.4, I spent several hours debugging an
> integration test that mysteriously failed under 2.4.4. It took me a lot of
> time to figure out that the offending line was this one:
>
> https://github.com/gradle/gradle/blob/9d9798019a47aaf034cdb85a931eb8fefdd4060f/subprojects/launcher/src/integTest/groovy/org/gradle/launcher/daemon/testing/DaemonsState.groovy#L44
>
> In Groovy 2.3, `wildcard` called `isWildcard`. Since we added support for
> multiple setters (I think it is related, but not 100% sure), the behavior
> seem to have changed: now Groovy calls `getWildcard`.
>
> But as you can see, there's a culprit: `getWildcard` is a static method,
> while `isWildcard` is an instance one. How silly!
>
> I think this should be a compile time error (having both a static `get`
> method and an instance `is` one, and the reverse).
>
> It shouldn't be a problem to have both static or both instance methods, as
> long as the behavior is deterministic. That is, if you find both:
>
> boolean getA()
> boolean isA()
>
> and that you access the property using:
>
> foo.a
>
> It should *always* be the same method which is called. My preference would
> go with the "is" version first, but I'm not sure we make any guarantee on
> this.
>
> WDYT?