Skip to content

Commit

Permalink
[SPARK-3873][BUILD] Add style checker to enforce import ordering.
Browse files Browse the repository at this point in the history
The checker tries to follow as closely as possible the guidelines of
the code style document, and makes some decisions where the guide is
not clear. In particular:

- wildcard imports come first when there are other imports in the
  same package
- multi-import blocks come before single imports
- lower-case names inside multi-import blocks come before others

In some projects, such as graphx, there seems to be a convention to
separate o.a.s imports from the project's own; to simplify the
checker, I chose not to allow that, which is a strict interpretation
of the code style guide, even though I think it makes sense.

Since the checks are based on syntax only, some edge cases may
generate spurious warnings; for example, when class names start
with a lower case letter (and are thus treated as a package name
by the checker).

The checker is currently only generating warnings, and since there
are many of those, the build output does get a little noisy. The
idea is to fix the code (and the checker, as needed) little by little
instead of having a huge change that touches everywhere.

Author: Marcelo Vanzin <[email protected]>

Closes #6502 from vanzin/SPARK-3873.
  • Loading branch information
Marcelo Vanzin committed Dec 8, 2015
1 parent 06746b3 commit 2ff17bc
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion scalastyle-config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ This file is divided into 3 sections:
<check customId="visiblefortesting" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
<parameters><parameter name="regex">@VisibleForTesting</parameter></parameters>
<customMessage><![CDATA[
@VisibleForTesting auses classpath issues. Please note this in the java doc instead (SPARK-11615).
@VisibleForTesting causes classpath issues. Please note this in the java doc instead (SPARK-11615).
]]></customMessage>
</check>

Expand Down Expand Up @@ -203,6 +203,18 @@ This file is divided into 3 sections:
<!-- Should turn this on, but we have a few places that need to be fixed first -->
<check level="error" class="org.scalastyle.scalariform.EqualsHashCodeChecker" enabled="false"></check>

<!-- Import ordering. Currently warning only since there are lots of violations. -->
<check level="warning" class="org.scalastyle.scalariform.ImportOrderChecker" enabled="true">
<parameters>
<parameter name="groups">java,scala,3rdParty,spark</parameter>
<parameter name="group.java">javax?\..+</parameter>
<parameter name="group.scala">scala\..+</parameter>
<parameter name="group.3rdParty">(?!org\.apache\.spark\.).*</parameter>
<parameter name="group.spark">org\.apache\.spark\..*</parameter>
</parameters>
</check>


<!-- ================================================================================ -->
<!-- rules we don't want -->
<!-- ================================================================================ -->
Expand Down

0 comments on commit 2ff17bc

Please sign in to comment.