Warn unused unused annotation

Author: som-snytt

build.sbt

@@ -489,6 +489,7 @@ lazy val compiler = configureAsSubproject(project)
     Compile / scalacOptions ++= Seq(
       "-Xlint",
       "-feature",
+      //"-Wconf:cat=unused-unused&site=scala.tools.nsc.reporters.Reporter.info.force:s",
       "-Wconf:cat=deprecation&msg=early initializers:s", // compiler heavily relies upon early initializers
     ),
     Compile / doc / scalacOptions ++= Seq(

src/compiler/scala/tools/nsc/Reporting.scala

@@ -21,6 +21,7 @@ import scala.reflect.internal.util.StringOps.countElementsAsString
 import scala.reflect.internal.util.{NoSourceFile, Position, SourceFile}
 import scala.tools.nsc.Reporting.Version.{NonParseableVersion, ParseableVersion}
 import scala.tools.nsc.Reporting._
+import scala.tools.nsc.settings.ScalaVersion
 import scala.util.matching.Regex
 
 /** Provides delegates to the reporter doing the actual work.
@@ -80,15 +81,25 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
       suppressions.getOrElse(pos.source, Nil).exists(_.annotPos.point == pos.point)
 
     def runFinished(hasErrors: Boolean): Unit = {
+      val warningNowarn = settings.warnUnusedNowarn
+      val warningUnused = settings.warnUnusedUnused && ScalaVersion.current >= ScalaVersion("2.13.11")
+      def maybeWarn(s: Suppression): Unit =
+        if (!s.used)
+          if (!s.synthetic) {
+            if (warningNowarn)
+              issueWarning(Message.Plain(s.annotPos, "@nowarn annotation does not suppress any warnings", WarningCategory.UnusedNowarn, ""))
+          }
+          else if (warningUnused && s.filters.exists { case MessageFilter.Category(WarningCategory.Unused) => true case _ => false })
+            issueWarning(Message.Plain(s.annotPos, "@unused annotation does not suppress any warnings", WarningCategory.UnusedUnused, ""))
       // report suspended messages (in case the run finished before typer)
       suspendedMessages.valuesIterator.foreach(_.foreach(issueWarning))
       // report unused nowarns only if all all phases are done. scaladoc doesn't run all phases.
-      if (!hasErrors && settings.warnUnusedNowarn && !settings.isScaladoc)
+      if (!hasErrors && (warningNowarn || warningUnused) && !settings.isScaladoc)
         for {
           source <- suppressions.keysIterator.toList
           sups   <- suppressions.remove(source)
           sup    <- sups.reverse
-        } if (!sup.used && !sup.synthetic) issueWarning(Message.Plain(sup.annotPos, "@nowarn annotation does not suppress any warnings", WarningCategory.UnusedNowarn, ""))
+        } maybeWarn(sup)
     }
 
     def reportSuspendedMessages(unit: CompilationUnit): Unit = {
@@ -257,7 +268,7 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
       } else warning(pos, msg, featureCategory(featureTrait.nameString), site)
     }
 
-    // Used in the optimizer where we don't have no symbols, the site string is created from the class internal name and method name.
+    // Used in the optimizer where we have no symbols, the site string is created from the class internal name and method name.
     def warning(pos: Position, msg: String, category: WarningCategory, site: String): Unit =
       issueIfNotSuppressed(Message.Plain(pos, msg, category, site))
 
@@ -380,6 +391,7 @@ object Reporting {
     object UnusedLocals extends Unused; add(UnusedLocals)
     object UnusedParams extends Unused; add(UnusedParams)
     object UnusedNowarn extends Unused; add(UnusedNowarn)
+    object UnusedUnused extends Unused; add(UnusedUnused)
 
     sealed trait Lint extends WarningCategory { override def summaryCategory: WarningCategory = Lint }
     object Lint extends Lint { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Lint] }; add(Lint)

src/compiler/scala/tools/nsc/ast/TreeDSL.scala

@@ -13,7 +13,6 @@
 package scala.tools.nsc
 package ast
 
-import scala.annotation.unused
 import scala.language.implicitConversions
 
 /** A DSL for generating scala code.  The goal is that the
@@ -79,13 +78,9 @@ trait TreeDSL {
       def INT_-   (other: Tree)     = fn(target, getMember(IntClass, nme.MINUS), other)
 
       // generic operations on ByteClass, IntClass, LongClass
-      @unused("avoid warning for multiple parameters")
       def GEN_|   (other: Tree, kind: ClassSymbol)  = fn(target, getMember(kind, nme.OR), other)
-      @unused("avoid warning for multiple parameters")
       def GEN_&   (other: Tree, kind: ClassSymbol)  = fn(target, getMember(kind, nme.AND), other)
-      @unused("avoid warning for multiple parameters")
       def GEN_==  (other: Tree, kind: ClassSymbol)  = fn(target, getMember(kind, nme.EQ), other)
-      @unused("avoid warning for multiple parameters")
       def GEN_!=  (other: Tree, kind: ClassSymbol)  = fn(target, getMember(kind, nme.NE), other)
 
       /** Apply, Select, Match **/

src/compiler/scala/tools/nsc/settings/ScalaVersion.scala

@@ -126,8 +126,14 @@ object ScalaVersion {
   }
 }
 
