From 22f98d5189b61200aaf11cec7a0a96d5cfa86a5e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 19 Sep 2016 00:46:17 -0700 Subject: SI-8040 Warn unused parameters One can `-Ywarn-unused:params` or more narrowly warn only for unused implicit parameters with `-Ywarn-unused:implicits`. Params includes constructor parameters. The settings for privates and locals are not yet distinguished. ``` $ skalac -Ywarn-unused:help Enable or disable specific `unused' warnings imports Warn if an import selector is not referenced. patvars Warn if a variable bound in a pattern is unused. privates Warn if a private member is unused. locals Warn if a local definition is unused. params Warn if a value parameter is unused. implicits Warn if an implicit parameter is unused. ``` --- .../scala/tools/nsc/settings/Warnings.scala | 2 +- .../tools/nsc/typechecker/TypeDiagnostics.scala | 115 +++++++++++++-------- test/files/neg/warn-unused-implicits.check | 9 ++ test/files/neg/warn-unused-implicits.flags | 1 + test/files/neg/warn-unused-implicits.scala | 32 ++++++ test/files/neg/warn-unused-params.check | 18 ++++ test/files/neg/warn-unused-params.flags | 1 + test/files/neg/warn-unused-params.scala | 61 +++++++++++ test/files/neg/warn-unused-patvars.check | 6 +- test/files/neg/warn-unused-patvars.scala | 2 + test/files/neg/warn-unused-privates.flags | 2 +- test/files/neg/warn-unused-privates.scala | 4 + 12 files changed, 207 insertions(+), 46 deletions(-) create mode 100644 test/files/neg/warn-unused-implicits.check create mode 100644 test/files/neg/warn-unused-implicits.flags create mode 100644 test/files/neg/warn-unused-implicits.scala create mode 100644 test/files/neg/warn-unused-params.check create mode 100644 test/files/neg/warn-unused-params.flags create mode 100644 test/files/neg/warn-unused-params.scala diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 1a16480149..29138c78d1 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -15,7 +15,7 @@ trait Warnings { // Warning semantics. val fatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") - // Non-lint warnings + // Non-lint warnings. val warnDeadCode = BooleanSetting("-Ywarn-dead-code", "Warn when dead code is identified.") val warnValueDiscard = BooleanSetting("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.") diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index b263a0fffd..529d68901d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -476,6 +476,7 @@ trait TypeDiagnostics { val setVars = mutable.Set[Symbol]() val treeTypes = mutable.Set[Type]() val atBounds = mutable.Set[Symbol]() + val params = mutable.Set[Symbol]() def defnSymbols = defnTrees.toList map (_.symbol) def localVars = defnSymbols filter (t => t.isLocalToBlock && t.isVar) @@ -496,8 +497,16 @@ trait TypeDiagnostics { override def traverse(t: Tree): Unit = { t match { - case t: MemberDef if qualifies(t.symbol) => defnTrees += t - case t: RefTree if t.symbol ne null => targets += t.symbol + case m: MemberDef if qualifies(t.symbol) => defnTrees += m + t match { + case DefDef(mods@_, name@_, tparams@_, vparamss, tpt@_, rhs@_) if !t.symbol.isAbstract && !t.symbol.isDeprecated => + if (t.symbol.isPrimaryConstructor) + for (cpa <- t.symbol.owner.constrParamAccessors if cpa.isPrivateLocal) params += cpa + else if (!t.symbol.isConstructor) + for (vs <- vparamss) params ++= vs.map(_.symbol) + case _ => + } + case _: RefTree if t.symbol ne null => targets += t.symbol case Assign(lhs, _) if lhs.symbol != null => setVars += lhs.symbol case Bind(n, _) if atBounded(t) => atBounds += t.symbol case _ => @@ -538,11 +547,16 @@ trait TypeDiagnostics { && (m.isPrivate || m.isLocalToBlock) && !targets(m) && !(m.name == nme.WILDCARD) // e.g. val _ = foo - && !ignoreNames(m.name.toTermName) // serialization methods + && (m.isValueParameter || !ignoreNames(m.name.toTermName)) // serialization methods && !isConstantType(m.info.resultType) // subject to constant inlining && !treeTypes.exists(_ contains m) // e.g. val a = new Foo ; new a.Bar ) - def unusedTypes = defnTrees.toList filter (t => isUnusedType(t.symbol)) + 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)) @@ -551,10 +565,11 @@ trait TypeDiagnostics { all.filterNot(v => v.symbol.isSetter && all.exists(g => g.symbol.isGetter && g.symbol.pos.point == v.symbol.pos.point) || atBounds.exists(x => v.symbol.pos.point == x.pos.point) - ) + ).sortBy(treepos) } // local vars which are never set, except those already returned in unused - def unsetVars = localVars filter (v => !setVars(v) && !isUnusedTerm(v)) + def unsetVars = localVars.filter(v => !setVars(v) && !isUnusedTerm(v)).sortBy(sympos) + def unusedParams = params.toList.filter(isUnusedTerm).sortBy(sympos) } private def warningsEnabled: Boolean = { @@ -566,43 +581,61 @@ trait TypeDiagnostics { def apply(unit: CompilationUnit): Unit = if (warningsEnabled) { val p = new UnusedPrivates p traverse unit.body - for (defn: DefTree <- p.unusedTerms) { - val sym = defn.symbol - val pos = ( - if (defn.pos.isDefined) defn.pos - else if (sym.pos.isDefined) sym.pos - else sym match { - case sym: TermSymbol => sym.referenced.pos - case _ => NoPosition + if (settings.warnUnusedLocals || settings.warnUnusedPrivates) { + for (defn: DefTree <- p.unusedTerms) { + val sym = defn.symbol + val pos = ( + if (defn.pos.isDefined) defn.pos + else if (sym.pos.isDefined) sym.pos + else sym match { + case sym: TermSymbol => sym.referenced.pos + case _ => NoPosition + } + ) + val why = if (sym.isPrivate) "private" else "local" + val what = ( + if (sym.isDefaultGetter) "default argument" + else if (sym.isConstructor) "constructor" + else if ( + sym.isVar + || sym.isGetter && (sym.accessed.isVar || (sym.owner.isTrait && !sym.hasFlag(STABLE))) + ) s"var ${sym.name.getterName.decoded}" + else if ( + sym.isVal + || sym.isGetter && (sym.accessed.isVal || (sym.owner.isTrait && sym.hasFlag(STABLE))) + || sym.isLazy + ) s"val ${sym.name.decoded}" + else if (sym.isSetter) s"setter of ${sym.name.getterName.decoded}" + else if (sym.isMethod) s"method ${sym.name.decoded}" + else if (sym.isModule) s"object ${sym.name.decoded}" + else "term" + ) + reporter.warning(pos, s"$why $what in ${sym.owner} is never used") + } + for (v <- p.unsetVars) { + reporter.warning(v.pos, s"local var ${v.name} in ${v.owner} is never set - it could be a val") + } + for (t <- p.unusedTypes) { + val sym = t.symbol + val wrn = if (sym.isPrivate) settings.warnUnusedPrivates else settings.warnUnusedLocals + if (wrn) { + val why = if (sym.isPrivate) "private" else "local" + reporter.warning(t.pos, s"$why ${sym.fullLocationString} is never used") } - ) - val why = if (sym.isPrivate) "private" else "local" - val what = ( - if (sym.isDefaultGetter) "default argument" - else if (sym.isConstructor) "constructor" - else if ( - sym.isVar - || sym.isGetter && (sym.accessed.isVar || (sym.owner.isTrait && !sym.hasFlag(STABLE))) - ) s"var ${sym.name.getterName.decoded}" - else if ( - sym.isVal - || sym.isGetter && (sym.accessed.isVal || (sym.owner.isTrait && sym.hasFlag(STABLE))) - || sym.isLazy - ) s"val ${sym.name.decoded}" - else if (sym.isSetter) s"setter of ${sym.name.getterName.decoded}" - else if (sym.isMethod) s"method ${sym.name.decoded}" - else if (sym.isModule) s"object ${sym.name.decoded}" - else "term" - ) - reporter.warning(pos, s"$why $what in ${sym.owner} is never used") - } - for (v <- p.unsetVars) { - reporter.warning(v.pos, s"local var ${v.name} in ${v.owner} is never set - it could be a val") + } } - for (t <- p.unusedTypes) { - val sym = t.symbol - val why = if (sym.isPrivate) "private" else "local" - reporter.warning(t.pos, s"$why ${sym.fullLocationString} is never used") + if (settings.warnUnusedParams || settings.warnUnusedImplicits) { + def classOf(s: Symbol): Symbol = if (s.isClass || s == NoSymbol) s else classOf(s.owner) + def isImplementation(m: Symbol): Boolean = { + val opc = new overridingPairs.Cursor(classOf(m)) + opc.iterator.exists(pair => pair.low == m) + } + def warnable(s: Symbol) = ( + (settings.warnUnusedParams || s.isImplicit) + && !isImplementation(s.owner) + ) + for (s <- p.unusedParams if warnable(s)) + reporter.warning(s.pos, s"parameter $s in ${s.owner} is never used") } } } diff --git a/test/files/neg/warn-unused-implicits.check b/test/files/neg/warn-unused-implicits.check new file mode 100644 index 0000000000..4cc5836800 --- /dev/null +++ b/test/files/neg/warn-unused-implicits.check @@ -0,0 +1,9 @@ +warn-unused-implicits.scala:11: warning: parameter value s in method f is never used + )(implicit s: String): Int = { // warn + ^ +warn-unused-implicits.scala:31: warning: parameter value s in method i is never used + def i(implicit s: String, t: Int) = t // yes, warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +two warnings found +one error found diff --git a/test/files/neg/warn-unused-implicits.flags b/test/files/neg/warn-unused-implicits.flags new file mode 100644 index 0000000000..18169f3218 --- /dev/null +++ b/test/files/neg/warn-unused-implicits.flags @@ -0,0 +1 @@ +-Ywarn-unused:implicits -Xfatal-warnings diff --git a/test/files/neg/warn-unused-implicits.scala b/test/files/neg/warn-unused-implicits.scala new file mode 100644 index 0000000000..54f924eac0 --- /dev/null +++ b/test/files/neg/warn-unused-implicits.scala @@ -0,0 +1,32 @@ + +trait InterFace { + /** Call something. */ + def call(a: Int, b: String, c: Double)(implicit s: String): Int +} + +trait BadAPI extends InterFace { + def f(a: Int, + b: String, + c: Double + )(implicit s: String): Int = { // warn + println(b + c) + a + } + @deprecated ("no warn in deprecated API", since="yesterday") + def g(a: Int, + b: String, + c: Double + )(implicit s: String): Int = { // no warn + println(b + c) + a + } + override def call(a: Int, + b: String, + c: Double + )(implicit s: String): Int = { // no warn, required by superclass + println(b + c) + a + } + + def i(implicit s: String, t: Int) = t // yes, warn +} diff --git a/test/files/neg/warn-unused-params.check b/test/files/neg/warn-unused-params.check new file mode 100644 index 0000000000..ca6320ccd9 --- /dev/null +++ b/test/files/neg/warn-unused-params.check @@ -0,0 +1,18 @@ +warn-unused-params.scala:9: warning: parameter value b in method f is never used + b: String, // warn + ^ +warn-unused-params.scala:32: warning: parameter value s in method i is never used + def i(implicit s: String) = 42 // yes, warn + ^ +warn-unused-params.scala:49: warning: parameter value u in class Unusing is never used +class Unusing(u: Int) { // warn + ^ +warn-unused-params.scala:57: warning: parameter value s in class CaseyAtTheBat is never used +case class CaseyAtTheBat(k: Int)(s: String) // warn + ^ +warn-unused-params.scala:60: warning: parameter value readResolve in method f is never used + def f(readResolve: Int) = 42 // warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +5 warnings found +one error found diff --git a/test/files/neg/warn-unused-params.flags b/test/files/neg/warn-unused-params.flags new file mode 100644 index 0000000000..795fb74272 --- /dev/null +++ b/test/files/neg/warn-unused-params.flags @@ -0,0 +1 @@ +-Ywarn-unused:params -Xfatal-warnings diff --git a/test/files/neg/warn-unused-params.scala b/test/files/neg/warn-unused-params.scala new file mode 100644 index 0000000000..c7578e53a4 --- /dev/null +++ b/test/files/neg/warn-unused-params.scala @@ -0,0 +1,61 @@ + +trait InterFace { + /** Call something. */ + def call(a: Int, b: String, c: Double): Int +} + +trait BadAPI extends InterFace { + def f(a: Int, + b: String, // warn + c: Double): Int = { + println(c) + a + } + @deprecated ("no warn in deprecated API", since="yesterday") + def g(a: Int, + b: String, // no warn + c: Double): Int = { + println(c) + a + } + override def call(a: Int, + b: String, // no warn, required by superclass + c: Double): Int = { + println(c) + a + } + + def meth(x: Int) = x + + override def equals(other: Any): Boolean = true // no warn + + def i(implicit s: String) = 42 // yes, warn + + /* + def future(x: Int): Int = { + val y = 42 + val x = y // maybe option to warn only if shadowed + x + } + */ +} + +// mustn't alter warnings in super +trait PoorClient extends BadAPI { + override def meth(x: Int) = ??? // no warn + override def f(a: Int, b: String, c: Double): Int = a + b.toInt + c.toInt +} + +class Unusing(u: Int) { // warn + def f = ??? +} + +class Valuing(val u: Int) // no warn + +case class CaseyKasem(k: Int) // no warn + +case class CaseyAtTheBat(k: Int)(s: String) // warn + +trait Ignorance { + def f(readResolve: Int) = 42 // warn +} diff --git a/test/files/neg/warn-unused-patvars.check b/test/files/neg/warn-unused-patvars.check index 002f7a07ca..2665126a36 100644 --- a/test/files/neg/warn-unused-patvars.check +++ b/test/files/neg/warn-unused-patvars.check @@ -1,10 +1,10 @@ -warn-unused-patvars.scala:7: warning: private val x in trait Boundings is never used +warn-unused-patvars.scala:9: warning: private val x in trait Boundings is never used private val x = 42 // warn, sanity check ^ -warn-unused-patvars.scala:26: warning: local val x in method v is never used +warn-unused-patvars.scala:28: warning: local val x in method v is never used val D(x) = d // warn, fixme ^ -warn-unused-patvars.scala:30: warning: local val x in method w is never used +warn-unused-patvars.scala:32: warning: local val x in method w is never used val D(x @ _) = d // warn, fixme (valdef pos is different) ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/warn-unused-patvars.scala b/test/files/neg/warn-unused-patvars.scala index 6f4620c0c7..3d35dfedd6 100644 --- a/test/files/neg/warn-unused-patvars.scala +++ b/test/files/neg/warn-unused-patvars.scala @@ -1,4 +1,6 @@ +// verify no warning when -Ywarn-unused:-patvars + case class C(a: Int, b: String, c: Option[String]) case class D(a: Int) diff --git a/test/files/neg/warn-unused-privates.flags b/test/files/neg/warn-unused-privates.flags index 25474aefb3..5ab4f36371 100644 --- a/test/files/neg/warn-unused-privates.flags +++ b/test/files/neg/warn-unused-privates.flags @@ -1 +1 @@ --Ywarn-unused -Xfatal-warnings +-Ywarn-unused:privates,locals,patvars -Xfatal-warnings diff --git a/test/files/neg/warn-unused-privates.scala b/test/files/neg/warn-unused-privates.scala index bc2799067e..f0580f02d5 100644 --- a/test/files/neg/warn-unused-privates.scala +++ b/test/files/neg/warn-unused-privates.scala @@ -189,3 +189,7 @@ trait Forever { } yield 42 // val emitted only if needed, hence nothing unused } } + +trait Ignorance { + private val readResolve = 42 // ignore +} -- cgit v1.2.3