Skip to content
Snippets Groups Projects
Commit 2ff17bcf authored by Marcelo Vanzin's avatar Marcelo Vanzin
Browse files

[SPARK-3873][BUILD] Add style checker to enforce import ordering.

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 <vanzin@cloudera.com>

Closes #6502 from vanzin/SPARK-3873.
parent 06746b30
No related branches found
No related tags found
No related merge requests found
...@@ -153,7 +153,7 @@ This file is divided into 3 sections: ...@@ -153,7 +153,7 @@ This file is divided into 3 sections:
<check customId="visiblefortesting" level="error" class="org.scalastyle.file.RegexChecker" enabled="true"> <check customId="visiblefortesting" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
<parameters><parameter name="regex">@VisibleForTesting</parameter></parameters> <parameters><parameter name="regex">@VisibleForTesting</parameter></parameters>
<customMessage><![CDATA[ <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> ]]></customMessage>
</check> </check>
...@@ -203,6 +203,18 @@ This file is divided into 3 sections: ...@@ -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 --> <!-- 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> <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 --> <!-- rules we don't want -->
<!-- ================================================================================ --> <!-- ================================================================================ -->
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment