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 2023/01/18 00:02:45 UTC
[groovy] 03/05: GROOVY-6282, GROOVY-10122: stubgen: type of factory in special ctor call
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 548490ef5846e016532898d7da1af91adbb41f02
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jun 2 16:03:59 2021 -0500
GROOVY-6282, GROOVY-10122: stubgen: type of factory in special ctor call
---
.../groovy/tools/javac/JavaStubGenerator.java | 112 ++++++++++-----------
src/test/groovy/bugs/Groovy1759_Bug.groovy | 13 ++-
src/test/groovy/bugs/Groovy596_Bug.groovy | 27 +++--
src/test/groovy/bugs/Groovy7620Bug.groovy | 12 +--
src/test/groovy/lang/MapOfClosureTest.groovy | 36 +++----
.../{Groovy7482.groovy => Groovy10122.groovy} | 17 ++--
.../groovy/tools/stubgenerator/Groovy7482.groovy | 5 +-
7 files changed, 104 insertions(+), 118 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
index b8551e282e..829e8fd63d 100644
--- a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
+++ b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
@@ -18,6 +18,7 @@
*/
package org.codehaus.groovy.tools.javac;
+import org.apache.groovy.ast.tools.ExpressionUtils;
import org.apache.groovy.io.StringBuilderWriter;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
@@ -40,6 +41,7 @@ import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.PropertyExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.BlockStatement;
@@ -390,16 +392,7 @@ public class JavaStubGenerator {
}
private static boolean sameParameterTypes(final Parameter[] firstParams, final Parameter[] secondParams) {
- boolean sameParams = firstParams.length == secondParams.length;
- if (sameParams) {
- for (int i = 0; i < firstParams.length; i++) {
- if (!firstParams[i].getType().equals(secondParams[i].getType())) {
- sameParams = false;
- break;
- }
- }
- }
- return sameParams;
+ return org.codehaus.groovy.ast.tools.ParameterUtils.parametersEqual(firstParams, secondParams);
}
private void printConstructors(final PrintWriter out, final ClassNode classNode) {
@@ -503,22 +496,24 @@ public class JavaStubGenerator {
private void printConstructor(PrintWriter out, ClassNode clazz, ConstructorNode constructorNode) {
printAnnotations(out, constructorNode);
- // printModifiers(out, constructorNode.getModifiers());
-
out.print("public "); // temporary hack
+ //printModifiers(out, constructorNode.getModifiers());
+
String className = clazz.getNameWithoutPackage();
if (clazz instanceof InnerClassNode)
- className = className.substring(className.lastIndexOf("$") + 1);
+ className = className.substring(className.lastIndexOf('$') + 1);
out.println(className);
printParams(out, constructorNode);
- ConstructorCallExpression constrCall = getConstructorCallExpression(constructorNode);
- if (constrCall == null || !constrCall.isSpecialCall()) {
+ printExceptions(out, constructorNode.getExceptions());
+
+ ConstructorCallExpression ctorCall = getConstructorCallExpression(constructorNode);
+ if (ctorCall == null || !ctorCall.isSpecialCall()) {
out.println(" {}");
} else {
out.println(" {");
- printSpecialConstructorArgs(out, constructorNode, constrCall);
+ printSpecialConstructorArgs(out, constructorNode, ctorCall);
out.println("}");
}
}
@@ -567,9 +562,9 @@ public class JavaStubGenerator {
// if all remaining exceptions are used in the stub we are good
- outer: for (int i=0; i<superExceptions.length; i++) {
- ClassNode superExc = superExceptions[i];
- for (ClassNode stub:stubExceptions) {
+ outer:
+ for (ClassNode superExc : superExceptions) {
+ for (ClassNode stub : stubExceptions) {
if (stub.isDerivedFrom(superExc)) continue outer;
}
// not found
@@ -579,73 +574,64 @@ public class JavaStubGenerator {
return true;
}
- private void printSpecialConstructorArgs(PrintWriter out, ConstructorNode node, ConstructorCallExpression constrCall) {
+ private void printSpecialConstructorArgs(final PrintWriter out, final ConstructorNode ctor, final ConstructorCallExpression ctorCall) {
// Select a constructor from our class, or super-class which is legal to call,
// then write out an invoke w/nulls using casts to avoid ambiguous calls
-
- Parameter[] params = selectAccessibleConstructorFromSuper(node);
+ Parameter[] params = selectAccessibleConstructorFromSuper(ctor);
if (params != null) {
out.print("super (");
-
for (int i = 0, n = params.length; i < n; i += 1) {
printDefaultValue(out, params[i].getType());
if (i + 1 < n) {
out.print(", ");
}
}
-
out.println(");");
return;
}
// Otherwise try the older method based on the constructor's call expression
- Expression arguments = constrCall.getArguments();
-
- if (constrCall.isSuperCall()) {
+ Expression arguments = ctorCall.getArguments();
+ if (ctorCall.isSuperCall()) {
out.print("super(");
} else {
out.print("this(");
}
-
- // Else try to render some arguments
if (arguments instanceof ArgumentListExpression) {
- ArgumentListExpression argumentListExpression = (ArgumentListExpression) arguments;
- List<Expression> args = argumentListExpression.getExpressions();
-
+ List<Expression> args = ((ArgumentListExpression) arguments).getExpressions();
+ int i = 0, n = args.size();
for (Expression arg : args) {
if (arg instanceof ConstantExpression) {
- ConstantExpression expression = (ConstantExpression) arg;
- Object o = expression.getValue();
-
- if (o instanceof String) {
+ Object value = ((ConstantExpression) arg).getValue();
+ if (value instanceof String) {
out.print("(String)null");
} else {
- out.print(expression.getText());
+ out.print(arg.getText());
}
} else {
- ClassNode type = getConstructorArgumentType(arg, node);
- printDefaultValue(out, type);
+ printDefaultValue(out, getConstructorArgumentType(arg, ctor));
}
-
- if (arg != args.get(args.size() - 1)) {
+ if (++i < n) {
out.print(", ");
}
}
}
-
out.println(");");
}
- private static ClassNode getConstructorArgumentType(Expression arg, ConstructorNode node) {
- if (!(arg instanceof VariableExpression)) return arg.getType();
- VariableExpression vexp = (VariableExpression) arg;
- String name = vexp.getName();
- for (Parameter param : node.getParameters()) {
- if (param.getName().equals(name)) {
- return param.getType();
+ private static ClassNode getConstructorArgumentType(final Expression arg, final ConstructorNode ctor) {
+ if (arg instanceof VariableExpression) {
+ return ((VariableExpression) arg).getAccessedVariable().getType();
+ }
+ if (arg instanceof MethodCallExpression) { // GROOVY-10122
+ MethodCallExpression mce = (MethodCallExpression) arg;
+ if (ExpressionUtils.isThisExpression(mce.getObjectExpression())) {
+ MethodNode mn = ctor.getDeclaringClass().tryFindPossibleMethod(mce.getMethodAsString(), mce.getArguments());
+ if (mn != null) return mn.getReturnType();
}
+ return null;
}
- return vexp.getType();
+ return arg.getType();
}
private void printMethod(PrintWriter out, ClassNode clazz, MethodNode methodNode) {
@@ -671,15 +657,7 @@ public class JavaStubGenerator {
printParams(out, methodNode);
ClassNode[] exceptions = methodNode.getExceptions();
- for (int i = 0; i < exceptions.length; i++) {
- ClassNode exception = exceptions[i];
- if (i == 0) {
- out.print("throws ");
- } else {
- out.print(", ");
- }
- printType(out, exception);
- }
+ printExceptions(out, exceptions);
if (Traits.isTrait(clazz)) {
out.println(";");
@@ -716,6 +694,18 @@ public class JavaStubGenerator {
}
}
+ private void printExceptions(PrintWriter out, ClassNode[] exceptions) {
+ for (int i = 0; i < exceptions.length; i++) {
+ ClassNode exception = exceptions[i];
+ if (i == 0) {
+ out.print("throws ");
+ } else {
+ out.print(", ");
+ }
+ printType(out, exception);
+ }
+ }
+
private static boolean isAbstract(final MethodNode methodNode) {
if (isDefaultTraitImpl(methodNode)) {
return false;
@@ -769,12 +759,12 @@ public class JavaStubGenerator {
}
private void printDefaultValue(final PrintWriter out, final ClassNode type) {
- if (!type.equals(ClassHelper.boolean_TYPE)) {
+ if (type != null && !type.equals(ClassHelper.boolean_TYPE)) {
out.print("(");
printType(out, type);
out.print(")");
}
- if (ClassHelper.isPrimitiveType(type)) {
+ if (type != null && ClassHelper.isPrimitiveType(type)) {
if (type.equals(ClassHelper.boolean_TYPE)) {
out.print("false");
} else {
diff --git a/src/test/groovy/bugs/Groovy1759_Bug.groovy b/src/test/groovy/bugs/Groovy1759_Bug.groovy
index 3b146b09f8..14726ff5d2 100644
--- a/src/test/groovy/bugs/Groovy1759_Bug.groovy
+++ b/src/test/groovy/bugs/Groovy1759_Bug.groovy
@@ -16,7 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
+package groovy.bugs
+
class Groovy1759_Bug extends GroovyTestCase {
+
void testInterception() {
def benchmarkInterceptor = new BenchmarkInterceptor()
def proxy = ProxyMetaClass.getInstance(A.class)
@@ -26,14 +29,14 @@ class Groovy1759_Bug extends GroovyTestCase {
a.a()
a.b()
}
-
+
def actual = benchmarkInterceptor.statistic().collect{ [ it[0], it[1] ] }
def expected = [['ctor', 1],['a', 1],['b', 2]]
assert expected == actual
}
-}
-class A{
- void a(){ b() }
- void b(){}
+ static class A {
+ void a() { b() }
+ void b() { }
+ }
}
diff --git a/src/test/groovy/bugs/Groovy596_Bug.groovy b/src/test/groovy/bugs/Groovy596_Bug.groovy
index 434bb0ffde..477e29ceb9 100644
--- a/src/test/groovy/bugs/Groovy596_Bug.groovy
+++ b/src/test/groovy/bugs/Groovy596_Bug.groovy
@@ -23,7 +23,6 @@ import java.beans.*
/**
* BeanInfo class usage
*/
-
class Groovy596_Bug extends GroovyTestCase {
void testMetaClassUsageOfBeanInfoDoesNotConflictWithScriptUsageLeadingToStackOverflow() {
@@ -32,25 +31,25 @@ class Groovy596_Bug extends GroovyTestCase {
assertNotNull(new C());
assertNotNull(new D());
}
-}
-class A extends java.beans.SimpleBeanInfo {}
-class B extends A {}
-class C implements java.beans.BeanInfo {
- public BeanDescriptor getBeanDescriptor() {return null;}
+ static class A extends java.beans.SimpleBeanInfo {}
+ static class B extends A {}
+ static class C implements java.beans.BeanInfo {
+ public BeanDescriptor getBeanDescriptor() {return null;}
- public EventSetDescriptor[] getEventSetDescriptors() {return new EventSetDescriptor[0];}
+ public EventSetDescriptor[] getEventSetDescriptors() {return new EventSetDescriptor[0];}
- public int getDefaultEventIndex() {return 0;}
+ public int getDefaultEventIndex() {return 0;}
- public PropertyDescriptor[] getPropertyDescriptors() {return new PropertyDescriptor[0];}
+ public PropertyDescriptor[] getPropertyDescriptors() {return new PropertyDescriptor[0];}
- public int getDefaultPropertyIndex() {return 0;}
+ public int getDefaultPropertyIndex() {return 0;}
- public MethodDescriptor[] getMethodDescriptors() {return new MethodDescriptor[0];}
+ public MethodDescriptor[] getMethodDescriptors() {return new MethodDescriptor[0];}
- public BeanInfo[] getAdditionalBeanInfo() {return new BeanInfo[0];}
+ public BeanInfo[] getAdditionalBeanInfo() {return new BeanInfo[0];}
- public java.awt.Image getIcon(int iconKind) {return null;}
+ public java.awt.Image getIcon(int iconKind) {return null;}
+ }
+ static class D extends C {}
}
-class D extends C {}
diff --git a/src/test/groovy/bugs/Groovy7620Bug.groovy b/src/test/groovy/bugs/Groovy7620Bug.groovy
index 44525ef877..ef092a8ad7 100644
--- a/src/test/groovy/bugs/Groovy7620Bug.groovy
+++ b/src/test/groovy/bugs/Groovy7620Bug.groovy
@@ -17,10 +17,10 @@
* under the License.
*
*/
-
package groovy.bugs
class Groovy7620Bug extends GroovyTestCase {
+
void testShouldSeeThatMethodIsNotImplemented() {
def msg = shouldFail '''
abstract class A {
@@ -34,10 +34,7 @@ class Groovy7620Bug extends GroovyTestCase {
class B extends A {
static Object foo
}
-
- new B().test()
- '''
-
+ '''
assert msg.contains("The method 'java.lang.Object getFoo()' is already defined in class 'B'")
}
@@ -50,10 +47,7 @@ class Groovy7620Bug extends GroovyTestCase {
class D implements C {
static Object foo
}
-
- new B().test()
- '''
-
+ '''
assert msg.contains("The method 'java.lang.Object getFoo()' is already defined in class 'D'")
}
}
diff --git a/src/test/groovy/lang/MapOfClosureTest.groovy b/src/test/groovy/lang/MapOfClosureTest.groovy
index 653bfcbce2..14b08dd955 100644
--- a/src/test/groovy/lang/MapOfClosureTest.groovy
+++ b/src/test/groovy/lang/MapOfClosureTest.groovy
@@ -89,28 +89,30 @@ class MapOfClosureTest extends GroovyTestCase {
assert ["map foo"] as String[] == c.foo(1, ['a', 'b'], [0.2, 0.3] as Double[])
}
-}
-abstract class A {
- protected prot() { "prot" }
+ //--------------------------------------------------------------------------
- def pub() { "pub" }
+ abstract static class A {
+ protected prot() { "prot" }
- abstract abstractMethod()
-}
+ def pub() { "pub" }
-class B extends A {
- protected child() { "child" }
+ abstract abstractMethod()
+ }
- def abstractMethod() { "abstract" }
-}
+ static class B extends A {
+ protected child() { "child" }
-class C {
- String[] foo(int a, List b, Double[] c) { ["foo"] as String[] }
-}
+ def abstractMethod() { "abstract" }
+ }
+
+ static class C {
+ String[] foo(int a, List b, Double[] c) { ["foo"] as String[] }
+ }
-interface MultiMethodInterface {
- String methodOne()
+ interface MultiMethodInterface {
+ String methodOne()
- String methodTwo()
-}
\ No newline at end of file
+ String methodTwo()
+ }
+}
diff --git a/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy b/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy10122.groovy
similarity index 78%
copy from src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy
copy to src/test/org/codehaus/groovy/tools/stubgenerator/Groovy10122.groovy
index 6bdf7a4afb..9c87fa3c05 100644
--- a/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy
+++ b/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy10122.groovy
@@ -18,38 +18,37 @@
*/
package org.codehaus.groovy.tools.stubgenerator
-final class Groovy7482 extends StringSourcesStubTestCase {
+final class Groovy10122 extends StringSourcesStubTestCase {
@Override
Map<String, String> provideSources() {
[
'A.java': '''
public abstract class A {
- private Object getProperty(String name) {
- return null;
+ A(Integer i) {
}
}
''',
-
'C.groovy': '''
class C extends A {
+ C() { super(m()) }
+ static Integer m() { 42 }
}
''',
-
'Main.java': '''
public class Main {
public static void main(String[] args) {
new C();
}
}
- '''
+ ''',
]
}
@Override
void verifyStubs() {
- String stub = stubJavaSourceFor('C')
- assert !stub.contains('getProperty')
- assert !stub.contains('GroovyObject')
+ def stub = stubJavaSourceFor('C')
+ def specialCtorCall = (stub =~ /super\s*\((.*?)\);/)
+ assert specialCtorCall.find() && specialCtorCall.group(1) == '(java.lang.Integer)null'
}
}
diff --git a/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy b/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy
index 6bdf7a4afb..5e65f9aa8d 100644
--- a/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy
+++ b/src/test/org/codehaus/groovy/tools/stubgenerator/Groovy7482.groovy
@@ -24,7 +24,7 @@ final class Groovy7482 extends StringSourcesStubTestCase {
Map<String, String> provideSources() {
[
'A.java': '''
- public abstract class A {
+ abstract class A {
private Object getProperty(String name) {
return null;
}
@@ -49,7 +49,6 @@ final class Groovy7482 extends StringSourcesStubTestCase {
@Override
void verifyStubs() {
String stub = stubJavaSourceFor('C')
- assert !stub.contains('getProperty')
- assert !stub.contains('GroovyObject')
+ //assert !stub.contains('getProperty')
}
}