Skip to content

Commit 2977e23

Browse files
xuanyuankingsrowen
xuanyuanking
authored and
srowen
committed
[SPARK-25986][BUILD] Add rules to ban throw Errors in application code
## What changes were proposed in this pull request? Add scala and java lint check rules to ban the usage of `throw new xxxErrors` and fix up all exists instance followed by apache#22989 (comment). See more details in apache#22969. ## How was this patch tested? Local test with lint-scala and lint-java. Closes apache#22989 from xuanyuanking/SPARK-25986. Authored-by: Yuanjian Li <[email protected]> Signed-off-by: Sean Owen <[email protected]>
1 parent 2b671e7 commit 2977e23

File tree

39 files changed

+128
-87
lines changed

39 files changed

+128
-87
lines changed

‎common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java

+4
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ public static int getSize(Object object, long offset) {
3939
case 8:
4040
return (int)Platform.getLong(object, offset);
4141
default:
42+
// checkstyle.off: RegexpSinglelineJava
4243
throw new AssertionError("Illegal UAO_SIZE");
44+
// checkstyle.on: RegexpSinglelineJava
4345
}
4446
}
4547

@@ -52,7 +54,9 @@ public static void putSize(Object object, long offset, int value) {
5254
Platform.putLong(object, offset, value);
5355
break;
5456
default:
57+
// checkstyle.off: RegexpSinglelineJava
5558
throw new AssertionError("Illegal UAO_SIZE");
59+
// checkstyle.on: RegexpSinglelineJava
5660
}
5761
}
5862
}

‎core/src/main/java/org/apache/spark/memory/MemoryConsumer.java

+2
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ private void throwOom(final MemoryBlock page, final long required) {
154154
taskMemoryManager.freePage(page, this);
155155
}
156156
taskMemoryManager.showMemoryUsage();
157+
// checkstyle.off: RegexpSinglelineJava
157158
throw new SparkOutOfMemoryError("Unable to acquire " + required + " bytes of memory, got " +
158159
got);
160+
// checkstyle.on: RegexpSinglelineJava
159161
}
160162
}

‎core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java

+4
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,10 @@ public long acquireExecutionMemory(long required, MemoryConsumer consumer) {
194194
throw new RuntimeException(e.getMessage());
195195
} catch (IOException e) {
196196
logger.error("error while calling spill() on " + c, e);
197+
// checkstyle.off: RegexpSinglelineJava
197198
throw new SparkOutOfMemoryError("error while calling spill() on " + c + " : "
198199
+ e.getMessage());
200+
// checkstyle.on: RegexpSinglelineJava
199201
}
200202
}
201203
}
@@ -215,8 +217,10 @@ public long acquireExecutionMemory(long required, MemoryConsumer consumer) {
215217
throw new RuntimeException(e.getMessage());
216218
} catch (IOException e) {
217219
logger.error("error while calling spill() on " + consumer, e);
220+
// checkstyle.off: RegexpSinglelineJava
218221
throw new SparkOutOfMemoryError("error while calling spill() on " + consumer + " : "
219222
+ e.getMessage());
223+
// checkstyle.on: RegexpSinglelineJava
220224
}
221225
}
222226

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

