You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2021/05/30 13:45:04 UTC
[groovy] branch master updated: GROOVY-5239: prefer outer class
method over static import method
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new a556398 GROOVY-5239: prefer outer class method over static import method
a556398 is described below
commit a5563982e437dfe173fc0fc00f3da91df5524137
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu May 27 15:14:27 2021 -0500
GROOVY-5239: prefer outer class method over static import method
---
.../groovy/ast/expr/MethodCallExpression.java | 4 +-
.../groovy/control/StaticImportVisitor.java | 20 ++---
src/test/groovy/bugs/Groovy5239.groovy | 100 +++++++++++++++++++++
3 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/MethodCallExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/MethodCallExpression.java
index f0c2bee..1eef39e 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/MethodCallExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/MethodCallExpression.java
@@ -119,9 +119,7 @@ public class MethodCallExpression extends Expression implements MethodCall {
*/
@Override
public String getMethodAsString() {
- if (!(method instanceof ConstantExpression)) return null;
- ConstantExpression constant = (ConstantExpression) method;
- return constant.getText();
+ return (method instanceof ConstantExpression ? method.getText() : null);
}
public Expression getObjectExpression() {
diff --git a/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java b/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
index 694213a..d7777ab 100644
--- a/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
@@ -59,6 +59,8 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.hasPossibleStaticProper
import static org.apache.groovy.ast.tools.ClassNodeUtils.hasStaticProperty;
import static org.apache.groovy.ast.tools.ClassNodeUtils.isInnerClass;
import static org.apache.groovy.ast.tools.ClassNodeUtils.isValidAccessorName;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstants;
import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
@@ -242,16 +244,16 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
Expression args = transform(mce.getArguments());
if (mce.isImplicitThis()) {
- if (currentClass.tryFindPossibleMethod(mce.getMethodAsString(), args) == null) {
+ String name = mce.getMethodAsString();
+ if (currentClass.tryFindPossibleMethod(name, args) == null
+ && currentClass.getOuterClasses().stream().noneMatch(oc -> oc.tryFindPossibleMethod(name, args) != null)) {
Expression result = findStaticMethodImportFromModule(method, args);
if (result != null) {
setSourcePosition(result, mce);
return result;
}
- if (method instanceof ConstantExpression && !inLeftExpression) {
- // could be a closure field
- String methodName = (String) ((ConstantExpression) method).getValue();
- result = findStaticFieldOrPropAccessorImportFromModule(methodName);
+ if (name != null && !inLeftExpression) { // maybe a closure field
+ result = findStaticFieldOrPropAccessorImportFromModule(name);
if (result != null) {
result = new MethodCallExpression(result, "call", args);
result.setSourcePosition(mce);
@@ -259,14 +261,13 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
}
}
}
- } else if (currentMethod != null && currentMethod.isStatic() && (object instanceof VariableExpression && ((VariableExpression) object).isSuperExpression())) {
+ } else if (currentMethod != null && currentMethod.isStatic() && isSuperExpression(object)) {
Expression result = new MethodCallExpression(new ClassExpression(currentClass.getSuperClass()), method, args);
result.setSourcePosition(mce);
return result;
}
- if (method instanceof ConstantExpression && ((ConstantExpression) method).getValue() instanceof String && (mce.isImplicitThis()
- || (object instanceof VariableExpression && (((VariableExpression) object).isThisExpression() || ((VariableExpression) object).isSuperExpression())))) {
+ if (method instanceof ConstantExpression && ((ConstantExpression) method).getValue() instanceof String && (mce.isImplicitThis() || isThisOrSuper(object))) {
String methodName = (String) ((ConstantExpression) method).getValue();
boolean foundInstanceMethod = (currentMethod != null && !currentMethod.isStatic() && currentClass.hasPossibleMethod(methodName, args));
@@ -353,8 +354,7 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
protected Expression transformPropertyExpression(PropertyExpression pe) {
if (currentMethod != null && currentMethod.isStatic()
- && pe.getObjectExpression() instanceof VariableExpression
- && ((VariableExpression) pe.getObjectExpression()).isSuperExpression()) {
+ && isSuperExpression(pe.getObjectExpression())) {
PropertyExpression pexp = new PropertyExpression(
new ClassExpression(currentClass.getUnresolvedSuperClass()),
transform(pe.getProperty())
diff --git a/src/test/groovy/bugs/Groovy5239.groovy b/src/test/groovy/bugs/Groovy5239.groovy
new file mode 100644
index 0000000..e76281c
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy5239.groovy
@@ -0,0 +1,100 @@
+/*
+ * 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 groovy.bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy5239 {
+
+ static class PublicStatic {
+ static who() { 'PublicStatic' }
+ }
+
+ @Test @groovy.test.NotYetImplemented
+ void testStaticImportVersusDelegateMethod() {
+ assertScript '''
+ import static groovy.bugs.Groovy5239.PublicStatic.who
+
+ class C {
+ def who() {
+ 'C'
+ }
+ }
+
+ new C().with {
+ assert 'C' == who()
+ }
+ '''
+ }
+
+ @Test
+ void testStaticImportVersusOuterClassMethod1() {
+ assertScript '''
+ import static groovy.bugs.Groovy5239.PublicStatic.who
+
+ class C {
+ def who() {
+ 'C'
+ }
+ void test() {
+ assert 'C' == who()
+ new D().test()
+ }
+
+ class D {
+ void test() {
+ assert 'C' == who() // resolves to static import
+ }
+ }
+ }
+
+ new C().test()
+ '''
+ }
+
+ @Test
+ void testStaticImportVersusOuterClassMethod2() {
+ assertScript '''
+ import static groovy.bugs.Groovy5239.PublicStatic.who
+
+ class C {
+ def who() {
+ 'C'
+ }
+ }
+
+ class D extends C {
+ void test() {
+ assert 'C' == who()
+ new E().test()
+ }
+
+ class E {
+ void test() {
+ assert 'C' == who() // resolves to static import
+ }
+ }
+ }
+
+ new D().test()
+ '''
+ }
+}