-/**
- * Represents the data after the dash in major.minor.rev-build
+/** Represents the data after the dash in "major.minor.rev-build".
+ *
+ *  Builds are ordered from Development, Milestone, RC, to Final.
+ *
+ *  Development builds are ordered on their arbitrary string ID
+ *  (which may be a timestamp and git hash, such as "20230101-003230-4d2b33e").
+ *
+ *  Other builds are numbered (M0, M1, RC0, RC1).
  */
 abstract class ScalaBuild extends Ordered[ScalaBuild] {
   /**
@@ -136,62 +142,54 @@ abstract class ScalaBuild extends Ordered[ScalaBuild] {
    */
   def unparse: String
 }
-/**
- * A development, test, integration, snapshot or other "unofficial" build
+/** A development, test, integration, snapshot or other "unofficial" build.
  */
 case class Development(id: String) extends ScalaBuild {
   def unparse = s"-${id}"
 
   def compare(that: ScalaBuild) = that match {
-    // sorting two development builds based on id is reasonably valid for two versions created with the same schema
-    // otherwise it's not correct, but since it's impossible to put a total ordering on development build versions
-    // this is a pragmatic compromise
+    // sorting two development builds based on id is reasonably valid
+    // if the two version strings were created with the same schema.
     case Development(thatId) => id compare thatId
-    // assume a development build is newer than anything else, that's not really true, but good luck
-    // mapping development build versions to other build types
-    case _ => 1
+    // assume a development build is older than anything else.
+    case _ => -1
   }
 }
-/**
- * A final final
+/** A final final.
  */
 case object Final extends ScalaBuild {
   def unparse = ""
 
   def compare(that: ScalaBuild) = that match {
     case Final => 0
-    // a final is newer than anything other than a development build or another final
-    case Development(_) => -1
+    // a final is newer than anything else.
     case _ => 1
   }
 }
 
-/**
- * A candidate for final release
+/** A candidate for final release.
  */
 case class RC(n: Int) extends ScalaBuild {
   def unparse = s"-RC${n}"
 
   def compare(that: ScalaBuild) = that match {
+    case Final => -1
     // compare two rcs based on their RC numbers
     case RC(thatN) => n - thatN
-    // an rc is older than anything other than a milestone or another rc
-    case Milestone(_) => 1
-    case _ => -1
+    // an rc is newer than anything else
+    case _ => 1
   }
 }
 
-/**
- * An intermediate release
+/** An intermediate release.
  */
 case class Milestone(n: Int) extends ScalaBuild {
   def unparse = s"-M${n}"
 
   def compare(that: ScalaBuild) = that match {
     // compare two milestones based on their milestone numbers
     case Milestone(thatN) => n - thatN
-    // a milestone is older than anything other than another milestone
+    case Development(_)   => 1
     case _ => -1
-
   }
 }

src/compiler/scala/tools/nsc/settings/Warnings.scala

@@ -144,8 +144,9 @@ trait Warnings {
     val Implicits = Choice("implicits", "Warn if an implicit parameter is unused.")
     val Synthetics = Choice("synthetics", "Warn if a synthetic implicit parameter (context bound) is unused.")
     val Nowarn    = Choice("nowarn",    "Warn if a @nowarn annotation does not suppress any warnings.")
+    val Unused    = Choice("unused",    "Warn if an @unused annotation does not suppress any warnings.")
     val Params    = Choice("params",    "Enable -Wunused:explicits,implicits,synthetics.", expandsTo = List(Explicits, Implicits, Synthetics))
-    val Linted    = Choice("linted",    "-Xlint:unused.", expandsTo = List(Imports, Privates, Locals, Implicits, Nowarn))
+    val Linted    = Choice("linted",    "-Xlint:unused.", expandsTo = List(Imports, Privates, Locals, Implicits, Nowarn, Unused))
   }
 
   // The -Ywarn-unused warning group.
