[GitHub] groovy pull request #441: GROOVY-7951: MethodCallExpression.transformExpress...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/groovy/pull/441


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #438: GROOVY-7948: fix completion of static imports in g...

[jwagenleitner]

Github user jwagenleitner commented on a diff in the pull request:

https://github.com/apache/groovy/pull/438#discussion_r83096173
  
--- Diff: 
subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy
 ---
@@ -87,51 +78,49 @@ class ImportsSyntaxCompletor implements 
IdentifierCompletor {
 return foundMatch
 }
 
-private static final String STATIC_IMPORT_PATTERN = ~/^import static 
([a-z0-9]+\.)+[A-Z][a-zA-Z0-9]*(\.(\*|[^.]+))?$/
+private static final String STATIC_IMPORT_PATTERN = ~/^static 
([a-zA-Z_][a-zA-Z_0-9]*\.)+([a-zA-Z_][a-zA-Z_0-9]*|\*)$/
 
 /**
  * finds matching imported classes or static methods
- * @param prefix
- * @param importSpec
- * @param matches
- * @return
+ * @param importSpec an import statement without the leading 'import ' 
or trailing semicolon
+ * @return all names matching the importSpec
  */
-void collectImportedSymbols(final String importSpec, final 
Collection matches) {
+SortedSet collectImportedSymbols(final String importSpec) {
--- End diff --

While I don't think `collectImportedSymbols` was meant to be called outside 
this class there is a chance it could be since it's a public method.  So to 
avoid a breaking change and to have the chance to include this in the next 2.4 
release (2.4.8) it would be necessary to keep the `void 
collectImportedSymbols(String,Collection)` method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GROOVY-7948) tab completion for static imports in groovysh does not work

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-7948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15569883#comment-15569883
 ] 

ASF GitHub Bot commented on GROOVY-7948:


Github user jwagenleitner commented on a diff in the pull request:

https://github.com/apache/groovy/pull/438#discussion_r83096173
  
--- Diff: 
subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy
 ---
@@ -87,51 +78,49 @@ class ImportsSyntaxCompletor implements 
IdentifierCompletor {
 return foundMatch
 }
 
-private static final String STATIC_IMPORT_PATTERN = ~/^import static 
([a-z0-9]+\.)+[A-Z][a-zA-Z0-9]*(\.(\*|[^.]+))?$/
+private static final String STATIC_IMPORT_PATTERN = ~/^static 
([a-zA-Z_][a-zA-Z_0-9]*\.)+([a-zA-Z_][a-zA-Z_0-9]*|\*)$/
 
 /**
  * finds matching imported classes or static methods
- * @param prefix
- * @param importSpec
- * @param matches
- * @return
+ * @param importSpec an import statement without the leading 'import ' 
or trailing semicolon
+ * @return all names matching the importSpec
  */
