From 8d68d07093d9b2fda7fec72eadf1f8a4fac3a89b Mon Sep 17 00:00:00 2001 From: Tai Le Manh Date: Sat, 18 May 2024 22:37:30 +0700 Subject: [PATCH] [INFRA] Improve the java checkstyle checks to log the errors to the sbt console Signed-off-by: Tai Le Manh --- build.sbt | 64 ++++++-------------------- project/Checkstyle.scala | 99 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 51 deletions(-) create mode 100644 project/Checkstyle.scala diff --git a/build.sbt b/build.sbt index 98c85d1bcf4..cf100b2fff8 100644 --- a/build.sbt +++ b/build.sbt @@ -17,8 +17,8 @@ // scalastyle:off line.size.limit import java.nio.file.Files -import Mima._ -import Unidoc._ +import Mima.* +import Unidoc.* // Scala versions val scala212 = "2.12.18" @@ -192,7 +192,7 @@ lazy val spark = (project in file("spark")) .settings ( name := "delta-spark", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, sparkMimaSettings, releaseSettings, crossSparkSettings(), @@ -272,7 +272,7 @@ lazy val contribs = (project in file("contribs")) .settings ( name := "delta-contribs", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, releaseSettings, Compile / packageBin / mappings := (Compile / packageBin / mappings).value ++ listPythonFiles(baseDirectory.value.getParentFile / "python"), @@ -310,7 +310,7 @@ lazy val sharing = (project in file("sharing")) .settings( name := "delta-sharing-spark", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, releaseSettings, Test / javaOptions ++= Seq("-ea"), libraryDependencies ++= Seq( @@ -334,7 +334,7 @@ lazy val kernelApi = (project in file("kernel/kernel-api")) .settings( name := "delta-kernel-api", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, javaOnlyReleaseSettings, Test / javaOptions ++= Seq("-ea"), libraryDependencies ++= Seq( @@ -374,7 +374,7 @@ lazy val kernelApi = (project in file("kernel/kernel-api")) |""".stripMargin) Seq(file) }, - javaCheckstyleSettings("dev/kernel-checkstyle.xml"), + Checkstyle.javaCheckstyleSettings("dev/kernel-checkstyle.xml"), // Unidoc settings unidocSourceFilePatterns := Seq(SourceFilePattern("io/delta/kernel/")), ).configureUnidoc(docTitle = "Delta Kernel") @@ -388,7 +388,7 @@ lazy val kernelDefaults = (project in file("kernel/kernel-defaults")) .settings( name := "delta-kernel-defaults", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, javaOnlyReleaseSettings, Test / javaOptions ++= Seq("-ea"), libraryDependencies ++= Seq( @@ -412,7 +412,7 @@ lazy val kernelDefaults = (project in file("kernel/kernel-defaults")) "org.apache.spark" %% "spark-core" % defaultSparkVersion % "test" classifier "tests", "org.apache.spark" %% "spark-catalyst" % defaultSparkVersion % "test" classifier "tests", ), - javaCheckstyleSettings("dev/kernel-checkstyle.xml"), + Checkstyle.javaCheckstyleSettings("dev/kernel-checkstyle.xml"), // Unidoc settings unidocSourceFilePatterns += SourceFilePattern("io/delta/kernel/"), ).configureUnidoc(docTitle = "Delta Kernel Defaults") @@ -502,7 +502,7 @@ lazy val iceberg = (project in file("iceberg")) .settings ( name := "delta-iceberg", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, releaseSettings, libraryDependencies ++= Seq( // Fix Iceberg's legacy java.lang.NoClassDefFoundError: scala/jdk/CollectionConverters$ error @@ -606,7 +606,7 @@ lazy val hudi = (project in file("hudi")) .settings ( name := "delta-hudi", commonSettings, - scalaStyleSettings, + Checkstyle.scalaStyleSettings, releaseSettings, libraryDependencies ++= Seq( "org.apache.hudi" % "hudi-java-client" % "0.14.0" % "compile" excludeAll( @@ -1072,7 +1072,7 @@ lazy val standalone = (project in file("connectors/standalone")) // Unidoc setting unidocSourceFilePatterns += SourceFilePattern("io/delta/standalone/"), - javaCheckstyleSettings("dev/connectors-checkstyle.xml") + Checkstyle.javaCheckstyleSettings("dev/connectors-checkstyle.xml") ).configureUnidoc() @@ -1257,7 +1257,7 @@ lazy val flink = (project in file("connectors/flink")) unidocSourceFilePatterns += SourceFilePattern("io/delta/flink/"), // TODO: this is the config that was used before archiving connectors but it has // standalone-specific import orders - javaCheckstyleSettings("dev/connectors-checkstyle.xml") + Checkstyle.javaCheckstyleSettings("dev/connectors-checkstyle.xml") ).configureUnidoc() /** @@ -1309,44 +1309,6 @@ lazy val kernelGroup = project } ).configureUnidoc(docTitle = "Delta Kernel") -/* - *********************** - * ScalaStyle settings * - *********************** - */ -ThisBuild / scalastyleConfig := baseDirectory.value / "scalastyle-config.xml" - -lazy val compileScalastyle = taskKey[Unit]("compileScalastyle") -lazy val testScalastyle = taskKey[Unit]("testScalastyle") - -lazy val scalaStyleSettings = Seq( - compileScalastyle := (Compile / scalastyle).toTask("").value, - - Compile / compile := ((Compile / compile) dependsOn compileScalastyle).value, - - testScalastyle := (Test / scalastyle).toTask("").value, - - Test / test := ((Test / test) dependsOn testScalastyle).value -) - -/* - **************************** - * Java checkstyle settings * - **************************** - */ - -def javaCheckstyleSettings(checkstyleFile: String): Def.SettingsDefinition = { - // Can be run explicitly via: build/sbt $module/checkstyle - // Will automatically be run during compilation (e.g. build/sbt compile) - // and during tests (e.g. build/sbt test) - Seq( - checkstyleConfigLocation := CheckstyleConfigLocation.File(checkstyleFile), - checkstyleSeverityLevel := CheckstyleSeverityLevel.Error, - (Compile / compile) := ((Compile / compile) dependsOn (Compile / checkstyle)).value, - (Test / test) := ((Test / test) dependsOn (Test / checkstyle)).value - ) -} - /* ******************** * Release settings * diff --git a/project/Checkstyle.scala b/project/Checkstyle.scala new file mode 100644 index 00000000000..8abd3c4a560 --- /dev/null +++ b/project/Checkstyle.scala @@ -0,0 +1,99 @@ +/* + * Copyright (2021) The Delta Lake Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import com.etsy.sbt.checkstyle.CheckstylePlugin.autoImport._ +import org.scalastyle.sbt.ScalastylePlugin.autoImport._ +import sbt._ +import sbt.Keys._ + +object Checkstyle { + + /* + ***************************** + * Scala checkstyle settings * + ***************************** + */ + + ThisBuild / scalastyleConfig := baseDirectory.value / "scalastyle-config.xml" + + private lazy val compileScalastyle = taskKey[Unit]("compileScalastyle") + private lazy val testScalastyle = taskKey[Unit]("testScalastyle") + + lazy val scalaStyleSettings = Seq( + compileScalastyle := (Compile / scalastyle).toTask("").value, + + Compile / compile := ((Compile / compile) dependsOn compileScalastyle).value, + + testScalastyle := (Test / scalastyle).toTask("").value, + + Test / test := ((Test / test) dependsOn testScalastyle).value + ) + + /* + **************************** + * Java checkstyle settings * + **************************** + */ + + private lazy val compileJavastyle = taskKey[Unit]("compileJavastyle") + private lazy val testJavastyle = taskKey[Unit]("testJavastyle") + + def javaCheckstyleSettings(checkstyleFile: String): Def.SettingsDefinition = { + // Can be run explicitly via: build/sbt $module/checkstyle + // Will automatically be run during compilation (e.g. build/sbt compile) + // and during tests (e.g. build/sbt test) + Seq( + checkstyleConfigLocation := CheckstyleConfigLocation.File(checkstyleFile), + checkstyleSeverityLevel := CheckstyleSeverityLevel.Ignore, + + compileJavastyle := { + (Compile / checkstyle).value + javaCheckstyle(streams.value.log, checkstyleOutputFile.value) + }, + (Compile / compile) := ((Compile / compile) dependsOn compileJavastyle).value, + + testJavastyle := { + (Test / checkstyle).value + javaCheckstyle(streams.value.log, (Compile / target).value / "checkstyle-test-report.xml") + }, + (Test / test) := ((Test / test) dependsOn (Test / testJavastyle)).value + ) + } + + private def javaCheckstyle(log: Logger, reportFile: File): Unit = { + val report = scala.xml.XML.loadFile(reportFile) + + val errors = (report \\ "file").flatMap { fileNode => + val file = fileNode.attribute("name").get.head.text + (fileNode \ "error").map { error => + val line = error.attribute("line").get.head.text + val message = error.attribute("message").get.head.text + (file, line, message) + } + } + + if (errors.nonEmpty) { + var errorMsg = "Found checkstyle errors" + errors.foreach { case (file, line, message) => + val lineError = s"File: $file, Line: $line, Message: $message" + log.error(lineError) + errorMsg += ("\n" + lineError) + } + sys.error(errorMsg + "\n") + } + } + +}