diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CategoryTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CategoryTests.java index 46d523ad3a..d5cd4f0ff3 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CategoryTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CategoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2019 the original author or authors. + * Copyright 2009-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,7 @@ public final class CategoryTests extends GroovyCompilerTestSuite { public void testCategory0() { //@formatter:off String[] sources = { - "Demo.groovy", + "Main.groovy", "use(NumberCategory) {\n" + " def dist = 300.meters\n" + " \n" + @@ -58,7 +58,7 @@ public void testCategory0() { public void testCategory1() { //@formatter:off String[] sources = { - "Demo.groovy", + "Main.groovy", "use(NumberCategory) {\n" + " def dist = 300.meters\n" + " \n" + @@ -89,7 +89,7 @@ public void testCategory1() { public void testCategory2() { //@formatter:off String[] sources = { - "Foo.groovy", + "Main.groovy", "assert new Plane().fly() ==\n" + " \"I'm the Concorde and I fly!\"\n" + "assert new Submarine().dive() ==\n" + @@ -139,11 +139,35 @@ public void testCategory2() { runConformTest(sources, "I'm the James Bond's vehicle and I dive!"); } + @Test + public void testCategory8433() { + //@formatter:off + String[] sources = { + "Main.groovy", + "use(NumberCategory) {\n" + + " print 1.something()\n" + + "}\n", + + "NumberCategory.groovy", + "@Category(Number) class NumberCategory {\n" + + " def something() {\n" + + " String variable = 'works'\n" + + " new Object() {\n" + // cast exception due to implicit first param "this" + " String toString() { variable }\n" + + " }\n" + + " }\n" + + "}\n", + }; + //@formatter:on + + runConformTest(sources, "works"); + } + @Test // not a great test, needs work public void testCategory_STS3822() { //@formatter:off String[] sources = { - "bad.groovy", + "Bad.groovy", "@Category(C.class) \n" + "@ScriptMixin(C.class)\n" + "class Bad {\n" + @@ -156,37 +180,37 @@ public void testCategory_STS3822() { runNegativeTest(sources, "----------\n" + - "1. ERROR in bad.groovy (at line 1)\n" + + "1. ERROR in Bad.groovy (at line 1)\n" + "\t@Category(C.class) \n" + "\t^^^^^^^^^^^^^^^^^^\n" + "Groovy:@groovy.lang.Category must define \'value\' which is the class to apply this category to\n" + "----------\n" + - "2. ERROR in bad.groovy (at line 1)\n" + + "2. ERROR in Bad.groovy (at line 1)\n" + "\t@Category(C.class) \n" + "\t ^\n" + "Groovy:unable to find class \'C.class\' for annotation attribute constant\n" + "----------\n" + - "3. ERROR in bad.groovy (at line 1)\n" + + "3. ERROR in Bad.groovy (at line 1)\n" + "\t@Category(C.class) \n" + "\t ^^^^^^^\n" + "Groovy:Only classes and closures can be used for attribute \'value\' in @groovy.lang.Category\n" + "----------\n" + - "4. ERROR in bad.groovy (at line 2)\n" + + "4. ERROR in Bad.groovy (at line 2)\n" + "\t@ScriptMixin(C.class)\n" + "\t^^^^^^^^^^^^\n" + "Groovy:class ScriptMixin is not an annotation in @ScriptMixin\n" + "----------\n" + - "5. ERROR in bad.groovy (at line 2)\n" + + "5. ERROR in Bad.groovy (at line 2)\n" + "\t@ScriptMixin(C.class)\n" + "\t ^^^^^^^^^^^\n" + "Groovy:unable to resolve class ScriptMixin for annotation\n" + "----------\n" + - "6. ERROR in bad.groovy (at line 2)\n" + + "6. ERROR in Bad.groovy (at line 2)\n" + "\t@ScriptMixin(C.class)\n" + "\t ^\n" + "Groovy:unable to find class \'C.class\' for annotation attribute constant\n" + "----------\n" + - "7. ERROR in bad.groovy (at line 4)\n" + + "7. ERROR in Bad.groovy (at line 4)\n" + "\t@Override\n" + "\t^^^^^^^^^\n" + "Groovy:Method \'toString\' from class \'Bad\' does not override method from its superclass or interfaces but is annotated with @Override.\n" + diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/InnerClassVisitor.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/InnerClassVisitor.java index 9191d92db9..057f374d1b 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/InnerClassVisitor.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/InnerClassVisitor.java @@ -220,7 +220,7 @@ public void visitConstructorCallExpression(ConstructorCallExpression call) { innerClass.addConstructor(ACC_SYNTHETIC, parameters.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, block); } - private boolean isStatic(InnerClassNode innerClass, VariableScope scope, final ConstructorCallExpression call) { + private boolean isStatic(final ClassNode innerClass, final VariableScope scope, final ConstructorCallExpression call) { boolean isStatic = innerClass.isStaticClass(); if (!isStatic) { if (currentMethod != null) { @@ -255,6 +255,10 @@ public void visitConstructorCallExpression(ConstructorCallExpression cce) { isStatic = currentField.isStatic(); } } + // GRECLIPSE add -- GROOVY-8433: Category transform applies static + isStatic = isStatic || innerClass.getOuterClass().getAnnotations().stream() + .anyMatch(a -> a.getClassNode().getName().equals("groovy.lang.Category")); + // GRECLIPSE end return isStatic; } diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/InnerClassVisitor.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/InnerClassVisitor.java index da30fa2291..4809a59ce2 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/InnerClassVisitor.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/InnerClassVisitor.java @@ -225,7 +225,7 @@ public void visitConstructorCallExpression(ConstructorCallExpression call) { innerClass.addConstructor(ACC_SYNTHETIC, parameters.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, block); } - private boolean isStatic(InnerClassNode innerClass, VariableScope scope, ConstructorCallExpression call) { + private boolean isStatic(final ClassNode innerClass, final VariableScope scope, final ConstructorCallExpression call) { boolean isStatic = innerClass.isStaticClass(); if (!isStatic) { if (currentMethod != null) { @@ -253,6 +253,10 @@ public void visitConstructorCallExpression(ConstructorCallExpression cce) { isStatic = currentField.isStatic(); } } + // GRECLIPSE add -- GROOVY-8433: Category transform applies static + isStatic = isStatic || innerClass.getOuterClass().getAnnotations().stream() + .anyMatch(a -> a.getClassNode().getName().equals("groovy.lang.Category")); + // GRECLIPSE end return isStatic; } diff --git a/base/org.codehaus.groovy40/.checkstyle b/base/org.codehaus.groovy40/.checkstyle index 1f70a5bba3..c49fd999a2 100644 --- a/base/org.codehaus.groovy40/.checkstyle +++ b/base/org.codehaus.groovy40/.checkstyle @@ -35,7 +35,7 @@ - + diff --git a/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/InnerClassVisitor.java b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/InnerClassVisitor.java new file mode 100644 index 0000000000..b82ecf1075 --- /dev/null +++ b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/InnerClassVisitor.java @@ -0,0 +1,325 @@ +/* + * 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 org.codehaus.groovy.classgen; + +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.CodeVisitorSupport; +import org.codehaus.groovy.ast.ConstructorNode; +import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.GroovyCodeVisitor; +import org.codehaus.groovy.ast.InnerClassNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; +import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.Variable; +import org.codehaus.groovy.ast.VariableScope; +import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.ConstructorCallExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.TupleExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; +import org.codehaus.groovy.ast.stmt.BlockStatement; +import org.codehaus.groovy.ast.stmt.ExpressionStatement; +import org.codehaus.groovy.control.CompilationUnit; +import org.codehaus.groovy.control.SourceUnit; +import org.codehaus.groovy.transform.trait.Traits; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; + +import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; +import static groovyjarjarasm.asm.Opcodes.ACC_FINAL; +import static groovyjarjarasm.asm.Opcodes.ACC_PUBLIC; +import static groovyjarjarasm.asm.Opcodes.ACC_STATIC; +import static groovyjarjarasm.asm.Opcodes.ACC_SYNTHETIC; + +public class InnerClassVisitor extends InnerClassVisitorHelper { + + private ClassNode classNode; + private FieldNode currentField; + private MethodNode currentMethod; + private final SourceUnit sourceUnit; + private boolean inClosure, processingObjInitStatements; + + public InnerClassVisitor(CompilationUnit cu, SourceUnit su) { + sourceUnit = su; + } + + @Override + protected SourceUnit getSourceUnit() { + return sourceUnit; + } + + @Override + public void visitClass(ClassNode node) { + classNode = node; + InnerClassNode innerClass = null; + if (!node.isEnum() && !node.isInterface() && node instanceof InnerClassNode) { + innerClass = (InnerClassNode) node; + if (innerClass.getVariableScope() == null && (innerClass.getModifiers() & ACC_STATIC) == 0) { + innerClass.addField("this$0", ACC_FINAL | ACC_SYNTHETIC, node.getOuterClass().getPlainNodeReference(), null); + } + } + + super.visitClass(node); + + if (node.isEnum() || node.isInterface()) return; + if (innerClass == null) return; + + if (node.getSuperClass().isInterface() || Traits.isAnnotatedWithTrait(node.getSuperClass())) { + node.addInterface(node.getUnresolvedSuperClass()); + node.setUnresolvedSuperClass(ClassHelper.OBJECT_TYPE); + } + } + + @Override + public void visitClosureExpression(ClosureExpression expression) { + boolean inClosureOld = inClosure; + inClosure = true; + super.visitClosureExpression(expression); + inClosure = inClosureOld; + } + + @Override + protected void visitObjectInitializerStatements(ClassNode node) { + processingObjInitStatements = true; + super.visitObjectInitializerStatements(node); + processingObjInitStatements = false; + } + + @Override + protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) { + currentMethod = node; + visitAnnotations(node); + visitClassCodeContainer(node.getCode()); + // GROOVY-5681: initial expressions should be visited too! + for (Parameter param : node.getParameters()) { + if (param.hasInitialExpression()) { + param.getInitialExpression().visit(this); + } + visitAnnotations(param); + } + currentMethod = null; + } + + @Override + public void visitField(FieldNode node) { + currentField = node; + super.visitField(node); + currentField = null; + } + + @Override + public void visitProperty(PropertyNode node) { + final FieldNode field = node.getField(); + final Expression init = field.getInitialExpression(); + field.setInitialValueExpression(null); + super.visitProperty(node); + field.setInitialValueExpression(init); + } + + @Override + public void visitConstructorCallExpression(ConstructorCallExpression call) { + super.visitConstructorCallExpression(call); + if (!call.isUsingAnonymousInnerClass()) { + passThisReference(call); + return; + } + + InnerClassNode innerClass = (InnerClassNode) call.getType(); + ClassNode outerClass = innerClass.getOuterClass(); + ClassNode superClass = innerClass.getSuperClass(); + if (!superClass.isInterface() && superClass.getOuterClass() != null + && !(superClass.isStaticClass() || (superClass.getModifiers() & ACC_STATIC) != 0)) { + insertThis0ToSuperCall(call, innerClass); + } + if (!innerClass.getDeclaredConstructors().isEmpty()) return; + if ((innerClass.getModifiers() & ACC_STATIC) != 0) return; + + VariableScope scope = innerClass.getVariableScope(); + if (scope == null) return; + boolean isStatic = !inClosure && isStatic(innerClass, scope, call); + + // expressions = constructor call arguments + List expressions = ((TupleExpression) call.getArguments()).getExpressions(); + // block = init code for the constructor we produce + BlockStatement block = new BlockStatement(); + // parameters = parameters of the constructor + int additionalParamCount = (isStatic ? 0 : 1) + scope.getReferencedLocalVariablesCount(); + List parameters = new ArrayList<>(expressions.size() + additionalParamCount); + // superCallArguments = arguments for the super call == the constructor call arguments + List superCallArguments = new ArrayList<>(expressions.size()); + + // first we add a super() call for all expressions given in the constructor call expression + for (int i = 0, n = expressions.size(); i < n; i += 1) { + // add one parameter for each expression in the constructor call + Parameter param = new Parameter(ClassHelper.OBJECT_TYPE, "p" + additionalParamCount + i); + parameters.add(param); + // add the corresponding argument to the super constructor call + superCallArguments.add(new VariableExpression(param)); + } + + // add the super call + ConstructorCallExpression cce = new ConstructorCallExpression( + ClassNode.SUPER, + new TupleExpression(superCallArguments) + ); + + block.addStatement(new ExpressionStatement(cce)); + + int pCount = 0; + if (!isStatic) { + // need to pass "this" to access unknown methods/properties + ClassNode enclosingType = (inClosure ? ClassHelper.CLOSURE_TYPE : outerClass).getPlainNodeReference(); + expressions.add(pCount, new VariableExpression("this", enclosingType)); + Parameter thisParameter = new Parameter(enclosingType, "p" + pCount); + parameters.add(pCount++, thisParameter); + + // "this" reference is saved in a field named "this$0" + FieldNode thisField = innerClass.addField("this$0", ACC_FINAL | ACC_SYNTHETIC, enclosingType, null); + addFieldInit(thisParameter, thisField, block); + } + + // for each shared variable, add a Reference field + for (Iterator it = scope.getReferencedLocalVariablesIterator(); it.hasNext();) { + Variable var = it.next(); + + VariableExpression ve = new VariableExpression(var); + ve.setClosureSharedVariable(true); + ve.setUseReferenceDirectly(true); + expressions.add(pCount, ve); + + ClassNode referenceType = ClassHelper.REFERENCE_TYPE.getPlainNodeReference(); + Parameter p = new Parameter(referenceType, "p" + pCount); + p.setOriginType(var.getOriginType()); + parameters.add(pCount++, p); + + VariableExpression initial = new VariableExpression(p); + initial.setSynthetic(true); + initial.setUseReferenceDirectly(true); + FieldNode pField = innerClass.addFieldFirst(ve.getName(), ACC_PUBLIC | ACC_SYNTHETIC, referenceType, initial); + pField.setHolder(true); + pField.setOriginType(ClassHelper.getWrapper(var.getOriginType())); + } + + innerClass.addConstructor(ACC_SYNTHETIC, parameters.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, block); + } + + private boolean isStatic(final ClassNode innerClass, final VariableScope scope, final ConstructorCallExpression call) { + boolean isStatic = innerClass.isStaticClass(); + if (!isStatic) { + if (currentMethod != null) { + if (currentMethod instanceof ConstructorNode) { + boolean[] precedesSuperOrThisCall = new boolean[1]; + ConstructorNode ctor = (ConstructorNode) currentMethod; + GroovyCodeVisitor visitor = new CodeVisitorSupport() { + @Override + public void visitConstructorCallExpression(ConstructorCallExpression cce) { + if (cce == call) { + precedesSuperOrThisCall[0] = true; + } else { + super.visitConstructorCallExpression(cce); + } + } + }; + if (ctor.firstStatementIsSpecialConstructorCall()) currentMethod.getFirstStatement().visit(visitor); + Arrays.stream(ctor.getParameters()).filter(Parameter::hasInitialExpression).forEach(p -> p.getInitialExpression().visit(visitor)); + + isStatic = precedesSuperOrThisCall[0]; + } else { + isStatic = currentMethod.isStatic(); + } + } else if (currentField != null) { + isStatic = currentField.isStatic(); + } + } + // GRECLIPSE add -- GROOVY-8433: Category transform applies static + isStatic = isStatic || innerClass.getOuterClass().getAnnotations().stream() + .anyMatch(a -> a.getClassNode().getName().equals("groovy.lang.Category")); + // GRECLIPSE end + return isStatic; + } + + // this is the counterpart of addThisReference(). To non-static inner classes, outer this should be + // passed as the first argument implicitly. + private void passThisReference(final ConstructorCallExpression call) { + ClassNode cn = call.getType().redirect(); + if (!shouldHandleImplicitThisForInnerClass(cn)) return; + + boolean isInStaticContext; + if (currentMethod != null) + isInStaticContext = currentMethod.isStatic(); + else if (currentField != null) + isInStaticContext = currentField.isStatic(); + else + isInStaticContext = !processingObjInitStatements; + + // GROOVY-10289 + ClassNode enclosing = classNode; + while (!isInStaticContext && !enclosing.equals(cn.getOuterClass())) { + isInStaticContext = (enclosing.getModifiers() & ACC_STATIC) != 0; + // TODO: if enclosing is a local type, also test field or method + enclosing = enclosing.getOuterClass(); + if (enclosing == null) break; + } + + if (isInStaticContext) { + // constructor call is in static context and the inner class is non-static - 1st arg is supposed to be + // passed as enclosing "this" instance + Expression args = call.getArguments(); + if (args instanceof TupleExpression && ((TupleExpression) args).getExpressions().isEmpty()) { + addError("No enclosing instance passed in constructor call of a non-static inner class", call); + } + } else { + insertThis0ToSuperCall(call, cn); + } + } + + private void insertThis0ToSuperCall(final ConstructorCallExpression call, final ClassNode cn) { + // calculate outer class which we need for this$0 + ClassNode parent = classNode; + int level = 0; + for (; parent != null && parent != cn.getOuterClass(); parent = parent.getOuterClass()) { + level++; + } + + // if constructor call is not in outer class, don't pass 'this' implicitly. Return. + if (parent == null) return; + + Expression args = call.getArguments(); + if (args instanceof TupleExpression) { + Expression this0 = varX("this"); // bypass closure + for (int i = 0; i != level; ++i) { + this0 = attrX(this0, constX("this$0")); + // GROOVY-8104: an anonymous inner class may still have closure nesting + if (i == 0 && classNode.getDeclaredField("this$0").getType().equals(ClassHelper.CLOSURE_TYPE)) { + this0 = callX(this0, "getThisObject"); + } + } + + ((TupleExpression) args).getExpressions().add(0, this0); + } + } +}