You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Daniel Sun <re...@hotmail.com> on 2017/12/29 02:40:17 UTC

[VOTE]Support package scope via `package` keyword

Hi all,

      In order to support package scope, we have to add annotation
`@PackageScope` to the target(e.g. class, field, method), which is verbose.
Nathan Harvey has started a thread to discuss specifying the package scope
via `package` keyword[1]. The feature is feasible and has been implemented
in my labs project[2]. 

      Please vote on add the new feature to Apache Groovy 3.0.0 and 2.6.0
releases.

      The vote is open for the next 72 hours and passes if a majority of at
least three +1 PMC votes are cast. 

[ ] +1  The feature sounds good
[ ]   0  I don't have a strong opinion about this, but I assume it's ok 
[ ]  -1  Because...

Cheers,
Daniel.Sun

[1]
http://groovy.329449.n5.nabble.com/Package-specific-syntax-tp5745613.html
[2] https://github.com/danielsun1106/groovy-parser/tree/package-scope




--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by MG <mg...@arscreat.com>.
Not PMC, of course, but I would vote -1 on the original proposal, based on:

 1. package scope is rare (unless someone proves otherwise ;-) )
 2. Having a keyword and annotation is confusing for users
 3. Dropping the annotation will break existing code (unless no one uses
    the feature), and functionality cannot be simulated by someone who
    depends on this
 4. Keeping the annotation gives no benefit for IDEs (IntelliJ)
    (although, after thinking about this, I feel this is a bit "The need
    of the many outweigh..." in any case)
 5. Giving you a little bit extra (in this case: flexibility with
    regards to package scope granularity) to me typically Groovy, and
    one thing that seperates it from other languages
 6. In Groovy one already uses a lot of annotations in some cases
    anyway, so I do not see the problem with having another one on a
    class for a rare case (I do agree, however, that the frequent case
    should be made as simple as possible in the annotation
    implementation, if possible)

This modified idea I find better, but it still only fixes the 
"functionality cannot be simulated by someone who depends on this" part 
of my arguments above, and introduces the problem that the 
fine-granularity feature of package scope in Groovy gets overlooked 
completely, making it a dead feature.

Cheers,
mg


On 31.12.2017 05:50, Nathan Harvey wrote:
> Perhaps another alternative could be introducing a @Scope annotation to apply
> the scoping behavior used by @PackageScope. Eg @Scope(Modifier.PRIVATE) or
> @Scope(value=PRIVATE, target=[CLASSES]).
>
> I suppose the "package" keyword doesn't indicate the full "package private"
> but I don't think that's necessary. My original point remains that using an
> annotation to declare scope is awkward and cumbersome.
>
>
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
>


Re: [VOTE]Support package scope via `package` keyword

Posted by Guillaume Laforge <gl...@gmail.com>.
I'm feeling like being in between -1 and 0.
For the very rare cases where it's used, I (personally) don't care to have
to use an annotation, even if that might seem a bit awkward or cumbersome
(but we use annotations in plenty other contexts anyway because of AST
transformations).
I don't see much value adding this 'package' notation, apart from adding a
bit more confusion for Java developers coming to Groovy and wondering what
the package keyword does in that place.

Guillaume


On Sun, Dec 31, 2017 at 7:24 AM, Paul King <pa...@asert.com.au> wrote:

>
> On Sun, Dec 31, 2017 at 2:50 PM, Nathan Harvey <na...@gmail.com>
> wrote:
>
>> [...] My original point remains that using an
>> annotation to declare scope is awkward and cumbersome.
>
>
> The thinking behind that at the time was that it is comparatively rare to
> use package private
> scope so let's not care if it's a little bit awkward and cumbersome if it
> makes the much more
> common case (public) much more concise. If we think the assumptions have
> changed around
> package private usage since then, it is possibly worth looking at again
> but it isn't something
> that I've noticed. The trend I was noticing some time back was that
> inheritance (protected)
> and package private (predominantly for ease of testing) were becoming less
> common with
> dependency injection and various agile practices reducing usage but
> perhaps things have
> swung the other away recently and I haven't noticed.
>
> Anyway, I'm happy to vote +1 on something that I see as not having a
> downside but for
> something that seems just like a sideways move with different but it's own
> pros/cons then
> I am less enthusiastic.
>
> Cheers, Paul.
>
>


-- 
Guillaume Laforge
Apache Groovy committer & PMC Vice-President
Developer Advocate @ Google Cloud Platform

Blog: http://glaforge.appspot.com/
Social: @glaforge <http://twitter.com/glaforge> / Google+
<https://plus.google.com/u/0/114130972232398734985/posts>

Re: [VOTE]Support package scope via `package` keyword

Posted by Paul King <pa...@asert.com.au>.
On Sun, Dec 31, 2017 at 2:50 PM, Nathan Harvey <na...@gmail.com>
wrote:

> [...] My original point remains that using an
> annotation to declare scope is awkward and cumbersome.


The thinking behind that at the time was that it is comparatively rare to
use package private
scope so let's not care if it's a little bit awkward and cumbersome if it
makes the much more
common case (public) much more concise. If we think the assumptions have
changed around
package private usage since then, it is possibly worth looking at again but
it isn't something
that I've noticed. The trend I was noticing some time back was that
inheritance (protected)
and package private (predominantly for ease of testing) were becoming less
common with
dependency injection and various agile practices reducing usage but perhaps
things have
swung the other away recently and I haven't noticed.

Anyway, I'm happy to vote +1 on something that I see as not having a
downside but for
something that seems just like a sideways move with different but it's own
pros/cons then
I am less enthusiastic.

Cheers, Paul.

Re: [VOTE]Support package scope via `package` keyword

Posted by Nathan Harvey <na...@gmail.com>.
Perhaps another alternative could be introducing a @Scope annotation to apply
the scoping behavior used by @PackageScope. Eg @Scope(Modifier.PRIVATE) or
@Scope(value=PRIVATE, target=[CLASSES]).

I suppose the "package" keyword doesn't indicate the full "package private"
but I don't think that's necessary. My original point remains that using an
annotation to declare scope is awkward and cumbersome. 



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by Paul King <pa...@asert.com.au>.
On Fri, Dec 29, 2017 at 9:35 PM, Jochen Theodorou <bl...@gmx.org> wrote:

> On 29.12.2017 11:58, Paul King wrote:
> [...]
>
>> I am unsure if I made myself clear. I think the default should remain as
>> CLASS but there could be an additional ALL enum value. Then again we could
>> just have a PackageScopeTarget[] ALL constant (though we have an
>> outstanding issue around using constants in annotation attributes we'd have
>> to check didn't get in the way).
>>
>
> It doesn´t make sense on local variables, the package, parameters or
> parameter types... Maybe I misunderstood what you mean with "ALL"
>

When you annotate a class like this:

```
import groovy.transform.PackageScope
import static groovy.transform.PackageScopeTarget.METHODS

@PackageScope(METHODS)
class Foo {
    def method1() {}
    def method2() {}
    def method3() {}
}
```

Then, the three methods are package private and the class is public.

You can alternatively have: @PackageScope([METHODS, CONSTRUCTORS]) or
whatever combinations
you like. For a class with many methods, fields, constructors this is much
more concise than an extra keyword on each member.
I was suggesting ALL as a constant [METHODS, CONSTRUCTORS, FIELDS, CLASS].
Like Cédric, I am not a huge fan of using just package as the keyword and
if we use `package private`, then deprecating the
shorthand variant could lead to ugly verbose code.

Cheers, Paul.


> bye Jochen
>

Re: [VOTE]Support package scope via `package` keyword

Posted by Cédric Champeau <ce...@gmail.com>.
I'd tend to be -1 on the name "package" and 0 on the feature. The -1 is
because "package" is not a modifier by itself. I'd prefer "package private"
(the official Java name for this). So it leads to the 0, because "package
private" is kind of verbose, and doesn't save much from "PackageScope".
However, it drives me to another topic that is related and that we talked
about a few years back: having the ability to use annotations without the
`@`. So you would say `compilestatic class Foo`, or `packagescope Foo foo`.

2017-12-29 12:35 GMT+01:00 Jochen Theodorou <bl...@gmx.org>:

> On 29.12.2017 11:58, Paul King wrote:
> [...]
>
>> I am unsure if I made myself clear. I think the default should remain as
>> CLASS but there could be an additional ALL enum value. Then again we could
>> just have a PackageScopeTarget[] ALL constant (though we have an
>> outstanding issue around using constants in annotation attributes we'd have
>> to check didn't get in the way).
>>
>
> It doesn´t make sense on local variables, the package, parameters or
> parameter types... Maybe I misunderstood what you mean with "ALL"
>
>
> bye Jochen
>

Re: [VOTE]Support package scope via `package` keyword

Posted by Jochen Theodorou <bl...@gmx.org>.
On 29.12.2017 11:58, Paul King wrote:
[...]
> I am unsure if I made myself clear. I think the default should remain as 
> CLASS but there could be an additional ALL enum value. Then again we 
> could just have a PackageScopeTarget[] ALL constant (though we have an 
> outstanding issue around using constants in annotation attributes we'd 
> have to check didn't get in the way).

It doesn´t make sense on local variables, the package, parameters or 
parameter types... Maybe I misunderstood what you mean with "ALL"


bye Jochen

Re: [VOTE]Support package scope via `package` keyword

Posted by Paul King <pa...@asert.com.au>.
On Fri, Dec 29, 2017 at 8:34 PM, Daniel.Sun <su...@apache.org> wrote:

> Hi Paul,
>
> > Just so I understand, the intention is to keep @PackageScope with
> > `package` being an alias for when no target is required. Correct?
>
> Yeah, correct :-)
>
> > Come to think of it, I wonder if there should be an `ALL` target?
> I agree with you that the target of @PackageScope should be `ALL`
>


I am unsure if I made myself clear. I think the default should remain as
CLASS but there could be an additional ALL enum value. Then again we could
just have a PackageScopeTarget[] ALL constant (though we have an
outstanding issue around using constants in annotation attributes we'd have
to check didn't get in the way).


> your vote is ?
>

I am at least a +0, just pondering the real merit given that it doesn't
really save much work for IDE writers and it won't be more concise for a
range of typical use cases. It doesn't scream a +1 for me but I'll ponder
for a few more days. I'm keen to hear more thoughts.

Cheers, Paul.


>
> Cheers,
> Daniel.Sun
>
>
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
>

Re: [VOTE]Support package scope via `package` keyword

Posted by "Daniel.Sun" <su...@apache.org>.
Hi Paul,

> Just so I understand, the intention is to keep @PackageScope with
> `package` being an alias for when no target is required. Correct?

Yeah, correct :-)

> Come to think of it, I wonder if there should be an `ALL` target?
I agree with you that the target of @PackageScope should be `ALL`

your vote is ?

Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by Paul King <pa...@asert.com.au>.
Just so I understand, the intention is to keep @PackageScope with `package`
being an alias for when no target is required. Correct?

If that's correct, it won't eliminate the need for IDEs to scan for
@PackageScope in general but in some common cases, some preliminary
assumptions could be made.

Also, when documenting, we should note that `package` will be more concise
when used in one or two places within a class but @PackageScope will be
more concise if used with a PackageScopeTarget when a few (or more) members
should be made package private. Come to think of it, I wonder if there
should be an `ALL` target?

Paul.

On Fri, Dec 29, 2017 at 12:40 PM, Daniel Sun <re...@hotmail.com>
wrote:

> Hi all,
>
>       In order to support package scope, we have to add annotation
> `@PackageScope` to the target(e.g. class, field, method), which is verbose.
> Nathan Harvey has started a thread to discuss specifying the package scope
> via `package` keyword[1]. The feature is feasible and has been implemented
> in my labs project[2].
>
>       Please vote on add the new feature to Apache Groovy 3.0.0 and 2.6.0
> releases.
>
>       The vote is open for the next 72 hours and passes if a majority of at
> least three +1 PMC votes are cast.
>
> [ ] +1  The feature sounds good
> [ ]   0  I don't have a strong opinion about this, but I assume it's ok
> [ ]  -1  Because...
>
> Cheers,
> Daniel.Sun
>
> [1]
> http://groovy.329449.n5.nabble.com/Package-specific-syntax-tp5745613.html
> [2] https://github.com/danielsun1106/groovy-parser/tree/package-scope
>
>
>
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
>

Re: [VOTE]Support package scope via `package` keyword

Posted by "Daniel.Sun" <su...@apache.org>.
We should collect at least three +1 from PMC(e.g. Guillaume, Jochen, Cédric,
Paul, John, etc. )   ;-)

Here is the complete list :
http://people.apache.org/phonebook.html?pmc=groovy


Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by Nathan Harvey <na...@gmail.com>.
Great work, Daniel. Very happy to see my ideas be made a reality! I won't
vote either way.



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by "Daniel.Sun" <su...@apache.org>.
Hi Jochen,

      Thanks for your voting :-)

> but under the condition to deprecate @PackageScope in 2.6.0 and then I
> would like to have it removed in 3.0.0 (if we want to be serious about
> cleanups, that is) 

       Yeah, we can do better step by step.

Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

Posted by Jochen Theodorou <bl...@gmx.org>.
On 29.12.2017 03:40, Daniel Sun wrote:
> Hi all,
> 
>        In order to support package scope, we have to add annotation
> `@PackageScope` to the target(e.g. class, field, method), which is verbose.
> Nathan Harvey has started a thread to discuss specifying the package scope
> via `package` keyword[1]. The feature is feasible and has been implemented
> in my labs project[2].

Just to be clear, this feature, this is a change for IDEs, not for the 
user directly. For example a project I am working on with like 90k lines 
of code, I think I used package private 3 times...no 2 times, because 
the other got changed.

>        Please vote on add the new feature to Apache Groovy 3.0.0 and 2.6.0
> releases.
> 
>        The vote is open for the next 72 hours and passes if a majority of at
> least three +1 PMC votes are cast.
> 
> [ ] +1  The feature sounds good
> [ ]   0  I don't have a strong opinion about this, but I assume it's ok
> [ ]  -1  Because...

You get a +1 from me, but under the condition to deprecate @PackageScope 
in 2.6.0 and then I would like to have it removed in 3.0.0 (if we want 
to be serious about cleanups, that is)

bye Jochen

Re: [VOTE]Support package scope via `package` keyword

Posted by Daniel Sun <re...@hotmail.com>.
Here is a simple test:
https://github.com/danielsun1106/groovy-parser/blob/package-scope/src/test/resources/core/PackageScope_01x.groovy

My vote is +1

Cheers,
Daniel.Sun




--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html