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 2022/02/21 22:16:35 UTC

[groovy] branch GROOVY_3_0_X updated: GROOVY-6510: `@Category`: preserve implicit-this semantic within closure

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
     new 6d31855  GROOVY-6510: `@Category`: preserve implicit-this semantic within closure
6d31855 is described below

commit 6d318555f5b70304fa16b5b8b916c3d64c70162b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Feb 21 16:16:27 2022 -0600

    GROOVY-6510: `@Category`: preserve implicit-this semantic within closure
---
 .../transform/CategoryASTTransformation.java       | 292 +++++++++-----------
 src/test/groovy/lang/CategoryAnnotationTest.groovy | 307 ++++++++++++---------
 2 files changed, 304 insertions(+), 295 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/CategoryASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/CategoryASTTransformation.java
index c85908d..6a371b9 100644
--- a/src/main/java/org/codehaus/groovy/transform/CategoryASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/CategoryASTTransformation.java
@@ -32,8 +32,8 @@ import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.PropertyExpression;
-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.CatchStatement;
@@ -42,15 +42,15 @@ import org.codehaus.groovy.ast.stmt.ForStatement;
 import org.codehaus.groovy.classgen.VariableScopeVisitor;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
-import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
 import org.codehaus.groovy.syntax.SyntaxException;
 import org.objectweb.asm.Opcodes;
 
 import java.util.HashSet;
 import java.util.LinkedList;
-import java.util.List;
 import java.util.Set;
 
+import static java.util.Collections.addAll;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
 import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
 import static org.codehaus.groovy.ast.tools.ClosureUtils.hasImplicitParameter;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
@@ -66,96 +66,140 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
  * </ul>
  */
 @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
