Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address deprecations in FeedbackTest #709

Merged
merged 5 commits into from
Nov 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,17 @@ crossTarget := {
ThisBuild / useCoursier := false

val scalac13Options = Seq(
"-Xlint:adapted-args",
"-Xlint:inaccessible",
"-Xlint:infer-any",
"-Xlint:nullary-unit",
"-Xlint:strict-unsealed-patmat",
"-Yrangepos",
"-Ywarn-unused",
"-Xsource:3"
)
val scalac12Options = Seq(
"-Xlint:adapted-args",
"-Ywarn-inaccessible",
"-Ywarn-infer-any",
"-Xlint:nullary-override",
"-Xlint:nullary-unit",
"-Xmax-classfile-name",
"254"
)
Expand All @@ -61,15 +57,13 @@ scalacOptions := {
"-feature",
"-encoding",
"utf8",
"-Xlint"
"-Xlint",
"-Xlint:adapted-args",
"-Xlint:nullary-unit"
)
common ++ (scalaBinaryVersion.value match {
case "2.12" => scalac12Options
case "2.13" =>
scalac13Options ++ (scalaVersion.value.split('.') match {
case Array(_, _, patch) if Set("0", "1", "2")(patch) => Seq("-Xlint:nullary-override")
case _ => Seq.empty[String]
})
case "2.13" => scalac13Options
})
}
javacOptions ++= Seq("-source", "1.8", "-target", "1.8")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.sksamuel.scapegoat.inspections.collections

import com.sksamuel.scapegoat.{Inspection, InspectionContext, Inspector, Levels}
import com.sksamuel.scapegoat.{isScala213, Inspection, InspectionContext, Inspector, Levels}