@@ -166,6 +167,7 @@ trait Warnings {
   def warnUnusedImplicits = warnUnused contains UnusedWarnings.Implicits
   def warnUnusedSynthetics = warnUnused contains UnusedWarnings.Synthetics
   def warnUnusedNowarn    = warnUnused contains UnusedWarnings.Nowarn
+  def warnUnusedUnused    = warnUnused contains UnusedWarnings.Unused
 
   val warnExtraImplicit   = BooleanSetting("-Wextra-implicit", "Warn when more than one implicit parameter section is defined.") withAbbreviation "-Ywarn-extra-implicit"
 

src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala

@@ -20,7 +20,7 @@ import scala.util.control.Exception.ultimately
 import symtab.Flags._
 import PartialFunction.{cond, condOpt}
 import scala.annotation.{nowarn, tailrec}
-import scala.tools.nsc.Reporting.WarningCategory
+import scala.tools.nsc.Reporting.{MessageFilter, Suppression, WarningCategory}
 
 /** An interface to enable higher configurability of diagnostic messages
  *  regarding type errors.  This is barely a beginning as error messages are
@@ -500,10 +500,20 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
     val ignoreNames: Set[TermName] = Set(
       "readResolve", "readObject", "writeObject", "writeReplace"
     ).map(TermName(_))
+
+    // is this a getter-setter pair? cf g.setterIn(g.owner) == s
+    def sameReference(g: Symbol, s: Symbol) =
+      if (g.accessed.exists && s.accessed.exists) g.accessed == s.accessed
+      else g.owner == s.owner && g.setterName == s.name         //sympos(g) == sympos(s)
+
+    def sympos(s: Symbol): Int =
+      if (s.pos.isDefined) s.pos.point else if (s.isTerm) s.asTerm.referenced.pos.point else -1
+    def treepos(t: Tree): Int =
+      if (t.pos.isDefined) t.pos.point else sympos(t.symbol)
   }
 
   class UnusedPrivates extends Traverser {
-    import UnusedPrivates.ignoreNames
+    import UnusedPrivates._
     def isEffectivelyPrivate(sym: Symbol): Boolean = false
     val defnTrees = ListBuffer[MemberDef]()
     val targets   = mutable.Set[Symbol]()
@@ -538,6 +548,11 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
 
     override def traverse(t: Tree): Unit = {
       val sym = t.symbol
+      if (sym != null && sym.pos.isDefined)
+        sym.getAnnotation(UnusedClass).foreach { annot =>
+          if (!runReporting.suppressionExists(annot.pos))
+            runReporting.addSuppression(Suppression(annot.pos, List(MessageFilter.Category(WarningCategory.Unused)), sym.pos.start, sym.pos.end, synthetic = true))
+        }
       t match {
         case m: MemberDef if qualifies(sym) && !t.isErrorTyped =>
           t match {
@@ -594,10 +609,8 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
       }
       super.traverse(t)
     }
-    def isSuppressed(sym: Symbol): Boolean = sym.hasAnnotation(UnusedClass)
     def isUnusedType(m: Symbol): Boolean = (
       m.isType
-        && !isSuppressed(m)
         && !m.isTypeParameterOrSkolem // would be nice to improve this
         && (m.isPrivate || m.isLocalToBlock || isEffectivelyPrivate(m))
         && !(treeTypes.exists(_.exists(_.typeSymbolDirect == m)))
@@ -612,7 +625,6 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
     }
     def isUnusedTerm(m: Symbol): Boolean = (
       m.isTerm
-        && !isSuppressed(m)
         && (!m.isSynthetic || isSyntheticWarnable(m))
         && ((m.isPrivate && !(m.isConstructor && m.owner.isAbstract)) || m.isLocalToBlock || isEffectivelyPrivate(m))
         && !targets(m)
@@ -631,26 +643,16 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
             && s.name == m.name && s.owner.isConstructor && s.owner.owner == m.owner) // exclude ctor params
         ))
       )
-    def sympos(s: Symbol): Int =
-      if (s.pos.isDefined) s.pos.point else if (s.isTerm) s.asTerm.referenced.pos.point else -1
-    def treepos(t: Tree): Int =
-      if (t.pos.isDefined) t.pos.point else sympos(t.symbol)
-
     def unusedTypes = defnTrees.toList.filter(t => isUnusedType(t.symbol)).sortBy(treepos)
     def unusedTerms = {
       val all = defnTrees.toList.filter(v => isUnusedTerm(v.symbol))
 
-      // is this a getter-setter pair? and why is this a difficult question for traits?
-      def sameReference(g: Symbol, s: Symbol) =
-        if (g.accessed.exists && s.accessed.exists) g.accessed == s.accessed
-        else g.owner == s.owner && g.setterName == s.name         //sympos(g) == sympos(s)
-
       // filter out setters if already warning for getter.
       val clean = all.filterNot(v => v.symbol.isSetter && all.exists(g => g.symbol.isGetter && sameReference(g.symbol, v.symbol)))
       clean.sortBy(treepos)
     }
     // local vars which are never set, except those already returned in unused
-    def unsetVars = varsWithoutSetters.filter(v => !isSuppressed(v) && !setVars(v) && !isUnusedTerm(v)).toList.sortBy(sympos)
+    def unsetVars = varsWithoutSetters.filter(v => !setVars(v) && !isUnusedTerm(v)).toList.sortBy(sympos)
     def unusedParams = params.iterator.filter(isUnusedParam).toList.sortBy(sympos)
     def inDefinedAt(p: Symbol) = p.owner.isMethod && p.owner.name == nme.isDefinedAt && p.owner.owner.isAnonymousFunction
     def unusedPatVars = patvars.toList.filter(p => isUnusedTerm(p) && !inDefinedAt(p)).sortBy(sympos)
@@ -681,7 +683,8 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
 
     // `checkUnused` is invoked after type checking. we have to avoid using `typer.context.warning`, which uses
     // `context.owner` as the `site` of the warning, but that's the root symbol at this point.
-    def emitUnusedWarning(pos: Position, msg: String, category: WarningCategory, site: Symbol): Unit = runReporting.warning(pos, msg, category, site)
+    def emitUnusedWarning(pos: Position, msg: String, category: WarningCategory, site: Symbol): Unit =
+      runReporting.warning(pos, msg, category, site)
 
     def run(unusedPrivates: UnusedPrivates)(body: Tree): Unit = {
       unusedPrivates.traverse(body)
@@ -723,9 +726,8 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
         for (defn <- unusedPrivates.unusedTerms if shouldWarnOn(defn.symbol)) { termWarning(defn) }
         for (defn <- unusedPrivates.unusedTypes if shouldWarnOn(defn.symbol)) { typeWarning(defn) }
 
-        for (v <- unusedPrivates.unsetVars) {
+        for (v <- unusedPrivates.unsetVars)
           emitUnusedWarning(v.pos, s"local var ${v.name} in ${v.owner} ${valAdvice}", WarningCategory.UnusedPrivates, v)
-        }
       }
       if (settings.warnUnusedPatVars) {
         for (v <- unusedPrivates.unusedPatVars)
@@ -774,7 +776,6 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
     }
   }
 
-
   trait TyperDiagnostics {
     self: Typer =>
 

src/library/scala/collection/mutable/StringBuilder.scala

@@ -21,7 +21,7 @@ import scala.Predef.{ // unimport char-related implicit conversions to avoid tri
   genericWrapArray => _,
   wrapCharArray => _,
   wrapString => _,
- //_
+  //_  : @unused  // note that Predef is unimported already by single masking import
 }
 
 /** A builder for mutable sequence of characters.  This class provides an API

test/files/neg/t11631.check

@@ -0,0 +1,18 @@
+t11631.scala:13: warning: private method z in class C is never used
+  private def z = 37              // unused
+              ^
+t11631.scala:18: warning: local var w in method q is never used
+    var w = 27                    // unused
+        ^
+t11631.scala:9: warning: @unused annotation does not suppress any warnings
+  def g(@unused i: Int) = i + 1   // unused unused
+         ^
+t11631.scala:11: warning: @unused annotation does not suppress any warnings
+  @unused private def x = 42      // unused unused
+   ^
+t11631.scala:17: warning: @unused annotation does not suppress any warnings
+    @unused var v = 42            // unused unused
+     ^
+error: No warnings can be incurred under -Werror.
+5 warnings
+1 error

test/files/neg/t11631.scala

@@ -0,0 +1,22 @@
+
+// scalac: -Xsource:3 -Werror -Wunused
+
+import annotation.*
+
+class C {
+  def n: Int = 42
+  def f(@unused i: Int) = n + 1
+  def g(@unused i: Int) = i + 1   // unused unused
+
+  @unused private def x = 42      // unused unused
+  @unused private def y = 27
+  private def z = 37              // unused
+  def p() = println(x)
+
+  def q() = {
+    @unused var v = 42            // unused unused
+    var w = 27                    // unused
+    v += 1      // write it
+    println(v)  // read it
+  }
+}

test/files/neg/t11631b.check

@@ -0,0 +1,9 @@
+t11631b.scala:5: warning: Unused import
+import annotation.*
+                  ^
+t11631b.scala:9: warning: private object X in object D is never used
+  private object X {
+                 ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/t11631b.scala

@@ -0,0 +1,22 @@
+
+// scalac: -Xsource:3 -Werror -Wunused
+// debug: -Xsource:3 -Werror -Wunused -Vdebug -Vlog:_ -Vtyper
+
+import annotation.*
+
+object D {
+  //@unused
+  private object X {
+    private final val x = 42
+  }
+  class X {
+    def f = X.x
+  }
+  private object Y {
+    private final val y = 42
+  }
+  class Y {
+    import Y._
+    def f = y
+  }
+}