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

Adjust deprecation config after Wconf #10750

Open
wants to merge 2 commits into
base: 2.13.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 29 additions & 10 deletions src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -28,7 +28,7 @@ import scala.tools.nsc.Reporting._
import scala.util.matching.Regex

/** Provides delegates to the reporter doing the actual work.
* PerRunReporting implements per-Run stateful info tracking and reporting
* PerRunReporting implements per-Run stateful info tracking and reporting
*/
trait Reporting extends internal.Reporting { self: ast.Positions with CompilationUnits with internal.Symbols =>
def settings: Settings
Expand All @@ -53,11 +53,29 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
globalError(s"Failed to parse `-Wconf` configuration: ${settings.Wconf.value}\n${msgs.mkString("\n")}$multiHelp")
WConf(Nil)
case Right(conf) =>
// configure cat=scala3-migration if it isn't yet
val Migration = MessageFilter.Category(WarningCategory.Scala3Migration)
val boost = (settings.isScala3: @nowarn) && !conf.filters.exists(_._1.exists(_ == Migration))
if (boost) conf.copy(filters = conf.filters :+ (Migration :: Nil, Action.Error))
else conf
// default is "cat=deprecation:ws,cat=feature:ws,cat=optimizer:ws"
// under -deprecation, "cat=deprecation:w", but under -deprecation:false or -nowarn, "cat=deprecation:s"
// similarly for -feature, -Wopt (?)
val needsDefaultAdjustment = settings.deprecation.isSetByUser
val adjusted =
if (needsDefaultAdjustment || settings.nowarn.value) {
val (user, defaults) = conf.filters.splitAt(conf.filters.length - settings.WconfDefault.length)
val Deprecation = MessageFilter.Category(WarningCategory.Deprecation)
val action = if (settings.deprecation.value) Action.Warning else Action.Silent
val fixed = defaults.map {
case (cat @ Deprecation :: Nil, Action.WarningSummary) => cat -> action
case other => other
}
conf.copy(filters = user ::: fixed)
}
else conf
// configure any:s for -nowarn or cat=scala3-migration:e for -Xsource:3
def Migration = MessageFilter.Category(WarningCategory.Scala3Migration)
if (settings.nowarn.value)
adjusted.copy(filters = adjusted.filters :+ (MessageFilter.Any :: Nil, Action.Silent))
else if (settings.isScala3: @nowarn)
adjusted.copy(filters = adjusted.filters :+ (Migration :: Nil, Action.Error))
else adjusted
}

private lazy val quickfixFilters = {
Expand Down Expand Up @@ -386,19 +404,20 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
var seenMacroExpansionsFallingBack = false

// i.e., summarize warnings
def summarizeErrors(): Unit = if (!reporter.hasErrors && !settings.nowarn.value) {
for (c <- summarizedWarnings.keys.toList.sortBy(_.name))
def summarizeErrors(): Unit = if (!reporter.hasErrors) {
val warnOK = !settings.nowarn.value
if (warnOK) for (c <- summarizedWarnings.keys.toList.sortBy(_.name))
summarize(Action.WarningSummary, c)
for (c <- summarizedInfos.keys.toList.sortBy(_.name))
summarize(Action.InfoSummary, c)

if (seenMacroExpansionsFallingBack)
if (warnOK && seenMacroExpansionsFallingBack)
warning(NoPosition, "some macros could not be expanded and code fell back to overridden methods;"+
"\nrecompiling with generated classfiles on the classpath might help.", WarningCategory.Other, site = "")

// todo: migrationWarnings

if (settings.fatalWarnings.value && reporter.hasWarnings)
if (warnOK && settings.fatalWarnings.value && reporter.hasWarnings)
reporter.error(NoPosition, "No warnings can be incurred under -Werror.")
}

Expand Down
Expand Up @@ -26,7 +26,7 @@ class ConsoleReporter(val settings: Settings, val reader: BufferedReader, val wr

override def finish(): Unit = {
import reflect.internal.util.StringOps.countElementsAsString
if (!settings.nowarn.value && hasWarnings)
if (hasWarnings && !settings.nowarn.value)
writer.println(countElementsAsString(warningCount, WARNING.toString.toLowerCase))
if (hasErrors)
writer.println(countElementsAsString(errorCount, ERROR.toString.toLowerCase))
Expand Down
1 change: 0 additions & 1 deletion src/compiler/scala/tools/nsc/reporters/Reporter.scala
Expand Up @@ -60,7 +60,6 @@ object Reporter {
/** The reporter used in a Global instance.
*
* It filters messages based on
* - settings.nowarn
* - settings.maxerrs / settings.maxwarns
* - positions (only one error at a position, no duplicate messages on a position)
*/
Expand Down
23 changes: 23 additions & 0 deletions src/compiler/scala/tools/nsc/settings/MutableSettings.scala
Expand Up @@ -355,6 +355,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
private[this] var _deprecationMessage: Option[String] = None
override def deprecationMessage = _deprecationMessage
def withDeprecationMessage(msg: String): this.type = { _deprecationMessage = Some(msg) ; this }

def reset(): Unit
}

/** A setting represented by an integer. */
Expand Down Expand Up @@ -419,6 +421,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
else List(name, value.toString)

withHelpSyntax(s"$name <n>")

override def reset() = v = default
}

/** A setting that is a boolean flag, with default as specified. */
Expand All @@ -443,6 +447,10 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
} else errorAndValue(s"'$x' is not a valid choice for '$name'", None)
case _ => errorAndValue(s"'$name' accepts only one boolean value", None)
}
override def reset() = {
v = default
setByUser = false
}
}

