You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/08/22 16:09:05 UTC
[groovy] 01/01: GROOVY-8947: convert "new A().new B()" into "new
A.B(new A())"
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-8947
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit bf3e88ee0bddbabbd2ed21f2a909dfc1ff873d93
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Aug 22 10:53:08 2021 -0500
GROOVY-8947: convert "new A().new B()" into "new A.B(new A())"
---
.../apache/groovy/parser/antlr4/AstBuilder.java | 7 +-
.../codehaus/groovy/control/ResolveVisitor.java | 32 --------
src/test/groovy/bugs/Groovy8947.groovy | 86 +++++++++++-----------
3 files changed, 49 insertions(+), 76 deletions(-)
diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 9a7aa5d..ae5a123 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -3448,7 +3448,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
Expression arguments = this.visitArguments(ctx.arguments());
Expression enclosingInstanceExpression = ctx.getNodeMetaData(ENCLOSING_INSTANCE_EXPRESSION);
- if (null != enclosingInstanceExpression) {
+ if (enclosingInstanceExpression != null) {
if (arguments instanceof ArgumentListExpression) {
((ArgumentListExpression) arguments).getExpressions().add(0, enclosingInstanceExpression);
} else if (arguments instanceof TupleExpression) {
@@ -3458,6 +3458,9 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
} else {
throw createParsingFailedException("Unsupported arguments", arguments); // should never reach here
}
+ if (enclosingInstanceExpression instanceof ConstructorCallExpression && classNode.getName().indexOf('.') < 0) {
+ classNode.setName(enclosingInstanceExpression.getType().getName() + '.' + classNode.getName()); // GROOVY-8947
+ }
}
if (asBoolean(ctx.anonymousInnerClassDeclaration())) {
@@ -3465,7 +3468,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
InnerClassNode anonymousInnerClassNode = this.visitAnonymousInnerClassDeclaration(ctx.anonymousInnerClassDeclaration());
List<InnerClassNode> anonymousInnerClassList = anonymousInnerClassesDefinedInMethodStack.peek();
- if (null != anonymousInnerClassList) { // if the anonymous class is created in a script, no anonymousInnerClassList is available.
+ if (anonymousInnerClassList != null) { // if the anonymous class is created in a script, no anonymousInnerClassList is available.
anonymousInnerClassList.add(anonymousInnerClassNode);
}
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 8d2c254..d4ce72e 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -22,7 +22,6 @@ import groovy.lang.Tuple2;
import org.apache.groovy.ast.tools.ExpressionUtils;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
import org.codehaus.groovy.ast.ClassHelper;
@@ -42,7 +41,6 @@ import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.AnnotationConstantExpression;
-import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.CastExpression;
import org.codehaus.groovy.ast.expr.ClassExpression;
@@ -113,7 +111,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private boolean inPropertyExpression = false;
private boolean inClosure = false;
- private final Map<ClassNode, ClassNode> possibleOuterClassNodeMap = new HashMap<>();
private Map<GenericsTypeName, GenericsType> genericParameterNames = new HashMap<>();
private final Set<FieldNode> fieldTypesChecked = new HashSet<>();
private boolean checkingVariableTypeInDeclaration;
@@ -528,10 +525,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
// GROOVY-9866: unresolvable interfaces
}
- // GROOVY-8947: non-static inner class outside of outer class
- cn = possibleOuterClassNodeMap.get(type);
- if (cn != null && setRedirect(type, cn)) return true;
-
// Another case we want to check here is if we are in a
// nested class A$B$C and want to access B without
// qualifying it by A.B. A alone will work, since that
@@ -1235,8 +1228,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
}
protected Expression transformConstructorCallExpression(final ConstructorCallExpression cce) {
- findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(cce);
-
ClassNode type = cce.getType();
if (cce.isUsingAnonymousInnerClass()) { // GROOVY-9642
resolveOrFail(type.getUnresolvedSuperClass(false), type);
@@ -1250,28 +1241,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
return cce.transformExpression(this);
}
- private void findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(final ConstructorCallExpression cce) {
- // GROOVY-8947: Fail to resolve non-static inner class outside of outer class
- // `new Computer().new Cpu(4)` will be parsed to `new Cpu(new Computer(), 4)`
- // so non-static inner class instantiation expression's first argument is a constructor call of outer class
- // but the first argument is constructor call can not be non-static inner class instantiation expression, e.g.
- // `new HashSet(new ArrayList())`, so we add "possible" to the variable name
- Expression argumentExpression = cce.getArguments();
- if (argumentExpression instanceof ArgumentListExpression) {
- ArgumentListExpression argumentListExpression = (ArgumentListExpression) argumentExpression;
- List<Expression> expressionList = argumentListExpression.getExpressions();
- if (!expressionList.isEmpty()) {
- Expression firstExpression = expressionList.get(0);
-
- if (firstExpression instanceof ConstructorCallExpression) {
- ConstructorCallExpression constructorCallExpression = (ConstructorCallExpression) firstExpression;
- ClassNode possibleOuterClassNode = constructorCallExpression.getType();
- possibleOuterClassNodeMap.put(cce.getType(), possibleOuterClassNode);
- }
- }
- }
- }
-
private static String getDescription(final ClassNode node) {
return (node.isInterface() ? "interface" : "class") + " '" + node.getName() + "'";
}
@@ -1680,6 +1649,5 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
genericsType.setResolved(genericsType.getType().isResolved());
}
return genericsType.isResolved();
-
}
}
diff --git a/src/test/groovy/bugs/Groovy8947.groovy b/src/test/groovy/bugs/Groovy8947.groovy
index f11e565..cba5a65 100644
--- a/src/test/groovy/bugs/Groovy8947.groovy
+++ b/src/test/groovy/bugs/Groovy8947.groovy
@@ -30,73 +30,75 @@ final class Groovy8947 {
@Test
void testResolvingNonStaticInnerClass() {
assertScript '''
- public class Computer {
- public class Cpu {
- int coreNumber
- public Cpu(int coreNumber) {
- this.coreNumber = coreNumber
- }
+ class Foo {
+ @groovy.transform.TupleConstructor(defaults=false)
+ class Bar {
+ def baz
}
- public static newCpuInstance(int coreNumber) {
- return new Computer().new Cpu(coreNumber)
+ static newBar(def baz) {
+ new Foo().new Bar(baz)
}
}
- assert 4 == new Computer().new Cpu(4).coreNumber
- assert 4 == Computer.newCpuInstance(4).coreNumber
- assert 0 == new HashSet(new ArrayList()).size()
-
- def coreNumber = new Computer().new Cpu(4).coreNumber
- assert 4 == coreNumber
- def cpu = new Computer().new Cpu(4)
- assert 4 == cpu.coreNumber
+ assert Foo.newBar('baz').baz == 'baz'
+ assert new Foo().new Bar('baz').baz == 'baz'
'''
}
@Test
void testResolvingNonStaticInnerClass2() {
assertScript '''
- public class Computer {
- public class Cpu {
- int coreNumber
- public Cpu(int coreNumber) {
- this.coreNumber = coreNumber
- }
+ class Foo {
+ @groovy.transform.TupleConstructor(defaults=false)
+ class Bar {
+ def baz
}
- static newComputerInstance() {
- return new Computer()
+ static newFoo() {
+ new Foo()
}
- static newCpuInstance(int coreNumber) {
- // `new Cpu(coreNumber)` is inside of the outer class `Computer`, so we can resolve `Cpu`
- return newComputerInstance().new Cpu(coreNumber)
+ static newBar(def baz) {
+ return newFoo().new Bar(baz)
}
}
- assert 4 == Computer.newCpuInstance(4).coreNumber
+ assert Foo.newBar('baz').baz == 'baz'
'''
}
@Test
void testResolvingNonStaticInnerClass3() {
- def err = shouldFail '''
- public class Computer {
- public class Cpu {
- int coreNumber
-
- public Cpu(int coreNumber) {
- this.coreNumber = coreNumber
- }
+ assertScript '''
+ class Foo {
+ @groovy.transform.TupleConstructor(defaults=false)
+ class Bar {
+ def baz
+ }
+ static newFoo() {
+ new Foo()
}
}
- def newComputerInstance() {
- return new Computer()
+
+ assert Foo.newFoo().new Foo.Bar('baz').baz == 'baz'
+ '''
+ }
+
+ @Test
+ void testResolvingNonStaticInnerClass4() {
+ def err = shouldFail '''
+ class Foo {
+ @groovy.transform.TupleConstructor(defaults=false)
+ class Bar {
+ def baz
+ }
+ static newFoo() {
+ new Foo()
+ }
}
- // `new Cpu(4)` is outside of outer class `Computer` and the return type of `newComputerInstance()` can not be resolved,
- // so we does not support this syntax outside of outer class
- assert 4 == newComputerInstance().new Cpu(4).coreNumber
+ // this form isn't supported outside of enclosing class
+ Foo.newFoo().new Bar('baz')
'''
- assert err =~ 'unable to resolve class Cpu'
+ assert err =~ 'unable to resolve class Bar'
}
}