JDK 10/11 RFR of JDK-8189146: Have use of "var" in 9 and earlier source versions issue a warning

[joe darcy]

Hi Maurizio,

On 1/9/2018 4:39 PM, Maurizio Cimadamore wrote:
> Thanks for the update. This is close, but compared to the existing 
> golden file, as you noticed, there are missing warnings, esp. when 
> 'var' is used as an array element type.
>
> I believe the issue is with this code:
>
> if (Feature.LOCAL_VARIABLE_TYPE_INFERENCE.allowedInSource(source) && 
> elemType.hasTag(IDENT)) {
> 2998             Name typeName = ((JCIdent)elemType).name;
> 2999             if (isRestrictedLocalVarTypeName(typeName, pos)) {
> 3000                 if (type.hasTag(TYPEARRAY)) {
> 3001                     //error - 'var' and arrays
> 3002                     reportSyntaxError(pos, "var.not.allowed.array");
> 3003                 } else {
> 3004                     startPos = TreeInfo.getStartPos(mods);
> 3005                     if (startPos == Position.NOPOS)
> 3006                         startPos = TreeInfo.getStartPos(type);
> 3007                     //implicit type
> 3008                     type = null;
> 3009                 }
> 3010             }
> 3011         }
>
>
> Here, if LVTI is disabled, the 'isRestrictedLocalVarTypeName' call 
> would not even take place, hence the missing warnings. I suggest to 
> shuffle as follows:
>
> if (elemType.hasTag(IDENT)) {
> 2998             Name typeName = ((JCIdent)elemType).name;
> 2999             if (isRestrictedLocalVarTypeName(typeName, pos)) {
> 3000                 if (type.hasTag(TYPEARRAY)) {
> 3001                     //error - 'var' and arrays
> 3002                     reportSyntaxError(pos, "var.not.allowed.array");
> 3003                 } else {
> 3004                     startPos = TreeInfo.getStartPos(mods);
> 3005                     if (startPos == Position.NOPOS)
> 3006                         startPos = TreeInfo.getStartPos(type);
> 3007                     //implicit type
> 3008                     type = null;
> 3009                 }
> 3010             }
> 3011         }

When I tried that out, there were more warnings; too many actually, 
there were double warnings issued in some cases:

/test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
     var x = null; //illegal
     ^
/test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
     var x = null; //illegal
         ^
and

/home/darcy/JDK/10/hg/test/langtools/tools/javac/lvti/ParserTest.java:49: 
warning: as of release 10, 'var' is a restricted local variable type and 
cannot be used for type declarations or as the element type of an array
         var x9 = null, y = null; //illegal
             ^
/home/darcy/JDK/10/hg/test/langtools/tools/javac/lvti/ParserTest.java:49: 
warning: as of release 10, 'var' is a restricted local variable type and 
cannot be used for type declarations or as the element type of an array
         var x9 = null, y = null; //illegal
                        ^

Stepping back a bit, if "var" were added as a full new keyword then the 
check for a use of "var" could occur within the ident() method, as is 
done for underscore. However, since "var" is allowed its traditional use 
in some contexts, the error/warning checks need to be dependent on the 
syntactic context. For pre-10, only a syntactic check needs occur, e.g. 
a construct like "var x = 5, y = 6" does not merit a warning about y 
having a var type; although that check is needed in 10 and later. 
Therefore I moved a check out to the method for the containing grammar 
production:

     http://cr.openjdk.java.net/~darcy/8189146.3/

I also changed the wording of the warning message slightly to be more 
inclusive of the situations it was covering. (More precisely worded 
warning could be given for different situations, but I don't think that 
is strictly necessary.)

One "var" warning/error check occurs now in typeName, which catches 
class/interface/enum/annotation/type var declaration usage, and needs to 
occur in some way for at least certain idents occurring in variable 
declarations, including in try-with-resources.

I made some updates to the ParserTest to better cover try-with-resources 
and took a somewhat different tack to attempt to get the right set of 
warnings issues. Unfortunately, there were duplicates here as well:

/test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
     var x = null; //illegal
     ^
/test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
     var x = null; //illegal
     ^

/test/langtools/tools/javac/lvti/ParserTest.java:50: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
         final @DA var x10 = m(); //ok
                   ^
/test/langtools/tools/javac/lvti/ParserTest.java:50: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
         final @DA var x10 = m(); //ok
                   ^
/test/langtools/tools/javac/lvti/ParserTest.java:51: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
         @DA final var x11 = m(); //ok
                   ^
/test/langtools/tools/javac/lvti/ParserTest.java:51: warning: as of 
release 10, 'var' is a restricted local variable type and cannot be used 
for type declarations or as the element type of an array
         @DA final var x11 = m(); //ok

Hopefully any additional modifications needed will be minor!

Thanks,

-Joe