ARROW-1304: [Java] Fix Indentation, WhitespaceAround and EmptyLineSeparator checkstyle warnings in Java

[icexelloss]

No description provided.

[icexelloss]

This is auto formatted with intellij + google_checks.xml.

I manually eyeballed the changes and it looks good. @wesm do you know someone that can help me review this change?

[icexelloss]

I also reverted all the changes to templates/*.java because intellij auto format doesn't work well with those. I need to fix them manually or use some other tool. Consider that this is a big PR that changes a lot of files, I think it's easier if we try to merge this one and address templates as follow up.

[wesm]

cc @siddharthteotia

Are these changes in line with the checkstyle checks? I seem to recall a prior reformat patch, and I'm curious since so many imports are moving around

[icexelloss]

@wesm , good point. I reverted import changes. I might need to fix them by hand. Luckily there are not too many warning for imports. I am looking into fixing import warnings better.

[icexelloss]

To provide more details, I compared the warning of checkstyle before and after this patch. It turns out this patch fixes identation , whitespace and EmptyLineSeparator warning and doesn't fix the rest. I think fixing all checkstyle warning is going to be more work (unless there is some magic that I missed). How about we merge this change to fix indentation , whitespace and EmptyLineSeparator for now?

For referrence:
Before:

     40 [AbbreviationAsWordInName]
      2 [ArrayTypeStyle]
     26 [CatchParameterName]
      6 [CommentsIndentation]
    376 [CustomImportOrder]
    119 [EmptyLineSeparator]
      5 [FallThrough]
    441 [Indentation]
    111 [JavadocMethod]
     43 [JavadocParagraph]
      8 [JavadocTagContinuationIndentation]
     17 [LeftCurly]
    279 [LineLength]
     35 [LocalVariableName]
      5 [MemberName]
      3 [MethodName]
      2 [MethodParamPad]
      4 [MissingSwitchDefault]
     15 [ModifierOrder]
      2 [MultipleVariableDeclarations]
     34 [NeedBraces]
      2 [OneStatementPerLine]
     47 [OperatorWrap]
     25 [OverloadMethodsDeclarationOrder]
     58 [ParameterName]
     10 [RightCurly]
      4 [SeparatorWrap]
      1 [SingleLineJavadoc]
     60 [SummaryJavadoc]
      9 [UpperEll]
      7 [VariableDeclarationUsageDistance]
    123 [WhitespaceAround]

After:

     40 [AbbreviationAsWordInName]
      2 [ArrayTypeStyle]
     26 [CatchParameterName]
      3 [CommentsIndentation]
    376 [CustomImportOrder]
      5 [FallThrough]
      3 [Indentation]
    111 [JavadocMethod]
     43 [JavadocParagraph]
      8 [JavadocTagContinuationIndentation]
    278 [LineLength]
     35 [LocalVariableName]
      5 [MemberName]
      3 [MethodName]
      2 [MethodParamPad]
      4 [MissingSwitchDefault]
     15 [ModifierOrder]
      2 [MultipleVariableDeclarations]
     47 [OperatorWrap]
     25 [OverloadMethodsDeclarationOrder]
     58 [ParameterName]
      4 [SeparatorWrap]
     60 [SummaryJavadoc]
      9 [UpperEll]
      7 [VariableDeclarationUsageDistance]
      2 [WhitespaceAround]

[siddharthteotia]

Are we dependent on people using the same IDE with same formatting settings?

For example, in my PR #925 even though I didn't touch many lines of code, formatting was still disturbed and different than what is present in the code-line.

Can this happen in future? Like, if someone uses a completely different IDE with different formatting, are we again going to run into this problem again?

Can we have some generic recommendations/best-practices for people to follow/impose in their respective IDEs before creating PRs?

[icexelloss]

@siddharthteotia The formatting changes you saw in #925 are because of incorrect whitespace/indentation. It doesn't matter which ide you are using, it will all cause whitespace/indentation issue. The PR fixes those.

Can this happen in future? Like, if someone uses a completely different IDE with different formatting, are we again going to run into this problem again?

Not with indentation/whitespace. Those are pretty standardized with IDEs. However, we should maybe standardize/publish formatter configs for IDEs in the repo.

For as I can tell, this patch is not dependent on Intellij format, since I imported google_checks.xml into intellij instead of using the default formatter.

[siddharthteotia]

I don't think we should have settings like:

(typecast) variable_name instead of (typecast)variable_name. The latter is preferable

Similarly, we should have for() instead of for ().

@StevenMPhillips , any thoughts?

[icexelloss]

typecast followed by whitespace is not defined the google_checks. I am leaning towards having white space after cast because it's the default behavior of eclipse and intellij, but if there is already established style guide in Arrow, I am happy to follow.

whitespace after for is defined in google_checks.xml, so we shouldn't violent it.

[icexelloss]

Ping @StevenMPhillips

[wesm]

This LGTM. @icexelloss can you resolve the rebase conflict?

[icexelloss]

@wesm, I resolved the conflict.

[wesm]

Thanks. I'm going to hold off on merging this until #898 is in

[wesm]

+1