8174243: incorrect error message for nested service provider
authorvromero
Tue, 14 Feb 2017 15:45:17 -0800
changeset 43865 532c50fea155
parent 43864 fc233ac29255
child 43866 3195ea126044
8174243: incorrect error message for nested service provider Reviewed-by: jjg, jlahoda
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/ServiceImplNotPublic.java
langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/example/ServiceImpl.java
langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/example/SomeService.java
langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/module-info.java
langtools/test/tools/javac/modules/ProvidesTest.java
langtools/test/tools/javac/modules/WrongErrorMessageForNestedServiceProviderTest.java
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java	Tue Feb 14 16:18:38 2017 +0300
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java	Tue Feb 14 15:45:17 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2017, 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
@@ -79,6 +79,10 @@
      */
     boolean isNewClass = false;
 
+    /** Indicate if the type being visited is a service implementation
+     */
+    boolean visitingServiceImplementation = false;
+
     /** Are arguments to current function applications boxed into an array for varargs?
      */
     Resolve.MethodResolutionPhase pendingResolutionPhase = null;
@@ -127,6 +131,7 @@
         info.isAnonymousDiamond = isAnonymousDiamond;
         info.isNewClass = isNewClass;
         info.preferredTreeForDiagnostics = preferredTreeForDiagnostics;