-void collectImportedSymbols(final String importSpec, final 
Collection matches) {
+SortedSet collectImportedSymbols(final String importSpec) {
--- End diff --

While I don't think `collectImportedSymbols` was meant to be called outside 
this class there is a chance it could be since it's a public method.  So to 
avoid a breaking change and to have the chance to include this in the next 2.4 
release (2.4.8) it would be necessary to keep the `void 
collectImportedSymbols(String,Collection)` method.


> tab completion for static imports in groovysh does not work
> ---
>
> Key: GROOVY-7948
> URL: https://issues.apache.org/jira/browse/GROOVY-7948
> Project: Groovy
>  Issue Type: Bug
>  Components: Groovysh
>Affects Versions: 2.4.7
>Reporter: Abraham Grief
>
> Currently, if I run groovysh, and enter the following:
> {noformat}
> class MyMath {
> static int square(int x) {return x*x}
> }
> import static MyMath.*
> {noformat}
> Then enter {noformat}sq{noformat} followed by hitting the Tab key, groovysh 
> does not provided any possible auto-completions.
> Instead, hitting the Tab key should auto-complete to the square method in 
> MyMath.
> I fixed this already in my local groovy installation and can submit it as a 
> pull request if that helps.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (GROOVY-7969) Incorrect modifers on setter for volatile property with @Bindable/@Vetoable

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-7969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15569528#comment-15569528
 ] 

ASF GitHub Bot commented on GROOVY-7969:


Github user blackdrag commented on a diff in the pull request:

https://github.com/apache/groovy/pull/448#discussion_r83071079
  
--- Diff: src/main/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java ---
@@ -0,0 +1,50 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.ast.tools;
+
+import org.codehaus.groovy.ast.PropertyNode;
+
+import java.lang.reflect.Modifier;
+
+public class PropertyNodeUtils {
+/**
+ * Fields within the AST that have no explicit visibility are deemed 
to be properties
+ * and represented by a PropertyNode. The Groovy compiler creates 
accessor methods and
+ * a backing field for such property nodes. During this process, all 
modifiers
+ * from the property are carried over to the backing field (so a 
property marked as
+ * {@code transient} will have a {@code transient} backing field) but 
when creating
+ * the accessor methods we don't carry over modifier values which 
don't make sense for
+ * methods (this includes VOLATILE and TRANSIENT) but other modifiers 
are carried over,
+ * for example {@code static}.
+ *
+ * @param propNode the original property node
+ * @return the modifiers which make sense for an accessor method
+ */
+public static int adjustPropertyModifiersForMethod(PropertyNode 
propNode) {
+int propNodeModifiers = propNode.getModifiers();
+// GROOVY-3726: clear volatile, transient modifiers so that they 
don't get applied to methods
+if ((propNodeModifiers & Modifier.VOLATILE) != 0) {
+propNodeModifiers -= Modifier.VOLATILE;
+}
+if ((propNodeModifiers & Modifier.TRANSIENT) != 0) {
+propNodeModifiers -= Modifier.TRANSIENT;
+}
--- End diff --

or: propNodeModifiers = ~(Modifier.TRANSIENT | Modifier.VOLATILE) & 
propNodeModifiers


> Incorrect modifers on setter for volatile property with @Bindable/@Vetoable
> ---
>
> Key: GROOVY-7969
> URL: https://issues.apache.org/jira/browse/GROOVY-7969
> Project: Groovy
>  Issue Type: Bug
>Reporter: Paul King
>
> As part of GROOVY-3726 we fixed getter/setter method modifiers for volatile 
> (or transient) properties. The @Bindable and @Vetoable transforms however 
> generate their own setters and bypass that fix.
> Here is a script to reproduce the problem:
> {code}
> import static java.lang.reflect.Modifier.toString
> import groovy.beans.Bindable
> class Foo {
>   volatile Date now
> }
> @Bindable class Bar {
>   volatile Date then
> }
> void pretty(int mod) { println "${mod.toString().padRight(10)}" + 
> toString(mod) }
> pretty(Foo.getMethod('getNow').modifiers)
> pretty(Foo.getMethod('setNow', Date).modifiers)
> pretty(Bar.getMethod('getThen').modifiers)
> pretty(Bar.getMethod('setThen', Date).modifiers)
> {code}
> which currently produces:
> {noformat}
> 1 public
> 1 public
> 1 public
> 65public volatile
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (GROOVY-7969) Incorrect modifers on setter for volatile property with @Bindable/@Vetoable

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-7969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15568490#comment-15568490
 ] 

ASF GitHub Bot commented on GROOVY-7969:


GitHub user paulk-asert opened a pull request:

https://github.com/apache/groovy/pull/448

GROOVY-7969: Incorrect modifers on setter for volatile property with …

…@Bindable/@Vetoable

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/paulk-asert/groovy groovy7969

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/groovy/pull/448.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #448


commit 254e6038e033b29b7fd9d0512dc11e13997d8730
Author: paulk 
Date:   2016-10-12T11:42:17Z

GROOVY-7969: Incorrect modifers on setter for volatile property with 
@Bindable/@Vetoable




> Incorrect modifers on setter for volatile property with @Bindable/@Vetoable
> ---
>
> Key: GROOVY-7969
> URL: https://issues.apache.org/jira/browse/GROOVY-7969
> Project: Groovy
>  Issue Type: Bug
>Reporter: Paul King
>
> As part of GROOVY-3726 we fixed getter/setter method modifiers for volatile 
> (or transient) properties. The @Bindable and @Vetoable transforms however 
> generate their own setters and bypass that fix.
> Here is a script to reproduce the problem:
> {code}
> import static java.lang.reflect.Modifier.toString
> import groovy.beans.Bindable
> class Foo {
>   volatile Date now
> }
> @Bindable class Bar {
>   volatile Date then
> }
> void pretty(int mod) { println "${mod.toString().padRight(10)}" + 
> toString(mod) }
> pretty(Foo.getMethod('getNow').modifiers)
> pretty(Foo.getMethod('setNow', Date).modifiers)
> pretty(Bar.getMethod('getThen').modifiers)
> pretty(Bar.getMethod('setThen', Date).modifiers)
> {code}
> which currently produces:
> {noformat}
> 1 public
> 1 public
> 1 public
> 65public volatile
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] groovy pull request #448: GROOVY-7969: Incorrect modifers on setter for vola...

[paulk-asert]

GitHub user paulk-asert opened a pull request:

https://github.com/apache/groovy/pull/448

GROOVY-7969: Incorrect modifers on setter for volatile property with …

…@Bindable/@Vetoable

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/paulk-asert/groovy groovy7969

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/groovy/pull/448.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #448


commit 254e6038e033b29b7fd9d0512dc11e13997d8730
Author: paulk 
Date:   2016-10-12T11:42:17Z

GROOVY-7969: Incorrect modifers on setter for volatile property with 
@Bindable/@Vetoable




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GROOVY-7967) AstNodeToScriptAdapter should output source using the recommended modifier order

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/GROOVY-7967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15568349#comment-15568349
 ] 

ASF GitHub Bot commented on GROOVY-7967:


Github user asfgit closed the pull request at:

https://github.com/apache/groovy/pull/447


> AstNodeToScriptAdapter should output source using the recommended modifier 
> order
> 
>
> Key: GROOVY-7967
> URL: https://issues.apache.org/jira/browse/GROOVY-7967
> Project: Groovy
>  Issue Type: Bug
>Reporter: Paul King
>Assignee: Paul King
>  Labels: breaking
>
> This source file:
> {code}
> interface Foo { }
> {code}
> when 'reprinted' via the groovyConsole shows up as:
> {code}
> abstract interface public class Foo extends java.lang.Object {  }
> {code}
> We don't need to have both 'interface' and 'class' and the modifier order 
> should follow the JLS recommendations.
> In some sense this is a trviial change but we do have numerous tests which 
> output the resulting source and check the value. We'll need to refactor those.
> It is a breaking change only if the old source order was being relied upon.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] groovy pull request #447: GROOVY-7967: AstNodeToScriptAdapter should output ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/groovy/pull/447


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---