8172807: Javac doesn't report errors on duplicate provides with different service implementations
authorjjg
Thu, 02 Feb 2017 14:34:21 -0800
changeset 43577 1a1b1242f7aa
parent 43576 bd55b00a4cb3
child 43578 640eb9781ce5
8172807: Javac doesn't report errors on duplicate provides with different service implementations Reviewed-by: jlahoda
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
langtools/src/jdk.jshell/share/classes/module-info.java
langtools/test/tools/javac/diags/examples/DuplicateProvides/module-info.java
langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/RepeatedProvides.java
langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/module-info.java
langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/A.java
langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/B.java
langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/I.java
langtools/test/tools/javac/modules/ProvidesTest.java
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Thu Feb 02 21:55:45 2017 +0000
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Thu Feb 02 14:34:21 2017 -0800
@@ -931,6 +931,9 @@
         public void visitProvides(JCProvides tree) {
             Type st = attr.attribType(tree.serviceName, env, syms.objectType);
             ClassSymbol service = (ClassSymbol) st.tsym;
+            if (allProvides.containsKey(service)) {
+                log.error(tree.serviceName.pos(), Errors.RepeatedProvidesForService(service));
+            }
             ListBuffer<ClassSymbol> impls = new ListBuffer<>();
             for (JCExpression implName : tree.implNames) {
                 Type it = attr.attribType(implName, env, syms.objectType);
@@ -959,8 +962,6 @@
                     }
                 }
                 if (it.hasTag(CLASS)) {
-                    // For now, we just check the pair (service-type, impl-type) is unique
-                    // TODO, check only one provides per service type as well
                     if (allProvides.computeIfAbsent(service, s -> new HashSet<>()).add(impl)) {
                         impls.append(impl);
                     } else {
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu Feb 02 21:55:45 2017 +0000
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Thu Feb 02 14:34:21 2017 -0800
@@ -2882,6 +2882,10 @@
 compiler.err.no.opens.unless.strong=\
     ''opens'' only allowed in strong modules
 
+# 0: symbol
+compiler.err.repeated.provides.for.service=\
+    multiple ''provides'' for service {0}
+
 # 0: symbol, 1: symbol
 compiler.err.duplicate.provides=\
     duplicate provides: service {0}, implementation {1}
--- a/langtools/src/jdk.jshell/share/classes/module-info.java	Thu Feb 02 21:55:45 2017 +0000
+++ b/langtools/src/jdk.jshell/share/classes/module-info.java	Thu Feb 02 14:34:21 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 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
@@ -70,11 +70,10 @@
     uses jdk.jshell.spi.ExecutionControlProvider;
     uses jdk.internal.editor.spi.BuildInEditorProvider;
 
-    provides javax.tools.Tool with jdk.internal.jshell.tool.JShellToolProvider;
-    provides jdk.jshell.spi.ExecutionControlProvider
-        with jdk.jshell.execution.JdiExecutionControlProvider;
+    provides javax.tools.Tool
+        with jdk.internal.jshell.tool.JShellToolProvider;
     provides jdk.jshell.spi.ExecutionControlProvider
-        with jdk.jshell.execution.LocalExecutionControlProvider;
-    provides jdk.jshell.spi.ExecutionControlProvider
-        with jdk.jshell.execution.FailOverExecutionControlProvider;
+        with jdk.jshell.execution.JdiExecutionControlProvider,
+             jdk.jshell.execution.LocalExecutionControlProvider,
+             jdk.jshell.execution.FailOverExecutionControlProvider;
 }
--- a/langtools/test/tools/javac/diags/examples/DuplicateProvides/module-info.java	Thu Feb 02 21:55:45 2017 +0000
+++ b/langtools/test/tools/javac/diags/examples/DuplicateProvides/module-info.java	Thu Feb 02 14:34:21 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 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
@@ -25,6 +25,6 @@
 
 module DuplicateExports {
      exports exported;
-     provides exported.Service with impl.ServiceImplementation;
-     provides exported.Service with impl.ServiceImplementation;
+     provides exported.Service
+        with impl.ServiceImplementation, impl.ServiceImplementation;
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/RepeatedProvides.java	Thu Feb 02 14:34:21 2017 -0800
@@ -0,0 +1,25 @@
+/*
+ * 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.repeated.provides.for.service
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/module-info.java	Thu Feb 02 14:34:21 2017 -0800
@@ -0,0 +1,6 @@
+module m {
+    uses p.I;
+    provides p.I with p.A;
+    provides p.I with p.B;
+}
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/A.java	Thu Feb 02 14:34:21 2017 -0800
@@ -0,0 +1,3 @@
+package p;
+public class A implements I { }
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/B.java	Thu Feb 02 14:34:21 2017 -0800
@@ -0,0 +1,3 @@
+package p;
+public class B implements I { }
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/RepeatedProvidesForService/modulesourcepath/m/p/I.java	Thu Feb 02 14:34:21 2017 -0800
@@ -0,0 +1,3 @@
+package p;
+public interface I { }
+
--- a/langtools/test/tools/javac/modules/ProvidesTest.java	Thu Feb 02 21:55:45 2017 +0000
+++ b/langtools/test/tools/javac/modules/ProvidesTest.java	Thu Feb 02 14:34:21 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 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
@@ -24,7 +24,7 @@
 /**
  * @test
  * @summary simple tests of module provides
- * @bug 8168854
+ * @bug 8168854 8172807
  * @library /tools/lib
  * @modules
  *      jdk.compiler/com.sun.tools.javac.api
@@ -109,21 +109,56 @@
     }
 
     @Test
-    public void testDuplicateProvides(Path base) throws Exception {
+    public void testDuplicateImplementations1(Path base) throws Exception {
         Path src = base.resolve("src");
         tb.writeJavaFiles(src,
-                "module m { provides p1.C1 with p2.C2; provides p1.C1 with p2.C2; }",
+                "module m { exports p1; exports p2; provides p1.C1 with p2.C2, p2.C2; }",
                 "package p1; public class C1 { }",
                 "package p2; public class C2 extends p1.C1 { }");
         Path classes = base.resolve("classes");
         Files.createDirectories(classes);
 
-        new JavacTask(tb)
-                .options("-XDrawDiagnostic")
+        List<String> output = new JavacTask(tb)
+                .options("-XDrawDiagnostics")
                 .outdir(classes)
                 .files(findJavaFiles(src))
                 .run(Task.Expect.FAIL)
-                .writeAll();
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList(
+                "module-info.java:1:65: compiler.err.duplicate.provides: p1.C1, p2.C2",
+                "1 error");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
+    }
+
+    @Test
+    public void testDuplicateImplementations2(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                "module m { exports p1; provides p1.C1 with p2.C2; provides p1.C1 with p2.C2; }",
+                "package p1; public class C1 { }",
+                "package p2; public class C2 extends p1.C1 { }");
+        Path classes = base.resolve("classes");
+        Files.createDirectories(classes);
+
+        List<String> output = new JavacTask(tb)
+                .options("-XDrawDiagnostics")
+                .outdir(classes)
+                .files(findJavaFiles(src))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList(
+                "module-info.java:1:62: compiler.err.repeated.provides.for.service: p1.C1",
+                "module-info.java:1:73: compiler.err.duplicate.provides: p1.C1, p2.C2",
+                "2 errors");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
     }
 
     @Test
@@ -228,7 +263,7 @@
     public void testSeveralImplementations(Path base) throws Exception {
         Path src = base.resolve("src");
         tb.writeJavaFiles(src,
-                "module m { provides p.C with p.Impl1; provides p.C with p.Impl2; }",
+                "module m { provides p.C with p.Impl1, p.Impl2; }",
                 "package p; public class C { }",
                 "package p; public class Impl1 extends p.C { }",
                 "package p; public class Impl2 extends p.C { }");
@@ -241,6 +276,30 @@
     }
 
     @Test
+    public void testRepeatedProvides(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                "module m { exports p; provides p.C with p.Impl1; provides p.C with p.Impl2; }",
+                "package p; public class C { }",
+                "package p; public class Impl1 extends p.C { }",
+                "package p; public class Impl2 extends p.C { }");
+
+        List<String> output = new JavacTask(tb)
+                .options("-XDrawDiagnostics")
+                .outdir(Files.createDirectories(base.resolve("classes")))
+                .files(findJavaFiles(src))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList("module-info.java:1:60: compiler.err.repeated.provides.for.service: p.C",
+                "1 error");
+        if (!output.containsAll(expected)) {
+            throw new Exception("Expected output not found");
+        }
+    }
+
+    @Test
     public void testOneImplementationsForServices(Path base) throws Exception {
         Path src = base.resolve("src");
         tb.writeJavaFiles(src,