JDK 16 RFR of JDK-8251921: Expand default constructor warning to cover more cases

[Jonathan Gibbons]

There's a possible minor typo:

146 public static class StaticBaryNest {

The extra "y" looks incorrect.

Otherwise OK,

-- Jon

On 8/25/20 4:31 PM, Joe Darcy wrote:
> Hello,
>
> As a follow-up to JDK-8071961: "Add javac lint warning when a default 
> constructor is created," Phil observed that there are cases in the JDK 
> where a *protected* class has a default constructor appearing in the 
> JDK. Please review the changes and CSR to augment the warning to cover 
> both public and protected classes. For nested classes, the enclosing 
> types must be either public or protected all the way up for the 
> warning to be issued.
>
>     JDK-8251921: Expand default constructor warning to cover more cases
>     CSR: https://bugs.openjdk.java.net/browse/JDK-8252348
>     webrev: http://cr.openjdk.java.net/~darcy/8251921.0/
>
> Patch below. Clean langtools test run with this change.
>
> Thanks,
>
> -Joe
>
> --- 
> old/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java 
> 2020-08-25 15:04:59.413000000 -0700
> +++ 
> new/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java 
> 2020-08-25 15:04:58.685000000 -0700
> @@ -3835,7 +3835,7 @@
>          if (lint.isEnabled(LintCategory.MISSING_EXPLICIT_CTOR) &&
>              ((c.flags() & (ENUM | RECORD)) == 0) &&
>              !c.isAnonymous() &&
> -            ((c.flags() & PUBLIC) != 0) &&
> +            ((c.flags() & (PUBLIC | PROTECTED)) != 0) &&
>              Feature.MODULES.allowedInSource(source)) {
>              NestingKind nestingKind = c.getNestingKind();
>              switch (nestingKind) {
> @@ -3844,10 +3844,10 @@
>                  case TOP_LEVEL -> {;} // No additional checks needed
>                  case MEMBER -> {
>                      // For nested member classes, all the enclosing
> -                    // classes must be public.
> +                    // classes must be public or protected.
>                      Symbol owner = c.owner;
>                      while (owner != null && owner.kind == TYP) {
> -                        if ((owner.flags() & PUBLIC) == 0)
> +                        if ((owner.flags() & (PUBLIC | PROTECTED)) == 0)
>                              return;
>                          owner = owner.owner;
>                      }
> --- 
> old/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties 
> 2020-08-25 15:05:01.037000000 -0700
> +++ 
> new/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties 
> 2020-08-25 15:05:00.189000000 -0700
> @@ -183,7 +183,7 @@
>      Warn about issues related to classfile contents.
>
>  javac.opt.Xlint.desc.missing-explicit-ctor=\
> -    Warn about missing explicit constructors in public classes in 
> exported packages.
> +    Warn about missing explicit constructors in public and protected 
> classes in exported packages.
>
>  javac.opt.Xlint.desc.deprecation=\
>      Warn about use of deprecated items.
> --- 
> old/test/langtools/tools/javac/warnings/DefaultCtor/DefaultCtorWarningToolBox.java 
> 2020-08-25 15:05:02.313000000 -0700
> +++ 
> new/test/langtools/tools/javac/warnings/DefaultCtor/DefaultCtorWarningToolBox.java 
> 2020-08-25 15:05:01.597000000 -0700
> @@ -97,7 +97,9 @@
>              List.of("Foo.java:4:8: 
> compiler.warn.missing-explicit-ctor: pkg1.Foo, pkg1, mod",
>                      "Foo.java:12:12: 
> compiler.warn.missing-explicit-ctor: pkg1.Foo.FooNest, pkg1, mod",
>                      "Foo.java:16:19: 
> compiler.warn.missing-explicit-ctor: pkg1.Foo.StaticFooNest, pkg1, mod",
> -                    "3 warnings");
> +            "Foo.java:25:15: compiler.warn.missing-explicit-ctor: 
> pkg1.Foo.ProtectedFooNest, pkg1, mod",
> +            "Foo.java:27:19: compiler.warn.missing-explicit-ctor: 
> pkg1.Foo.ProtectedFooNest.ProtectedFooNestNest, pkg1, mod",
> +                    "5 warnings");
>
>          // Warning enable,
>          log = new JavacTask(tb)
> @@ -137,30 +139,34 @@
>          class Bar {
>
>              // No explicit constructor; use a default.
> -            public class FooNest {
> +            public class BarNest {
>              }
>
>              // No explicit constructor; use a default.
> -            public static class StaticFooNest {
> +            public static class StaticBaryNest {
> +            }
> +
> +            // No explicit constructor; use a default.
> +            protected class ProtectedBarNest {
>              }
>
>              // Package-access classes
>
>              // No explicit constructor; use a default.
> -            /*package*/ class PkgFooNest {
> +            /*package*/ class PkgBarNest {
>              }
>
>              // No explicit constructor; use a default.
> -            /*package*/ static class PkgStaticFooNest {
> +            /*package*/ static class PkgStaticBarNest {
>              }
>              // Private classes
>
>              // No explicit constructor; use a default.
> -            private class PrvFooNest {
> +            private class PrvBarNest {
>              }
>
>              // No explicit constructor; use a default.
> -            private static class PrvStaticFooNest {
> +            private static class PrvStaticBarNest {
>              }
>          }
>          """;
> @@ -190,10 +196,18 @@
>              public static class SuppressedStaticFooNest {
>              }
>
> +            // No explicit constructor; use a default.
> +            protected class ProtectedFooNest {
> +                // No explicit constructor; use a default.
> +                protected class ProtectedFooNestNest {}
> +            }
> +
>              // Package-access classes
>
>              // No explicit constructor; use a default.
>              /*package*/ class PkgFooNest {
> +                // No explicit constructor; use a default.
> +                protected class PkgFooNestNest {}
>              }
>
>              // No explicit constructor; use a default.
> @@ -203,6 +217,8 @@
>
>              // No explicit constructor; use a default.
>              private class PrvFooNest {
> +                // No explicit constructor; use a default.
> +                protected class PrvFooNestNest {}
>              }
>
>              // No explicit constructor; use a default.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200827/6366c883/attachment.htm>