Qualified class name inside qualified instance creation expression

[Tagir Valeev]

Hello!

I filed an issue:
https://bugs.openjdk.java.net/browse/JDK-8218194
Thank you for looking into this.

With best regards,
Tagir Valeev

On Fri, Feb 1, 2019 at 10:01 PM B. Blaser <bsrbnd at gmail.com> wrote:
>
> Hi,
>
> On Thu, 31 Jan 2019 at 23:05, Alex Buckley <alex.buckley at oracle.com> wrote:
> >
> > On 1/31/2019 3:33 AM, Tagir Valeev wrote:
> > > Consider the following code:
> > >
> > > class Test {
> > >      class Inner {}
> > >
> > >      void test() {
> > >          new Test.Inner(); // compiles successfully
> > >          this.new Test.Inner(); // error
> > >      }
> > > }
> >
> > Both the unqualified class instance creation expression and the
> > qualified class instance creation expression should compile. Each has
> > `this` as the immediately enclosing instance.
> >
> > > javac reports:
> > > Test.java:6: error: '(' expected
> > >          this.new Test.Inner();
> > >                       ^
> > > 1 error
> > >
> > > I read JLS 15.9 [1]:
> > > ClassInstanceCreationExpression:
> > >      UnqualifiedClassInstanceCreationExpression
> > >      ExpressionName . UnqualifiedClassInstanceCreationExpression
> > >      Primary . UnqualifiedClassInstanceCreationExpression
> > > UnqualifiedClassInstanceCreationExpression:
> > >      new [TypeArguments] ClassOrInterfaceTypeToInstantiate (
> > > [ArgumentList] ) [ClassBody]
> > > ClassOrInterfaceTypeToInstantiate:
> > >      {Annotation} Identifier {. {Annotation} Identifier} [TypeArgumentsOrDiamond]
> > >
> > > Thus this.new Test.Inner() could be parsed as Primary .
> > > UnqualifiedClassInstanceCreationExpression where
> > > ClassOrInterfaceTypeToInstantiate could be parsed as Identifier .
> > > Identifier, so there should not be parse error.
> >
> > Correct. There has been shimmer in this part of the language. In Java SE
> > 7, `this.new Test.Inner()` was illegal; only `this.new SIMPLENAME()` was
> > allowed. (See
> > https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.9)
> > In Java SE 8, the grammar of `ClassInstanceCreationExpression` was
> > modified to support type annotations, and `this.new QUALIFIEDNAME()` was
> > allowed. In principle, there is no reason to disallow a qualified name
> > in a qualified CICE. The phrase `new Test.Inner()` is meaningful as an
> > unqualified CICE, and enriching the phrase to be a qualified CICE, with
> > an explicit immediately enclosing instance, doesn't make it
> > unmeaningful. I suspect javac is still expecting (modulo type arguments)
> > a simple name after `new`.
>
> Right, I note that the parser doesn't conform to the current JLS grammar:
>
> http://hg.openjdk.java.net/jdk/jdk/file/c53a3355dbb4/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java#l2272
>
> InnerCreator = [Annotations] Ident [TypeArguments] ClassCreatorRest
>
> It seems 'InnerCreator' is missing '{. [Annotations] Ident}'.
> Is there any open JBS issue for this?
>
> I tried a quick fix, here under, in 'JavacParser' and 'Attr' which
> solve the problem; any feedback is welcome.
> Langtools:tier1 is OK but I'm not sure all is right with type
> annotations, etc...
> I might dig a bit more and send out a RFR soon but please let me know
> the priority of this before investing too much time now?
>
> Thanks,
> Bernard
>
>
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
> @@ -2311,7 +2311,9 @@
>              // TODO 308: in <expr>.new C, do we also want to add the
> type annotations
>              // from expr to the combined type, or not? Yes, do this.
>              clazzid1 = make.at(clazz.pos).Select(make.Type(encltype),
> -                                                 ((JCIdent) clazzid).name);
> +                    clazzid.hasTag(SELECT) ?
> +                    ((JCFieldAccess) clazzid).name :
> +                    ((JCIdent) clazzid).name);
>
>              EndPosTable endPosTable = this.env.toplevel.endPositions;
>              endPosTable.storeEnd(clazzid1, tree.getEndPosition(endPosTable));
> @@ -2351,7 +2353,16 @@
>
>          clazztype = chk.checkDiamond(tree, clazztype);
>          chk.validate(clazz, localEnv);
> -        if (tree.encl != null) {
> +        if (tree.encl != null && clazzid.hasTag(SELECT)) {
> +            try {
> +                Type qualified = attribType(tree.clazz, env);
> +                if (!types.isSameType(clazztype, qualified))
> +                    log.error(tree.pos(),
> Errors.IncomparableTypes(clazztype, qualified));
> +            } finally {
> +                env.info.isNewClass = false;
> +            }
> +        }
> +        else if (tree.encl != null) {
>              // We have to work in this case to store
>              // symbol + type back into the attributed tree.
>              tree.clazz.type = clazztype;
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 1999, 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
> @@ -2269,7 +2269,7 @@
>          }
>      }
>
> -    /** InnerCreator = [Annotations] Ident [TypeArguments] ClassCreatorRest
> +    /** InnerCreator = [Annotations] Ident {. [Annotations] Ident}
> [TypeArguments] ClassCreatorRest
>       */
>      JCExpression innerCreator(int newpos, List<JCExpression>
> typeArgs, JCExpression encl) {
>          List<JCAnnotation> newAnnotations = typeAnnotationsOpt();
> @@ -2280,6 +2280,15 @@
>              t =
> toP(F.at(newAnnotations.head.pos).AnnotatedType(newAnnotations, t));
>          }
>
> +        while (token.kind == DOT) {
> +            nextToken();
> +            newAnnotations = typeAnnotationsOpt();
> +            t = toP(F.at(token.pos).Select(t, ident()));
> +            if (newAnnotations.nonEmpty()) {
> +                t =
> toP(F.at(newAnnotations.head.pos).AnnotatedType(newAnnotations, t));
> +            }
> +        }
> +
>          if (token.kind == LT) {
>              int oldmode = mode;
>              t = typeArguments(t, true);
> diff --git a/test/langtools/tools/javac/QualifiedAccess/QualifiedAccess_4.out
> b/test/langtools/tools/javac/QualifiedAccess/QualifiedAccess_4.out
> --- a/test/langtools/tools/javac/QualifiedAccess/QualifiedAccess_4.out
> +++ b/test/langtools/tools/javac/QualifiedAccess/QualifiedAccess_4.out
> @@ -1,2 +1,2 @@
> -QualifiedAccess_4.java:17:28: compiler.err.expected: '('
> +QualifiedAccess_4.java:17:19: compiler.err.cant.resolve.location:
> kindname.variable, x, , , (compiler.misc.location: kindname.class,
> CMain, null)
>  1 error