6945418: Project Coin: Simplified Varargs Method Invocation

Summary: Add new mandatory warning for unsafe vararg method declaration. Warning can be suppressed as usual (@SuppressWarnings("varargs")/-Xlint:-varargs)
Reviewed-by: jjg, darcy

src/share/classes/com/sun/tools/javac/code/Lint.java

@@ -203,7 +203,12 @@ public class Lint
         /**
          * Warn about issues relating to use of statics
          */
-        STATIC("static");
+        STATIC("static"),
+
+        /**
+         * Warn about potentially unsafe vararg methods
+         */
+        VARARGS("varargs");
 
         LintCategory(String option) {
             this(option, false);

src/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -657,6 +657,8 @@ public class Attr extends JCTree.Visitor {
                 attribStat(l.head, localEnv);
             }
 
+            chk.checkVarargMethodDecl(tree);
+
             // Check that type parameters are well-formed.
             chk.validate(tree.typarams, localEnv);
             if ((owner.flags() & ANNOTATION) != 0 &&
@@ -2634,10 +2636,11 @@ public class Attr extends JCTree.Visitor {
             }
             if (useVarargs) {
                 JCTree tree = env.tree;
+                Type argtype = owntype.getParameterTypes().last();
                 if (owntype.getReturnType().tag != FORALL || warned) {
-                    chk.checkVararg(env.tree.pos(), owntype.getParameterTypes());
+                    chk.checkVararg(env.tree.pos(), owntype.getParameterTypes(), sym, env);
                 }
-                Type elemtype = types.elemtype(owntype.getParameterTypes().last());
+                Type elemtype = types.elemtype(argtype);
                 switch (tree.getTag()) {
                 case JCTree.APPLY:
                     ((JCMethodInvocation) tree).varargsElement = elemtype;

src/share/classes/com/sun/tools/javac/comp/Check.java

@@ -106,6 +106,7 @@ public class Check {
 
         boolean verboseDeprecated = lint.isEnabled(LintCategory.DEPRECATION);
         boolean verboseUnchecked = lint.isEnabled(LintCategory.UNCHECKED);
+        boolean verboseVarargs = lint.isEnabled(LintCategory.VARARGS);
         boolean verboseSunApi = lint.isEnabled(LintCategory.SUNAPI);
         boolean enforceMandatoryWarnings = source.enforceMandatoryWarnings();
 
@@ -113,6 +114,8 @@ public class Check {
                 enforceMandatoryWarnings, "deprecated");
         uncheckedHandler = new MandatoryWarningHandler(log, verboseUnchecked,
                 enforceMandatoryWarnings, "unchecked");
+        unsafeVarargsHandler = new MandatoryWarningHandler(log, verboseVarargs,
+                enforceMandatoryWarnings, "varargs");
         sunApiHandler = new MandatoryWarningHandler(log, verboseSunApi,
                 enforceMandatoryWarnings, "sunapi");
     }
@@ -150,6 +153,10 @@ public class Check {
      */
     private MandatoryWarningHandler uncheckedHandler;
 
+    /** A handler for messages about unchecked or unsafe vararg method decl.
+     */
+    private MandatoryWarningHandler unsafeVarargsHandler;
+
     /** A handler for messages about using Sun proprietary API.
      */
     private MandatoryWarningHandler sunApiHandler;
@@ -182,6 +189,15 @@ public class Check {
             uncheckedHandler.report(pos, msg, args);
     }
 
+    /** Warn about unsafe vararg method decl.
+     *  @param pos        Position to be used for error reporting.
+     *  @param sym        The deprecated symbol.
+     */
+    void warnUnsafeVararg(DiagnosticPosition pos, Type elemType) {
+        if (!lint.isSuppressed(LintCategory.VARARGS))
+            unsafeVarargsHandler.report(pos, "varargs.non.reifiable.type", elemType);
+    }
+
     /** Warn about using Sun proprietary API.
      *  @param pos        Position to be used for error reporting.
      *  @param msg        A string describing the problem.
@@ -202,6 +218,7 @@ public class Check {
     public void reportDeferredDiagnostics() {
         deprecationHandler.reportDeferredDiagnostic();
         uncheckedHandler.reportDeferredDiagnostic();
+        unsafeVarargsHandler.reportDeferredDiagnostic();
         sunApiHandler.reportDeferredDiagnostic();
     }
 
@@ -680,17 +697,33 @@ public class Check {
         }
     }
 
+    void checkVarargMethodDecl(JCMethodDecl tree) {
+        MethodSymbol m = tree.sym;
+        //check the element type of the vararg
+        if (m.isVarArgs()) {
+            Type varargElemType = types.elemtype(tree.params.last().type);
+            if (!types.isReifiable(varargElemType)) {
+                warnUnsafeVararg(tree.params.head.pos(), varargElemType);
+            }
+        }
+    }
+
     /**
      * Check that vararg method call is sound
      * @param pos Position to be used for error reporting.
      * @param argtypes Actual arguments supplied to vararg method.
      */
-    void checkVararg(DiagnosticPosition pos, List<Type> argtypes) {
+    void checkVararg(DiagnosticPosition pos, List<Type> argtypes, Symbol msym, Env<AttrContext> env) {
+        Env<AttrContext> calleeLintEnv = env;
+        while (calleeLintEnv.info.lint == null)
+            calleeLintEnv = calleeLintEnv.next;
+        Lint calleeLint = calleeLintEnv.info.lint.augment(msym.attributes_field, msym.flags());
         Type argtype = argtypes.last();
-        if (!types.isReifiable(argtype))
+        if (!types.isReifiable(argtype) && !calleeLint.isSuppressed(Lint.LintCategory.VARARGS)) {
             warnUnchecked(pos,
                               "unchecked.generic.array.creation",
                               argtype);
+        }
     }
 
     /** Check that given modifiers are legal for given symbol and

src/share/classes/com/sun/tools/javac/comp/Infer.java

@@ -290,6 +290,7 @@ public class Infer {
     public Type instantiateMethod(final Env<AttrContext> env,
                                   List<Type> tvars,
                                   MethodType mt,
+                                  final Symbol msym,
                                   final List<Type> argtypes,
                                   final boolean allowBoxing,
                                   final boolean useVarargs,
@@ -418,7 +419,7 @@ public class Infer {
                     checkWithinBounds(all_tvars,
                            types.subst(inferredTypes, tvars, inferred), warn);
                     if (useVarargs) {
-                        chk.checkVararg(env.tree.pos(), formals);
+                        chk.checkVararg(env.tree.pos(), formals, msym, env);
                     }
                     return super.inst(inferred, types);
             }};

src/share/classes/com/sun/tools/javac/comp/Resolve.java

@@ -348,6 +348,7 @@ public class Resolve {
             infer.instantiateMethod(env,
                                     tvars,
                                     (MethodType)mt,
+                                    m,
                                     argtypes,
                                     allowBoxing,
                                     useVarargs,

src/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -593,6 +593,20 @@ compiler.note.unchecked.filename.additional=\
 compiler.note.unchecked.plural.additional=\
     Some input files additionally use unchecked or unsafe operations.
 
+compiler.note.varargs.filename=\
+    {0} declares unsafe vararg methods.
+compiler.note.varargs.plural=\
+    Some input files declare unsafe vararg methods.
+# The following string may appear after one of the above unsafe varargs
+# messages.
+compiler.note.varargs.recompile=\
+    Recompile with -Xlint:varargs for details.
+
+compiler.note.varargs.filename.additional=\
+    {0} declares additional unsafe vararg methods.
+compiler.note.varargs.plural.additional=\
+    Some input files additionally declares unsafe vararg methods.
+
 compiler.note.sunapi.filename=\
     {0} uses Sun proprietary API that may be removed in a future release.
 compiler.note.sunapi.plural=\
@@ -796,6 +810,9 @@ compiler.warn.unchecked.meth.invocation.applied=\
 compiler.warn.unchecked.generic.array.creation=\
     [unchecked] unchecked generic array creation for varargs parameter of type {0}
 
+compiler.warn.varargs.non.reifiable.type=\
+    [varargs] Possible heap pollution from parameterized vararg type {0}
+
 compiler.warn.missing.deprecated.annotation=\
     [dep-ann] deprecated item is not annotated with @Deprecated
 

src/share/classes/com/sun/tools/javac/util/List.java

@@ -103,6 +103,7 @@ public class List<A> extends AbstractCollection<A> implements java.util.List<A>
 
     /** Construct a list consisting of given elements.
      */
+    @SuppressWarnings("varargs")
     public static <A> List<A> of(A x1, A x2, A x3, A... rest) {
         return new List<A>(x1, new List<A>(x2, new List<A>(x3, from(rest))));
     }

test/tools/javac/varargs/6730476/T6730476a.java

@@ -27,7 +27,7 @@
  *
  * @summary invalid "unchecked generic array" warning
  * @author mcimadamore
- * @compile T6730476a.java -Xlint -Werror
+ * @compile T6730476a.java -Xlint:unchecked -Werror
  *
  */
 

test/tools/javac/varargs/6806876/T6806876.out

 T6806876.java:11:32: compiler.warn.unchecked.generic.array.creation: java.lang.Number&java.lang.Comparable<? extends java.lang.Number&java.lang.Comparable<?>>[]
 - compiler.err.warnings.and.werror
+- compiler.note.varargs.filename: T6806876.java
+- compiler.note.varargs.recompile
 1 error
 1 warning

test/tools/javac/varargs/warning/Warn4.java

+/*
+ * Copyright 2010 Sun Microsystems, Inc.  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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/**
+ * @test
+ * @bug     6945418
+ * @summary Project Coin: Simplified Varargs Method Invocation
+ * @author  mcimadamore
+ * @run main Warn4
+ */
+import com.sun.source.util.JavacTask;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.HashSet;
+import javax.tools.Diagnostic;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+
+public class Warn4 {
+
+    final static Warning[] error = null;
+    final static Warning[] none = new Warning[] {};
+    final static Warning[] vararg = new Warning[] { Warning.VARARGS };
+    final static Warning[] unchecked = new Warning[] { Warning.UNCHECKED };
+    final static Warning[] both = new Warning[] { Warning.VARARGS, Warning.UNCHECKED };
+
+    enum Warning {
+        UNCHECKED("unchecked"),
+        VARARGS("varargs");
+
+        String category;
+
+        Warning(String category) {
+            this.category = category;
+        }
+
+        boolean isEnabled(XlintOption xlint, SuppressLevel suppressLevel) {
+            return Arrays.asList(xlint.enabledWarnings).contains(this);
+        }
+
+        boolean isSuppressed(SuppressLevel suppressLevel) {
+            return Arrays.asList(suppressLevel.suppressedWarnings).contains(VARARGS);
+        }
+    }
+
+    enum XlintOption {
+        NONE(),
+        UNCHECKED(Warning.UNCHECKED),
+        VARARGS(Warning.VARARGS),
+        ALL(Warning.UNCHECKED, Warning.VARARGS);
+
+        Warning[] enabledWarnings;
+
+        XlintOption(Warning... enabledWarnings) {
+            this.enabledWarnings = enabledWarnings;
+        }
+
+        String getXlintOption() {
+            StringBuilder buf = new StringBuilder();
+            String sep = "";
+            for (Warning w : enabledWarnings) {
+                buf.append(sep);
+                buf.append(w.category);
+                sep=",";
+            }
+            return "-Xlint:" +
+                    (this == NONE ? "none" : buf.toString());
+        }
+    }
+
+    enum SuppressLevel {
+        NONE(),
+        UNCHECKED(Warning.UNCHECKED),
+        VARARGS(Warning.VARARGS),
+        ALL(Warning.UNCHECKED, Warning.VARARGS);
+
+        Warning[] suppressedWarnings;
+
+        SuppressLevel(Warning... suppressedWarnings) {
+            this.suppressedWarnings = suppressedWarnings;
+        }
+
+        String getSuppressAnnotation() {
+            StringBuilder buf = new StringBuilder();
+            String sep = "";
+            for (Warning w : suppressedWarnings) {
+                buf.append(sep);
+                buf.append("\"");
+                buf.append(w.category);
+                buf.append("\"");
+                sep=",";
+            }
+            return this == NONE ? "" :
+                "@SuppressWarnings({" + buf.toString() + "})";
+        }
+    }
+
+    enum Signature {
+
+        EXTENDS_TVAR("<Z> void #name(List<? extends Z>#arity arg) { #body }",
+            new Warning[][] {both, both, both, both, error, both, both, both, error}),
+        SUPER_TVAR("<Z> void #name(List<? super Z>#arity arg) { #body }",
+            new Warning[][] {error, both, error, both, error, error, both, both, error}),
+        UNBOUND("void #name(List<?>#arity arg) { #body }",
+            new Warning[][] {none, none, none, none, none, none, none, none, error}),
+        INVARIANT_TVAR("<Z> void #name(List<Z>#arity arg) { #body }",
+            new Warning[][] {both, both, both, both, error, both, both, both, error}),
+        TVAR("<Z> void #name(Z#arity arg) { #body }",
+            new Warning[][] {both, both, both, both, both, both, both, both, vararg}),
+        EXTENDS("void #name(List<? extends String>#arity arg) { #body }",
+            new Warning[][] {error, error, error, error, error, both, error, both, error}),
+        SUPER("void #name(List<? super String>#arity arg) { #body }",
+            new Warning[][] {error, error, error, error, error, error, both, both, error}),
+        INVARIANT("void #name(List<String>#arity arg) { #body }",
+            new Warning[][] {error, error, error, error, error, error, error, both, error}),
+        UNPARAMETERIZED("void #name(String#arity arg) { #body }",
+            new Warning[][] {error, error, error, error, error, error, error, error, none});
+
+        String template;
+        Warning[][] warnings;
+
+        Signature(String template, Warning[][] warnings) {
+            this.template = template;
+            this.warnings = warnings;
+        }
+
+        boolean isApplicableTo(Signature other) {
+            return warnings[other.ordinal()] != null;
+        }
+
+        boolean giveUnchecked(Signature other) {
+            return warnings[other.ordinal()] == unchecked ||
+                    warnings[other.ordinal()] == both;
+        }
+
+        boolean giveVarargs(Signature other) {
+            return warnings[other.ordinal()] == vararg ||
+                    warnings[other.ordinal()] == both;
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        for (XlintOption xlint : XlintOption.values()) {
+            for (SuppressLevel suppressLevel : SuppressLevel.values()) {
+                for (Signature vararg_meth : Signature.values()) {
+                    for (Signature client_meth : Signature.values()) {
+                        if (vararg_meth.isApplicableTo(client_meth)) {
+                            test(xlint,
+                                    suppressLevel,
+                                    vararg_meth,
+                                    client_meth);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    static void test(XlintOption xlint, SuppressLevel suppressLevel,
+            Signature vararg_meth, Signature client_meth) throws Exception {
+        final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+        JavaSource source = new JavaSource(suppressLevel, vararg_meth, client_meth);
+        DiagnosticChecker dc = new DiagnosticChecker();
+        JavacTask ct = (JavacTask)tool.getTask(null, null, dc,
+                Arrays.asList(xlint.getXlintOption()), null, Arrays.asList(source));
+        ct.generate(); //to get mandatory notes
+        check(dc.warnings,
+                dc.notes,
+                new boolean[] {vararg_meth.giveUnchecked(client_meth),
+                               vararg_meth.giveVarargs(client_meth)},
+                source, xlint, suppressLevel);
+    }
+
+    static void check(Set<Warning> warnings, Set<Warning> notes, boolean[] warnArr, JavaSource source, XlintOption xlint, SuppressLevel suppressLevel) {
+        boolean badOutput = false;
+        for (Warning wkind : Warning.values()) {
+            badOutput |= (warnArr[wkind.ordinal()] && !wkind.isSuppressed(suppressLevel)) !=
+                    (wkind.isEnabled(xlint, suppressLevel) ?
+                        warnings.contains(wkind) :
+                        notes.contains(wkind));
+        }
+        if (badOutput) {
+            throw new Error("invalid diagnostics for source:\n" +
+                    source.getCharContent(true) +
+                    "\nOptions: " + xlint.getXlintOption() +
+                    "\nExpected unchecked warning: " + warnArr[0] +
+                    "\nExpected unsafe vararg warning: " + warnArr[1] +
+                    "\nWarnings: " + warnings +
+                    "\nNotes: " + notes);
+        }
+    }
+
+    static class JavaSource extends SimpleJavaFileObject {
+
+        String source;
+
+        public JavaSource(SuppressLevel suppressLevel, Signature vararg_meth, Signature client_meth) {
+            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+            String meth1 = vararg_meth.template.replace("#arity", "...");
+            meth1 = meth1.replace("#name", "m");
+            meth1 = meth1.replace("#body", "");
+            meth1 = suppressLevel.getSuppressAnnotation() + meth1;
+            String meth2 = client_meth.template.replace("#arity", "");
+            meth2 = meth2.replace("#name", "test");
+            meth2 = meth2.replace("#body", "m(arg);");
+            source = "import java.util.List;\n" +
+               "class Test {\n" + meth1 +
+               "\n" + meth2 + "\n}\n";
+        }
+
+        @Override
+        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+            return source;
+        }
+    }
+
+    static class DiagnosticChecker implements javax.tools.DiagnosticListener<JavaFileObject> {
+
+        Set<Warning> warnings = new HashSet<>();
+        Set<Warning> notes = new HashSet<>();
+
+        public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+            if (diagnostic.getKind() == Diagnostic.Kind.MANDATORY_WARNING ||
+                    diagnostic.getKind() == Diagnostic.Kind.WARNING) {
+                warnings.add(diagnostic.getCode().contains("varargs") ?
+                    Warning.VARARGS :
+                    Warning.UNCHECKED);
+            }
+            else if (diagnostic.getKind() == Diagnostic.Kind.NOTE) {
+                notes.add(diagnostic.getCode().contains("varargs") ?
+                    Warning.VARARGS :
+                    Warning.UNCHECKED);
+            }
+        }
+    }
+}