From b99ee3920271686d4b97450a7ee3ae85ad1b7ec9 Mon Sep 17 00:00:00 2001 From: Eugene Sypachev Date: Fri, 12 Jun 2020 13:29:05 +0200 Subject: [PATCH] Improved reports, markdown report (#376) * draft interface * refactor reporters * markdown reporter * clean up report execution * readme and accidental changes * scalafix --- README.md | 2 +- .../scapegoat/io/HtmlReportWriter.scala | 16 ++++--- .../com/sksamuel/scapegoat/io/IOUtils.scala | 41 ++++-------------- .../scapegoat/io/MarkdownReportWriter.scala | 43 +++++++++++++++++++ .../sksamuel/scapegoat/io/ReportWriter.scala | 25 +++++++++++ .../scapegoat/io/ScalastyleReportWriter.scala | 11 ++--- .../scapegoat/io/XmlReportWriter.scala | 8 +++- .../scala/com/sksamuel/scapegoat/plugin.scala | 33 +++++++------- .../com/sksamuel/scapegoat/FeedbackTest.scala | 3 +- .../com/sksamuel/scapegoat/PluginRunner.scala | 6 +-- .../FinalModifierOnCaseClassTest.scala | 2 +- .../collections/MapGetAndGetOrElseTest.scala | 2 +- 12 files changed, 123 insertions(+), 69 deletions(-) create mode 100644 src/main/scala/com/sksamuel/scapegoat/io/MarkdownReportWriter.scala create mode 100644 src/main/scala/com/sksamuel/scapegoat/io/ReportWriter.scala diff --git a/README.md b/README.md index 87ccd2b8..6875f7ac 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ You can pass other configuration flags adding it to the `additionalParameters` l |`-P:scapegoat:ignoredFiles:`|Colon separated list of regexes to match files to ignore.|false| |`-P:scapegoat:verbose:`|Boolean flag that enables/disables verbose console messages.|false| |`-P:scapegoat:consoleOutput:`|Boolean flag that enables/disables console report output.|false| -|`-P:scapegoat:reports:`|Colon separated list of reports to generate. Valid options are `none`, `xml`, `html`, `scalastyle`, or `all`.|false| +|`-P:scapegoat:reports:`|Colon separated list of reports to generate. Valid options are `none`, `xml`, `html`, `scalastyle`, `markdown`, or `all`.|false| |`-P:scapegoat:overrideLevels:`|Overrides the built in warning levels. Should be a colon separated list of `name=level` expressions.|false| |`-P:scapegoat:sourcePrefix:`|Overrides source prefix if it differs from `src/main/scala`, for ex. `app/` for Play applications.|false| |`-P:scapegoat:minimalWarnLevel:`|Provides minimal level of inspection displayed in reports.|false| diff --git a/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala index 93857448..db605640 100644 --- a/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala +++ b/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala @@ -5,9 +5,11 @@ import scala.xml.Unparsed import com.sksamuel.scapegoat.{Feedback, Levels} /** @author Stephen Samuel */ -object HtmlReportWriter { +object HtmlReportWriter extends ReportWriter { - val css = + override protected def fileName: String = "scapegoat.html" + + private val css = """ | body { | font-family: 'Ubuntu', sans-serif; @@ -60,7 +62,7 @@ object HtmlReportWriter { | """.stripMargin - def header = + private def header = Scapegoat Inspection Reporter{ Unparsed( @@ -79,7 +81,7 @@ object HtmlReportWriter { - def body(reporter: Feedback) = + private def body(reporter: Feedback) =

Scapegoat Inspections

@@ -92,7 +94,7 @@ object HtmlReportWriter {

{warnings(reporter)} - def warnings(reporter: Feedback) = { + private def warnings(reporter: Feedback) = { reporter.warningsWithMinimalLevel.map { warning => val source = warning.sourceFileNormalized + ":" + warning.line
@@ -125,8 +127,10 @@ object HtmlReportWriter { } } - def generate(reporter: Feedback) = + private def toHTML(reporter: Feedback) = {header}{body(reporter)} + + override protected def generate(feedback: Feedback): String = toHTML(feedback).toString() } diff --git a/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala b/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala index 6002fe7d..6232c28d 100644 --- a/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala +++ b/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala @@ -1,45 +1,22 @@ package com.sksamuel.scapegoat.io -import java.io.{BufferedWriter, File, FileWriter} +import java.io.File import com.sksamuel.scapegoat.Feedback /** * @author Stephen Samuel - * @author Eugene Sypachev (Axblade) */ object IOUtils { + def writeHTMLReport(targetDir: File, reporter: Feedback): File = + HtmlReportWriter.write(targetDir, reporter) - private val XmlFile = "scapegoat.xml" - private val ScalastyleXmlFile = "scapegoat-scalastyle.xml" - private val HtmlFile = "scapegoat.html" + def writeXMLReport(targetDir: File, reporter: Feedback): File = + XmlReportWriter.write(targetDir, reporter) - def serialize(file: File, str: String) = { - val out = new BufferedWriter(new FileWriter(file)) - out.write(str) - out.close() - } - - def writeHTMLReport(targetDir: File, reporter: Feedback): File = { - val html = HtmlReportWriter.generate(reporter).toString() - writeFile(targetDir, html, HtmlFile) - } - - def writeXMLReport(targetDir: File, reporter: Feedback): File = { - val xml = XmlReportWriter.toXML(reporter).toString() - writeFile(targetDir, xml, XmlFile) - } - - def writeScalastyleReport(targetDir: File, reporter: Feedback): File = { - val xml = ScalastyleReportWriter.toXML(reporter).toString() - writeFile(targetDir, xml, ScalastyleXmlFile) - } - - private def writeFile(targetDir: File, data: String, fileName: String) = { - targetDir.mkdirs() - val file = new File(targetDir.getAbsolutePath + "/" + fileName) - serialize(file, data) - file - } + def writeScalastyleReport(targetDir: File, reporter: Feedback): File = + ScalastyleReportWriter.write(targetDir, reporter) + def writeMarkdownReport(targetDir: File, reporter: Feedback): File = + MarkdownReportWriter.write(targetDir, reporter) } diff --git a/src/main/scala/com/sksamuel/scapegoat/io/MarkdownReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/MarkdownReportWriter.scala new file mode 100644 index 00000000..27463a54 --- /dev/null +++ b/src/main/scala/com/sksamuel/scapegoat/io/MarkdownReportWriter.scala @@ -0,0 +1,43 @@ +package com.sksamuel.scapegoat.io + +import com.sksamuel.scapegoat.{Feedback, Levels, Warning} + +object MarkdownReportWriter extends ReportWriter { + override protected def fileName: String = "scapegoat.md" + + override protected def generate(reporter: Feedback): String = { + s"""# Scapegoat Inspections + | + |**Errors**: ${reporter.warnings(Levels.Error).size.toString} + | + |**Warnings**: ${reporter.warnings(Levels.Warning).size.toString} + | + |**Infos**: ${reporter.warnings(Levels.Info).size.toString} + | + |## Report + | + |${renderAll(reporter)} + |""".stripMargin + } + + private def renderAll(reporter: Feedback): String = + reporter.warningsWithMinimalLevel.map(renderWarning).mkString("\n") + + private def renderWarning(warning: Warning): String = { + val source = warning.sourceFileNormalized + ":" + warning.line + val md = + s"""### $source + | + |**Level**: ${warning.level.toString} + | + |**Inspection**: ${warning.inspection} + | + |${warning.text} + | + |${warning.explanation} + | + |${warning.snippet.map(snippet => s"\n```scala\n$snippet\n```").getOrElse("")} + |""".stripMargin + md + } +} diff --git a/src/main/scala/com/sksamuel/scapegoat/io/ReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/ReportWriter.scala new file mode 100644 index 00000000..bbf501e8 --- /dev/null +++ b/src/main/scala/com/sksamuel/scapegoat/io/ReportWriter.scala @@ -0,0 +1,25 @@ +package com.sksamuel.scapegoat.io + +import java.io.{BufferedWriter, File, FileWriter} + +import com.sksamuel.scapegoat.Feedback + +trait ReportWriter { + + protected def fileName: String + + protected def generate(feedback: Feedback): String + + private def serialize(file: File, str: String): Unit = { + val out = new BufferedWriter(new FileWriter(file)) + out.write(str) + out.close() + } + + def write(targetDir: File, feedback: Feedback): File = { + targetDir.mkdirs() + val file = new File(s"${targetDir.getAbsolutePath}/$fileName") + serialize(file, generate(feedback)) + file + } +} diff --git a/src/main/scala/com/sksamuel/scapegoat/io/ScalastyleReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/ScalastyleReportWriter.scala index cde1bf83..112a5175 100644 --- a/src/main/scala/com/sksamuel/scapegoat/io/ScalastyleReportWriter.scala +++ b/src/main/scala/com/sksamuel/scapegoat/io/ScalastyleReportWriter.scala @@ -4,19 +4,19 @@ import scala.xml.Node import com.sksamuel.scapegoat.{Feedback, Warning} -/** @author Eugene Sypachev (Axblade) */ -object ScalastyleReportWriter { +object ScalastyleReportWriter extends ReportWriter { private val checkstyleVersion = "5.0" private val scapegoat = "scapegoat" - def toXML(feedback: Feedback): Node = { + override protected val fileName = "scapegoat-scalastyle.xml" + + private def toXML(feedback: Feedback): Node = {feedback.warningsWithMinimalLevel.groupBy(_.sourceFileFull).map(fileToXml)} - } - private def fileToXml(fileWarningMapEntry: (String, Seq[Warning])) = { + private def fileToXml(fileWarningMapEntry: (String, Seq[Warning])): Node = { val (file, warnings) = fileWarningMapEntry {warnings.map(warningToXml)} @@ -28,4 +28,5 @@ object ScalastyleReportWriter { warning.inspection } snippet={warning.snippet.orNull} explanation={warning.explanation}> + override protected def generate(feedback: Feedback): String = toXML(feedback).toString() } diff --git a/src/main/scala/com/sksamuel/scapegoat/io/XmlReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/XmlReportWriter.scala index a686fd70..e7b46e7c 100644 --- a/src/main/scala/com/sksamuel/scapegoat/io/XmlReportWriter.scala +++ b/src/main/scala/com/sksamuel/scapegoat/io/XmlReportWriter.scala @@ -5,9 +5,11 @@ import scala.xml.Node import com.sksamuel.scapegoat.{Feedback, Warning} /** @author Stephen Samuel */ -object XmlReportWriter { +object XmlReportWriter extends ReportWriter { - def toXML(feedback: Feedback): Node = { + override protected val fileName = "scapegoat.xml" + + private def toXML(feedback: Feedback): Node = { @@ -19,4 +21,6 @@ object XmlReportWriter { + + override protected def generate(feedback: Feedback): String = toXML(feedback).toString() } diff --git a/src/main/scala/com/sksamuel/scapegoat/plugin.scala b/src/main/scala/com/sksamuel/scapegoat/plugin.scala index dbba6c70..ab63936b 100644 --- a/src/main/scala/com/sksamuel/scapegoat/plugin.scala +++ b/src/main/scala/com/sksamuel/scapegoat/plugin.scala @@ -58,14 +58,17 @@ class ScapegoatPlugin(val global: Global) extends Plugin { case "xml" => component.disableXML = false case "html" => component.disableHTML = false case "scalastyle" => component.disableScalastyleXML = false + case "markdown" => component.disableMarkdown = false case "all" => component.disableXML = false component.disableHTML = false component.disableScalastyleXML = false + component.disableMarkdown = false case "none" => component.disableXML = true component.disableHTML = true component.disableScalastyleXML = true + component.disableMarkdown = true case _ => } case None => @@ -122,7 +125,7 @@ class ScapegoatPlugin(val global: Global) extends Plugin { "-P:scapegoat:verbose: enable/disable verbose console messages", "-P:scapegoat:consoleOutput: enable/disable console report output", "-P:scapegoat:reports: colon separated list of reports to generate.", - " Valid options are `xml', `html', `scalastyle',", + " Valid options are `xml', `html', `scalastyle', 'markdown',", " or `all'. Use `none' to disable reports.", "-P:scapegoat:overrideLevels: override the built in warning levels, e.g. to", " downgrade a Error to a Warning.", @@ -164,6 +167,7 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection]) var disableXML = true var disableHTML = true var disableScalastyleXML = true + var disableMarkdown = true var customInpections: Seq[Inspection] = Nil var sourcePrefix = "src/main/scala/" var minimalLevel: Level = Levels.Info @@ -186,6 +190,14 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection]) } lazy val feedback = new Feedback(consoleOutput, global.reporter, sourcePrefix, minimalLevel) + def writeReport(isDisabled: Boolean, reportName: String, writer: (File, Feedback) => File): Unit = { + if (!isDisabled) { + val output = writer(dataDir, feedback) + if (verbose) + reporter.echo(s"[info] [scapegoat] Written $reportName report [$output]") + } + } + override def newPhase(prev: scala.tools.nsc.Phase): Phase = new Phase(prev) { override def run(): Unit = { @@ -208,21 +220,10 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection]) ) } - if (!disableHTML) { - val html = IOUtils.writeHTMLReport(dataDir, feedback) - if (verbose) - reporter.echo(s"[info] [scapegoat] Written HTML report [$html]") - } - if (!disableXML) { - val xml = IOUtils.writeXMLReport(dataDir, feedback) - if (verbose) - reporter.echo(s"[info] [scapegoat] Written XML report [$xml]") - } - if (!disableScalastyleXML) { - val xml = IOUtils.writeScalastyleReport(dataDir, feedback) - if (verbose) - reporter.echo(s"[info] [scapegoat] Written Scalastyle XML report [$xml]") - } + writeReport(disableHTML, "HTML", IOUtils.writeHTMLReport) + writeReport(disableXML, "XML", IOUtils.writeXMLReport) + writeReport(disableScalastyleXML, "Scalastyle XML", IOUtils.writeScalastyleReport) + writeReport(disableMarkdown, "Markdown", IOUtils.writeMarkdownReport) } } } diff --git a/src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala b/src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala index af7390ac..e4ffc1f0 100644 --- a/src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala @@ -147,7 +147,8 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit val feedback = new Feedback(false, reporter, defaultSourcePrefix, Levels.Warning) inspections.foreach(inspection => feedback.warn(position, inspection)) feedback.warningsWithMinimalLevel.length should be(2) - feedback.warningsWithMinimalLevel.map(_.level) should contain only (Seq(Levels.Warning, Levels.Error): _*) + feedback.warningsWithMinimalLevel + .map(_.level) should contain only (Seq(Levels.Warning, Levels.Error): _*) } "for `error`" in { diff --git a/src/test/scala/com/sksamuel/scapegoat/PluginRunner.scala b/src/test/scala/com/sksamuel/scapegoat/PluginRunner.scala index 51afd862..2d0d1709 100644 --- a/src/test/scala/com/sksamuel/scapegoat/PluginRunner.scala +++ b/src/test/scala/com/sksamuel/scapegoat/PluginRunner.scala @@ -67,10 +67,8 @@ trait PluginRunner { val jarPath = sbtHome + "/cache/" + groupId + "/" + artifactId + "/jars/" + artifactId + "-" + version + ".jar" val file = new File(jarPath) - if (file.exists) { - // println(s"Located ivy jar [$file]") - file - } else throw new FileNotFoundException(s"Could not locate [$jarPath].") + if (file.exists) file + else throw new FileNotFoundException(s"Could not locate [$jarPath].") } def sbtCompileDir: File = { diff --git a/src/test/scala/com/sksamuel/scapegoat/inspections/FinalModifierOnCaseClassTest.scala b/src/test/scala/com/sksamuel/scapegoat/inspections/FinalModifierOnCaseClassTest.scala index 89457a77..6db0bebb 100644 --- a/src/test/scala/com/sksamuel/scapegoat/inspections/FinalModifierOnCaseClassTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/inspections/FinalModifierOnCaseClassTest.scala @@ -8,7 +8,7 @@ class FinalModifierOnCaseClassTest extends InspectionTest { private def assertFinalModOnCaseClass(code: String): Unit = { compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 - compiler.scapegoat.feedback.warnings.head.text shouldBe ("Missing final modifier on case class") + compiler.scapegoat.feedback.warnings.head.text shouldBe "Missing final modifier on case class" } private def assertNoFinalModOnCaseClass(code: String): Unit = { diff --git a/src/test/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElseTest.scala b/src/test/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElseTest.scala index 682a26f1..61eb54e1 100644 --- a/src/test/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElseTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElseTest.scala @@ -8,7 +8,7 @@ class MapGetAndGetOrElseTest extends InspectionTest { private def getOrElseAssertion(code: String): Unit = { compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 - compiler.scapegoat.feedback.warnings.head.text shouldBe ("Use of Map.get().getOrElse instead of Map.getOrElse") + compiler.scapegoat.feedback.warnings.head.text shouldBe "Use of Map.get().getOrElse instead of Map.getOrElse" } "Map with get followed by getOrElse" - {