Skip to content

Commit d0d8222

Browse files
dskrvkJoshRosen
dskrvk
authored and
JoshRosen
committed
[SPARK-6990][BUILD] Add Java linting script; fix minor warnings
This replaces apache#9696 Invoke Checkstyle and print any errors to the console, failing the step. Use Google's style rules modified according to https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide Some important checks are disabled (see TODOs in `checkstyle.xml`) due to multiple violations being present in the codebase. Suggest fixing those TODOs in a separate PR(s). More on Checkstyle can be found on the [official website](http://checkstyle.sourceforge.net/). Sample output (from [build 46345](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46345/consoleFull)) (duplicated because I run the build twice with different profiles): > Checkstyle checks failed at following occurrences: [ERROR] src/main/java/org/apache/spark/sql/execution/datasources/parquet/UnsafeRowParquetRecordReader.java:[217,7] (coding) MissingSwitchDefault: switch without "default" clause. > [ERROR] src/main/java/org/apache/spark/sql/execution/datasources/parquet/SpecificParquetRecordReaderBase.java:[198,10] (modifier) ModifierOrder: 'protected' modifier out of order with the JLS suggestions. > [ERROR] src/main/java/org/apache/spark/sql/execution/datasources/parquet/UnsafeRowParquetRecordReader.java:[217,7] (coding) MissingSwitchDefault: switch without "default" clause. > [ERROR] src/main/java/org/apache/spark/sql/execution/datasources/parquet/SpecificParquetRecordReaderBase.java:[198,10] (modifier) ModifierOrder: 'protected' modifier out of order with the JLS suggestions. > [error] running /home/jenkins/workspace/SparkPullRequestBuilder2/dev/lint-java ; received return code 1 Also fix some of the minor violations that didn't require sweeping changes. Apologies for the previous botched PRs - I finally figured out the issue. cr: JoshRosen, pwendell > I state that the contribution is my original work, and I license the work to the project under the project's open source license. Author: Dmitry Erastov <[email protected]> Closes apache#9867 from dskrvk/master.
1 parent 95296d9 commit d0d8222

File tree

31 files changed

+368
-70
lines changed

31 files changed

+368
-70
lines changed

‎checkstyle-suppressions.xml

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<!--
2+
~ Licensed to the Apache Software Foundation (ASF) under one or more
3+
~ contributor license agreements. See the NOTICE file distributed with
4+
~ this work for additional information regarding copyright ownership.
5+
~ The ASF licenses this file to You under the Apache License, Version 2.0
6+
~ (the "License"); you may not use this file except in compliance with
7+
~ the License. You may obtain a copy of the License at
8+
~
9+
~ http://www.apache.org/licenses/LICENSE-2.0
10+
~
11+
~ Unless required by applicable law or agreed to in writing, software
12+
~ distributed under the License is distributed on an "AS IS" BASIS,
13+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
~ See the License for the specific language governing permissions and
15+
~ limitations under the License.
16+
-->
17+
18+
<!DOCTYPE suppressions PUBLIC
19+
"-//Puppy Crawl//DTD Suppressions 1.1//EN"
20+
"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
21+
22+
<!--
23+
24+
This file contains suppression rules for Checkstyle checks.
25+
Ideally only files that cannot be modified (e.g. third-party code)
26+
should be added here. All other violations should be fixed.
27+
28+
-->
29+
30+
<suppressions>
31+
<suppress checks=".*"
32+
files="core/src/main/java/org/apache/spark/util/collection/TimSort.java"/>
33+
</suppressions>

‎checkstyle.xml

