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 2019/11/22 02:20:46 UTC
[groovy] branch master updated: GROOVY-9245: exclude synthetic
constructors from CachedClass
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 9ee6f86 GROOVY-9245: exclude synthetic constructors from CachedClass
9ee6f86 is described below
commit 9ee6f869762f377d542732bd5eb46e448d3a2654
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'
'''
}
-
}