+2
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ public boolean hasSpaceForAnotherRecord() {
214214

215215
public void expandPointerArray(LongArray newArray) {
216216
if (newArray.size() < array.size()) {
217+
// checkstyle.off: RegexpSinglelineJava
217218
throw new SparkOutOfMemoryError("Not enough memory to grow pointer array");
219+
// checkstyle.on: RegexpSinglelineJava
218220
}
219221
Platform.copyMemory(
220222
array.getBaseObject(),

‎core/src/main/scala/org/apache/spark/util/random/RandomSampler.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ trait RandomSampler[T, U] extends Pseudorandom with Cloneable with Serializable
4949

5050
/** return a copy of the RandomSampler object */
5151
override def clone: RandomSampler[T, U] =
52-
throw new NotImplementedError("clone() is not implemented.")
52+
throw new UnsupportedOperationException("clone() is not implemented.")
5353
}
5454

5555
private[spark]

‎core/src/test/scala/org/apache/spark/FailureSuite.scala

+2
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,9 @@ class FailureSuite extends SparkFunSuite with LocalSparkContext {
257257
sc = new SparkContext("local[1,2]", "test")
258258
intercept[SparkException] {
259259
sc.parallelize(1 to 2).foreach { i =>
260+
// scalastyle:off throwerror
260261
throw new LinkageError()
262+
// scalastyle:on throwerror
261263
}
262264
}
263265
}

‎core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala

+2
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,9 @@ class FetchFailureHidingRDD(
467467
} catch {
468468
case t: Throwable =>
469469
if (throwOOM) {
470+
// scalastyle:off throwerror
470471
throw new OutOfMemoryError("OOM while handling another exception")
472+
// scalastyle:on throwerror
471473
} else if (interrupt) {
472474
// make sure our test is setup correctly
473475
assert(TaskContext.get().asInstanceOf[TaskContextImpl].fetchFailed.isDefined)

‎core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala

+2
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,9 @@ class TaskResultGetterSuite extends SparkFunSuite with BeforeAndAfter with Local
265265

266266
private class UndeserializableException extends Exception {
267267
private def readObject(in: ObjectInputStream): Unit = {
268+
// scalastyle:off throwerror
268269
throw new NoClassDefFoundError()
270+
// scalastyle:on throwerror
269271
}
270272
}
271273

‎core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
574574
"list1",
575575
StorageLevel.MEMORY_ONLY,
576576
ClassTag.Any,
577-
() => throw new AssertionError("attempted to compute locally")).isLeft)
577+
() => fail("attempted to compute locally")).isLeft)
578578
}
579579

