8001114: Container annotation is not checked for semantic correctness
authorjfranck
Mon, 03 Dec 2012 11:16:32 +0100
changeset 14804 f93a8d60b9a4
parent 14803 88347e495d34
child 14805 ec5621e36729
child 14948 6a2008d8e9ba
8001114: Container annotation is not checked for semantic correctness Reviewed-by: jjg
langtools/src/share/classes/com/sun/tools/javac/code/Annotations.java
langtools/src/share/classes/com/sun/tools/javac/comp/Annotate.java
langtools/src/share/classes/com/sun/tools/javac/comp/Check.java
langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties
langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.java
langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.out
langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.java
langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.out
langtools/test/tools/javac/annotations/repeatingAnnotations/NoRepeatableAnno.out
langtools/test/tools/javac/annotations/repeatingAnnotations/RepeatingTargetNotAllowed.java
langtools/test/tools/javac/annotations/repeatingAnnotations/RepeatingTargetNotAllowed.out
langtools/test/tools/javac/diags/examples/ContainedByNonDefault.java
langtools/test/tools/javac/diags/examples/InvalidDuplicateAnnotation.java
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Annotations.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Annotations.java	Mon Dec 03 11:16:32 2012 +0100
@@ -74,12 +74,12 @@
      */
     private List<Attribute.Compound> attributes = NOT_STARTED;
     /*
-     * The Symbol this Annotatios belong to
+     * The Symbol this Annotations belong to
      */
-    private final Symbol s;
+    private final Symbol sym;
 