-public class CategoryASTTransformation implements ASTTransformation, Opcodes {
-    // should not use a static variable because of possible changes to node metadata
-    // which would be visible to other compilation units
-    private final VariableExpression thisExpression = createThisExpression();
+public class CategoryASTTransformation implements ASTTransformation {
 
-    private static VariableExpression createThisExpression() {
-        VariableExpression expr = new VariableExpression("$this");
-        expr.setClosureSharedVariable(true);
-        return expr;
-    }
-
-    /**
-     * Property invocations done on 'this' reference are transformed so that the invocations at runtime are
-     * done on the additional parameter 'self'
-     */
-    public void visit(ASTNode[] nodes, final SourceUnit source) {
+    @Override
+    public void visit(final ASTNode[] nodes, final SourceUnit sourceUnit) {
         if (nodes.length != 2 || !(nodes[0] instanceof AnnotationNode) || !(nodes[1] instanceof ClassNode)) {
-            source.getErrorCollector().addError(
-                    new SyntaxErrorMessage(new SyntaxException("@Category can only be added to a ClassNode but got: " + (nodes.length==2?nodes[1]:"nothing"),
-                        nodes[0].getLineNumber(), nodes[0].getColumnNumber()), source));
+            sourceUnit.addError(new SyntaxException("@Category can only be added to a ClassNode but got: " + (nodes.length == 2 ? nodes[1] : "nothing"), nodes[0].getLineNumber(), nodes[0].getColumnNumber()));
         }
 
-        AnnotationNode annotation = (AnnotationNode) nodes[0];
-        ClassNode parent = (ClassNode) nodes[1];
-
-        ClassNode targetClass = getTargetClass(source, annotation);
-        thisExpression.setType(targetClass);
+        ClassNode sourceClass = (ClassNode) nodes[1];
+        ClassNode targetClass; // the declared type of "self"
+        Expression value = ((AnnotationNode) nodes[0]).getMember("value");
+        if (value instanceof ClassExpression) {
+            targetClass = value.getType();
+        } else {
+            targetClass = ClassHelper.OBJECT_TYPE; // TODO: ClassHelper.make((Class<?>)groovy.lang.Category.class.getMethod("value").getDefaultValue());
+            sourceUnit.addErrorAndContinue(new SyntaxException("@Category must define 'value' which is the class to apply this category to", nodes[0]));
+        }
 
-        final LinkedList<Set<String>> varStack = new LinkedList<Set<String>>();
-        if (!ensureNoInstanceFieldOrProperty(source, parent)) return;
+        if (ensureNoInstanceFieldOrProperty(sourceClass, sourceUnit)) {
+            transformReferencesToThis(targetClass, sourceClass, sourceUnit);
+            new VariableScopeVisitor(sourceUnit, true).visitClass(sourceClass);
+        }
+    }
 
-        Set<String> names = new HashSet<String>();
-        for (FieldNode field : parent.getFields()) {
-            names.add(field.getName());
+    private static boolean ensureNoInstanceFieldOrProperty(final ClassNode sourceClass, final SourceUnit sourceUnit) {
+        boolean valid = true;
+        for (FieldNode fieldNode : sourceClass.getFields()) {
+            if (!fieldNode.isStatic() && fieldNode.getLineNumber() > 0) { // if < 1, probably an AST transform or internal code (like generated metaclass field, ...)
+                addUnsupportedInstanceMemberError(fieldNode.getName(), fieldNode,  sourceUnit);
+                valid = false;
+            }
         }
-        for (PropertyNode field : parent.getProperties()) {
-            names.add(field.getName());
+        for (PropertyNode propertyNode : sourceClass.getProperties()) {
+            if (!propertyNode.isStatic() && propertyNode.getLineNumber() > 0) { // if < 1, probably an AST transform or internal code (like generated metaclass field, ...)
+                addUnsupportedInstanceMemberError(propertyNode.getName(), propertyNode, sourceUnit);
+                valid = false;
+            }
         }
+        return valid;
+    }
+
+    private static void addUnsupportedInstanceMemberError(final String name, final ASTNode node, final SourceUnit unit) {
+        unit.addErrorAndContinue(new SyntaxException("The @Category transformation does not support instance " + (node instanceof FieldNode ? "fields" : "properties") + " but found [" + name + "]", node));
+    }
+
+    //--------------------------------------------------------------------------
+
+    private void transformReferencesToThis(final ClassNode targetClass, final ClassNode sourceClass, final SourceUnit sourceUnit) {
+        final Reference<Parameter> selfParameter = new Reference<>();
+        final LinkedList<Set<String>> varStack = new LinkedList<>();
+
+        Set<String> names = new HashSet<>();
+        for (FieldNode fn : sourceClass.getFields()) names.add(fn.getName());
+        for (PropertyNode pn : sourceClass.getProperties()) names.add(pn.getName());
         varStack.add(names);
 
-        final Reference parameter = new Reference();
-        final ClassCodeExpressionTransformer expressionTransformer = new ClassCodeExpressionTransformer() {
-            protected SourceUnit getSourceUnit() {
-                return source;
-            }
+        ClassCodeExpressionTransformer transformer = new ClassCodeExpressionTransformer() {
+            private boolean inClosure; // GROOVY-6510: track closure containment
 
-            private void addVariablesToStack(Parameter[] params) {
-                Set<String> names = new HashSet<String>(varStack.getLast());
-                for (Parameter param : params) {
-                    names.add(param.getName());
-                }
+            private void addVariablesToStack(final Parameter[] parameter) {
+                Set<String> names = new HashSet<>(varStack.getLast());
+                for (Parameter p : parameter) names.add(p.getName());
                 varStack.add(names);
             }
 
+            private Expression createThisExpression() {
+                VariableExpression ve = new VariableExpression("$this", targetClass);
+                ve.setClosureSharedVariable(true);
+                return ve;
+            }
+
             @Override
-            public void visitCatchStatement(CatchStatement statement) {
-                varStack.getLast().add(statement.getVariable().getName());
-                super.visitCatchStatement(statement);
-                varStack.getLast().remove(statement.getVariable().getName());
+            protected SourceUnit getSourceUnit() {
+                return sourceUnit;
             }
 
             @Override
-            public void visitMethod(MethodNode node) {
-                addVariablesToStack(node.getParameters());
-                super.visitMethod(node);
-                varStack.removeLast();
+            public Expression transform(final Expression expression) {
+                if (expression instanceof VariableExpression) {
+                    VariableExpression ve = (VariableExpression) expression;
+                    if (ve.isThisExpression()) {
+                        Expression thisExpression = createThisExpression();
+                        thisExpression.setSourcePosition(ve);
+                        return thisExpression;
+                    } else if (!inClosure && !ve.isSuperExpression() && !varStack.getLast().contains(ve.getName())) {
+                        PropertyExpression pe = new PropertyExpression(createThisExpression(), ve.getName());
+                        pe.setSourcePosition(ve);
+                        return pe;
+                    }
+                } else if (expression instanceof MethodCallExpression) {
+                    MethodCallExpression mce = (MethodCallExpression) expression;
+                    if (inClosure && mce.isImplicitThis() && isThisExpression(mce.getObjectExpression())) {
+                        // GROOVY-6510: preserve implicit-this semantics
+                        mce.setArguments(transform(mce.getArguments()));
+                        mce.setMethod(transform(mce.getMethod()));
+                        return mce;
+                    }
+                } else if (expression instanceof ClosureExpression) {
+                    ClosureExpression ce = (ClosureExpression) expression;
+                    addVariablesToStack(hasImplicitParameter(ce) ? params(param(ClassHelper.OBJECT_TYPE, "it")) : getParametersSafe(ce));
+                    ce.getVariableScope().putReferencedLocalVariable(selfParameter.get());
+                    addAll(varStack.getLast(), "owner", "delegate", "thisObject");
+                    boolean closure = inClosure; inClosure = true;
+                    ce.getCode().visit(this);
+                    varStack.removeLast();
+                    inClosure = closure;
+                }
+                return super.transform(expression);
             }
 
             @Override
-            public void visitBlockStatement(BlockStatement block) {
-                Set<String> names = new HashSet<String>(varStack.getLast());
+            public void visitBlockStatement(final BlockStatement statement) {
+                Set<String> names = new HashSet<>(varStack.getLast());
                 varStack.add(names);
-                super.visitBlockStatement(block);
+                super.visitBlockStatement(statement);
                 varStack.remove(names);
             }
 
             @Override
-            public void visitClosureExpression(ClosureExpression ce) {
-                addVariablesToStack(getParametersSafe(ce));
-                super.visitClosureExpression(ce);
-                varStack.removeLast();
+            public void visitCatchStatement(final CatchStatement statement) {
+                varStack.getLast().add(statement.getVariable().getName());
+                super.visitCatchStatement(statement);
+                varStack.getLast().remove(statement.getVariable().getName());
+            }
+
+            @Override
+            public void visitClosureExpression(final ClosureExpression expression) {
             }
 
             @Override
-            public void visitDeclarationExpression(DeclarationExpression expression) {
+            public void visitDeclarationExpression(final DeclarationExpression expression) {
                 if (expression.isMultipleAssignmentDeclaration()) {
-                    TupleExpression te = expression.getTupleExpression();
-                    List<Expression> list = te.getExpressions();
-                    for (Expression arg : list) {
-                        VariableExpression ve = (VariableExpression) arg;
+                    for (Expression e : expression.getTupleExpression().getExpressions()) {
+                        VariableExpression ve = (VariableExpression) e;
                         varStack.getLast().add(ve.getName());
                     }
                 } else {
@@ -166,120 +210,48 @@ public class CategoryASTTransformation implements ASTTransformation, Opcodes {
             }
 
             @Override
-            public void visitForLoop(ForStatement forLoop) {
-                Expression exp = forLoop.getCollectionExpression();
-                exp.visit(this);
-                Parameter loopParam = forLoop.getVariable();
-                if (loopParam != null) {
-                    varStack.getLast().add(loopParam.getName());
+            public void visitExpressionStatement(final ExpressionStatement statement) {
+                // GROOVY-3543: visit the declaration expressions so that declaration variables get added on the varStack
+                if (statement.getExpression() instanceof DeclarationExpression) {
+                    statement.getExpression().visit(this);
                 }
-                super.visitForLoop(forLoop);
+                super.visitExpressionStatement(statement);
             }
 
             @Override
-            public void visitExpressionStatement(ExpressionStatement es) {
-                // GROOVY-3543: visit the declaration expressions so that declaration variables get added on the varStack
-                Expression exp = es.getExpression();
-                if (exp instanceof DeclarationExpression) {
-                    exp.visit(this);
+            public void visitForLoop(final ForStatement statement) {
+                Expression exp = statement.getCollectionExpression();
+                exp.visit(this);
+                Parameter loopParam = statement.getVariable();
+                if (loopParam != null) {
+                    varStack.getLast().add(loopParam.getName());
                 }
-                super.visitExpressionStatement(es);
+                super.visitForLoop(statement);
             }
 
             @Override
-            public Expression transform(Expression exp) {
-                if (exp instanceof VariableExpression) {
-                    VariableExpression ve = (VariableExpression) exp;
-                    if (ve.getName().equals("this"))
-                        return thisExpression;
-                    else {
-                        if (!varStack.getLast().contains(ve.getName())) {
-                            PropertyExpression transformed = new PropertyExpression(thisExpression, ve.getName());
-                            transformed.setSourcePosition(ve);
-                            return transformed;
-                        }
-                    }
-                } else if (exp instanceof PropertyExpression) {
-                    PropertyExpression pe = (PropertyExpression) exp;
-                    if (pe.getObjectExpression() instanceof VariableExpression) {
-                        VariableExpression vex = (VariableExpression) pe.getObjectExpression();
-                        if (vex.isThisExpression()) {
-                            pe.setObjectExpression(thisExpression);
-                            return pe;
-                        }
-                    }
-                } else if (exp instanceof ClosureExpression) {
-                    ClosureExpression ce = (ClosureExpression) exp;
-                    ce.getVariableScope().putReferencedLocalVariable((Parameter) parameter.get());
-                    addVariablesToStack(
-                            hasImplicitParameter(ce)
-                                    ? params(param(ClassHelper.OBJECT_TYPE, "it"))
-                                    : getParametersSafe(ce));
-                    ce.getCode().visit(this);
-                    varStack.removeLast();
-                }
-                return super.transform(exp);
+            public void visitMethod(final MethodNode node) {
+                addVariablesToStack(node.getParameters());
+                super.visitMethod(node);
+                varStack.removeLast();
             }
         };
 
-        for (MethodNode method : parent.getMethods()) {
+        for (MethodNode method : sourceClass.getMethods()) {
             if (!method.isStatic()) {
-                method.setModifiers(method.getModifiers() | Opcodes.ACC_STATIC);
-                final Parameter[] origParams = method.getParameters();
-                final Parameter[] newParams = new Parameter[origParams.length + 1];
                 Parameter p = new Parameter(targetClass, "$this");
                 p.setClosureSharedVariable(true);
-                newParams[0] = p;
-                parameter.set(p);
-                System.arraycopy(origParams, 0, newParams, 1, origParams.length);
-                method.setParameters(newParams);
+                selfParameter.set(p);
 
-                expressionTransformer.visitMethod(method);
-            }
-        }
-        new VariableScopeVisitor(source, true).visitClass(parent);
-    }
+                Parameter[] oldParams = method.getParameters();
+                Parameter[] newParams = new Parameter[oldParams.length + 1];
+                newParams[0] = p;
+                System.arraycopy(oldParams, 0, newParams, 1, oldParams.length);
 
-    private static boolean ensureNoInstanceFieldOrProperty(final SourceUnit source, final ClassNode parent) {
-        boolean valid = true;
-        for (FieldNode fieldNode : parent.getFields()) {
-            if (!fieldNode.isStatic() && fieldNode.getLineNumber()>0) {
-                // if <0, probably an AST transform or internal code (like generated metaclass field, ...)
-                addUnsupportedError(fieldNode,  source);
-                valid = false;
-            }
-        }
-        for (PropertyNode propertyNode : parent.getProperties()) {
-            if (!propertyNode.isStatic() && propertyNode.getLineNumber()>0) {
-                // if <0, probably an AST transform or internal code (like generated metaclass field, ...)
-                addUnsupportedError(propertyNode, source);
-                valid = false;
+                method.setModifiers(method.getModifiers() | Opcodes.ACC_STATIC);
+                method.setParameters(newParams);
+                transformer.visitMethod(method);
             }
         }
-        return valid;
-    }
-
-    private static void addUnsupportedError(ASTNode node, SourceUnit unit) {
-        unit.getErrorCollector().addErrorAndContinue("The @Category transformation does not support instance "+
-                (node instanceof FieldNode?"fields":"properties")
-                + " but found ["+getName(node)+"]", node, unit);
-    }
-
-    private static String getName(ASTNode node) {
-        if (node instanceof FieldNode) return ((FieldNode) node).getName();
-        if (node instanceof PropertyNode) return ((PropertyNode) node).getName();
-        return node.getText();
-    }
-
-    private static ClassNode getTargetClass(SourceUnit source, AnnotationNode annotation) {
-        Expression value = annotation.getMember("value");
-        if (!(value instanceof ClassExpression)) {
-            //noinspection ThrowableInstanceNeverThrown
-            source.getErrorCollector().addErrorAndContinue("@groovy.lang.Category must define 'value' which is the class to apply this category to", annotation, source);
-            return null;
-        } else {
-            ClassExpression ce = (ClassExpression) value;
-            return ce.getType();
-        }
     }
 }
diff --git a/src/test/groovy/lang/CategoryAnnotationTest.groovy b/src/test/groovy/lang/CategoryAnnotationTest.groovy
index 1627838..9eda030 100644
--- a/src/test/groovy/lang/CategoryAnnotationTest.groovy
+++ b/src/test/groovy/lang/CategoryAnnotationTest.groovy
@@ -18,98 +18,91 @@
  */
 package groovy.lang
 
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-class CategoryAnnotationTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class CategoryAnnotationTest {
+
+    @Test // GROOVY-3367
     void testTransformationOfPropertyInvokedOnThis() {
-        //Test the fix for GROOVY-3367
-        assertScript """
-            @Category(Distance3367)
-            class DistanceCategory3367 {
-                Distance3367 plus(Distance3367 increment) {
-                    new Distance3367(number: this.number + increment.number)
-                }
-            }
-    
-            class Distance3367 {
+        assertScript '''
+            class Distance {
                 def number
             }
-    
-            use(DistanceCategory3367) {
-                def d1 = new Distance3367(number: 5)
-                def d2 = new Distance3367(number: 10)
-                def d3 = d1 + d2
-                assert d3.number == 15
-            }
-        """
-    }
-
-    void testTransformationWithCatchClause() {
-        //Test the fix for GROOVY-4801
-        assertScript """
-            class ExceptionHandler {
-                static def handled(Object self, Closure block) {
-                    try { block.call() }
-                    catch (Throwable t) { t.message }
+            @Category(Distance)
+            class DistanceCategory {
+                Distance plus(Distance increment) {
+                    new Distance(number: this.number + increment.number)
                 }
             }
 
-            @Mixin(ExceptionHandler)
-            class Caller {
-                def thrower() { handled { 1/0 } }
+            use(DistanceCategory) {
+                def d1 = new Distance(number: 5)
+                def d2 = new Distance(number: 10)
+                def d3 = d1 + d2
+                assert d3.number == 15
             }
-
-            assert new Caller().thrower() == 'Division by zero'
-        """
+        '''
     }
 
+    @Test // GROOVY-3543
     void testCategoryMethodsHavingDeclarationStatements() {
-        // GROOVY-3543: Declaration statements in category class' methods were not being visited by 
-        // CategoryASTTransformation's expressionTransformer resulting in declaration variables not being 
-        // defined on varStack resulting in compilation errors later
-        assertScript """
+        // Declaration statements in category class' methods were not being visited by
+        // CategoryASTTransformation's expressionTransformer resulting in declaration
+        // variables not being defined on varStack resulting in compilation errors later
+        assertScript '''
+            interface Test {
+                String getName()
+            }
+            class MyTest implements Test {
+                String getName() {
+                    return "Pre-"
+                }
+            }
             @Category(Test)
             class TestCategory {
-                String getSuperName1() { 
-                    String myname = "" 
-                    return myname + "hi from category" 
+                String getSuperName1() {
+                    String myname = ""
+                    return myname + "hi from category"
                 }
                 // 2nd test case of JIRA
-                String getSuperName2() { 
-                    String myname = this.getName() 
-                    for(int i = 0; i < 5; i++) myname += i 
+                String getSuperName2() {
+                    String myname = this.getName()
+                    for(int i = 0; i < 5; i++) myname += i
                     return myname + "-Post"
                 }
                 // 3rd test case of JIRA
-                String getSuperName3() { 
-                    String myname = this.getName() 
+                String getSuperName3() {
+                    String myname = this.getName()
                     for(i in 0..4) myname += i
                     return myname + "-Post"
                 }
             }
-    
-            interface Test { 
-                String getName() 
-            }
-    
-            class MyTest implements Test {
-                String getName() {
-                    return "Pre-"
-                }
-            }
-    
-            def onetest = new MyTest()
-            use(TestCategory) { 
-                assert onetest.getSuperName1() == "hi from category"
-                assert onetest.getSuperName2() == "Pre-01234-Post"
-                assert onetest.getSuperName3() == "Pre-01234-Post"
+
+            def test = new MyTest()
+            use(TestCategory) {
+                assert test.getSuperName1() == "hi from category"
+                assert test.getSuperName2() == "Pre-01234-Post"
+                assert test.getSuperName3() == "Pre-01234-Post"
             }
-        """
+        '''
     }
 
+    @Test // GROOVY-3543
     void testPropertyNameExpandingToGetterInsideCategoryMethod() {
-        //GROOVY-3543: Inside the category method, this.getType().name was failing but this.getType().getName() was not.
-        assertScript """
+        // Inside the category method, this.getType().name was failing but this.getType().getName() was not.
+        assertScript '''
+            class Type {
+                String name
+            }
+            interface Guy {
+                Type getType()
+            }
+            class McGuyver implements Guy {
+                Type type
+            }
             @Category(Guy)
             class Naming {
                 String getTypeName() {
@@ -119,60 +112,100 @@ class CategoryAnnotationTest extends GroovyTestCase {
                         ""
                 }
             }
-            
-            interface Guy {
-                Type getType()
-            }
-            
-            class Type {
-                String name
-            }
-            
-            class MyGuyver implements Guy {
-                Type type
-            }
-            
+
             def atype = new Type(name: 'String')
-            def onetest = new MyGuyver(type:atype)
-            
+            def onetest = new McGuyver(type:atype)
+
             use(Naming) {
                 assert onetest.getTypeName() == onetest.getType().getName()
             }
-        """
+        '''
     }
 
+    @Test
     void testClosureUsingThis() {
-        assertScript """
-            @Category(Guy)
-            class Filtering {
-                List process() {
-                    this.messages.findAll{it.name != this.getName()}
-                }
-            }
-            
+        assertScript '''
             interface Guy {
                 String getName()
                 List getMessages()
             }
-            
-            class MyGuyver implements Guy {
+            class McGuyver implements Guy {
                 List messages
                 String name
             }
-            
-            def onetest = new MyGuyver(
-                    name: 'coucou',
-                    messages : [['name':'coucou'], ['name':'test'], ['name':'salut']])
-            
-            Guy.mixin   Filtering
-            
-            assert onetest.process() == onetest.messages.findAll{it.name != onetest.getName()}        
-        """
+            @Category(Guy)
+            class Filtering {
+                List process() {
+                    this.messages.findAll{ it.name != this.getName() }
+                }
+            }
+
+            def onetest = new McGuyver(name: 'coucou',
+                messages: [['name':'coucou'], ['name':'test'], ['name':'salut']]
+            )
+
+            Guy.mixin(Filtering)
+
+            assert onetest.process() == onetest.messages.findAll{ it.name != onetest.getName() }
+        '''
     }
 
+    @Test // GROOVY-6510
+    void testClosureUsingImplicitThis() {
+        assertScript '''
+            @Category(Number)
+            class NumberCategory {
+                def foo() {
+                    def bar = { ->
+                        baz() // do not want "$this.baz()"
+                    }
+                    bar.resolveStrategy = Closure.DELEGATE_FIRST
+                    bar.delegate = new NumberDelegate(this)
+                    bar.call()
+                }
+            }
+
+            class NumberDelegate {
+                private final Number n
+                NumberDelegate(Number n) { this.n = n }
+                String baz() { 'number ' + n.intValue() }
+            }
+
+            use(NumberCategory) {
+                String result = 1.foo()
+                assert result == 'number 1'
+            }
+        '''
+
+        assertScript '''
+            @Category(Number)
+            class NumberCategory {
+                def foo() {
+                    def bar = { ->
+                        baz // do not want "$this.baz"
+                    }
+                    bar.resolveStrategy = Closure.DELEGATE_FIRST
+                    bar.delegate = new NumberDelegate(this)
+                    bar.call()
+                }
+            }
+
+            class NumberDelegate {
+                private final Number n
+                NumberDelegate(Number n) { this.n = n }
+                String getBaz() { 'number ' + n.intValue() }
+            }
+
+            use(NumberCategory) {
+                String result = 1.foo()
+                assert result == 'number 1'
+            }
+        '''
+    }
+
+    @Test // GROOVY-4546
     void testClosureWithinDeclarationExpressionAndMultipleAssignment() {
-        // GROOVY-4546
-        assertScript """
+        assertScript '''
             @Category(Integer)
             class MyOps {
                 def multiplesUpTo4() { [this * 2, this * 3, this * 4] }
@@ -195,16 +228,12 @@ class CategoryAnnotationTest extends GroovyTestCase {
                 assert 5.alsoMultiplesUpTo(6) == [10, 15, 20, 25, 30]
                 assert 5.twice() == 10
             }
-        """
+        '''
     }
 
-    // GROOVY-6120
+    @Test // GROOVY-6120
     void testFieldShouldNotBeAllowedInCategory() {
-        def message = shouldFail(RuntimeException) {
-            assertScript '''
-            @Mixin(Foo)
-            class Bar {  }
-
+        def err = shouldFail RuntimeException, '''
             @Category(Bar)
             class Foo {
                 public x = 5
@@ -212,20 +241,18 @@ class CategoryAnnotationTest extends GroovyTestCase {
                     x
                 }
             }
+            @Mixin(Foo)
+            class Bar {
+            }
 
             assert new Bar().foo() == 5
-            '''
-        }
-        assert message.contains('The @Category transformation does not support instance fields')
+        '''
+        assert err =~ /The @Category transformation does not support instance fields/
     }
 
-    // GROOVY-6120
+    @Test // GROOVY-6120
     void testPropertyShouldNotBeAllowedInCategory() {
-        def message = shouldFail(RuntimeException) {
-            assertScript '''
-            @Mixin(Foo)
-            class Bar {  }
-
+        def err = shouldFail RuntimeException, '''
             @Category(Bar)
             class Foo {
                 int x = 5
@@ -233,56 +260,67 @@ class CategoryAnnotationTest extends GroovyTestCase {
                     x
                 }
             }
+            @Mixin(Foo)
+            class Bar {
+            }
 
             assert new Bar().foo() == 5
-            '''
-        }
-        assert message.contains('The @Category transformation does not support instance properties')
+        '''
+        assert err =~ /The @Category transformation does not support instance properties/
     }
 
-    // GROOVY-6120
+    @Test // GROOVY-6120
     void testShouldNotThrowVerifyError() {
         assertScript '''
-            @Mixin(Foo)
-            class Bar { int x = 5 }
-
             @Category(Bar)
             class Foo {
                 def foo() {
                     x
                 }
             }
+            @Mixin(Foo)
+            class Bar {
+                int x = 5
+            }
 
             assert new Bar().foo() == 5
         '''
     }
 
-    // GROOVY-6120
+    @Test // GROOVY-6120
     void testCategoryShouldBeCompatibleWithCompileStatic() {
         assertScript '''
-            @Mixin(Foo)
-            class Bar { int x = 5 }
+            import groovy.transform.CompileStatic
 
+            @CompileStatic
             @Category(Bar)
-            @groovy.transform.CompileStatic
             class Foo {
                 def foo() {
                     x
                 }
             }
+            @Mixin(Foo)
+            class Bar {
+                int x = 5
+            }
 
             assert new Bar().foo() == 5
         '''
     }
 
-    void testCategoryShouldBeCompatibleWithCompileStatic_GROOVY6917() {
+    @Test // GROOVY-6917
+    void testCategoryShouldBeCompatibleWithCompileStatic2() {
         assertScript '''
-            @groovy.transform.CompileStatic
+            import groovy.transform.CompileStatic
+
+            @CompileStatic
             @Category(Integer)
             class IntegerCategory {
-                Integer twice() { this * 2 }
+                Integer twice() {
+                    this * 2
+                }
                 List<Integer> multiplesUpTo(Integer num) {
-                    (2..num).collect{ j -> this * j }
+                    (2..num).collect{ n -> this * n }
                 }
                 List<Integer> multiplyAll(List<Integer> nums) {
                     nums.collect{ it * this }
@@ -297,4 +335,3 @@ class CategoryAnnotationTest extends GroovyTestCase {
         '''
     }
 }
-