Skip to content

Commit

Permalink
SKIP level (#782)
Browse files Browse the repository at this point in the history
* ignore level

* test levels

* check ignore level as minimal

* fix matches in reporters

* suppressing warnings README
  • Loading branch information
eugene-sy authored Oct 18, 2023
1 parent ecfd860 commit c9777af
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 10 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ And if you prefer a prettier report, here is a screen shot of the type of HTML r

For instructions on suppressing warnings by file, by inspection or by line see [the sbt-scapegoat README](https://github.com/sksamuel/sbt-scapegoat).

To suppress warnings globally for the project, use `disabledInspections` or `overrideLevels` flags:

```
-P:scapegoat:disabledInspections:FinalModifierOnCaseClass
-P:scapegoat:overrideLevels:PreferSeqEmpty=ignore:AsInstanceOf=ignore
```

### Inspections

There are currently 121 inspections. An overview list is given, followed by a more detailed description of each inspection after the list (todo: finish rest of detailed descriptions)
Expand Down
4 changes: 4 additions & 0 deletions src/main/scala/com/sksamuel/scapegoat/Configuration.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ object Configuration {
val sourcePrefix = fromProperty("sourcePrefix", defaultValue = "src/main/scala/")(x => x)
val minimalLevel = fromProperty[Level]("minimalLevel", defaultValue = Levels.Info) { value =>
Levels.fromName(value)
} match {
case Levels.Ignore => throw new IllegalArgumentException(s"Minimal level cannot be set to 'ignore'")
case l => l
}

val dataDir = fromProperty[Option[File]](
"dataDir",
defaultValue = None
Expand Down
8 changes: 5 additions & 3 deletions src/main/scala/com/sksamuel/scapegoat/Feedback.scala
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class Feedback(
case Levels.Error => reporter.error(pos, report)
case Levels.Warning => reporter.warning(pos, report)
case Levels.Info => reporter.echo(pos, report)
case Levels.Ignore => ()
}
}
}
Expand All @@ -93,9 +94,10 @@ final case class Warning(
) {
def hasMinimalLevelOf(minimalLevel: Level): Boolean = {
minimalLevel match {
case Levels.Info => true
case Levels.Warning => this.level == Levels.Warning || this.level == Levels.Error
case Levels.Error => this.level == Levels.Error
case Levels.Ignore => throw new IllegalArgumentException("Ignore cannot be minimal level")
case Levels.Info => this.level.higherOrEqualTo(Levels.Info)
case Levels.Warning => this.level.higherOrEqualTo(Levels.Warning)
case Levels.Error => this.level.higherOrEqualTo(Levels.Error)
}
}
}
23 changes: 19 additions & 4 deletions src/main/scala/com/sksamuel/scapegoat/Level.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ package com.sksamuel.scapegoat
* @author
* Stephen Samuel
*/
sealed trait Level
sealed trait Level {
protected def weight: Short

def higherOrEqualTo(other: Level): Boolean = this.weight >= other.weight
}

object Levels {

Expand All @@ -13,7 +17,9 @@ object Levels {
*
* An example is use of nulls. Use of nulls can lead to NullPointerExceptions and should be avoided.
*/
case object Error extends Level
case object Error extends Level {
override protected def weight: Short = 30
}

/**
* Warnings are reserved for code that has bad semantics. This by itself does not necessarily mean the code
Expand All @@ -26,7 +32,9 @@ object Levels {
* Another example is a constant if. You can do things like if (true) { } if you want, but since the block
* will always evaluate, the if statement perhaps indicates a mistake.
*/
case object Warning extends Level
case object Warning extends Level {
override protected def weight: Short = 20
}

/**
* Infos are used for code which is semantically fine, but there exists a more idomatic way of writing it.
Expand All @@ -43,13 +51,20 @@ object Levels {
*
* def foo = a
*/
case object Info extends Level
case object Info extends Level {
override protected def weight: Short = 10
}

case object Ignore extends Level {
override protected def weight: Short = 0
}

def fromName(name: String): Level =
name.toLowerCase() match {
case "error" => Error
case "warning" => Warning
case "info" => Info
case "ignore" => Ignore
case _ => throw new IllegalArgumentException(s"Unrecognised level '$name'")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ object HtmlReportWriter extends ReportWriter {
case Levels.Info => <span class="label label-info">Info</span>
case Levels.Warning => <span class="label label-warning">Warning</span>
case Levels.Error => <span class="label label-danger">Error</span>
case Levels.Ignore => ()
}
}&nbsp;{decorateCode(warning.text)}&nbsp; <span class="inspection">
{warning.inspection}
Expand Down
4 changes: 4 additions & 0 deletions src/test/scala/com/sksamuel/scapegoat/ConfigurationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,9 @@ class ConfigurationTest extends AnyFreeSpec with Matchers {
}
}
}

"throw an exception on 'ignore' as minimal level" in {
the[IllegalArgumentException] thrownBy Configuration.fromPluginOptions(List("minimalLevel:ignore"))
}
}
}
24 changes: 21 additions & 3 deletions src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,13 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val inspectionIgnored = new DummyInspection(
"My default is Ignore",
Levels.Ignore,
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo, inspectionIgnored)
val reporter = new StoreReporter(new Settings())
val feedback =
new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix, Levels.Info))
Expand All @@ -153,7 +159,13 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val inspectionIgnored = new DummyInspection(
"My default is Ignore",
Levels.Ignore,
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo, inspectionIgnored)
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(
reporter,
Expand Down Expand Up @@ -184,7 +196,13 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val inspectionIgnored = new DummyInspection(
"My default is Ignore",
Levels.Ignore,
"This is description.",
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo, inspectionIgnored)
val reporter = new StoreReporter(new Settings())
val feedback =
new Feedback(reporter, testConfiguration(consoleOutput = false, defaultSourcePrefix, Levels.Error))
Expand Down
69 changes: 69 additions & 0 deletions src/test/scala/com/sksamuel/scapegoat/LevelsTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.sksamuel.scapegoat

import org.scalatest.freespec.AnyFreeSpec
import org.scalatest.matchers.should.Matchers

class LevelsTest extends AnyFreeSpec with Matchers {
"Levels" - {
"#fromName" - {
"should return correct object" - {
"for 'error'" in {
Levels.fromName("error") should be(Levels.Error)
}

"for 'warning'" in {
Levels.fromName("warning") should be(Levels.Warning)
}

"for 'info'" in {
Levels.fromName("info") should be(Levels.Info)
}

"for 'ignore'" in {
Levels.fromName("ignore") should be(Levels.Ignore)
}
}

"throw an exception when uunknown level is provided" in {
the[IllegalArgumentException] thrownBy Levels.fromName(
"UNKNOWN"
) should have message "Unrecognised level 'UNKNOWN'"
}
}
}

"Level" - {
"#higherOrEqual" - {
"should be true for levels with higher weight" - {
val levels = Seq(Levels.Ignore, Levels.Info, Levels.Warning, Levels.Error)

"for ignore" in {
levels.map(other =>
Levels.Ignore.higherOrEqualTo(other)
) should contain theSameElementsInOrderAs Seq(true, false, false, false)
}

"for info" in {
levels.map(other => Levels.Info.higherOrEqualTo(other)) should contain theSameElementsInOrderAs Seq(
true,
true,
false,
false
)
}

"for warning" in {
levels.map(other =>
Levels.Warning.higherOrEqualTo(other)
) should contain theSameElementsInOrderAs Seq(true, true, true, false)
}

"for error" in {
levels.map(other =>
Levels.Error.higherOrEqualTo(other)
) should contain theSameElementsInOrderAs Seq(true, true, true, true)
}
}
}
}
}

0 comments on commit c9777af

Please sign in to comment.