8224177: Harden annotation processing framework to irregular behavior from processors
authordarcy
Thu, 23 May 2019 18:47:24 -0700
changeset 55021 d84176dd57b0
parent 55020 2bcb825c8edf
child 55022 9785b9fb328e
8224177: Harden annotation processing framework to irregular behavior from processors Reviewed-by: jjg
src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
test/langtools/tools/javac/diags/examples/DuplicateSupportedInfoFromProc/DuplicateSupportedInfoFromProc.java
test/langtools/tools/javac/diags/examples/DuplicateSupportedInfoFromProc/processors/AnnoProc.java
test/langtools/tools/javac/diags/examples/MalformedSupported/MalformedSupported.java
test/langtools/tools/javac/diags/examples/RedundantTypesWithWildcardProc/RedundantTypesWithWildcardProc.java
test/langtools/tools/javac/diags/examples/RedundantTypesWithWildcardProc/processors/AnnoProc.java
test/langtools/tools/javac/processing/warnings/TestRepeatedItemsRuntime.java
test/langtools/tools/javac/processing/warnings/auric_current.out
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java	Thu May 23 18:31:38 2019 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java	Thu May 23 18:47:24 2019 -0700
@@ -686,11 +686,12 @@
     static class ProcessorState {
         public Processor processor;
         public boolean   contributed;
-        private ArrayList<Pattern> supportedAnnotationPatterns;
-        private ArrayList<String>  supportedOptionNames;
+        private Set<String> supportedAnnotationStrings; // Used for warning generation
+        private Set<Pattern> supportedAnnotationPatterns;
+        private Set<String> supportedOptionNames;
 
         ProcessorState(Processor p, Log log, Source source, DeferredCompletionFailureHandler dcfh,
-                       boolean allowModules, ProcessingEnvironment env) {
+                       boolean allowModules, ProcessingEnvironment env, boolean lint) {
             processor = p;
             contributed = false;
 
@@ -700,18 +701,46 @@
 
                 checkSourceVersionCompatibility(source, log);
 
-                supportedAnnotationPatterns = new ArrayList<>();
-                for (String importString : processor.getSupportedAnnotationTypes()) {
-                    supportedAnnotationPatterns.add(importStringToPattern(allowModules,
-                                                                          importString,
-                                                                          processor,
-                                                                          log));
+
+                // Check for direct duplicates in the strings of
+                // supported annotation types. Do not check for
+                // duplicates that would result after stripping of
+                // module prefixes.
+                supportedAnnotationStrings = new LinkedHashSet<>();
+                supportedAnnotationPatterns = new LinkedHashSet<>();
+                for (String annotationPattern : processor.getSupportedAnnotationTypes()) {
+                    boolean patternAdded = supportedAnnotationStrings.add(annotationPattern);
+
+                    supportedAnnotationPatterns.
+                        add(importStringToPattern(allowModules, annotationPattern,
+                                                  processor, log, lint));
+                    if (lint && !patternAdded) {
+                        log.warning(Warnings.ProcDuplicateSupportedAnnotation(annotationPattern,
+                                                                              p.getClass().getName()));
+                    }
                 }
 
-                supportedOptionNames = new ArrayList<>();
+                // If a processor supports "*", that matches
+                // everything and other entries are redundant. With
+                // more work, it could be checked that the supported
+                // annotation types were otherwise non-overlapping
+                // with each other in other cases, for example "foo.*"
+                // and "foo.bar.*".
+                if (lint &&
+                    supportedAnnotationPatterns.contains(MatchingUtils.validImportStringToPattern("*")) &&
+                    supportedAnnotationPatterns.size() > 1) {
+                    log.warning(Warnings.ProcRedundantTypesWithWildcard(p.getClass().getName()));
+                }
+
+                supportedOptionNames = new LinkedHashSet<>();
                 for (String optionName : processor.getSupportedOptions() ) {
-                    if (checkOptionName(optionName, log))
-                        supportedOptionNames.add(optionName);
+                    if (checkOptionName(optionName, log)) {
+                        boolean optionAdded = supportedOptionNames.add(optionName);
+                        if (lint && !optionAdded) {
+                            log.warning(Warnings.ProcDuplicateOptionName(optionName,
+                                                                         p.getClass().getName()));
+                        }
+                    }
                 }
 
             } catch (ClientCodeException e) {
@@ -797,7 +826,8 @@
                     ProcessorState ps = new ProcessorState(psi.processorIterator.next(),
                                                            log, source, dcfh,
                                                            Feature.MODULES.allowedInSource(source),
-                                                           JavacProcessingEnvironment.this);
+                                                           JavacProcessingEnvironment.this,
+                                                           lint);
                     psi.procStateList.add(ps);
                     return ps;
                 } else
@@ -1705,7 +1735,7 @@
      * regex matching that string.  If the string is not a valid
      * import-style string, return a regex that won't match anything.
      */
-    private static Pattern importStringToPattern(boolean allowModules, String s, Processor p, Log log) {
+    private static Pattern importStringToPattern(boolean allowModules, String s, Processor p, Log log, boolean lint) {
         String module;
         String pkg;
         int slash = s.indexOf('/');
@@ -1716,15 +1746,26 @@
             module = allowModules ? ".*/" : "";
             pkg = s;
         } else {
-            module = Pattern.quote(s.substring(0, slash + 1));
+            String moduleName = s.substring(0, slash);
+            if (!SourceVersion.isIdentifier(moduleName)) {
+                return warnAndNoMatches(s, p, log, lint);
+            }
+            module = Pattern.quote(moduleName + "/");
+            // And warn if module is specified if modules aren't supported, conditional on -Xlint:proc?
             pkg = s.substring(slash + 1);
         }
         if (MatchingUtils.isValidImportString(pkg)) {
             return Pattern.compile(module + MatchingUtils.validImportStringToPatternString(pkg));
         } else {
+            return warnAndNoMatches(s, p, log, lint);
+        }
+    }
+
+    private static Pattern warnAndNoMatches(String s, Processor p, Log log, boolean lint) {
+        if (lint) {
             log.warning(Warnings.ProcMalformedSupportedString(s, p.getClass().getName()));
-            return noMatches; // won't match any valid identifier
         }
+        return noMatches; // won't match any valid identifier
     }
 
     /**
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu May 23 18:31:38 2019 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu May 23 18:47:24 2019 -0700
@@ -1911,6 +1911,18 @@
 compiler.warn.proc.processor.incompatible.source.version=\
     Supported source version ''{0}'' from annotation processor ''{1}'' less than -source ''{2}''
 
+# 0: string, 1: string
+compiler.warn.proc.duplicate.option.name=\
+    Duplicate supported option ''{0}'' returned by annotation processor ''{1}''
+
+# 0: string, 1: string
+compiler.warn.proc.duplicate.supported.annotation=\
+    Duplicate supported annotation type ''{0}'' returned by annotation processor ''{1}''
+
+# 0: string
+compiler.warn.proc.redundant.types.with.wildcard=\
+    Annotation processor ''{0}'' redundantly supports both ''*'' and other annotation types
+
 compiler.warn.proc.proc-only.requested.no.procs=\
     Annotation processing without compilation requested but no processors were found.
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/DuplicateSupportedInfoFromProc/DuplicateSupportedInfoFromProc.java	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2010, 2019, 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.warn.proc.duplicate.option.name
+// key: compiler.warn.proc.duplicate.supported.annotation
+// options: -processor AnnoProc -Xlint:processing
+
+class DuplicateSupportedInfoFromProc { }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/DuplicateSupportedInfoFromProc/processors/AnnoProc.java	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2010, 2019, 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.
+ */
+
+import java.util.*;
+import javax.annotation.processing.*;
+import javax.lang.model.*;
+import javax.lang.model.element.*;
+
+public class AnnoProc extends AbstractProcessor {
+    public boolean process(Set<? extends TypeElement> elems, RoundEnvironment renv) {
+        return true;
+    }
+
+    @Override
+    public Set<String> getSupportedOptions() {
+        IdentityHashMap<String, Integer> temp = new IdentityHashMap<>();
+        temp.put(new String("foo"), 1);
+        temp.put(new String("foo"), 2);
+        // Return a set with two copies of the option name "foo".
+        return temp.keySet();
+    }
+
+    @Override
+    public Set<String> getSupportedAnnotationTypes() {
+        IdentityHashMap<String, Integer> temp = new IdentityHashMap<>();
+        temp.put(new String("java.lang.SupressWarnings"), 1);
+        temp.put(new String("java.lang.SupressWarnings"), 2);
+        // Return a set with two copies of the annotation type name.
+        return temp.keySet();
+    }
+
+    @Override
+    public SourceVersion getSupportedSourceVersion() {
+        return SourceVersion.latest();
+    }
+}
--- a/test/langtools/tools/javac/diags/examples/MalformedSupported/MalformedSupported.java	Thu May 23 18:31:38 2019 -0700
+++ b/test/langtools/tools/javac/diags/examples/MalformedSupported/MalformedSupported.java	Thu May 23 18:47:24 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2019, 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
@@ -22,6 +22,6 @@
  */
 
 // key: compiler.warn.proc.malformed.supported.string
-// options: -processor AnnoProc
+// options: -processor AnnoProc -Xlint:processing
 
 class MalformedSupported { }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/RedundantTypesWithWildcardProc/RedundantTypesWithWildcardProc.java	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2010, 2019, 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.warn.proc.redundant.types.with.wildcard
+// options: -processor AnnoProc -Xlint:processing
+
+class RedundantTypesWithWildcardProc { }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/RedundantTypesWithWildcardProc/processors/AnnoProc.java	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2010, 2019, 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.
+ */
+
+import java.util.*;
+import javax.annotation.processing.*;
+import javax.lang.model.*;
+import javax.lang.model.element.*;
+
+@SupportedAnnotationTypes({"java.lang.SuppressWarnings", "*"})
+public class AnnoProc extends AbstractProcessor {
+    public boolean process(Set<? extends TypeElement> elems, RoundEnvironment renv) {
+        return true;
+    }
+
+    @Override
+    public SourceVersion getSupportedSourceVersion() {
+        return SourceVersion.latest();
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/processing/warnings/TestRepeatedItemsRuntime.java	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,177 @@
+ /*
+ * Copyright (c) 2006, 2019, 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
+ * @bug 8224177
+ * @summary Test warnings from the annotation processing runtime about malformed supported information from processors.
+ * @compile TestRepeatedItemsRuntime.java
+ * @compile/ref=gold_sv_none.out  -XDrawDiagnostics -processor TestRepeatedItemsRuntime -proc:only TestRepeatedItemsRuntime.java
+ * @compile/ref=auric_current.out -XDrawDiagnostics -processor TestRepeatedItemsRuntime -proc:only -Xlint:processing TestRepeatedItemsRuntime.java
+ */
+
+import java.lang.annotation.*;
+import java.util.*;
+import javax.annotation.processing.*;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.TypeElement;
+
+/**
+ * A warning should be issued by the logic in
+ * javax.annotation.processing.AbstractProcessor for the repeated
+ * information.  The "Foo" option warnings occur regardless of source
+ * level. The number of times the Baz annotation type is repeated
+ * depends on whether or not the source level supports modules.
+ */
+@Quux
+public class TestRepeatedItemsRuntime extends AbstractProcessor {
+
+    @Override
+    public SourceVersion getSupportedSourceVersion() {
+        return SourceVersion.latest();
+    }
+
+    @Override
+    public Set<String>getSupportedOptions() {
+        IdentityHashMap<String, Integer> temp = new IdentityHashMap<>();
+        // Use String constructor for identity map.
+        temp.put(new String("foo"), 1);
+        temp.put(new String("foo"), 2);
+
+        var returnValue = temp.keySet();
+        assert returnValue.size() == 2;
+        return returnValue;
+    }
+
+    /**
+     * Partial implementation of the Set interface with identity
+     * semantics and predictable iteration order.
+     *
+     * The javax.annotation.processing.Processor protocol relies on
+     * the iterator.
+     */
+    private static class ArrayBackedSet implements Set<String> {
+        private static String[] data = {"Quux",
+                                        "Quux",
+                                        "&&&/foo.Bar",
+                                        "foo.Bar",
+                                        "foo.Bar",
+                                        "quux/Quux",
+                                        "*"};
+        public ArrayBackedSet() {}
+
+        // Return an iterator of known iteration order so the set warning messages will be predictable.
+        @Override
+        public Iterator<String> iterator() {
+            return Arrays.asList(data).iterator();
+        }
+
+        @Override
+        public boolean add(String e) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean addAll(Collection<? extends String> c) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public void clear() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean contains(Object o){
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean containsAll(Collection<?> c) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            return o == this;
+        }
+
+        @Override
+        public int hashCode() {
+            int hash = 0;
+            for (String s : data) {
+                hash += s.hashCode();
+            }
+            return hash;
+        }
+
+        @Override
+        public boolean isEmpty() {
+            return data.length > 0;
+        }
+
+        @Override
+        public boolean remove(Object o) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean removeAll(Collection<?> c) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean retainAll(Collection<?> c) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public int size() {
+            return data.length;
+        }
+
+        @Override
+        public Object[] toArray() {
+            return data.clone();
+        }
+
+        @Override
+        public <T> T[] toArray(T[] a) {
+            throw new UnsupportedOperationException();
+        }
+    }
+
+    @Override
+    public Set<String>getSupportedAnnotationTypes() {
+        return new ArrayBackedSet();
+    }
+
+    public boolean process(Set<? extends TypeElement> annotations,
+                           RoundEnvironment roundEnvironment) {
+        return true;
+    }
+}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface Quux {
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/processing/warnings/auric_current.out	Thu May 23 18:47:24 2019 -0700
@@ -0,0 +1,6 @@
+- compiler.warn.proc.duplicate.supported.annotation: Quux, TestRepeatedItemsRuntime
+- compiler.warn.proc.malformed.supported.string: &&&/foo.Bar, TestRepeatedItemsRuntime
+- compiler.warn.proc.duplicate.supported.annotation: foo.Bar, TestRepeatedItemsRuntime
+- compiler.warn.proc.redundant.types.with.wildcard: TestRepeatedItemsRuntime
+- compiler.warn.proc.duplicate.option.name: foo, TestRepeatedItemsRuntime
+5 warnings