From 6d7b81a1e47960fbbc469108a34414f76a706342 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 16 Dec 2016 00:05:47 -0800 Subject: SI-8040 No warn args to super, main args `class B(x: X) extends A(x)` uses `x` in ctor, where it is detectable as an ordinary param. `implicit class C(val s: String)` may not use `s` in extension methods, so don't warn. Don't warn required args to main method. Don't warn about synthetic isDefinedAt in anonymous functions, or about defaultCase$. --- .../scala/tools/nsc/backend/jvm/opt/Inliner.scala | 1 - .../tools/nsc/typechecker/TypeDiagnostics.scala | 36 ++++++++++++++-------- src/repl/scala/tools/nsc/interpreter/IMain.scala | 10 +++--- test/files/neg/warn-unused-params.check | 4 +-- test/files/neg/warn-unused-params.scala | 8 +++++ test/files/neg/warn-unused-privates.check | 11 ++++++- test/files/neg/warn-unused-privates.flags | 2 +- test/files/neg/warn-unused-privates.scala | 7 +++++ 8 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala index 0ae8347dc5..1c29859f46 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -15,7 +15,6 @@ import scala.collection.JavaConverters._ import AsmUtils._ import BytecodeUtils._ import collection.mutable -import scala.tools.asm.tree.analysis.{Analyzer, SourceInterpreter} import BackendReporting._ import scala.tools.nsc.backend.jvm.BTypes.InternalName diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 042c58226a..fa0407792e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -500,26 +500,28 @@ trait TypeDiagnostics { ) override def traverse(t: Tree): Unit = { + val sym = t.symbol t match { 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.isSynthetic && t.symbol.isImplicit) return - else if (!t.symbol.isConstructor) + case DefDef(mods@_, name@_, tparams@_, vparamss, tpt@_, rhs@_) if !sym.isAbstract && !sym.isDeprecated && !sym.isMacro => + if (sym.isPrimaryConstructor) + for (cpa <- sym.owner.constrParamAccessors if cpa.isPrivateLocal) params += cpa + else if (sym.isSynthetic && sym.isImplicit) return + else if (!sym.isConstructor) for (vs <- vparamss) params ++= vs.map(_.symbol) case _ => } case CaseDef(pat, guard@_, rhs@_) if settings.warnUnusedPatVars => pat.foreach { - case b @ Bind(_, _) if !atBounded(b) => patvars += b.symbol + // TODO don't warn in isDefinedAt of $anonfun + case b @ Bind(n, _) if !atBounded(b) && n != nme.DEFAULT_CASE => patvars += b.symbol case _ => } - case _: RefTree if t.symbol ne null => targets += t.symbol + case _: RefTree if sym ne null => targets += sym case Assign(lhs, _) if lhs.symbol != null => setVars += lhs.symbol - case Bind(n, _) if atBounded(t) => atBounds += t.symbol + case Bind(_, _) if atBounded(t) => atBounds += sym case _ => } // Only record type references which don't originate within the @@ -555,15 +557,20 @@ trait TypeDiagnostics { ) def isUnusedTerm(m: Symbol): Boolean = ( - (m.isTerm) + m.isTerm && (!m.isSynthetic || isSyntheticWarnable(m)) - && ((m.isPrivate && !(m.isConstructor && m.owner.isAbstract))|| m.isLocalToBlock) + && ((m.isPrivate && !(m.isConstructor && m.owner.isAbstract)) || m.isLocalToBlock) && !targets(m) && !(m.name == nme.WILDCARD) // e.g. val _ = foo && (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 + //&& !(m.isVal && m.info.resultType =:= typeOf[Unit]) // Unit val is uninteresting ) + def isUnusedParam(m: Symbol): Boolean = isUnusedTerm(m) && !(m.isParamAccessor && ( + m.owner.isImplicit || + targets.exists(s => s.isParameter && 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 = @@ -582,8 +589,9 @@ trait TypeDiagnostics { } // local vars which are never set, except those already returned in unused def unsetVars = localVars.filter(v => !setVars(v) && !isUnusedTerm(v)).sortBy(sympos) - def unusedParams = params.toList.filter(isUnusedTerm).sortBy(sympos) - def unusedPatVars = patvars.toList.filter(isUnusedTerm).sortBy(sympos) + def unusedParams = params.toList.filter(isUnusedParam).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) } private def warningsEnabled: Boolean = { @@ -648,9 +656,13 @@ trait TypeDiagnostics { val opc = new overridingPairs.Cursor(classOf(m)) opc.iterator.exists(pair => pair.low == m) } + def isConvention(p: Symbol): Boolean = { + p.name.decoded == "args" && p.owner.name.decoded == "main" + } def warnable(s: Symbol) = ( (settings.warnUnusedParams || s.isImplicit) && !isImplementation(s.owner) + && !isConvention(s) ) for (s <- p.unusedParams if warnable(s)) reporter.warning(s.pos, s"parameter $s in ${s.owner} is never used") diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala index a351d2da95..b977ab0939 100644 --- a/src/repl/scala/tools/nsc/interpreter/IMain.scala +++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala @@ -751,11 +751,9 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends lazy val evalClass = load(evalPath) def evalEither = callEither(resultName) match { - case Left(ex) => ex match { - case ex: NullPointerException => Right(null) - case ex => Left(unwrap(ex)) - } - case Right(result) => Right(result) + case Right(result) => Right(result) + case Left(_: NullPointerException) => Right(null) + case Left(e) => Left(unwrap(e)) } def compile(source: String): Boolean = compileAndSaveRun(label, source) @@ -789,7 +787,7 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends } ((pos, msg)) :: loop(filtered) } - val warnings = loop(run.reporting.allConditionalWarnings.map{case (pos, (msg, since)) => (pos, msg)}) + val warnings = loop(run.reporting.allConditionalWarnings.map{ case (pos, (msg, since@_)) => (pos, msg) }) if (warnings.nonEmpty) mostRecentWarnings = warnings } diff --git a/test/files/neg/warn-unused-params.check b/test/files/neg/warn-unused-params.check index ca6320ccd9..373417ce08 100644 --- a/test/files/neg/warn-unused-params.check +++ b/test/files/neg/warn-unused-params.check @@ -7,10 +7,10 @@ warn-unused-params.scala:32: warning: parameter value s in method i is never use 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 +warn-unused-params.scala:59: 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 +warn-unused-params.scala:62: 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. diff --git a/test/files/neg/warn-unused-params.scala b/test/files/neg/warn-unused-params.scala index c7578e53a4..b166e8fae6 100644 --- a/test/files/neg/warn-unused-params.scala +++ b/test/files/neg/warn-unused-params.scala @@ -52,6 +52,8 @@ class Unusing(u: Int) { // warn class Valuing(val u: Int) // no warn +class Revaluing(u: Int) { def f = u } // no warn + case class CaseyKasem(k: Int) // no warn case class CaseyAtTheBat(k: Int)(s: String) // warn @@ -59,3 +61,9 @@ case class CaseyAtTheBat(k: Int)(s: String) // warn trait Ignorance { def f(readResolve: Int) = 42 // warn } + +class Reusing(u: Int) extends Unusing(u) // no warn + +class Main { + def main(args: Array[String]): Unit = println("hello, args") // no warn +} diff --git a/test/files/neg/warn-unused-privates.check b/test/files/neg/warn-unused-privates.check index d4853847fd..2a88d3e6c3 100644 --- a/test/files/neg/warn-unused-privates.check +++ b/test/files/neg/warn-unused-privates.check @@ -106,6 +106,15 @@ warn-unused-privates.scala:201: warning: pattern var z in method f is never used warn-unused-privates.scala:208: warning: pattern var z in method f is never used; `z@_' suppresses this warning case Some(z) => "warn" ^ +warn-unused-privates.scala:20: warning: parameter value msg0 in class B3 is never used +class B3(msg0: String) extends A("msg") + ^ +warn-unused-privates.scala:136: warning: parameter value i in method x_= is never used + private def x_=(i: Int): Unit = ??? + ^ +warn-unused-privates.scala:138: warning: parameter value i in method y_= is never used + private def y_=(i: Int): Unit = ??? + ^ error: No warnings can be incurred under -Xfatal-warnings. -36 warnings found +39 warnings found one error found diff --git a/test/files/neg/warn-unused-privates.flags b/test/files/neg/warn-unused-privates.flags index 5ab4f36371..25474aefb3 100644 --- a/test/files/neg/warn-unused-privates.flags +++ b/test/files/neg/warn-unused-privates.flags @@ -1 +1 @@ --Ywarn-unused:privates,locals,patvars -Xfatal-warnings +-Ywarn-unused -Xfatal-warnings diff --git a/test/files/neg/warn-unused-privates.scala b/test/files/neg/warn-unused-privates.scala index 6f16ab4138..f7640927fb 100644 --- a/test/files/neg/warn-unused-privates.scala +++ b/test/files/neg/warn-unused-privates.scala @@ -217,3 +217,10 @@ object `not even using companion privates` { def f = i } } + +class `no warn in patmat anonfun isDefinedAt` { + def f(pf: PartialFunction[String, Int]) = pf("42") + def g = f { + case s => s.length // no warn (used to warn case s => true in isDefinedAt) + } +} -- cgit v1.2.3