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 2019/11/18 19:11:54 UTC
[groovy] branch GROOVY-9245 created (now b8f23f1)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9245
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at b8f23f1 GROOVY-9245: exclude synthetic constructors from CachedClass
This branch includes the following new commits:
new b8f23f1 GROOVY-9245: exclude synthetic constructors from CachedClass
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-9245: exclude synthetic constructors from
CachedClass
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9245
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b8f23f1937e90c602bfe5ee7cb5f77b9ca4e1679
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 18 13:11:14 2019 -0600
GROOVY-9245: exclude synthetic constructors from CachedClass
---
.../transform/builder/InitializerStrategy.java | 26 ++++----
.../codehaus/groovy/reflection/CachedClass.java | 1 +
src/test/groovy/bugs/Groovy9245.groovy | 70 ++++++++++++++++++++++
.../groovy/transform/BuilderTransformTest.groovy | 3 +-
4 files changed, 84 insertions(+), 16 deletions(-)
diff --git a/src/main/java/groovy/transform/builder/InitializerStrategy.java b/src/main/java/groovy/transform/builder/InitializerStrategy.java
index b48d5af..6ce152a 100644
--- a/src/main/java/groovy/transform/builder/InitializerStrategy.java
+++ b/src/main/java/groovy/transform/builder/InitializerStrategy.java
@@ -133,7 +133,6 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
public abstract static class UNSET {
}
- private static final int PUBLIC_STATIC = ACC_PUBLIC | ACC_STATIC;
private static final Expression DEFAULT_INITIAL_VALUE = null;
private static final ClassNode TUPLECONS_TYPE = ClassHelper.make(TupleConstructor.class);
@@ -176,13 +175,13 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
" processing: includes/excludes only allowed on classes", anno);
}
if (mNode instanceof ConstructorNode) {
- mNode.setModifiers(ACC_PRIVATE | ACC_SYNTHETIC);
+ mNode.setModifiers(ACC_PRIVATE);
} else {
- if ((mNode.getModifiers() & ACC_STATIC) == 0) {
+ if (!mNode.isStatic()) {
transform.addError("Error during " + BuilderASTTransformation.MY_TYPE_NAME +
" processing: method builders only allowed on static methods", anno);
}
- mNode.setModifiers(ACC_PRIVATE | ACC_SYNTHETIC | ACC_STATIC);
+ mNode.setModifiers(ACC_SYNTHETIC | ACC_PRIVATE | ACC_STATIC);
}
ClassNode buildee = mNode.getDeclaringClass();
Parameter[] parameters = mNode.getParameters();
@@ -226,7 +225,7 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
private static List<FieldNode> convertParamsToFields(ClassNode builder, Parameter[] parameters) {
List<FieldNode> fieldNodes = new ArrayList<FieldNode>();
- for(Parameter parameter: parameters) {
+ for (Parameter parameter: parameters) {
Map<String,ClassNode> genericsSpec = createGenericsSpec(builder);
ClassNode correctedType = correctToGenericsSpecRecurse(genericsSpec, parameter.getType());
FieldNode fieldNode = new FieldNode(parameter.getName(), parameter.getModifiers(), correctedType, builder, DEFAULT_INITIAL_VALUE);
@@ -238,7 +237,7 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
private static ClassNode createInnerHelperClass(ClassNode buildee, String builderClassName, int fieldsSize) {
final String fullName = buildee.getName() + "$" + builderClassName;
- ClassNode builder = new InnerClassNode(buildee, fullName, PUBLIC_STATIC, OBJECT_TYPE);
+ ClassNode builder = new InnerClassNode(buildee, fullName, ACC_PUBLIC | ACC_STATIC, OBJECT_TYPE);
GenericsType[] gtypes = new GenericsType[fieldsSize];
for (int i = 0; i < gtypes.length; i++) {
gtypes[i] = makePlaceholder(i);
@@ -251,7 +250,7 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
final BlockStatement body = new BlockStatement();
body.addStatement(returnS(callX(builder, buildMethodName)));
ClassNode returnType = makeClassSafeWithGenerics(builder, unsetGenTypes(numFields));
- return new MethodNode(builderMethodName, PUBLIC_STATIC, returnType, NO_PARAMS, NO_EXCEPTIONS, body);
+ return new MethodNode(builderMethodName, ACC_PUBLIC | ACC_STATIC, returnType, NO_PARAMS, NO_EXCEPTIONS, body);
}
private static GenericsType[] unsetGenTypes(int numFields) {
@@ -272,8 +271,8 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
private static void createBuilderConstructors(ClassNode builder, ClassNode buildee, List<FieldNode> fields) {
addGeneratedConstructor(builder, ACC_PRIVATE, NO_PARAMS, NO_EXCEPTIONS, block(ctorSuperS()));
- final BlockStatement body = new BlockStatement();
- body.addStatement(ctorSuperS());
+
+ BlockStatement body = block(ctorSuperS());
initializeFields(fields, body, false);
addGeneratedConstructor(builder, ACC_PRIVATE, getParams(fields, buildee), NO_EXCEPTIONS, body);
}
@@ -281,10 +280,9 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
private static void createBuildeeConstructors(BuilderASTTransformation transform, ClassNode buildee, ClassNode builder, List<FieldNode> fields, boolean needsConstructor, boolean useSetters) {
createInitializerConstructor(buildee, builder, fields);
if (needsConstructor) {
- final BlockStatement body = new BlockStatement();
- body.addStatement(ctorSuperS());
+ BlockStatement body = block(ctorSuperS());
initializeFields(fields, body, useSetters);
- addGeneratedConstructor(buildee, ACC_PRIVATE | ACC_SYNTHETIC, getParams(fields, buildee), NO_EXCEPTIONS, body);
+ addGeneratedConstructor(buildee, ACC_PRIVATE, getParams(fields, buildee), NO_EXCEPTIONS, body);
}
}
@@ -296,7 +294,7 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
argsList.add(propX(varX(initParam), fieldNode.getName()));
}
String newName = "$" + mNode.getName(); // can't have private and public methods of the same name, so rename original
- addGeneratedMethod(buildee, mNode.getName(), PUBLIC_STATIC, mNode.getReturnType(), params(param(paramType, "initializer")), NO_EXCEPTIONS,
+ addGeneratedMethod(buildee, mNode.getName(), ACC_PUBLIC | ACC_STATIC, mNode.getReturnType(), params(param(paramType, "initializer")), NO_EXCEPTIONS,
block(stmt(callX(buildee, newName, args(argsList)))));
renameMethod(buildee, mNode, newName);
}
@@ -331,7 +329,7 @@ public class InitializerStrategy extends BuilderASTTransformation.AbstractBuilde
private static MethodNode createBuildMethod(ClassNode builder, String buildMethodName, List<FieldNode> fields) {
ClassNode returnType = makeClassSafeWithGenerics(builder, unsetGenTypes(fields.size()));
- return new MethodNode(buildMethodName, PUBLIC_STATIC, returnType, NO_PARAMS, NO_EXCEPTIONS, block(returnS(ctorX(returnType))));
+ return new MethodNode(buildMethodName, ACC_PUBLIC | ACC_STATIC, returnType, NO_PARAMS, NO_EXCEPTIONS, block(returnS(ctorX(returnType))));
}
private MethodNode createBuilderMethodForField(ClassNode builder, List<FieldNode> fields, String prefix, int fieldPos) {
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedClass.java b/src/main/java/org/codehaus/groovy/reflection/CachedClass.java
index 5752836..79a7756 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedClass.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedClass.java
@@ -72,6 +72,7 @@ public class CachedClass {
public CachedConstructor[] initValue() {
PrivilegedAction<CachedConstructor[]> action = () -> {
return Arrays.stream(getTheClass().getDeclaredConstructors())
+ .filter(c -> !c.isSynthetic()) // GROOVY-9245: exclude inner class ctors
.filter(c -> checkCanSetAccessible(c, CachedClass.class))
.map(c -> new CachedConstructor(CachedClass.this, c))
.toArray(CachedConstructor[]::new);
diff --git a/src/test/groovy/bugs/Groovy9245.groovy b/src/test/groovy/bugs/Groovy9245.groovy
new file mode 100644
index 0000000..d8dcc6c
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9245.groovy
@@ -0,0 +1,70 @@
+/*
+ * 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.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
+import org.junit.Test
+
+final class Groovy9245 {
+
+ @Test
+ void testConstructorSelection() {
+ def config = new CompilerConfiguration(
+ targetDirectory: File.createTempDir(),
+ jointCompilationOptions: [memStub: true]
+ )
+
+ def parentDir = File.createTempDir()
+ try {
+ def a = new File(parentDir, 'A.java')
+ a.write '''
+ public class A {
+ private A() {
+ }
+ public A(String s) {
+ }
+ private static class B extends A {
+ }
+ }
+ '''
+ def b = new File(parentDir, 'Main.groovy')
+ b.write '''
+ new A(null)
+ /*
+ groovy.lang.GroovyRuntimeException: Ambiguous method overloading for method A#<init>.
+ Cannot resolve which method to invoke for [null] due to overlapping prototypes between:
+ [class java.lang.String]
+ [class A$1]
+ */
+ '''
+
+ def loader = new GroovyClassLoader(this.class.classLoader)
+ def cu = new JavaAwareCompilationUnit(config, loader)
+ cu.addSources(a, b)
+ cu.compile()
+
+ Class clazz = loader.loadClass('Main')
+ assert clazz.newInstance().run()
+ } finally {
+ parentDir.deleteDir()
+ config.targetDirectory.deleteDir()
+ }
+ }
+}
diff --git a/src/test/org/codehaus/groovy/transform/BuilderTransformTest.groovy b/src/test/org/codehaus/groovy/transform/BuilderTransformTest.groovy
index 0a707ec..a200ff6 100644
--- a/src/test/org/codehaus/groovy/transform/BuilderTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/BuilderTransformTest.groovy
@@ -774,7 +774,7 @@ class BuilderTransformTest extends CompilableTestSupport {
'''
}
- void testInternalFieldsAreIncludedIfRequestedForInitializerStrategyStrategy_GROOVY6454() {
+ void testInternalFieldsAreIncludedIfRequestedForInitializerStrategy_GROOVY6454() {
assertScript '''
import groovy.transform.builder.*
@@ -806,5 +806,4 @@ class BuilderTransformTest extends CompilableTestSupport {
assert new FooBuilder().name('Mary').build().name == 'John'
'''
}
-
}