/**
* @author
Expand All @@ -15,6 +15,8 @@ class PredefIterableIsMutable
"Iterable aliases scala.collection.mutable.Iterable. Did you intend to use an immutable Iterable?"
) {

override def isEnabled: Boolean = !isScala213

def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser: context.Traverser =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.sksamuel.scapegoat.inspections.collections

import com.sksamuel.scapegoat.{Inspection, InspectionContext, Inspector, Levels}
import com.sksamuel.scapegoat.{isScala213, Inspection, InspectionContext, Inspector, Levels}

/**
* @author
Expand All @@ -15,6 +15,8 @@ class PredefTraversableIsMutable
"Traversable aliases scala.collection.mutable.Traversable. Did you intend to use an immutable Traversable?"
) {

override def isEnabled: Boolean = !isScala213

def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser: context.Traverser =
Expand Down
29 changes: 17 additions & 12 deletions src/test/scala/com/sksamuel/scapegoat/FeedbackTest.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.sksamuel.scapegoat

import scala.reflect.internal.util.NoPosition
import scala.tools.nsc.Settings
import scala.tools.nsc.reporters.StoreReporter

import org.scalatest.freespec.AnyFreeSpec
Expand Down Expand Up @@ -31,58 +32,61 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is description.",
"This is explanation."
)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix))
feedback.warn(position, inspection)
reporter.infos should contain(
reporter.Info(
StoreReporter.Info(
position,
"[scapegoat] [DummyInspection] My default is Error\n This is explanation.",
reporter.ERROR
)
)
}

"for warning" in {
val inspection = new DummyInspection(
"My default is Warning",
Levels.Warning,
"This is description.",
"This is explanation."
)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix))
feedback.warn(position, inspection)
reporter.infos should contain(
reporter.Info(
StoreReporter.Info(
position,
"[scapegoat] [DummyInspection] My default is Warning\n This is explanation.",
reporter.WARNING
)
)
}

"for info" in {
val inspection = new DummyInspection(
"My default is Info",
Levels.Info,
"This is description.",
"This is explanation."
)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix))
feedback.warn(position, inspection)
reporter.infos should contain(
reporter.Info(
StoreReporter.Info(
position,
"[scapegoat] [DummyInspection] My default is Info\n This is explanation.",
reporter.INFO
)
)
}
}

"should use proper paths" - {
"for `src/main/scala`" in {
val normalizeSourceFile = PrivateMethod[String](Symbol("normalizeSourceFile"))
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix))
val source = "src/main/scala/com/sksamuel/scapegoat/Test.scala"
val result = feedback invokePrivate normalizeSourceFile(source)
Expand All @@ -91,7 +95,7 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit

"for `app`" in {
val normalizeSourceFile = PrivateMethod[String](Symbol("normalizeSourceFile"))
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, "app/"))
val source = "app/com/sksamuel/scapegoat/Test.scala"
val result = feedback invokePrivate normalizeSourceFile(source)
Expand All @@ -100,13 +104,14 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit

"should add trailing / to the sourcePrefix automatically" in {
val normalizeSourceFile = PrivateMethod[String](Symbol("normalizeSourceFile"))
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback = new Feedback(reporter, testConfiguration(consoleOutput = true, "app/custom"))
val source = "app/custom/com/sksamuel/scapegoat/Test.scala"
val result = feedback invokePrivate normalizeSourceFile(source)
result should ===("com.sksamuel.scapegoat.Test.scala")
}
}

"should use minimal warning level in reports" - {
"for `info`" in {
val inspectionError = new DummyInspection(
Expand All @@ -128,7 +133,7 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback =
new Feedback(reporter, testConfiguration(consoleOutput = true, defaultSourcePrefix, Levels.Info))
inspections.foreach(inspection => feedback.warn(position, inspection))
Expand All @@ -155,7 +160,7 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback =
new Feedback(
reporter,
Expand Down Expand Up @@ -187,7 +192,7 @@ class FeedbackTest extends AnyFreeSpec with Matchers with OneInstancePerTest wit
"This is explanation."
)
val inspections = Seq(inspectionError, inspectionWarning, inspectionInfo)
val reporter = new StoreReporter
val reporter = new StoreReporter(new Settings())
val feedback =
new Feedback(reporter, testConfiguration(consoleOutput = false, defaultSourcePrefix, Levels.Error))
inspections.foreach(inspection => feedback.warn(position, inspection))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class BooleanParameterTest extends InspectionTest {
"should report info" - {
"for methods using Boolean parameter" in {
val code = """class Test {
def foo(bool: Boolean) = 4
} """.stripMargin
def foo(bool: Boolean) = 4
}""".stripMargin

compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,44 +1,51 @@
package com.sksamuel.scapegoat.inspections.collections

import com.sksamuel.scapegoat.InspectionTest
import com.sksamuel.scapegoat.{isScala213, Inspection, InspectionTest}

/** @author Stephen Samuel */
class PredefIterableIsMutableTest extends InspectionTest {

override val inspections = Seq(new PredefIterableIsMutable)
override val inspections: Seq[Inspection] = Seq(new PredefIterableIsMutable)

"PredefIterableIsMutable" - {
"should report warning" - {
"for Iterable apply" in {
val code = """object Test { val a = Iterable("sammy") }""".stripMargin
val expectedWarnings = if (isScala213) 0 else 1
compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 1
compiler.scapegoat.feedback.warnings.size shouldBe expectedWarnings
}

"for declaring Iterable as return type" in {
val code = """object Test { def foo : Iterable[String] = ??? }""".stripMargin
val expectedWarnings = if (isScala213) 0 else 1
compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 1
compiler.scapegoat.feedback.warnings.size shouldBe expectedWarnings
}
}

"should not report warning" - {
"for scala.collection.mutable usage" in {
val code = """import scala.collection.mutable.Iterable
|object Test { val a = Iterable("sammy") }""".stripMargin
compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 0
}

"for scala.collection.immutable usage" in {
val code = """import scala.collection.immutable.Iterable
|object Test { val a = Iterable("sammy") }""".stripMargin
compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 0
}

"for scala.collection.mutable defs" in {
val code = """import scala.collection.mutable.Iterable
|object Test { def foo : Iterable[String] = ??? }""".stripMargin
compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 0
}

"for scala.collection.immutable defs" in {
val code = """import scala.collection.immutable.Iterable
|object Test { def foo : Iterable[String] = ??? }""".stripMargin
Expand Down