java: implement checkstyle

[zklapow]

Apologies for the enormous diff.

This PR adds checkstyle linting to the java codebase based on google's checkstyle rules (googles defaults were chosen because A) they are included with checkstyle B) they are widely used and thus have good IDE support and C) this was originally a youtube project)

The pom has been updated to fail the build on checkstyle violations, and all of the existing code has been updated to conform to checkstyle so that it will build.

This PR contains a huge number of whitespace changes (because these files were inconsistently using tabs/spaces) as well as a number of other minor refactorings (renaming single letter variables, moving oddly placed variable declarations) but no logic has been changed.

This should make working on and contributing to this project much easier.

[deepthi]

LGTM

[harshit-gangal]

It will be good to enable git pre commit checkstyle java linter plugin. Can you also add this as part of this PR?

[zklapow]

👍 definitely, just added a pre commit hook that looks for modified files in the java client and runs checkstyle:check for affected modules.

[harshit-gangal]

Looks Good.
Thanks for adding git hook.

[sougou]

Can you resolve the conflicts?

[zklapow]

just resolved the conflicts, should be good to go now.

[deepthi]

@zklapow there is now an error from the pre-commit hooks:

Not a git repository
To compare two paths outside a working tree:
usage: git diff [--no-index] <path> <path>

This is coming from the checkstyle hook (confirmed by removing it in my local workspace before trying to commit). Can you please look into and fix this?

[leoxlin]

I'm unable to repro @deepthi, can you give me the

• pwd
• Modify the hook to do set -ex b5973f5#diff-dbad1d8ff567a43f822895eed3ac8e6aR3

[zklapow]

@deepthi the above info would be very useful. I also can't repro this behavior, but would be happy to do some more digging if I had a better idea of what your setup is like.

[deepthi]

I was doing a `git commit -a -m "some commit message"` from vitess.io/vitess in the planetscale fork. There is a .git directory in there, and this directory is not a symlink to anywhere else. Once I do `set -ex`, I get more output: + cd java + modules=() ++ git diff --relative --name-only --cached '*.java' Not a git repository To compare two paths outside a working tree: usage: git diff [--no-index] <path> <path> + '[' 0 -eq 0 ']' + exit

…

On Wed, Feb 20, 2019 at 11:59 AM zklapow ***@***.***> wrote: @deepthi <https://github.com/deepthi> the above info would be very useful. I also can't repro this behavior, but would be happy to do some more digging if I had a better idea of what your setup is like. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4599 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXs16IR7fSzVo7_gQWYHc9Ea_R_cAs2ks5vPak4gaJpZM4anie0> .

[zklapow]

@deepthi can you try applying this patch to the git hook and see if that fixes things? I think my cd'ing might be overly naive and this should help:

diff --git a/misc/git/hooks/checkstyle b/misc/git/hooks/checkstyle
index 777dd5124..e603222c5 100755
--- a/misc/git/hooks/checkstyle
+++ b/misc/git/hooks/checkstyle
@@ -15,7 +15,8 @@ function get_module() {
   done
 }

-cd java;
+DIR=$(PWD)
+cd $DIR/java;

 modules=();

@@ -41,4 +42,4 @@ export MAVEN_OPTS="-client

 mvn -q -pl "$modules_arg" checkstyle:check;

-cd -;
+cd $DIR;

If that works for you I can PR that in. Sorry for the inconvenience!

[deepthi]

I had to try it slightly differently: $(pwd) instead of $(PWD) but it still errors out:

++ pwd
+ DIR=/home/deepthi/go/src/vitess.io/vitess
+ cd /home/deepthi/go/src/vitess.io/vitess/java
+ modules=()
++ git diff --relative --name-only --cached '*.java'
Not a git repository
To compare two paths outside a working tree:
usage: git diff [--no-index] <path> <path>
+ '[' 0 -eq 0 ']'
+ exit