Skip to content

Commit

Permalink
Replace silencer plugin with built-in warning configuration (#12543)
Browse files Browse the repository at this point in the history
Since Scala 2.13.2, Scala introduced built-in support to
manage warnings in a more granular fashion, thus making
the silencer plugin we are currently using no longer
strictly useful. Removing compiler plugins also removes
friction from migrating to Scala 3 in the future. As a
cherry on top, the built-in warning configuration also
allows to check whether a `@nowarn` actually does
anything, allowing us to proactively remove unused
warnings should the need arise.

[Here][1] is s a blog post by the Scala team about it.

Warnings have been either solved or preserved if useful,
trying to minimize the scope (keeping it at the single
expression scope if possible). In particular, all
remaining usages of the Scala Collection API compatibility
module have been removed.

Using the silencer plugin also apparently hid a few
remaining usages of compatibility libraries that were used
as part of the transition from Scala 2.12 to Scala 2.13
that are no longer needed. Removing those warnings
highlighted those.

changelog_begin
changelog_end

[1]: https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html
  • Loading branch information
stefanobaghino-da authored Jan 24, 2022
1 parent 93cfe04 commit aec3390
Show file tree
Hide file tree
Showing 122 changed files with 58 additions and 455 deletions.
39 changes: 7 additions & 32 deletions bazel_tools/scala.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,9 @@ def resolve_scala_deps(deps, scala_deps = [], versioned_deps = {}, versioned_sca
versioned_scala_deps.get(scala_major_version, [])
]

def extra_scalacopts(scala_deps, plugins):
return (["-P:silencer:lineContentFilters=import scala.collection.compat._"] if (scala_major_version != "2.12" and
silencer_plugin in plugins and
"@maven//:org_scala_lang_modules_scala_collection_compat" in scala_deps) else [])

# Please don't remove, this will be useful in the future to transition to Scala 3
version_specific = {
"2.12": [
# these two flags turn on source-incompatible enhancements that are always
# on in Scala 2.13. Despite the naming, though, the most impactful and
# 2.13-like change is -Ypartial-unification. -Xsource:2.13 only turns on
# some minor, but in one specific case (scala/bug#10283) essential bug fixes
"-Xsource:2.13",
"-Ypartial-unification",
# adapted args is a deprecated feature:
# `def foo(a: (A, B))` can be called with `foo(a, b)`.
# properly it should be `foo((a,b))`
"-Yno-adapted-args",
"-Xlint:unsound-match",
"-Xlint:by-name-right-associative", # will never be by-name if used correctly
"-Xfuture",
"-language:higherKinds",
],
"2.13": [],
}

common_scalacopts = version_specific.get(scala_major_version, []) + [
Expand Down Expand Up @@ -96,8 +77,7 @@ common_scalacopts = version_specific.get(scala_major_version, []) + [
# Gives a warning for functions declared as returning Unit, but the body returns a value
"-Ywarn-value-discard",
"-Ywarn-unused:imports",
# Allow `@nowarn` annotations that allegedly do nothing (necessary because of false positives)
"-Ywarn-unused:-nowarn",
"-Ywarn-unused:nowarn",
"-Ywarn-unused",
]

Expand All @@ -109,12 +89,9 @@ common_plugins = [
"@maven//:org_wartremover_wartremover_{}".format(scala_version_suffix),
]

# Please don't remove, this will be useful in the future to transition to Scala 3
version_specific_warts = {
"2.12": [
# On 2.13, this also triggers in string interpolation
# https://github.com/wartremover/wartremover/issues/447
"StringPlusAny",
],
"2.13": [],
}

plugin_scalacopts = [
Expand Down Expand Up @@ -233,10 +210,9 @@ def _wrap_rule(
exports = resolve_scala_deps(exports, scala_exports)
if (len(exports) > 0):
kwargs["exports"] = exports
compat_scalacopts = extra_scalacopts(scala_deps = scala_deps, plugins = plugins)
rule(
name = name,
scalacopts = common_scalacopts + plugin_scalacopts + compat_scalacopts + scalacopts,
scalacopts = common_scalacopts + plugin_scalacopts + scalacopts,
plugins = common_plugins + plugins,
deps = deps,
runtime_deps = runtime_deps,
Expand Down Expand Up @@ -542,12 +518,11 @@ def _create_scaladoc_jar(
# Limit execution to Linux and MacOS
if is_windows == False:
deps = resolve_scala_deps(deps, scala_deps, versioned_deps, versioned_scala_deps)
compat_scalacopts = extra_scalacopts(scala_deps = scala_deps, plugins = plugins)
scaladoc_jar(
name = name + "_scaladoc",
deps = deps,
srcs = srcs,
scalacopts = common_scalacopts + plugin_scalacopts + compat_scalacopts + scalacopts,
scalacopts = common_scalacopts + plugin_scalacopts + scalacopts,
plugins = common_plugins + plugins,
generated_srcs = generated_srcs,
tags = ["scaladoc"],
Expand Down
6 changes: 1 addition & 5 deletions compiler/scenario-service/server/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright (c) 2022 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

load("//bazel_tools:scala.bzl", "da_scala_binary", "silencer_plugin")
load("//bazel_tools:scala.bzl", "da_scala_binary")

genrule(
name = "scenario_service_jar",
Expand All @@ -15,12 +15,8 @@ da_scala_binary(
name = "scenario-service-raw",
srcs = glob(["src/main/scala/**/*.scala"]),
main_class = "com.daml.lf.scenario.ScenarioServiceMain",
plugins = [
silencer_plugin,
],
resources = glob(["src/main/resources/*"]),
scala_deps = [
"@maven//:org_scala_lang_modules_scala_collection_compat",
"@maven//:com_typesafe_scala_logging_scala_logging",
"@maven//:org_scalaz_scalaz_core",
"@maven//:com_github_scopt_scopt",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import com.daml.lf.engine.script.{Participants, Runner, Script, ScriptF, ScriptI

import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.collection.compat._
import scala.collection.immutable.HashMap
import scala.util.{Failure, Success}

Expand Down
5 changes: 0 additions & 5 deletions daml-lf/archive/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ load(
"da_scala_test_suite",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)
load("//daml-lf/language:daml-lf.bzl", "LF_MAJOR_VERSIONS", "PROTO_LF_VERSIONS", "mangle_for_java", "versions")
load(
Expand Down Expand Up @@ -96,12 +95,8 @@ da_haskell_library(
da_scala_library(
name = "daml_lf_archive_reader",
srcs = glob(["src/main/scala/**/*.scala"]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:org_scalaz_scalaz_core",
"@maven//:org_scala_lang_modules_scala_collection_compat",
],
scalacopts = lf_scalacopts_stricter,
tags = ["maven_coordinates=com.daml:daml-lf-archive-reader:__VERSION__"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import com.daml.nameof.NameOf
import com.daml.scalautil.Statement.discard

import scala.Ordering.Implicits.infixOrderingOps
import scala.collection.compat._
import scala.collection.mutable
import scala.jdk.CollectionConverters._

Expand Down
15 changes: 2 additions & 13 deletions daml-lf/data/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ load(
"kind_projector_plugin",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)

da_scala_library(
Expand All @@ -17,14 +16,11 @@ da_scala_library(
glob(["src/main/scala/**/*.scala"]),
plugins = [
kind_projector_plugin,
silencer_plugin,
],
scala_deps = [
"@maven//:org_scalaz_scalaz_core",
],
scalacopts = lf_scalacopts_stricter + [
"-P:silencer:lineContentFilters=import ImmArraySeq.Implicits._",
],
scalacopts = lf_scalacopts_stricter,
tags = ["maven_coordinates=com.daml:daml-lf-data:__VERSION__"],
visibility = [
"//visibility:public",
Expand All @@ -41,20 +37,13 @@ da_scala_library(
da_scala_test(
name = "data-test",
size = "small",
srcs = glob(["src/test/**/*.scala"]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:org_scalacheck_scalacheck",
"@maven//:org_scalatestplus_scalacheck_1_15",
"@maven//:org_scalaz_scalaz_core",
"@maven//:org_scalaz_scalaz_scalacheck_binding",
],
scalacopts = lf_scalacopts + [
"-P:silencer:lineContentFilters=import ImmArraySeq.Implicits._",
"-P:silencer:lineContentFilters=signum",
],
scalacopts = lf_scalacopts,
deps = [
":data",
"//daml-lf/data-scalacheck",
Expand Down
5 changes: 0 additions & 5 deletions daml-lf/interface/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,13 @@ load(
"da_scala_test",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)

da_scala_library(
name = "interface",
srcs = glob(["src/main/**/*.scala"]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:org_scalaz_scalaz_core",
"@maven//:org_scala_lang_modules_scala_collection_compat",
],
scalacopts = lf_scalacopts_stricter,
tags = ["maven_coordinates=com.daml:daml-lf-interface:__VERSION__"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package reader
import com.daml.lf.data.Ref.{DottedName, Name}

import scala.language.implicitConversions
import scala.collection.compat._
import scala.collection.immutable.Map
import scalaz.{-\/, ==>>, @@, Applicative, Cord, Monoid, Order, Semigroup, Tag, Traverse, \/, \/-}
import scalaz.std.map._
Expand Down
8 changes: 1 addition & 7 deletions daml-lf/language/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ load(
"da_scala_test",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)

da_scala_library(
Expand All @@ -29,12 +28,7 @@ da_scala_test(
name = "language-test",
size = "small",
srcs = glob(["src/test/**/*.scala"]),
plugins = [
silencer_plugin,
],
scalacopts = lf_scalacopts + [
"-P:silencer:lineContentFilters=signum",
],
scalacopts = lf_scalacopts,
deps = [
":language",
"//daml-lf/data",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class LanguageVersionSpec extends AnyWordSpec with Matchers with TableDrivenProp
forEvery(versions)(v2 =>
LV.Ordering
.compare(v1, v2)
.signum shouldBe (versionRank(v1) compareTo versionRank(v2)).signum
.sign shouldBe (versionRank(v1) compareTo versionRank(v2)).sign
)
)
}
Expand Down
8 changes: 1 addition & 7 deletions daml-lf/parser/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,16 @@ load(
"da_scala_test",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)

da_scala_library(
name = "parser",
srcs = glob(["src/main/**/*.scala"]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:org_scala_lang_modules_scala_parser_combinators",
"@maven//:org_scalaz_scalaz_core",
],
scalacopts = lf_scalacopts_stricter + [
"-P:silencer:lineContentFilters=standardInterpolator",
],
scalacopts = lf_scalacopts_stricter,
visibility = [
"//daml-lf:__subpackages__",
"//ledger:__subpackages__",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ object Implicits {

@SuppressWarnings(Array("org.wartremover.warts.Any"))
def n(args: Any*): Ref.Name =
Ref.Name.assertFromString(sc.standardInterpolator(identity, args.map(prettyPrint)))
Ref.Name.assertFromString(
StringContext.standardInterpolator(identity, args.map(prettyPrint), sc.parts)
)

@SuppressWarnings(Array("org.wartremover.warts.Any"))
private def interpolate[T](p: Parsers.Parser[T])(args: Seq[Any]): T =
Parsers.parseAll(Parsers.phrase(p), sc.standardInterpolator(identity, args.map(prettyPrint)))
Parsers.parseAll(
Parsers.phrase(p),
StringContext.standardInterpolator(identity, args.map(prettyPrint), sc.parts),
)
}

private def toString(x: BigDecimal) =
Expand Down
8 changes: 1 addition & 7 deletions daml-lf/transaction-test-lib/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@ load(
"da_scala_library",
"kind_projector_plugin",
"lf_scalacopts_stricter",
"silencer_plugin",
)

da_scala_library(
name = "transaction-test-lib",
srcs = glob(["src/main/**/*.scala"]),
plugins = [
kind_projector_plugin,
silencer_plugin,
],
scala_deps = [
"@maven//:com_chuusai_shapeless",
Expand All @@ -23,11 +21,7 @@ da_scala_library(
"@maven//:org_scalaz_scalaz_scalacheck_binding",
"@maven//:org_scala_lang_modules_scala_collection_compat",
],
scalacopts = lf_scalacopts_stricter + [
"-P:silencer:lineContentFilters=import elt.injshrink",
# Forced upon us by Shrink
"-P:silencer:lineContentFilters=Stream.empty",
],
scalacopts = lf_scalacopts_stricter,
tags = ["maven_coordinates=com.daml:daml-lf-transaction-test-lib:__VERSION__"],
visibility = ["//visibility:public"],
deps = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ object TypedValueGenerators {
Tag unsubst implicitly[Arbitrary[Vector[elt.Inj] @@ Div3]]
}
override def injshrink(implicit shr: Shrink[Value.ContractId]) = {
import elt.injshrink
implicitly[Shrink[Vector[elt.Inj]]]
}
}
Expand All @@ -169,7 +168,6 @@ object TypedValueGenerators {
implicitly[Arbitrary[Option[elt.Inj]]]
}
override def injshrink(implicit cid: Shrink[Value.ContractId]) = {
import elt.injshrink
implicitly[Shrink[Option[elt.Inj]]]
}
}
Expand Down Expand Up @@ -306,7 +304,7 @@ object TypedValueGenerators {
override def injshrink(implicit shr: Shrink[Value.ContractId]) =
Shrink { ev =>
if (!(values.headOption contains ev)) values.headOption.toStream
else Stream.empty
else Stream.empty: @annotation.nowarn("cat=deprecation")
}
},
)
Expand Down
12 changes: 0 additions & 12 deletions daml-lf/transaction/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ load(
"da_scala_test",
"lf_scalacopts",
"lf_scalacopts_stricter",
"silencer_plugin",
)

#
Expand Down Expand Up @@ -47,12 +46,8 @@ proto_jars(
da_scala_library(
name = "transaction",
srcs = glob(["src/main/**/*.scala"]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:org_scalaz_scalaz_core",
"@maven//:org_scala_lang_modules_scala_collection_compat",
],
scalacopts = lf_scalacopts_stricter,
tags = ["maven_coordinates=com.daml:daml-lf-transaction:__VERSION__"],
Expand All @@ -77,16 +72,12 @@ da_scala_test(
"src/test/**/value/*.scala",
"src/test/**/transaction/*.scala",
]),
plugins = [
silencer_plugin,
],
scala_deps = [
"@maven//:com_chuusai_shapeless",
"@maven//:org_scalacheck_scalacheck",
"@maven//:org_scalatestplus_scalacheck_1_15",
"@maven//:org_scalaz_scalaz_core",
"@maven//:org_scalaz_scalaz_scalacheck_binding",
"@maven//:org_scala_lang_modules_scala_collection_compat",
],
scalacopts = lf_scalacopts,
deps = [
Expand All @@ -107,9 +98,6 @@ da_scala_test(
srcs = glob([
"src/test/**/validation/*.scala",
]),
plugins = [
silencer_plugin,
],
scalacopts = lf_scalacopts,
deps = [
":transaction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package com.daml.lf

import scala.collection.compat._
import scala.collection.BuildFrom

package object transaction {

Expand Down
Loading

0 comments on commit aec3390

Please sign in to comment.