+        info.visitingServiceImplementation = visitingServiceImplementation;
         return info;
     }
 
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Tue Feb 14 16:18:38 2017 +0300
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Tue Feb 14 15:45:17 2017 -0800
@@ -996,8 +996,18 @@
             }
             ListBuffer<ClassSymbol> impls = new ListBuffer<>();
             for (JCExpression implName : tree.implNames) {
-                Type it = attr.attribType(implName, env, syms.objectType);
+                Type it;
+                boolean prevVisitingServiceImplementation = env.info.visitingServiceImplementation;
+                try {
+                    env.info.visitingServiceImplementation = true;
+                    it = attr.attribType(implName, env, syms.objectType);
+                } finally {
+                    env.info.visitingServiceImplementation = prevVisitingServiceImplementation;
+                }
                 ClassSymbol impl = (ClassSymbol) it.tsym;
+                if ((impl.flags_field & PUBLIC) == 0) {
+                    log.error(implName.pos(), Errors.NotDefPublic(impl, impl.location()));
+                }
                 //find provider factory:
                 MethodSymbol factory = factoryMethod(impl);
                 if (factory != null) {
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java	Tue Feb 14 16:18:38 2017 +0300
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java	Tue Feb 14 15:45:17 2017 -0800
@@ -307,6 +307,11 @@
         if (env.enclMethod != null && (env.enclMethod.mods.flags & ANONCONSTR) != 0)
             return true;
 
+        if (env.info.visitingServiceImplementation &&
+            env.toplevel.modle == c.packge().modle) {
+            return true;
+        }
+
         boolean isAccessible = false;
         switch ((short)(c.flags() & AccessFlags)) {
             case PRIVATE:
@@ -389,6 +394,11 @@
         if (env.enclMethod != null && (env.enclMethod.mods.flags & ANONCONSTR) != 0)
             return true;
 
+        if (env.info.visitingServiceImplementation &&
+            env.toplevel.modle == sym.packge().modle) {
+            return true;
+        }
+
         switch ((short)(sym.flags() & AccessFlags)) {
         case PRIVATE:
             return
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Tue Feb 14 16:18:38 2017 +0300
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Tue Feb 14 15:45:17 2017 -0800
@@ -915,6 +915,10 @@
     {0} is not public in {1}; cannot be accessed from outside package
 
 # 0: symbol, 1: symbol
+compiler.err.not.def.public=\
+    {0} is not public in {1}
+
+# 0: symbol, 1: symbol
 compiler.misc.not.def.public.cant.access=\
     {0} is not public in {1}; cannot be accessed from outside package
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/ServiceImplNotPublic.java	Tue Feb 14 15:45:17 2017 -0800
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2017, 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.not.def.public
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/example/ServiceImpl.java	Tue Feb 14 15:45:17 2017 -0800
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+package example;
+class ServiceImpl implements example.SomeService {
+    public ServiceImpl() {}
+    public void foo() {}
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/example/SomeService.java	Tue Feb 14 15:45:17 2017 -0800
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+package example;
+public interface SomeService {
+    public void foo();
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/ServiceImplNotPublic/module-info.java	Tue Feb 14 15:45:17 2017 -0800
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+module m {
+    exports example;
+    provides example.SomeService with example.ServiceImpl;
+}
--- a/langtools/test/tools/javac/modules/ProvidesTest.java	Tue Feb 14 16:18:38 2017 +0300
+++ b/langtools/test/tools/javac/modules/ProvidesTest.java	Tue Feb 14 15:45:17 2017 -0800
@@ -377,7 +377,7 @@
                 .writeAll()
                 .getOutputLines(Task.OutputKind.DIRECT);
 
-        List<String> expected = Arrays.asList("module-info.java:1:34: compiler.err.not.def.public.cant.access: p2.C2, p2",
+        List<String> expected = Arrays.asList("module-info.java:1:34: compiler.err.not.def.public: p2.C2, p2",
                 "1 error");
         if (!output.containsAll(expected)) {
             throw new Exception("Expected output not found");
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/modules/WrongErrorMessageForNestedServiceProviderTest.java	Tue Feb 14 15:45:17 2017 -0800
@@ -0,0 +1,181 @@
+/*
+ * Copyright (c) 2017, 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 8174243
+ * @summary incorrect error message for nested service provider
+ * @library /tools/lib
+ * @modules
+ *      jdk.compiler/com.sun.tools.javac.api
+ *      jdk.compiler/com.sun.tools.javac.main
+ * @build toolbox.ToolBox toolbox.JavacTask ModuleTestBase
+ * @run main WrongErrorMessageForNestedServiceProviderTest
+ */
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+
+import toolbox.JavacTask;
+import toolbox.Task;
+import toolbox.ToolBox;
+
+public class WrongErrorMessageForNestedServiceProviderTest extends ModuleTestBase {
+    public static void main(String... args) throws Exception {
+        WrongErrorMessageForNestedServiceProviderTest t = new WrongErrorMessageForNestedServiceProviderTest();
+        t.runTests();
+    }
+
+    private static final String twoServicesModuleDef =
+            "module m {\n" +
+            "    exports example;\n" +
+            "    provides example.SomeService with example.ServiceImpl;\n" +
+            "    provides example.SomeServiceOuter with example.Outer.ServiceImplOuter;\n" +
+            "}";
+
+    private static final String someServiceInt =
+            "package example;\n" +
+            "public interface SomeService {\n" +
+            "    public void foo();\n" +
+            "}";
+
+    private static final String someServiceIntOuter =
+            "package example;\n" +
+            "public interface SomeServiceOuter {\n" +
+            "    public void foo();\n" +
+            "}";
+
+    @Test
+    public void testPositive(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                twoServicesModuleDef,
+                someServiceInt,
+                someServiceIntOuter,
+                "package example;\n" +
+                "public class ServiceImpl implements example.SomeService {\n" +
+                "    public ServiceImpl() {}\n" +
+                "    public void foo() {}\n" +
+                "}",
+
+                "package example;\n" +
+                "class Outer {\n" +
+                "    public static class ServiceImplOuter implements example.SomeServiceOuter {\n" +
+                "        public ServiceImplOuter() {}\n" +
+                "        public void foo() {}\n" +
+                "    }\n" +
+                "}");
+        Path classes = base.resolve("classes");
+        Files.createDirectories(classes);
+
+        List<String> output = new JavacTask(tb)
+                .outdir(classes)
+                .options("-Werror", "-XDrawDiagnostics")
+                .files(findJavaFiles(src))
+                .run(Task.Expect.SUCCESS)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+        List<String> expected = Arrays.asList("");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
+    }
+
+    @Test
+    public void testNegative(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                twoServicesModuleDef,
+                someServiceInt,
+                someServiceIntOuter,
+
+                "package example;\n" +
+                "class ServiceImpl implements example.SomeService {\n" +
+                "    public ServiceImpl() {}\n" +
+                "    public void foo() {}\n" +
+                "}",
+
+                "package example;\n" +
+                "class Outer {\n" +
+                "    static class ServiceImplOuter implements example.SomeServiceOuter {\n" +
+                "        public ServiceImplOuter() {}\n" +
+                "        public void foo() {}\n" +
+                "    }\n" +
+                "}");
+        Path classes = base.resolve("classes");
+        Files.createDirectories(classes);
+
+        List<String> output = new JavacTask(tb)
+                .outdir(classes)
+                .options("-Werror", "-XDrawDiagnostics")
+                .files(findJavaFiles(src))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+        List<String> expected = Arrays.asList(
+                "module-info.java:3:46: compiler.err.not.def.public: example.ServiceImpl, example",
+                "module-info.java:4:57: compiler.err.not.def.public: example.Outer.ServiceImplOuter, example.Outer",
+                "2 errors");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
+    }
+
+    @Test
+    public void testClassWrappedByPrivateClass(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                "module m {\n" +
+                "    exports example;\n" +
+                "    provides example.SomeServiceOuter with example.Outer1.Outer2.ServiceImplOuter;\n" +
+                "}",
+
+                someServiceIntOuter,
+
+                "package example;\n" +
+                "class Outer1 {\n" +
+                "    static private class Outer2 {\n" +
+                "        public static class ServiceImplOuter implements example.SomeServiceOuter {\n" +
+                "            public ServiceImplOuter() {}\n" +
+                "            public void foo() {}\n" +
+                "        }\n" +
+                "    }\n" +
+                "}");
+        Path classes = base.resolve("classes");
+        Files.createDirectories(classes);
+
+        List<String> output = new JavacTask(tb)
+                .outdir(classes)
+                .options("-Werror", "-XDrawDiagnostics")
+                .files(findJavaFiles(src))
+                .run(Task.Expect.SUCCESS)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+        List<String> expected = Arrays.asList("");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
+    }
+}