Skip to content

Commit

Permalink
Enable -Xlint and -Werror
Browse files Browse the repository at this point in the history
Linting on 2.13 such as required empty args,
inferred Any in sameElements of Array[_],
Timeout is double-valued, avoid Any.equals,
some elements unused, deprecations.
Allow JavaConverters.

Can't use nowarn and unused annotations
under 2.11/2.12.
  • Loading branch information
som-snytt committed Dec 28, 2021
1 parent dd5d392 commit eff270d
Show file tree
Hide file tree
Showing 18 changed files with 46 additions and 29 deletions.
29 changes: 20 additions & 9 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ mimaPreviousArtifacts := Set.empty
crossScalaVersions := List()
addCommandAlias(
"scalafixAll",
"; ++2.12.10 ; scalafixEnable ; all scalafix test:scalafix"
s"; ++$scala212 ; scalafixEnable ; all scalafix test:scalafix"
)
addCommandAlias(
"scalafixCheckAll",
"; ++2.12.10 ; scalafixEnable ; scalafix --check ; test:scalafix --check"
s"; ++$scala212 ; scalafixEnable ; scalafix --check ; test:scalafix --check"
)
val isPreScala213 = Set[Option[(Long, Long)]](Some((2, 11)), Some((2, 12)))
val scala2Versions = List(scala213, scala212, scala211)
Expand Down Expand Up @@ -152,17 +152,28 @@ val sharedSettings = List(
"-Ywarn-unused-import",
"-target:jvm-1.8"
)
case Some((major, _)) if major != 2 =>
case Some((2, 12)) =>
List(
"-language:implicitConversions"
"-feature",
"-target:jvm-1.8",
"-Ywarn-unused-import",
"-Yrangepos"
)
case _ =>
case Some((2, _)) =>
List(
"-feature",
"-target:jvm-1.8",
"-Yrangepos",
// -Xlint is unusable because of
// https://github.com/scala/bug/issues/10448
"-Ywarn-unused:imports"
"-Wconf:cat=deprecation&msg=JavaConverters:s",
raw"-Wconf:cat=deprecation&site=munit\.internal\.MacroCompatScala2\..*:s",
"-Wconf:cat=unused-privates&msg=never used:s",
"-Wconf:cat=unused-params&msg=never used:s",
//"-Werror",
"-Xlint",
"-Yrangepos"
)
case _ =>
List(
"-language:implicitConversions"
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion docs/tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def check[T](

check("basic", List(1, 2), Some(1))
check("empty", List(), Some(1))
check("null", List(null, 2), Some(null))
check("null", List(null, "more"), Some(null))
```

When declaring tests in a helper function, it's useful to pass around an
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package munit

import language.implicitConversions

import org.scalacheck.Prop
import org.scalacheck.{Test => ScalaCheckTest}
import org.scalacheck.util.Pretty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ object PlatformCompat {
ec: ExecutionContext
): Future[T] = {
val onComplete = Promise[T]()
val timeoutHandle = timers.setTimeout(duration.toMillis) {
val timeoutHandle = timers.setTimeout(duration.toMillis.toDouble) {
onComplete.tryFailure(
new TimeoutException(s"test timed out after $duration")
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ final class JUnitEvent(
val throwable: OptionalThrowable = new OptionalThrowable,
val duration: Long = -1L
) extends Event {
def fingerprint: Fingerprint = taskDef.fingerprint
def fingerprint(): Fingerprint = taskDef.fingerprint()
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ final class JUnitReporter(
throwable: OptionalThrowable = new OptionalThrowable
): Unit = {
val testName =
taskDef.fullyQualifiedName + "." +
taskDef.fullyQualifiedName() + "." +
settings.decodeName(method)
val selector = new TestSelector(testName)
eventHandler.handle(
Expand Down Expand Up @@ -215,7 +215,7 @@ final class JUnitReporter(

private def findTestFileName(trace: Array[StackTraceElement]): String =
trace
.find(_.getClassName == taskDef.fullyQualifiedName)
.find(_.getClassName == taskDef.fullyQualifiedName())
.map(_.getFileName)
.orNull

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final class JUnitRunner(
def done(): String = ""

def serializeTask(task: Task, serializer: TaskDef => String): String =
serializer(task.taskDef)
serializer(task.taskDef())

def deserializeTask(task: String, deserializer: String => TaskDef): Task =
new JUnitTask(deserializer(task), runSettings, classLoader)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ final class JUnitTask(
classLoader: ClassLoader
) extends Task {

def tags: Array[String] = Array.empty
def tags(): Array[String] = Array.empty

def execute(
eventHandler: EventHandler,
Expand Down
2 changes: 2 additions & 0 deletions munit/shared/src/main/scala-2.13/munit/internal/Compat.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package munit.internal

import language.reflectiveCalls

object Compat {
type LazyList[+T] = scala.LazyList[T]
val LazyList = scala.LazyList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package munit.internal

import munit.Clue
import munit.Location
import scala.language.implicitConversions
import scala.language.experimental.macros
import scala.reflect.macros.blackbox.Context

Expand Down
2 changes: 1 addition & 1 deletion munit/shared/src/main/scala/munit/Assertions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
if (obtained != expected) {

(obtained, expected) match {
case (a: Array[_], b: Array[_]) if a.sameElements(b) =>
case (a: Array[_], b: Array[_]) if a.sameElements[Any](b) =>
// Special-case error message when comparing arrays. See
// https://github.com/scalameta/munit/pull/393 and
// https://github.com/scalameta/munit/issues/339 for a related
Expand Down
7 changes: 3 additions & 4 deletions munit/shared/src/main/scala/munit/MUnitRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class MUnitRunner(val cls: Class[_ <: Suite], newInstance: () => Suite)
with Configurable {

def this(cls: Class[_ <: Suite]) =
this(MUnitRunner.ensureEligibleConstructor(cls), () => cls.newInstance())
this(MUnitRunner.ensureEligibleConstructor(cls), () => cls.getDeclaredConstructor().newInstance())

val suite: Suite = newInstance()

Expand Down Expand Up @@ -90,7 +90,7 @@ class MUnitRunner(val cls: Class[_ <: Suite], newInstance: () => Suite)
val desc = Description.createTestDescription(
cls,
testName,
test.annotations: _*
test.annotations.toIndexedSeq: _*
)
desc
}
Expand Down Expand Up @@ -243,7 +243,6 @@ class MUnitRunner(val cls: Class[_ <: Suite], newInstance: () => Suite)
test: Test
): Future[BeforeAllResult] = {
val context = new BeforeEach(test)
val fixtures = mutable.ListBuffer.empty[AnyFixture[_]]
sequenceFutures(
munitFixtures.iterator.map(f =>
valueTransform(() => f.beforeEach(context)).map(_ => f)
Expand Down Expand Up @@ -369,7 +368,7 @@ class MUnitRunner(val cls: Class[_ <: Suite], newInstance: () => Suite)
private def foreachUnsafe(
thunks: Iterable[() => Any]
): ForeachUnsafeResult = {
var errors = mutable.ListBuffer.empty[Throwable]
val errors = mutable.ListBuffer.empty[Throwable]
val async = mutable.ListBuffer.empty[Future[Any]]
thunks.foreach { thunk =>
try {
Expand Down
2 changes: 2 additions & 0 deletions munit/shared/src/main/scala/munit/TestOptions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ object TestOptions extends TestOptionsConversions {

trait TestOptionsConversions {

import language.implicitConversions

/**
* Implicitly create a TestOptions given a test name.
* This allows writing `test("name") { ... }` even if `test` accepts a `TestOptions`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ object Diffs {
def create(obtained: String, expected: String): Diff =
new Diff(obtained, expected)

@deprecated("")
@deprecated("", since="")
def assertNoDiff(
obtained: String,
expected: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ trait Equalizer[T] {
object Equalizer {
def default[T]: Equalizer[T] = new Equalizer[T] {
override def equals(original: T, revised: T): Boolean = {
original.equals(revised)
original == revised //original.equals(revised)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import scala.concurrent.Future
import scala.concurrent.Promise

class AsyncFunFixtureOrderSuite extends FunSuite {
val latch: Promise[Unit] = Promise[Unit]
val latch: Promise[Unit] = Promise[Unit]()
var completedFromTest: Option[Boolean] = None
var completedFromTeardown: Option[Boolean] = None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ class ScalaCheckExceptionFrameworkSuites
tags = Set(OnlyJVM),
onEvent = { event =>
if (
event.throwable.isDefined &&
event.throwable().get.getCause() != null
event.throwable().isDefined() &&
event.throwable().get().getCause() != null
) {
event
.throwable()
.get
.get()
.getCause
.getStackTrace()
.take(2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ class ComparisonFailExceptionSuite extends BaseSuite {
// console still uses `munitPrint()`, which would have displayed `List("1",
// "2", "3")` instead of `List(1, 2, 3)`.
assertNoDiff(
e.getActual,
e.getActual(),
"List(1, 2, 3)"
)
assertNoDiff(
e.getExpected,
e.getExpected(),
"List(1, 2)"
)
assertEquals(e.expected, List(1, 2))
Expand Down

0 comments on commit eff270d

Please sign in to comment.