changeset 52299:e11a53698d57

8198945: Invalid RuntimeVisibleTypeAnnotations for annotation on anonymous class type parameter Reviewed-by: wmdietl, abuckley, martin
author cushon
date Mon, 17 Sep 2018 11:09:43 -0700
parents c2f38eb6b31a
children a8d239bdaaee
files src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java test/langtools/tools/javac/annotations/typeAnnotations/classfile/AnonymousClassTest.java
diffstat 4 files changed, 297 insertions(+), 46 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java	Wed Oct 24 15:45:09 2018 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java	Mon Sep 17 11:09:43 2018 -0700
@@ -1297,15 +1297,26 @@
             super.visitTypeParameter(tree);
         }
 
-        private void copyNewClassAnnotationsToOwner(JCNewClass tree) {
+        private void propagateNewClassAnnotationsToOwner(JCNewClass tree) {
             Symbol sym = tree.def.sym;
-            final TypeAnnotationPosition pos =
-                TypeAnnotationPosition.newObj(tree.pos);
+            // The anonymous class' synthetic class declaration is itself an inner class,
+            // so the type path is one INNER_TYPE entry deeper than that of the
+            // lexically enclosing class.
+            List<TypePathEntry> depth =
+                    locateNestedTypes(sym.owner.enclClass().type, new ListBuffer<>())
+                            .append(TypePathEntry.INNER_TYPE).toList();
+            TypeAnnotationPosition pos =
+                    TypeAnnotationPosition.newObj(depth, /* currentLambda= */ null, tree.pos);
+
             ListBuffer<Attribute.TypeCompound> newattrs = new ListBuffer<>();
-
+            List<TypePathEntry> expectedLocation =
+                    locateNestedTypes(tree.clazz.type, new ListBuffer<>()).toList();
             for (Attribute.TypeCompound old : sym.getRawTypeAttributes()) {
-                newattrs.append(new Attribute.TypeCompound(old.type, old.values,
-                                                           pos));
+                // Only propagate type annotations from the top-level supertype,
+                // (including if the supertype is an inner class).
+                if (old.position.location.equals(expectedLocation)) {
+                    newattrs.append(new Attribute.TypeCompound(old.type, old.values, pos));
+                }
             }
 
             sym.owner.appendUniqueTypeAttributes(newattrs.toList());
@@ -1313,27 +1324,8 @@
 
         @Override
         public void visitNewClass(JCNewClass tree) {
-            if (tree.def != null &&
-                    !tree.def.mods.annotations.isEmpty()) {
-                JCClassDecl classdecl = tree.def;
-                TypeAnnotationPosition pos;
-
-                if (classdecl.extending == tree.clazz) {
-                    pos = TypeAnnotationPosition.classExtends(tree.pos);
-                } else if (classdecl.implementing.contains(tree.clazz)) {
-                    final int index = classdecl.implementing.indexOf(tree.clazz);
-                    pos = TypeAnnotationPosition.classExtends(index, tree.pos);
-                } else {
-                    // In contrast to CLASS elsewhere, typarams cannot occur here.
-                    throw new AssertionError("Could not determine position of tree " + tree);
-                }
-                Type before = classdecl.sym.type;
-                separateAnnotationsKinds(classdecl, tree.clazz.type, classdecl.sym, pos);
-                copyNewClassAnnotationsToOwner(tree);
-                // classdecl.sym.type now contains an annotated type, which
-                // is not what we want there.
-                // TODO: should we put this type somewhere in the superclass/interface?
-                classdecl.sym.type = before;
+            if (tree.def != null && tree.def.sym != null) {
+                propagateNewClassAnnotationsToOwner(tree);
             }
 
             scan(tree.encl);
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java	Wed Oct 24 15:45:09 2018 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java	Mon Sep 17 11:09:43 2018 -0700
@@ -1115,7 +1115,9 @@
         public void visitNewClass(JCNewClass tree) {
             scan(tree.encl);
             scan(tree.typeargs);
-            scan(tree.clazz);
+            if (tree.def == null) {
+                scan(tree.clazz);
+            }
             scan(tree.args);
             // the anonymous class instantiation if any will be visited separately.
         }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Wed Oct 24 15:45:09 2018 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Mon Sep 17 11:09:43 2018 -0700
@@ -2255,25 +2255,11 @@
             }
             return e;
         } else if (token.kind == LPAREN) {
-            JCNewClass newClass = classCreatorRest(newpos, null, typeArgs, t);
-            if (newClass.def != null) {
-                assert newClass.def.mods.annotations.isEmpty();
-                if (newAnnotations.nonEmpty()) {
-                    // Add type and declaration annotations to the new class;
-                    // com.sun.tools.javac.code.TypeAnnotations.TypeAnnotationPositions.visitNewClass(JCNewClass)
-                    // will later remove all type annotations and only leave the
-                    // declaration annotations.
-                    newClass.def.mods.pos = earlier(newClass.def.mods.pos, newAnnotations.head.pos);
-                    newClass.def.mods.annotations = newAnnotations;
-                }
-            } else {
-                // handle type annotations for instantiations
-                if (newAnnotations.nonEmpty()) {
-                    t = insertAnnotationsToMostInner(t, newAnnotations, false);
-                    newClass.clazz = t;
-                }
+            // handle type annotations for instantiations and anonymous classes
+            if (newAnnotations.nonEmpty()) {
+                t = insertAnnotationsToMostInner(t, newAnnotations, false);
             }
-            return newClass;
+            return classCreatorRest(newpos, null, typeArgs, t);
         } else {
             setErrorEndPos(token.pos);
             reportSyntaxError(token.pos, Errors.Expected2(LPAREN, LBRACKET));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/annotations/typeAnnotations/classfile/AnonymousClassTest.java	Mon Sep 17 11:09:43 2018 -0700
@@ -0,0 +1,271 @@
+/*
+ * Copyright (c) 2018 Google LLC. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8198945 8207018
+ * @summary Invalid RuntimeVisibleTypeAnnotations for annotation on anonymous class type parameter
+ * @library /tools/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.jdeps/com.sun.tools.classfile
+ *          jdk.jdeps/com.sun.tools.javap
+ * @build toolbox.ToolBox toolbox.JavapTask
+ * @run compile -g AnonymousClassTest.java
+ * @run main AnonymousClassTest
+ */
+
+import static java.util.stream.Collectors.toSet;
+
+import com.sun.tools.classfile.Annotation;
+import com.sun.tools.classfile.Annotation.Annotation_element_value;
+import com.sun.tools.classfile.Annotation.Array_element_value;
+import com.sun.tools.classfile.Annotation.Class_element_value;
+import com.sun.tools.classfile.Annotation.Enum_element_value;
+import com.sun.tools.classfile.Annotation.Primitive_element_value;
+import com.sun.tools.classfile.Annotation.element_value;
+import com.sun.tools.classfile.Annotation.element_value.Visitor;
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
+import com.sun.tools.classfile.ConstantPool.CONSTANT_Integer_info;
+import com.sun.tools.classfile.ConstantPool.InvalidIndex;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Method;
+import com.sun.tools.classfile.RuntimeVisibleTypeAnnotations_attribute;
+import com.sun.tools.classfile.TypeAnnotation;
+import com.sun.tools.classfile.TypeAnnotation.Position;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import toolbox.ToolBox;
+
+public class AnonymousClassTest {
+
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target(ElementType.TYPE_USE)
+    public @interface TA {
+        int value() default 0;
+    }
+
+    private void f() {
+        new @TA(0) Callable<@TA(1) Object>() {
+            public Object call() {
+                return null;
+            }
+        };
+    }
+
+    class Inner {
+        private void g() {
+            // The annotation is propagated from the top-level class Object to NEW expression for
+            // the anonymous class' synthetic class declaration, which is an inner class of an inner
+            // class.
+            new @TA(2) Object() {};
+        }
+    }
+
+    private void g() {
+        new @TA(3) AnonymousClassTest.@TA(4) Inner() {};
+    }
+
+    public static void main(String args[]) throws Exception {
+        testAnonymousClassDeclaration();
+        testTopLevelMethod();
+        testInnerClassMethod();
+        testQualifiedSuperType();
+    }
+
+    static void testAnonymousClassDeclaration() throws Exception {
+        ClassFile cf = ClassFile.read(Paths.get(ToolBox.testClasses, "AnonymousClassTest$1.class"));
+        RuntimeVisibleTypeAnnotations_attribute rvta =
+                (RuntimeVisibleTypeAnnotations_attribute)
+                        cf.attributes.get(Attribute.RuntimeVisibleTypeAnnotations);
+        assertEquals(
+                Set.of(
+                        "@LAnonymousClassTest$TA;(1) CLASS_EXTENDS, offset=-1, location=[TYPE_ARGUMENT(0)]",
+                        "@LAnonymousClassTest$TA;(0) CLASS_EXTENDS, offset=-1, location=[]"),
+                Arrays.stream(rvta.annotations)
+                        .map(a -> annotationDebugString(cf, a))
+                        .collect(toSet()));
+    }
+
+    static void testTopLevelMethod() throws Exception {
+        ClassFile cf = ClassFile.read(Paths.get(ToolBox.testClasses, "AnonymousClassTest.class"));
+        Method method = findMethod(cf, "f");
+        Set<TypeAnnotation> annotations = getRuntimeVisibleTypeAnnotations(method);
+        assertEquals(
+                Set.of("@LAnonymousClassTest$TA;(0) NEW, offset=0, location=[INNER_TYPE]"),
+                annotations.stream().map(a -> annotationDebugString(cf, a)).collect(toSet()));
+    }
+
+    static void testInnerClassMethod() throws Exception {
+        ClassFile cf =
+                ClassFile.read(Paths.get(ToolBox.testClasses, "AnonymousClassTest$Inner.class"));
+        Method method = findMethod(cf, "g");
+        Set<TypeAnnotation> annotations = getRuntimeVisibleTypeAnnotations(method);
+        // The annotation needs two INNER_TYPE type path entries to apply to
+        // AnonymousClassTest$Inner$1.
+        assertEquals(
+                Set.of(
+                        "@LAnonymousClassTest$TA;(2) NEW, offset=0, location=[INNER_TYPE, INNER_TYPE]"),
+                annotations.stream().map(a -> annotationDebugString(cf, a)).collect(toSet()));
+    }
+
+    static void testQualifiedSuperType() throws Exception {
+        {
+            ClassFile cf =
+                    ClassFile.read(Paths.get(ToolBox.testClasses, "AnonymousClassTest.class"));
+            Method method = findMethod(cf, "g");
+            Set<TypeAnnotation> annotations = getRuntimeVisibleTypeAnnotations(method);
+            // Only @TA(4) is propagated to the anonymous class declaration.
+            assertEquals(
+                    Set.of("@LAnonymousClassTest$TA;(4) NEW, offset=0, location=[INNER_TYPE]"),
+                    annotations.stream().map(a -> annotationDebugString(cf, a)).collect(toSet()));
+        }
+
+        {
+            ClassFile cf =
+                    ClassFile.read(Paths.get(ToolBox.testClasses, "AnonymousClassTest$2.class"));
+            RuntimeVisibleTypeAnnotations_attribute rvta =
+                    (RuntimeVisibleTypeAnnotations_attribute)
+                            cf.attributes.get(Attribute.RuntimeVisibleTypeAnnotations);
+            assertEquals(
+                    Set.of(
+                            "@LAnonymousClassTest$TA;(3) CLASS_EXTENDS, offset=-1, location=[]",
+                            "@LAnonymousClassTest$TA;(4) CLASS_EXTENDS, offset=-1, location=[INNER_TYPE]"),
+                    Arrays.stream(rvta.annotations)
+                            .map(a -> annotationDebugString(cf, a))
+                            .collect(toSet()));
+        }
+    }
+
+    // Returns the Method's RuntimeVisibleTypeAnnotations, and asserts that there are no RVTIs
+    // erroneously associated with the Method instead of its Code attribute.
+    private static Set<TypeAnnotation> getRuntimeVisibleTypeAnnotations(Method method) {
+        if (method.attributes.get(Attribute.RuntimeVisibleTypeAnnotations) != null) {
+            throw new AssertionError(
+                    "expected no RuntimeVisibleTypeAnnotations attribute on enclosing method");
+        }
+        Code_attribute code = (Code_attribute) method.attributes.get(Attribute.Code);
+        RuntimeVisibleTypeAnnotations_attribute rvta =
+                (RuntimeVisibleTypeAnnotations_attribute)
+                        code.attributes.get(Attribute.RuntimeVisibleTypeAnnotations);
+        return Set.of(rvta.annotations);
+    }
+
+    private static Method findMethod(ClassFile cf, String name) {
+        return Arrays.stream(cf.methods)
+                .filter(
+                        m -> {
+                            try {
+                                return m.getName(cf.constant_pool).contentEquals(name);
+                            } catch (ConstantPoolException e) {
+                                throw new AssertionError(e);
+                            }
+                        })
+                .findFirst()
+                .get();
+    }
+
+    private static void assertEquals(Object expected, Object actual) {
+        if (!actual.equals(expected)) {
+            throw new AssertionError(String.format("expected: %s, saw: %s", expected, actual));
+        }
+    }
+
+    private static String annotationDebugString(ClassFile cf, TypeAnnotation annotation) {
+        Position pos = annotation.position;
+        String name;
+        try {
+            name = cf.constant_pool.getUTF8Info(annotation.annotation.type_index).value;
+        } catch (Exception e) {
+            throw new AssertionError(e);
+        }
+        return String.format(
+                "@%s(%s) %s, offset=%d, location=%s",
+                name,
+                annotationValueoDebugString(cf, annotation.annotation),
+                pos.type,
+                pos.offset,
+                pos.location);
+    }
+
+    private static String annotationValueoDebugString(ClassFile cf, Annotation annotation) {
+        if (annotation.element_value_pairs.length != 1) {
+            throw new UnsupportedOperationException();
+        }
+        try {
+            return elementValueDebugString(cf, annotation.element_value_pairs[0].value);
+        } catch (Exception e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    private static String elementValueDebugString(ClassFile cf, element_value value) {
+        class Viz implements Visitor<String, Void> {
+            @Override
+            public String visitPrimitive(Primitive_element_value ev, Void aVoid) {
+                try {
+                    switch (ev.tag) {
+                        case 'I':
+                            return Integer.toString(
+                                    ((CONSTANT_Integer_info)
+                                                    cf.constant_pool.get(ev.const_value_index))
+                                            .value);
+                        default:
+                            throw new UnsupportedOperationException(String.format("%c", ev.tag));
+                    }
+                } catch (InvalidIndex e) {
+                    throw new AssertionError(e);
+                }
+            }
+
+            @Override
+            public String visitEnum(Enum_element_value ev, Void aVoid) {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public String visitClass(Class_element_value ev, Void aVoid) {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public String visitAnnotation(Annotation_element_value ev, Void aVoid) {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public String visitArray(Array_element_value ev, Void aVoid) {
+                throw new UnsupportedOperationException();
+            }
+        }
+        return value.accept(new Viz(), null);
+    }
+}