-    public Annotations(Symbol s) {
-        this.s = s;
+    public Annotations(Symbol sym) {
+        this.sym = sym;
     }
 
     public List<Attribute.Compound> getAttributes() {
@@ -102,7 +102,7 @@
     }
 
     public void setAttributesWithCompletion(final Annotate.AnnotateRepeatedContext ctx) {
-        Assert.check(pendingCompletion() || (!isStarted() && s.kind == PCK));
+        Assert.check(pendingCompletion() || (!isStarted() && sym.kind == PCK));
 
         Map<Symbol.TypeSymbol, ListBuffer<Attribute.Compound>> annotated = ctx.annotated;
         boolean atLeastOneRepeated = false;
@@ -111,7 +111,7 @@
             if (lb.size() == 1) {
                 buf = buf.prepend(lb.first());
             } else { // repeated
-                buf = buf.prepend(new Placeholder(lb.toList(), s));
+                buf = buf.prepend(new Placeholder(lb.toList(), sym));
                 atLeastOneRepeated = true;
             }
         }
@@ -141,7 +141,7 @@
 
                 @Override
                 public String toString() {
-                    return "repeated annotation pass of: " + s + " in: " + s.owner;
+                    return "repeated annotation pass of: " + sym + " in: " + sym.owner;
                 }
 
                 @Override
@@ -253,7 +253,7 @@
 
         // Process repeated annotations
         Attribute.Compound validRepeated =
-                ctx.processRepeatedAnnotations(placeholder.getPlaceholderFor());
+            ctx.processRepeatedAnnotations(placeholder.getPlaceholderFor(), sym);
 
         if (validRepeated != null) {
             // Check that the container isn't manually
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Annotate.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Annotate.java	Mon Dec 03 11:16:32 2012 +0100
@@ -26,7 +26,6 @@
 package com.sun.tools.javac.comp;
 
 import java.util.Map;
-
 import com.sun.tools.javac.util.*;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
 import com.sun.tools.javac.code.*;
@@ -171,8 +170,8 @@
          * @param repeatingAnnotations a List of repeating annotations
          * @return a new Attribute.Compound that is the container for the repeatingAnnotations
          */
-        public Attribute.Compound processRepeatedAnnotations(List<Attribute.Compound> repeatingAnnotations) {
-            return Annotate.this.processRepeatedAnnotations(repeatingAnnotations, this);
+        public Attribute.Compound processRepeatedAnnotations(List<Attribute.Compound> repeatingAnnotations, Symbol sym) {
+            return Annotate.this.processRepeatedAnnotations(repeatingAnnotations, this, sym);
         }
 
         /**
@@ -339,10 +338,11 @@
      * annotation are invalid.  This method reports errors/warnings.
      */
     private Attribute.Compound processRepeatedAnnotations(List<Attribute.Compound> annotations,
-            AnnotateRepeatedContext ctx) {
+                                                          AnnotateRepeatedContext ctx,
+                                                          Symbol on) {
         Attribute.Compound firstOccurrence = annotations.head;
         List<Attribute> repeated = List.nil();
-        Type origAnnoType;
+        Type origAnnoType = null;
         Type arrayOfOrigAnnoType = null;
         Type targetContainerType = null;
         MethodSymbol containerValueSymbol = null;
@@ -390,6 +390,13 @@
                                                       new Attribute.Array(arrayOfOrigAnnoType, repeated));
             annoTree = m.Annotation(new Attribute.Compound(targetContainerType,
                     List.of(p)));
+
+            if (!chk.annotationApplicable(annoTree, on))
+                log.error(annoTree.pos(), "invalid.containedby.annotation.incompatible.target", targetContainerType, origAnnoType);
+
+            if (!chk.validateAnnotationDeferErrors(annoTree))
+                log.error(annoTree.pos(), "duplicate.annotation.invalid.repeated", origAnnoType);
+
             Attribute.Compound c = enterAnnotation(annoTree,
                                                    targetContainerType,
                                                    ctx.env);
@@ -410,7 +417,7 @@
         // annotation's declaration, or null if it has none
         Attribute.Compound ca = origAnnoDecl.attribute(syms.containedByType.tsym);
         if (ca == null) { // has no ContainedBy annotation
-            log.error(pos, "duplicate.annotation.missing.container", origAnnoType);
+            log.error(pos, "duplicate.annotation.missing.container", origAnnoType, syms.containedByType);
             return null;
         }
 
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java	Mon Dec 03 11:16:32 2012 +0100
@@ -2892,39 +2892,54 @@
     }
 
     /** Check an annotation value.
+     *
+     * @param a The annotation tree to check
+     * @return true if this annotation tree is valid, otherwise false
      */
-    public void validateAnnotation(JCAnnotation a) {
-        // collect an inventory of the members (sorted alphabetically)
-        Set<MethodSymbol> members = new TreeSet<MethodSymbol>(new Comparator<Symbol>() {
-            public int compare(Symbol t, Symbol t1) {
-                return t.name.compareTo(t1.name);
-            }
-        });
+    public boolean validateAnnotationDeferErrors(JCAnnotation a) {
+        boolean res = false;
+        final Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
+        try {
+            res = validateAnnotation(a);
+        } finally {
+            log.popDiagnosticHandler(diagHandler);
+        }
+        return res;
+    }
+
+    private boolean validateAnnotation(JCAnnotation a) {
+        boolean isValid = true;
+        // collect an inventory of the annotation elements
+        Set<MethodSymbol> members = new LinkedHashSet<MethodSymbol>();
         for (Scope.Entry e = a.annotationType.type.tsym.members().elems;
              e != null;
              e = e.sibling)
             if (e.sym.kind == MTH)
                 members.add((MethodSymbol) e.sym);
 
-        // count them off as they're annotated
+        // remove the ones that are assigned values
         for (JCTree arg : a.args) {
             if (!arg.hasTag(ASSIGN)) continue; // recovery
             JCAssign assign = (JCAssign) arg;
             Symbol m = TreeInfo.symbol(assign.lhs);
             if (m == null || m.type.isErroneous()) continue;
-            if (!members.remove(m))
+            if (!members.remove(m)) {
+                isValid = false;
                 log.error(assign.lhs.pos(), "duplicate.annotation.member.value",
                           m.name, a.type);
+            }
         }
 
         // all the remaining ones better have default values
-        ListBuffer<Name> missingDefaults = ListBuffer.lb();
+        List<Name> missingDefaults = List.nil();
         for (MethodSymbol m : members) {
             if (m.defaultValue == null && !m.type.isErroneous()) {
-                missingDefaults.append(m.name);
+                missingDefaults = missingDefaults.append(m.name);
             }
         }
+        missingDefaults = missingDefaults.reverse();
         if (missingDefaults.nonEmpty()) {
+            isValid = false;
             String key = (missingDefaults.size() > 1)
                     ? "annotation.missing.default.value.1"
                     : "annotation.missing.default.value";
@@ -2935,21 +2950,23 @@
         // repeated values in its value member
         if (a.annotationType.type.tsym != syms.annotationTargetType.tsym ||
             a.args.tail == null)
-            return;
+            return isValid;
 
-        if (!a.args.head.hasTag(ASSIGN)) return; // error recovery
+        if (!a.args.head.hasTag(ASSIGN)) return false; // error recovery
         JCAssign assign = (JCAssign) a.args.head;
         Symbol m = TreeInfo.symbol(assign.lhs);
-        if (m.name != names.value) return;
+        if (m.name != names.value) return false;
         JCTree rhs = assign.rhs;
-        if (!rhs.hasTag(NEWARRAY)) return;
+        if (!rhs.hasTag(NEWARRAY)) return false;
         JCNewArray na = (JCNewArray) rhs;
         Set<Symbol> targets = new HashSet<Symbol>();
         for (JCTree elem : na.elems) {
             if (!targets.add(TreeInfo.symbol(elem))) {
+                isValid = false;
                 log.error(elem.pos(), "repeated.annotation.target");
             }
         }
+        return isValid;
     }
 
     void checkDeprecatedAnnotation(DiagnosticPosition pos, Symbol s) {
--- a/langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java	Mon Dec 03 11:16:32 2012 +0100
@@ -1360,6 +1360,16 @@
     void attachAnnotationDefault(final Symbol sym) {
         final MethodSymbol meth = (MethodSymbol)sym; // only on methods
         final Attribute value = readAttributeValue();
+
+        // The default value is set later during annotation. It might
+        // be the case that the Symbol sym is annotated _after_ the
+        // repeating instances that depend on this default value,
+        // because of this we set an interim value that tells us this
+        // element (most likely) has a default.
+        //
+        // Set interim value for now, reset just before we do this
+        // properly at annotate time.
+        meth.defaultValue = value;
         annotate.normal(new AnnotationDefaultCompleter(meth, value));
     }
 
@@ -1680,6 +1690,9 @@
         public void enterAnnotation() {
             JavaFileObject previousClassFile = currentClassFile;
             try {
+                // Reset the interim value set earlier in
+                // attachAnnotationDefault().
+                sym.defaultValue = null;
                 currentClassFile = classFile;
                 sym.defaultValue = deproxy(sym.type.getReturnType(), value);
             } finally {
--- a/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties	Mon Dec 03 11:16:32 2012 +0100
@@ -307,13 +307,17 @@
 compiler.err.duplicate.annotation=\
     duplicate annotation
 
+# 0: type
+compiler.err.duplicate.annotation.invalid.repeated=\
+    annotation {0} cannot be repeated\nIt does not define a valid containing annotation.
+
 # 0: name, 1: type
 compiler.err.duplicate.annotation.member.value=\
     duplicate annotation member value {0} in {1}
 
-# 0: type
+# 0: type, 1: type
 compiler.err.duplicate.annotation.missing.container=\
-    duplicate annotation, the declaration of {0} does not have a ContainedBy annotation
+    duplicate annotation, the declaration of {0} does not have a valid {1} annotation
 
 # 0: type, 1: type
 compiler.err.invalid.container.no.containedby=\
--- a/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.java	Mon Dec 03 11:16:32 2012 +0100
@@ -20,4 +20,3 @@
 
 @Foo @Foo
 public class MissingDefaultCase1 {}
-
--- a/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.out	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase1.out	Mon Dec 03 11:16:32 2012 +0100
@@ -1,2 +1,3 @@
+MissingDefaultCase1.java:21:1: compiler.err.duplicate.annotation.invalid.repeated: Foo
 MissingDefaultCase1.java:12:1: compiler.err.invalid.containedby.annotation.elem.nondefault: FooContainer, other()
-1 error
+2 errors
--- a/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.java	Mon Dec 03 11:16:32 2012 +0100
@@ -20,4 +20,3 @@
 
 @Foo @Foo
 public class MissingDefaultCase2 {}
-
--- a/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.out	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/MissingDefaultCase2.out	Mon Dec 03 11:16:32 2012 +0100
@@ -1,2 +1,3 @@
+MissingDefaultCase2.java:21:1: compiler.err.duplicate.annotation.invalid.repeated: Foo
 MissingDefaultCase2.java:12:1: compiler.err.invalid.containedby.annotation.elem.nondefault: FooContainer, other()
-1 error
+2 errors
--- a/langtools/test/tools/javac/annotations/repeatingAnnotations/NoRepeatableAnno.out	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/NoRepeatableAnno.out	Mon Dec 03 11:16:32 2012 +0100
@@ -1,3 +1,3 @@
-NoRepeatableAnno.java:11:1: compiler.err.duplicate.annotation.missing.container: Foo
-NoRepeatableAnno.java:11:6: compiler.err.duplicate.annotation.missing.container: Foo
+NoRepeatableAnno.java:11:1: compiler.err.duplicate.annotation.missing.container: Foo, java.lang.annotation.ContainedBy
+NoRepeatableAnno.java:11:6: compiler.err.duplicate.annotation.missing.container: Foo, java.lang.annotation.ContainedBy
 2 errors
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/RepeatingTargetNotAllowed.java	Mon Dec 03 11:16:32 2012 +0100
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. 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
+ * @summary Container annotation is not checked for semantic correctness
+ * @bug 8001114
+ *
+ * @compile/fail/ref=RepeatingTargetNotAllowed.out -XDrawDiagnostics RepeatingTargetNotAllowed.java
+ */
+
+import java.lang.annotation.*;
+
+@ContainedBy(Foos.class)
+@interface Foo {}
+
+@ContainerFor(Foo.class)
+@Target(ElementType.ANNOTATION_TYPE)
+@interface Foos {
+    Foo[] value();
+}
+
+public class RepeatingTargetNotAllowed {
+    @Foo @Foo int f = 0;
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/annotations/repeatingAnnotations/RepeatingTargetNotAllowed.out	Mon Dec 03 11:16:32 2012 +0100
@@ -0,0 +1,2 @@
+RepeatingTargetNotAllowed.java:44:5: compiler.err.invalid.containedby.annotation.incompatible.target: Foos, Foo
+1 error
--- a/langtools/test/tools/javac/diags/examples/ContainedByNonDefault.java	Wed Dec 12 20:26:56 2012 +0100
+++ b/langtools/test/tools/javac/diags/examples/ContainedByNonDefault.java	Mon Dec 03 11:16:32 2012 +0100
@@ -31,6 +31,4 @@
 @ContainerFor(Anno.class)
 @interface Annos { Anno[] value(); String foo(); }
 
-@Anno
-@Anno
 class ContainedByNonDefault { }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/InvalidDuplicateAnnotation.java	Mon Dec 03 11:16:32 2012 +0100
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. 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.
+ */
+
+// key: compiler.err.duplicate.annotation.invalid.repeated
+// key: compiler.err.invalid.containedby.annotation.elem.nondefault
+//
+// We need an almost valid containing annotation. The easiest way to get
+// one close enough to valid is by forgetting a default.
+
+import java.lang.annotation.*;
+
+@ContainedBy(Annos.class)
+@interface Anno { }
+
+@ContainerFor(Anno.class)
+@interface Annos { Anno[] value(); String foo(); }
+
+@Anno
+@Anno
+class InvalidDuplicateAnnotation { }