580580
test("in-memory LRU storage") {

‎dev/checkstyle.xml

+9-4
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,13 @@
7171
If you wish to turn off checking for a section of code, you can put a comment in the source
7272
before and after the section, with the following syntax:
7373
74-
// checkstyle:off no.XXX (such as checkstyle.off: NoFinalizer)
74+
// checkstyle.off: XXX (such as checkstyle.off: NoFinalizer)
7575
... // stuff that breaks the styles
76-
// checkstyle:on
76+
// checkstyle.on: XXX (such as checkstyle.on: NoFinalizer)
7777
-->
7878
<module name="SuppressionCommentFilter">
79-
<property name="offCommentFormat" value="checkstyle.off\: ([\w\|]+)"/>
80-
<property name="onCommentFormat" value="checkstyle.on\: ([\w\|]+)"/>
79+
<property name="offCommentFormat" value="checkstyle\.off\: ([\w\|]+)"/>
80+
<property name="onCommentFormat" value="checkstyle\.on\: ([\w\|]+)"/>
8181
<property name="checkFormat" value="$1"/>
8282
</module>
8383
<module name="OuterTypeFilename"/>
@@ -180,5 +180,10 @@
180180
<module name="UnusedImports"/>
181181
<module name="RedundantImport"/>
182182
<module name="RedundantModifier"/>
183+
<module name="RegexpSinglelineJava">
184+
<property name="format" value="throw new \w+Error\("/>
185+
<property name="message" value="Avoid throwing error in application code."/>
186+
</module>
187+
183188
</module>
184189
</module>

‎external/kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaUtils.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ object KafkaUtils extends Logging {
5656
): RDD[ConsumerRecord[K, V]] = {
5757
val preferredHosts = locationStrategy match {
5858
case PreferBrokers =>
59-
throw new AssertionError(
59+
throw new IllegalArgumentException(
6060
"If you want to prefer brokers, you must provide a mapping using PreferFixed " +
6161
"A single KafkaRDD does not have a driver consumer and cannot look up brokers for you.")
6262
case PreferConsistent => ju.Collections.emptyMap[TopicPartition, String]()

‎mllib-local/src/main/scala/org/apache/spark/ml/linalg/Vectors.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ sealed trait Vector extends Serializable {
106106
*/
107107
@Since("2.0.0")
108108
def copy: Vector = {
109-
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
109+
throw new UnsupportedOperationException(s"copy is not implemented for ${this.getClass}.")
110110
}
111111

112112
/**

‎mllib/src/main/scala/org/apache/spark/ml/classification/NaiveBayes.scala

+4-4
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class NaiveBayes @Since("1.5.0") (
146146
requireZeroOneBernoulliValues
147147
case _ =>
148148
// This should never happen.
149-
throw new UnknownError(s"Invalid modelType: ${$(modelType)}.")
149+
throw new IllegalArgumentException(s"Invalid modelType: ${$(modelType)}.")
150150
}
151151
}
152152

@@ -196,7 +196,7 @@ class NaiveBayes @Since("1.5.0") (
196196
case Bernoulli => math.log(n + 2.0 * lambda)
197197
case _ =>
198198
// This should never happen.
199-
throw new UnknownError(s"Invalid modelType: ${$(modelType)}.")
199+
throw new IllegalArgumentException(s"Invalid modelType: ${$(modelType)}.")
200200
}
201201
var j = 0
202202
while (j < numFeatures) {
@@ -295,7 +295,7 @@ class NaiveBayesModel private[ml] (
295295
(Option(thetaMinusNegTheta), Option(negTheta.multiply(ones)))
296296
case _ =>
297297
// This should never happen.
298-
throw new UnknownError(s"Invalid modelType: ${$(modelType)}.")
298+
throw new IllegalArgumentException(s"Invalid modelType: ${$(modelType)}.")
299299
}
300300

301301
@Since("1.6.0")
@@ -329,7 +329,7 @@ class NaiveBayesModel private[ml] (
329329
bernoulliCalculation(features)
330330
case _ =>
331331
// This should never happen.
332-
throw new UnknownError(s"Invalid modelType: ${$(modelType)}.")
332+
throw new IllegalArgumentException(s"Invalid modelType: ${$(modelType)}.")
333333
}
334334
}
335335

‎mllib/src/main/scala/org/apache/spark/ml/param/params.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class Param[T](val parent: String, val name: String, val doc: String, val isVali
9797
case m: Matrix =>
9898
JsonMatrixConverter.toJson(m)
9999
case _ =>
100-
throw new NotImplementedError(
100+
throw new UnsupportedOperationException(
101101
"The default jsonEncode only supports string, vector and matrix. " +
102102
s"${this.getClass.getName} must override jsonEncode for ${value.getClass.getName}.")
103103
}
@@ -151,7 +151,7 @@ private[ml] object Param {
151151
}
152152

153153
case _ =>
154-
throw new NotImplementedError(
154+
throw new UnsupportedOperationException(
155155
"The default jsonDecode only supports string, vector and matrix. " +
156156
s"${this.getClass.getName} must override jsonDecode to support its value type.")
157157
}

‎mllib/src/main/scala/org/apache/spark/ml/tuning/ValidatorParams.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ private[ml] object ValidatorParams {
140140
"value" -> compact(render(JString(relativePath))),
141141
"isJson" -> compact(render(JBool(false))))
142142
case _: MLWritable =>
143-
throw new NotImplementedError("ValidatorParams.saveImpl does not handle parameters " +
144-
"of type: MLWritable that are not DefaultParamsWritable")
143+
throw new UnsupportedOperationException("ValidatorParams.saveImpl does not handle" +
144+
" parameters of type: MLWritable that are not DefaultParamsWritable")
145145
case _ =>
146146
Map("parent" -> p.parent, "name" -> p.name, "value" -> p.jsonEncode(v),
147147
"isJson" -> compact(render(JBool(true))))

‎mllib/src/main/scala/org/apache/spark/mllib/classification/NaiveBayes.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class NaiveBayesModel private[spark] (
8383
(Option(thetaMinusNegTheta), Option(negTheta.multiply(ones)))
8484
case _ =>
8585
// This should never happen.
86-
throw new UnknownError(s"Invalid modelType: $modelType.")
86+
throw new IllegalArgumentException(s"Invalid modelType: $modelType.")
8787
}
8888

8989
@Since("1.0.0")

‎mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ sealed trait Vector extends Serializable {
117117
*/
118118
@Since("1.1.0")
119119
def copy: Vector = {
120-
throw new NotImplementedError(s"copy is not implemented for ${this.getClass}.")
120+
throw new UnsupportedOperationException(s"copy is not implemented for ${this.getClass}.")
121121
}
122122

123123
/**

‎mllib/src/test/scala/org/apache/spark/ml/PredictorSuite.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ object PredictorSuite {
7373
}
7474

7575
override def copy(extra: ParamMap): MockPredictor =
76-
throw new NotImplementedError()
76+
throw new UnsupportedOperationException()
7777
}
7878

7979
class MockPredictionModel(override val uid: String)
@@ -82,9 +82,9 @@ object PredictorSuite {
8282
def this() = this(Identifiable.randomUID("mockpredictormodel"))
8383

8484
override def predict(features: Vector): Double =
85-
throw new NotImplementedError()
85+
throw new UnsupportedOperationException()
8686

8787
override def copy(extra: ParamMap): MockPredictionModel =
88-
throw new NotImplementedError()
88+
throw new UnsupportedOperationException()
8989
}
9090
}

‎mllib/src/test/scala/org/apache/spark/ml/classification/ClassifierSuite.scala

+6-5
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ object ClassifierSuite {
117117

118118
def this() = this(Identifiable.randomUID("mockclassifier"))
119119

120-
override def copy(extra: ParamMap): MockClassifier = throw new NotImplementedError()
120+
override def copy(extra: ParamMap): MockClassifier = throw new UnsupportedOperationException()
121121

122122
override def train(dataset: Dataset[_]): MockClassificationModel =
123-
throw new NotImplementedError()
123+
throw new UnsupportedOperationException()
124124

125125
// Make methods public
126126
override def extractLabeledPoints(dataset: Dataset[_], numClasses: Int): RDD[LabeledPoint] =
@@ -133,11 +133,12 @@ object ClassifierSuite {
133133

134134
def this() = this(Identifiable.randomUID("mockclassificationmodel"))
135135

136-
protected def predictRaw(features: Vector): Vector = throw new NotImplementedError()
136+
protected def predictRaw(features: Vector): Vector = throw new UnsupportedOperationException()
137137

138-
override def copy(extra: ParamMap): MockClassificationModel = throw new NotImplementedError()
138+
override def copy(extra: ParamMap): MockClassificationModel =
139+
throw new UnsupportedOperationException()
139140

140-
override def numClasses: Int = throw new NotImplementedError()
141+
override def numClasses: Int = throw new UnsupportedOperationException()
141142
}
142143

143144
}

‎mllib/src/test/scala/org/apache/spark/ml/classification/NaiveBayesSuite.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ class NaiveBayesSuite extends MLTest with DefaultReadWriteTest {
103103
case Bernoulli =>
104104
expectedBernoulliProbabilities(model, features)
105105
case _ =>
106-
throw new UnknownError(s"Invalid modelType: $modelType.")
106+
throw new IllegalArgumentException(s"Invalid modelType: $modelType.")
107107
}
108108
assert(probability ~== expected relTol 1.0e-10)
109109
}
@@ -378,7 +378,7 @@ object NaiveBayesSuite {
378378
counts.toArray.sortBy(_._1).map(_._2)
379379
case _ =>
380380
// This should never happen.
381-
throw new UnknownError(s"Invalid modelType: $modelType.")
381+
throw new IllegalArgumentException(s"Invalid modelType: $modelType.")
382382
}
383383

384384
LabeledPoint(y, Vectors.dense(xi))

‎mllib/src/test/scala/org/apache/spark/ml/classification/OneVsRestSuite.scala

+8-8
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ class OneVsRestSuite extends MLTest with DefaultReadWriteTest {
134134
assert(lrModel1.coefficients ~== lrModel2.coefficients relTol 1E-3)
135135
assert(lrModel1.intercept ~== lrModel2.intercept relTol 1E-3)
136136
case other =>
137-
throw new AssertionError(s"Loaded OneVsRestModel expected model of type" +
138-
s" LogisticRegressionModel but found ${other.getClass.getName}")
137+
fail("Loaded OneVsRestModel expected model of type LogisticRegressionModel " +
138+
s"but found ${other.getClass.getName}")
139139
}
140140
}
141141

@@ -247,8 +247,8 @@ class OneVsRestSuite extends MLTest with DefaultReadWriteTest {
247247
assert(lr.getMaxIter === lr2.getMaxIter)
248248
assert(lr.getRegParam === lr2.getRegParam)
249249
case other =>
250-
throw new AssertionError(s"Loaded OneVsRest expected classifier of type" +
251-
s" LogisticRegression but found ${other.getClass.getName}")
250+
fail("Loaded OneVsRest expected classifier of type LogisticRegression" +
251+
s" but found ${other.getClass.getName}")
252252
}
253253
}
254254

@@ -267,8 +267,8 @@ class OneVsRestSuite extends MLTest with DefaultReadWriteTest {
267267
assert(classifier.getMaxIter === lr2.getMaxIter)
268268
assert(classifier.getRegParam === lr2.getRegParam)
269269
case other =>
270-
throw new AssertionError(s"Loaded OneVsRestModel expected classifier of type" +
271-
s" LogisticRegression but found ${other.getClass.getName}")
270+
fail("Loaded OneVsRestModel expected classifier of type LogisticRegression" +
271+
s" but found ${other.getClass.getName}")
272272
}
273273

274274
assert(model.labelMetadata === model2.labelMetadata)
@@ -278,8 +278,8 @@ class OneVsRestSuite extends MLTest with DefaultReadWriteTest {
278278
assert(lrModel1.coefficients === lrModel2.coefficients)
279279
assert(lrModel1.intercept === lrModel2.intercept)
280280
case other =>
281-
throw new AssertionError(s"Loaded OneVsRestModel expected model of type" +
282-
s" LogisticRegressionModel but found ${other.getClass.getName}")
281+
fail(s"Loaded OneVsRestModel expected model of type LogisticRegressionModel" +
282+
s" but found ${other.getClass.getName}")
283283
}
284284
}
285285

‎mllib/src/test/scala/org/apache/spark/ml/feature/VectorIndexerSuite.scala

+3-1
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,9 @@ class VectorIndexerSuite extends MLTest with DefaultReadWriteTest with Logging {
283283
points.zip(rows.map(_(0))).foreach {
284284
case (orig: SparseVector, indexed: SparseVector) =>
285285
assert(orig.indices.length == indexed.indices.length)
286-
case _ => throw new UnknownError("Unit test has a bug in it.") // should never happen
286+
case _ =>
287+
// should never happen
288+
fail("Unit test has a bug in it.")
287289
}
288290
}
289291
}

‎mllib/src/test/scala/org/apache/spark/ml/tree/impl/RandomForestSuite.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,9 @@ class RandomForestSuite extends SparkFunSuite with MLlibTestSparkContext {
417417
case n: InternalNode => n.split match {
418418
case s: CategoricalSplit =>
419419
assert(s.leftCategories === Array(1.0))
420-
case _ => throw new AssertionError("model.rootNode.split was not a CategoricalSplit")
420+
case _ => fail("model.rootNode.split was not a CategoricalSplit")
421421
}
422-
case _ => throw new AssertionError("model.rootNode was not an InternalNode")
422+
case _ => fail("model.rootNode was not an InternalNode")
423423
}
424424
}
425425

@@ -444,7 +444,7 @@ class RandomForestSuite extends SparkFunSuite with MLlibTestSparkContext {
444444
assert(n.leftChild.isInstanceOf[InternalNode])
445445
assert(n.rightChild.isInstanceOf[InternalNode])
446446
Array(n.leftChild.asInstanceOf[InternalNode], n.rightChild.asInstanceOf[InternalNode])
447-
case _ => throw new AssertionError("rootNode was not an InternalNode")
447+
case _ => fail("rootNode was not an InternalNode")
448448
}
449449

450450
// Single group second level tree construction.

0 commit comments

Comments
 (0)