+164
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
<!--
2+
~ Licensed to the Apache Software Foundation (ASF) under one or more
3+
~ contributor license agreements. See the NOTICE file distributed with
4+
~ this work for additional information regarding copyright ownership.
5+
~ The ASF licenses this file to You under the Apache License, Version 2.0
6+
~ (the "License"); you may not use this file except in compliance with
7+
~ the License. You may obtain a copy of the License at
8+
~
9+
~ http://www.apache.org/licenses/LICENSE-2.0
10+
~
11+
~ Unless required by applicable law or agreed to in writing, software
12+
~ distributed under the License is distributed on an "AS IS" BASIS,
13+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
~ See the License for the specific language governing permissions and
15+
~ limitations under the License.
16+
-->
17+
18+
<!DOCTYPE module PUBLIC
19+
"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
20+
"http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
21+
22+
<!--
23+
24+
Checkstyle configuration based on the Google coding conventions from:
25+
26+
- Google Java Style
27+
https://google-styleguide.googlecode.com/svn-history/r130/trunk/javaguide.html
28+
29+
with Spark-specific changes from:
30+
31+
https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide
32+
33+
Checkstyle is very configurable. Be sure to read the documentation at
34+
http://checkstyle.sf.net (or in your downloaded distribution).
35+
36+
Most Checks are configurable, be sure to consult the documentation.
37+
38+
To completely disable a check, just comment it out or delete it from the file.
39+
40+
Authors: Max Vetrenko, Ruslan Diachenko, Roman Ivanov.
41+
42+
-->
43+
44+
<module name = "Checker">
45+
<property name="charset" value="UTF-8"/>
46+
47+
<property name="severity" value="error"/>
48+
49+
<property name="fileExtensions" value="java, properties, xml"/>
50+
51+
<module name="SuppressionFilter">
52+
<property name="file" value="checkstyle-suppressions.xml"/>
53+
</module>
54+
55+
<!-- Checks for whitespace -->
56+
<!-- See http://checkstyle.sf.net/config_whitespace.html -->
57+
<module name="FileTabCharacter">
58+
<property name="eachLine" value="true"/>
59+
</module>
60+
61+
<module name="TreeWalker">
62+
<module name="OuterTypeFilename"/>
63+
<module name="IllegalTokenText">
64+
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
65+
<property name="format" value="\\u00(08|09|0(a|A)|0(c|C)|0(d|D)|22|27|5(C|c))|\\(0(10|11|12|14|15|42|47)|134)"/>
66+
<property name="message" value="Avoid using corresponding octal or Unicode escape."/>
67+
</module>
68+
<module name="AvoidEscapedUnicodeCharacters">
69+
<property name="allowEscapesForControlCharacters" value="true"/>
70+
<property name="allowByTailComment" value="true"/>
71+
<property name="allowNonPrintableEscapes" value="true"/>
72+
</module>
73+
<!-- TODO: 11/09/15 disabled - the lengths are currently > 100 in many places -->
74+
<!--
75+
<module name="LineLength">
76+
<property name="max" value="100"/>
77+
<property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
78+
</module>
79+
-->
80+
<module name="NoLineWrap"/>
81+
<module name="EmptyBlock">
82+
<property name="option" value="TEXT"/>
83+
<property name="tokens" value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
84+
</module>
85+
<module name="NeedBraces">
86+
<property name="allowSingleLineStatement" value="true"/>
87+
</module>
88+
<module name="OneStatementPerLine"/>
89+
<module name="ArrayTypeStyle"/>
90+
<module name="FallThrough"/>
91+
<module name="UpperEll"/>
92+
<module name="ModifierOrder"/>
93+
<module name="SeparatorWrap">
94+
<property name="tokens" value="DOT"/>
95+
<property name="option" value="nl"/>
96+
</module>
97+
<module name="SeparatorWrap">
98+
<property name="tokens" value="COMMA"/>
99+
<property name="option" value="EOL"/>
100+
</module>
101+
<module name="PackageName">
102+
<property name="format" value="^[a-z]+(\.[a-z][a-z0-9]*)*$"/>
103+
<message key="name.invalidPattern"
104+
value="Package name ''{0}'' must match pattern ''{1}''."/>
105+
</module>
106+
<module name="ClassTypeParameterName">
107+
<property name="format" value="([A-Z][a-zA-Z0-9]*$)"/>
108+
<message key="name.invalidPattern"
109+
value="Class type name ''{0}'' must match pattern ''{1}''."/>
110+
</module>
111+
<module name="MethodTypeParameterName">
112+
<property name="format" value="([A-Z][a-zA-Z0-9]*)"/>
113+
<message key="name.invalidPattern"
114+
value="Method type name ''{0}'' must match pattern ''{1}''."/>
115+
</module>
116+
<module name="NoFinalizer"/>
117+
<module name="GenericWhitespace">
118+
<message key="ws.followed"
119+
value="GenericWhitespace ''{0}'' is followed by whitespace."/>
120+
<message key="ws.preceded"
121+
value="GenericWhitespace ''{0}'' is preceded with whitespace."/>
122+
<message key="ws.illegalFollow"
123+
value="GenericWhitespace ''{0}'' should followed by whitespace."/>
124+
<message key="ws.notPreceded"
125+
value="GenericWhitespace ''{0}'' is not preceded with whitespace."/>
126+
</module>
127+
<!-- TODO: 11/09/15 disabled - indentation is currently inconsistent -->
128+
<!--
129+
<module name="Indentation">
130+
<property name="basicOffset" value="4"/>
131+
<property name="braceAdjustment" value="0"/>
132+
<property name="caseIndent" value="4"/>
133+
<property name="throwsIndent" value="4"/>
134+
<property name="lineWrappingIndentation" value="4"/>
135+
<property name="arrayInitIndent" value="4"/>
136+
</module>
137+
-->
138+
<!-- TODO: 11/09/15 disabled - order is currently wrong in many places -->
139+
<!--
140+
<module name="ImportOrder">
141+
<property name="separated" value="true"/>
142+
<property name="ordered" value="true"/>
143+
<property name="groups" value="/^javax?\./,scala,*,org.apache.spark"/>
144+
</module>
145+
-->
146+
<module name="MethodParamPad"/>
147+
<module name="AnnotationLocation">
148+
<property name="tokens" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF"/>
149+
</module>
150+
<module name="AnnotationLocation">
151+
<property name="tokens" value="VARIABLE_DEF"/>
152+
<property name="allowSamelineMultipleAnnotations" value="true"/>
153+
</module>
154+
<module name="MethodName">
155+
<property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9_]*$"/>
156+
<message key="name.invalidPattern"
157+
value="Method name ''{0}'' must match pattern ''{1}''."/>
158+
</module>
159+
<module name="EmptyCatchBlock">
160+
<property name="exceptionVariableName" value="expected"/>
161+
</module>
162+
<module name="CommentsIndentation"/>
163+
</module>
164+
</module>

