Fix checkstyle issue with redundant public keyword

[smarr]

The latest Eclipse Checkstyle plugin (version 6.14) flagged public and private keywords as redundant on inner classes that are declared for instance as private.

It also flagged redundant static keywords for enum, because they are implicitly static.

Note, this is a rather crosscutting change that touches quite a bit of code. This should probably be carefully considered before merging.

Since there are quite a few places also in Graal that have these issues, it might not be desirable to change it all at once (Eclipse has a quick fix feature, but it didn't work for me). Here the patch to mx, that worked for me (note, no 6.14.1 jar on lafo yet.

diff --git a/mx.mx/suite.py b/mx.mx/suite.py
index 528c414..8e3640b 100644
--- a/mx.mx/suite.py
+++ b/mx.mx/suite.py
@@ -59,15 +59,15 @@ suite = {

     "CHECKSTYLE" : {
       "urls" : [
-        "https://lafo.ssw.uni-linz.ac.at/pub/graal-external-deps/checkstyle-6.0-all.jar",
-        "jar:http://sourceforge.net/projects/checkstyle/files/checkstyle/6.0/checkstyle-6.0-bin.zip/download!/checkstyle-6.0/checkstyle-6.0-all.jar",
+        "https://lafo.ssw.uni-linz.ac.at/pub/graal-external-deps/checkstyle-6.14.1-all.jar",
+        "jar:http://sourceforge.net/projects/checkstyle/files/checkstyle/6.14.1/checkstyle-6.14.1-bin.zip/download!/checkstyle-6.14.1/checkstyle-6.14.1-all.jar",
       ],
-      "sha1" : "2bedc7feded58b5fd65595323bfaf7b9bb6a3c7a",
+      "sha1" : "8e2c3a2bcef100c084e6ea80cc426b3443632d8c",
       "licence" : "LGPLv21",
       "maven" : {
         "groupId" : "com.puppycrawl.tools",
         "artifactId" : "checkstyle",
-        "version" : "6.0",
+        "version" : "6.14.1",
       }
     },

[woess]

Looks good to me. We'll have to fix all projects, though. I think Manuel already did that for Graal.

[lukasstadler]

to be honest, I think having default modifiers in all kinds of places will take some getting used to.

[smarr]

Yeah, the other option would be disabling the rule, I guess.
On the other hand, I always wondered what a private class with a public constructor is supposed to mean.

Perhaps @jtulach has a more informed opinion. One of the considerations I found is about code evolution. So, it indicates more explicitly the visibility of the constructor when the class visibility is changed: http://stackoverflow.com/a/243276/916546

[jtulach]

I like the removal of public constructors from non-public classes. Having them public is misleading.

[mrigger]

I fixed these issues for Graal and JVMCI. We probably still have to clean up new violations before upgrading Checkstyle.