/** A special setting for accumulating arguments like -Dfoo=bar. */
Expand All @@ -464,6 +472,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
def tryToSetColon(args: List[String]): Option[ResultOfTryToSet] = errorAndValue(s"bad argument for $name", None)
override def respondsTo(token: String) = token startsWith prefix
def unparse: List[String] = value
override def reset() = v = Nil
}

/** A setting represented by a string, (`default` unless set) */
Expand Down Expand Up @@ -495,6 +504,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
override def isHelping: Boolean = sawHelp

override def help = helpText.get
override def reset() = v = default
}

/** A setting represented by a Scala version.
Expand Down Expand Up @@ -536,6 +546,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
override def help = helpText.get

withHelpSyntax(s"${name}:<${arg}>")

override def reset() = v = initial
}

class PathSetting private[nsc](
Expand All @@ -555,6 +567,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
super.value,
appendPath.value
)
override def reset() = ()
}

/** Set the output directory for all sources. */
Expand Down Expand Up @@ -794,6 +807,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
}
def unparse: List[String] = value.toList.map(s => s"$name:$s")
def contains(s: String) = domain.values.find(_.toString == s).exists(value.contains)
override def reset() = clear()
}

/** A setting that accumulates all strings supplied to it,
Expand Down Expand Up @@ -838,6 +852,11 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
override def isHelping: Boolean = sawHelp

override def help = helpText.get

override def reset() = {
v = default
setByUser = false
}
}

/** A setting represented by a string in a given set of `choices`,
Expand Down Expand Up @@ -892,6 +911,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
override def tryToSetFromPropertyValue(s: String) = tryToSetColon(s::Nil) // used from ide

withHelpSyntax(name + ":<" + helpArg + ">")

override def reset() = v = default
}

private def mkPhasesHelp(descr: String, default: String) = {
Expand Down Expand Up @@ -972,6 +993,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
if (default == "") name + ":<phases>"
else name + "[:phases]"
)

override def reset() = clear()
}

/** Internal use - syntax enhancements. */
Expand Down
Expand Up @@ -36,10 +36,7 @@ trait StandardScalaSettings { _: MutableSettings =>
/** Other settings.
*/
val dependencyfile = StringSetting ("-dependencyfile", "file", "Set dependency tracking file.", ".scala_dependencies") withAbbreviation "--dependency-file"
val deprecation = BooleanSetting ("-deprecation", "Emit warning and location for usages of deprecated APIs. See also -Wconf.") withAbbreviation "--deprecation" withPostSetHook { s =>
if (s.value) Wconf.tryToSet(List(s"cat=deprecation:w"))
else Wconf.tryToSet(List(s"cat=deprecation:s"))
}
val deprecation = BooleanSetting ("-deprecation", "Emit warning and location for usages of deprecated APIs. See also -Wconf.").withAbbreviation("--deprecation")
val encoding = StringSetting ("-encoding", "encoding", "Specify character encoding used by source files.", Properties.sourceEncoding) withAbbreviation "--encoding"
val explaintypes = BooleanSetting ("-explaintypes", "Explain type errors in more detail.") withAbbreviation "--explain-types"
val feature = BooleanSetting ("-feature", "Emit warning and location for usages of features that should be imported explicitly. See also -Wconf.") withAbbreviation "--feature" withPostSetHook { s =>
Expand All @@ -48,7 +45,9 @@ trait StandardScalaSettings { _: MutableSettings =>
}
val g = ChoiceSetting ("-g", "level", "Set level of generated debugging info.", List("none", "source", "line", "vars", "notailcalls"), "vars")
val help = BooleanSetting ("-help", "Print a synopsis of standard options") withAbbreviation "--help" withAbbreviation("-h")
val nowarn = BooleanSetting ("-nowarn", "Generate no warnings.") withAbbreviation "--no-warnings" withPostSetHook { s => if (s.value) maxwarns.value = 0 }
val nowarn = BooleanSetting("-nowarn", "Silence warnings. (-Wconf:any:s)")
.withAbbreviation("--no-warnings")
.withPostSetHook(s => if (s.value) maxwarns.value = 0)
val optimise: BooleanSetting // depends on post hook which mutates other settings
val print = BooleanSetting ("-print", "Print program with Scala-specific features removed.") withAbbreviation "--print"
val quickfix = MultiStringSetting(
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -27,7 +27,7 @@ trait Warnings {
// Warning semantics.
val fatalWarnings = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.") withAbbreviation "-Xfatal-warnings"

private val WconfDefault = List("cat=deprecation:ws", "cat=feature:ws", "cat=optimizer:ws")
val WconfDefault = List("cat=deprecation:ws", "cat=feature:ws", "cat=optimizer:ws")
// Note: user-defined settings are added on the right, but the value is reversed before
// it's parsed, so that later defined settings take precedence.
val Wconf = MultiStringSetting(
Expand Down
4 changes: 2 additions & 2 deletions src/reflect/scala/reflect/internal/Reporting.scala
Expand Up @@ -122,8 +122,8 @@ abstract class Reporter {

private def filteredInfo(pos: Position, msg: String, severity: Severity, actions: List[CodeAction]): Unit = {
val f = filter(pos, msg, severity)
if (f <= 1) increment(severity)
if (f == 0) doReport(pos, msg, severity, actions)
if (f < Reporter.Suppress) increment(severity)
if (f == Reporter.Display) doReport(pos, msg, severity, actions)
}

def increment(severity: Severity): Unit = severity match {
Expand Down
11 changes: 11 additions & 0 deletions test/files/neg/t12664.check
@@ -0,0 +1,11 @@
t12664.scala:4: warning: side-effecting nullary methods are discouraged: suggest defining as `def f()` instead [quickfixable]
def f: Unit = 42
^
t12664.scala:4: warning: a pure expression does nothing in statement position
def f: Unit = 42
^
warning: 1 deprecation (since 1.0); re-run enabling -deprecation for details, or try -help
warning: 1 lint warning; change -Wconf for cat=lint to display individual messages
error: No warnings can be incurred under -Werror.
4 warnings
1 error
12 changes: 12 additions & 0 deletions test/files/neg/t12664.scala
@@ -0,0 +1,12 @@
//> using options -Wconf:cat=lint-missing-interpolator:ws,cat=deprecation:ws -Werror -Xlint

class C {
def f: Unit = 42

def oops = "$f"

@deprecated("old stuff", since="1.0")
def old = 17

def stale = old
}
6 changes: 6 additions & 0 deletions test/files/neg/t12984.check
@@ -0,0 +1,6 @@
t12984.scala:12: warning: class D is deprecated (since 2.0): Will be phased out eventually someday.
def d = new D
^
error: No warnings can be incurred under -Werror.
1 warning
1 error
13 changes: 13 additions & 0 deletions test/files/neg/t12984.scala
@@ -0,0 +1,13 @@

//> using options -deprecation -Wconf:cat=deprecation&origin=C:s -Werror -Xlint

@deprecated("Just say no.", since="1.0")
class C

@deprecated("Will be phased out eventually someday.", since="2.0")
class D

trait Test {
def c = new C
def d = new D
}
38 changes: 38 additions & 0 deletions test/files/pos/t12664.scala
@@ -0,0 +1,38 @@
//> using options -nowarn -Wconf:cat=lint-missing-interpolator:ws -Werror -Xlint -Xsource:3

/*
-nowarn and -Xlint are in contradiction. Which wins?
-nowarn is recognized by per-run reporting and by sink reporter (e.g., ConsoleReporter).
For per-run reporting, -nowarn means default wconf for deprecation must not be ws (summary, which would warn).
Instead, it is w or s depending on whether -deprecation is requested.
So from the perspective of per-run reporting, -deprecation means issue a diagnostic even if -nowarn.

For the sink reporter, too, -nowarn means "don't summarize with warning count".
In addition, -nowarn means -maxwarns:0, so any warning is filtered by FilteringReporter.
(Normally, displayed warnings is capped by -maxwarns and then summarized as a count when done.)
So from the perspective of the sink reporter, -nowarn means filter out warnings and don't print count of warnings.
It doesn't consider -deprecation at all.
In addition, -nowarn subverts -Werror.

In the test example, there are 2 lints, a non-lint, and a migration warning.
The wconf boosts a lint to ws summary, but -nowarn tells the sink not to print either a warning or a count.
Migration is boosted to e by default, but -nowarn says don't boost migration warnings.
A user-supplied wconf could boost migration despite -nowarn.
Other warnings are silenced by any:s installed by -nowarn.
*/
trait T {
def g[A]: Option[A]
}

class C extends T {
def f: Unit = 42 // suppressed other warning for expr, lint for parens

override def g[A] = None // suppressed migration warning, not boosted to error under --no-warnings

def oops = "$f" // summarized lint

@deprecated("old stuff", since="1.0")
def old = 17

def stale = old
}
3 changes: 1 addition & 2 deletions test/files/pos/t8410.scala
@@ -1,5 +1,4 @@
// scalac: -opt:inline:** -Wopt:none -Werror -deprecation:false
//
//> using options -opt:inline:** -Wopt:none -Werror -deprecation:false

object Test extends App {
@deprecated("","") def f = 42
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/nowarn.scala
Expand Up @@ -10,7 +10,7 @@ class Driven extends Driver {
true
}
protected def newCompiler(): Global = Global(settings, reporter)
override protected def doCompile(compiler: Global): Unit = reporter.warning(NoPosition, "I don't do anything.")
override protected def doCompile(compiler: Global): Unit = reporter.warning(NoPosition, s"I can't do anything for ${reporter.getClass}.")
def run(): Unit = process(Array("file.scala"))
}
object Test {
Expand Down