‎core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeInMemorySorter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ private SortedIterator(int numRecords) {
175175
this.position = 0;
176176
}
177177

178-
public SortedIterator clone () {
178+
public SortedIterator clone() {
179179
SortedIterator iter = new SortedIterator(numRecords);
180180
iter.position = position;
181181
iter.baseObject = baseObject;

‎core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,8 @@ public void iteratingOverDataPagesWithWastedSpace() throws Exception {
356356

357357
final java.util.BitSet valuesSeen = new java.util.BitSet(NUM_ENTRIES);
358358
final Iterator<BytesToBytesMap.Location> iter = map.iterator();
359-
final long key[] = new long[KEY_LENGTH / 8];
360-
final long value[] = new long[VALUE_LENGTH / 8];
359+
final long[] key = new long[KEY_LENGTH / 8];
360+
final long[] value = new long[VALUE_LENGTH / 8];
361361
while (iter.hasNext()) {
362362
final BytesToBytesMap.Location loc = iter.next();
363363
Assert.assertTrue(loc.isDefined());

‎dev/lint-java

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/usr/bin/env bash
2+
3+
#
4+
# Licensed to the Apache Software Foundation (ASF) under one or more
5+
# contributor license agreements. See the NOTICE file distributed with
6+
# this work for additional information regarding copyright ownership.
7+
# The ASF licenses this file to You under the Apache License, Version 2.0
8+
# (the "License"); you may not use this file except in compliance with
9+
# the License. You may obtain a copy of the License at
10+
#
11+
# http://www.apache.org/licenses/LICENSE-2.0
12+
#
13+
# Unless required by applicable law or agreed to in writing, software
14+
# distributed under the License is distributed on an "AS IS" BASIS,
15+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
# See the License for the specific language governing permissions and
17+
# limitations under the License.
18+
#
19+
20+
SCRIPT_DIR="$( cd "$( dirname "$0" )" && pwd )"
21+
SPARK_ROOT_DIR="$(dirname $SCRIPT_DIR)"
22+
23+
ERRORS=$($SCRIPT_DIR/../build/mvn -Pkinesis-asl -Pyarn -Phive -Phive-thriftserver checkstyle:check | grep ERROR)
24+
25+
if test ! -z "$ERRORS"; then
26+
echo -e "Checkstyle checks failed at following occurrences:\n$ERRORS"
27+
exit 1
28+
else
29+
echo -e "Checkstyle checks passed."
30+
fi

‎dev/run-tests-jenkins.py

+1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ def run_tests(tests_timeout):
119119
ERROR_CODES["BLOCK_GENERAL"]: 'some tests',
120120
ERROR_CODES["BLOCK_RAT"]: 'RAT tests',
121121
ERROR_CODES["BLOCK_SCALA_STYLE"]: 'Scala style tests',
122+
ERROR_CODES["BLOCK_JAVA_STYLE"]: 'Java style tests',
122123
ERROR_CODES["BLOCK_PYTHON_STYLE"]: 'Python style tests',
123124
ERROR_CODES["BLOCK_R_STYLE"]: 'R style tests',
124125
ERROR_CODES["BLOCK_DOCUMENTATION"]: 'to generate documentation',

‎dev/run-tests.py

+7
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,11 @@ def run_scala_style_checks():
198198
run_cmd([os.path.join(SPARK_HOME, "dev", "lint-scala")])
199199

200200

201+
def run_java_style_checks():
202+
set_title_and_block("Running Java style checks", "BLOCK_JAVA_STYLE")
203+
run_cmd([os.path.join(SPARK_HOME, "dev", "lint-java")])
204+
205+
201206
def run_python_style_checks():
202207
set_title_and_block("Running Python style checks", "BLOCK_PYTHON_STYLE")
203208
run_cmd([os.path.join(SPARK_HOME, "dev", "lint-python")])
@@ -522,6 +527,8 @@ def main():
522527
# style checks
523528
if not changed_files or any(f.endswith(".scala") for f in changed_files):
524529
run_scala_style_checks()
530+
if not changed_files or any(f.endswith(".java") for f in changed_files):
531+
run_java_style_checks()
525532
if not changed_files or any(f.endswith(".py") for f in changed_files):
526533
run_python_style_checks()
527534
if not changed_files or any(f.endswith(".R") for f in changed_files):

‎dev/sparktestsupport/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,6 @@
3131
"BLOCK_SPARK_UNIT_TESTS": 18,
3232
"BLOCK_PYSPARK_UNIT_TESTS": 19,
3333
"BLOCK_SPARKR_UNIT_TESTS": 20,
34+
"BLOCK_JAVA_STYLE": 21,
3435
"BLOCK_TIMEOUT": 124
3536
}

‎examples/src/main/java/org/apache/spark/examples/ml/JavaSimpleParamsExample.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static void main(String[] args) {
7777
ParamMap paramMap = new ParamMap();
7878
paramMap.put(lr.maxIter().w(20)); // Specify 1 Param.
7979
paramMap.put(lr.maxIter(), 30); // This overwrites the original maxIter.
80-
double thresholds[] = {0.45, 0.55};
80+
double[] thresholds = {0.45, 0.55};
8181
paramMap.put(lr.regParam().w(0.1), lr.thresholds().w(thresholds)); // Specify multiple Params.
8282

8383
// One can also combine ParamMaps.

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaLDAExample.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ public static void main(String[] args) {
4141
public Vector call(String s) {
4242
String[] sarray = s.trim().split(" ");
4343
double[] values = new double[sarray.length];
44-
for (int i = 0; i < sarray.length; i++)
44+
for (int i = 0; i < sarray.length; i++) {
4545
values[i] = Double.parseDouble(sarray[i]);
46+
}
4647
return Vectors.dense(values);
4748
}
4849
}

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaMultiLabelClassificationMetricsExample.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ public static void main(String[] args) {
5757

5858
// Stats by labels
5959
for (int i = 0; i < metrics.labels().length - 1; i++) {
60-
System.out.format("Class %1.1f precision = %f\n", metrics.labels()[i], metrics.precision
61-
(metrics.labels()[i]));
62-
System.out.format("Class %1.1f recall = %f\n", metrics.labels()[i], metrics.recall(metrics
63-
.labels()[i]));
64-
System.out.format("Class %1.1f F1 score = %f\n", metrics.labels()[i], metrics.f1Measure
65-
(metrics.labels()[i]));
60+
System.out.format("Class %1.1f precision = %f\n", metrics.labels()[i], metrics.precision(
61+
metrics.labels()[i]));
62+
System.out.format("Class %1.1f recall = %f\n", metrics.labels()[i], metrics.recall(
63+
metrics.labels()[i]));
64+
System.out.format("Class %1.1f F1 score = %f\n", metrics.labels()[i], metrics.f1Measure(
65+
metrics.labels()[i]));
6666
}
6767

6868
// Micro stats

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaMulticlassClassificationMetricsExample.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,12 @@ public Tuple2<Object, Object> call(LabeledPoint p) {
7474

7575
// Stats by labels
7676
for (int i = 0; i < metrics.labels().length; i++) {
77-
System.out.format("Class %f precision = %f\n", metrics.labels()[i],metrics.precision
78-
(metrics.labels()[i]));
79-
System.out.format("Class %f recall = %f\n", metrics.labels()[i], metrics.recall(metrics
80-
.labels()[i]));
81-
System.out.format("Class %f F1 score = %f\n", metrics.labels()[i], metrics.fMeasure
82-
(metrics.labels()[i]));
77+
System.out.format("Class %f precision = %f\n", metrics.labels()[i],metrics.precision(
78+
metrics.labels()[i]));
79+
System.out.format("Class %f recall = %f\n", metrics.labels()[i], metrics.recall(
80+
metrics.labels()[i]));
81+
System.out.format("Class %f F1 score = %f\n", metrics.labels()[i], metrics.fMeasure(
82+
metrics.labels()[i]));
8383
}
8484

8585
//Weighted stats

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaRankingMetricsExample.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ public List<Integer> call(Rating[] docs) {
120120
}
121121
}
122122
);
123-
JavaRDD<Tuple2<List<Integer>, List<Integer>>> relevantDocs = userMoviesList.join
124-
(userRecommendedList).values();
123+
JavaRDD<Tuple2<List<Integer>, List<Integer>>> relevantDocs = userMoviesList.join(
124+
userRecommendedList).values();
125125

126126
// Instantiate the metrics object
127127
RankingMetrics metrics = RankingMetrics.of(relevantDocs);

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaRecommendationExample.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
// $example off$
3030

3131
public class JavaRecommendationExample {
32-
public static void main(String args[]) {
32+
public static void main(String[] args) {
3333
// $example on$
3434
SparkConf conf = new SparkConf().setAppName("Java Collaborative Filtering Example");
3535
JavaSparkContext jsc = new JavaSparkContext(conf);

‎examples/src/main/java/org/apache/spark/examples/mllib/JavaRegressionMetricsExample.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ public static void main(String[] args) {
4343
public LabeledPoint call(String line) {
4444
String[] parts = line.split(" ");
4545
double[] v = new double[parts.length - 1];
46-
for (int i = 1; i < parts.length - 1; i++)
46+
for (int i = 1; i < parts.length - 1; i++) {
4747
v[i - 1] = Double.parseDouble(parts[i].split(":")[1]);
48+
}
4849
return new LabeledPoint(Double.parseDouble(parts[0]), Vectors.dense(v));
4950
}
5051
}

‎examples/src/main/java/org/apache/spark/examples/streaming/JavaSqlNetworkWordCount.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ public JavaRecord call(String word) {
112112

113113
/** Lazily instantiated singleton instance of SQLContext */
114114
class JavaSQLContextSingleton {
115-
static private transient SQLContext instance = null;
116-
static public SQLContext getInstance(SparkContext sparkContext) {
115+
private static transient SQLContext instance = null;
116+
public static SQLContext getInstance(SparkContext sparkContext) {
117117
if (instance == null) {
118118
instance = new SQLContext(sparkContext);
119119
}

0 commit comments

Comments
 (0)