[NEW BUG] jdk.internal.module.Checks seems to miss a check for 'var'

[Christoph Dreis]

I wonder if this should do the trick.

What do you think, Alan?

============ PATCH ============
diff -r 96c45aa61056 src/java.base/share/classes/jdk/internal/module/Checks.java
--- a/src/java.base/share/classes/jdk/internal/module/Checks.java       Fri Mar 22 13:42:45 2019 +0530
+++ b/src/java.base/share/classes/jdk/internal/module/Checks.java       Fri Mar 22 14:30:03 2019 +0100
@@ -111,6 +111,11 @@
         if (name.indexOf('.') == -1)
             throw new IllegalArgumentException(name + ": is not a qualified name of"
                                                + " a Java class in a named package");
+        String unqualifiedName = getUnqualifiedClassName(name);
+        if (!"var".equals(unqualifiedName)) {
+            throw new IllegalArgumentException(name + ": Invalid " + what
+                    + ": 'var' is not a valid Java class name");
+        }
         return name;
     }

@@ -118,7 +123,16 @@
      * Returns {@code true} if the given name is a legal class name.
      */
     public static boolean isClassName(String name) {
-        return isTypeName(name);
+        if (!isTypeName(name)) {
+            return false;
+        }
+        String unqualifiedName = getUnqualifiedClassName(name);
+        return !"var".equals(unqualifiedName);
+    }
+
+    private static String getUnqualifiedClassName(String name) {
+        int index = name.lastIndexOf('.');
+        return index != -1 ? name.substring(index) : name;
     }


> Thanks Alan for your fast response.
> 
> I'm "glad" the root issue remains even though the initial patch is wrong.
> 
> I guess "uses" could be affected here as well, not just "mainClass" and
> "provides".
> How do we move on from now? Can I help in anyway?
> 
> Christoph
> 
> > On 22/03/2019 11:30, Christoph Dreis wrote:
> > > Hi,
> > >
> > > I recently stumbled upon jdk.internal.module.Checks and was
> > > wondering if you could help me understanding if I face a bug. The
> > > mentioned class has a private static field containing a list of
> > > reserved keywords. When checking the list for completeness, I
> > > noticed that "var" seems to be missing here and I wonder if it should be
> added.
> > This class supports validation when building module descriptors with
> > the API, e.g.
> >
> >          var descriptor = ModuleDescriptor.newModule("var")
> >                  .exports("var", Set.of("var"))
> >                  .provides("var.var", List.of("var.var"))
> >                  .mainClass("var.var")
> >                  .build();
> >
> > It is also used when deriving module descriptors for automatic modules.
> >
> > So I think you found an issue as the "provides" and "mainClass"
> > builder methods should reject a class name named "var" (no issue with
> > the module or packages names which is why it shouldn't be added to the
> RESERVED set).
> >
> > -Alan