From a85521efbbf65161debc460ab5cb55562db051e9 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 21 Sep 2016 11:03:53 -0700 Subject: SI-8040 Warn patvars in casedefs Collect bindings in casedefs unless "@-bound to _". Also minor refactor to make it easier to see the cases of `id @ _`. Tupled matching is supposed to be efficient either now or soon. --- .../scala/tools/nsc/ast/parser/Parsers.scala | 35 ++++++++++------------ .../tools/nsc/typechecker/TypeDiagnostics.scala | 13 +++++++- test/files/neg/warn-unused-privates.check | 10 +++++-- test/files/neg/warn-unused-privates.scala | 16 ++++++++++ 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 68bd663ab8..01a083018c 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1947,27 +1947,22 @@ self => * | Pattern3 * }}} */ - def pattern2(): Tree = { - val p = pattern3() - - if (in.token != AT) p // var pattern upgraded later to x @ _ - else p match { - case Ident(nme.WILDCARD) => - in.nextToken() - pattern3() - case Ident(name) => - in.nextToken() - val body = pattern3() - atPos(p.pos.start, p.pos.start, body.pos.end) { - val t = Bind(name, body) - body match { - case Ident(nme.WILDCARD) => t updateAttachment AtBoundIdentifierAttachment - case _ if !settings.warnUnusedPatVars => t updateAttachment AtBoundIdentifierAttachment - case _ => t - } + def pattern2(): Tree = (pattern3(), in.token) match { + case (Ident(nme.WILDCARD), AT) => + in.nextToken() + pattern3() + case (p @ Ident(name), AT) => + in.nextToken() + val body = pattern3() + atPos(p.pos.start, p.pos.start, body.pos.end) { + val t = Bind(name, body) + body match { + case Ident(nme.WILDCARD) => t updateAttachment AtBoundIdentifierAttachment + case _ if !settings.warnUnusedPatVars => t updateAttachment AtBoundIdentifierAttachment + case _ => t } - case _ => p - } + } + case (p, _) => p } /** {{{ diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 529d68901d..ee2d00900a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -477,6 +477,7 @@ trait TypeDiagnostics { val treeTypes = mutable.Set[Type]() val atBounds = mutable.Set[Symbol]() val params = mutable.Set[Symbol]() + val patvars = mutable.Set[Symbol]() def defnSymbols = defnTrees.toList map (_.symbol) def localVars = defnSymbols filter (t => t.isLocalToBlock && t.isVar) @@ -506,6 +507,11 @@ trait TypeDiagnostics { 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 + 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 @@ -570,6 +576,7 @@ 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) } private def warningsEnabled: Boolean = { @@ -613,7 +620,7 @@ trait TypeDiagnostics { 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") + reporter.warning(v.pos, s"local var ${v.name} in ${v.owner} is never set: consider using immutable val") } for (t <- p.unusedTypes) { val sym = t.symbol @@ -624,6 +631,10 @@ trait TypeDiagnostics { } } } + if (settings.warnUnusedPatVars) { + for (v <- p.unusedPatVars) + reporter.warning(v.pos, s"pattern var ${v.name} in ${v.owner} is never used; `${v.name}@_' suppresses this warning") + } 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 = { diff --git a/test/files/neg/warn-unused-privates.check b/test/files/neg/warn-unused-privates.check index c77fcff420..490e070794 100644 --- a/test/files/neg/warn-unused-privates.check +++ b/test/files/neg/warn-unused-privates.check @@ -79,7 +79,7 @@ warn-unused-privates.scala:166: warning: local val x in method v is never used warn-unused-privates.scala:170: warning: local val x in method w is never used val D(x @ _) = d // warn, fixme (valdef pos is different) ^ -warn-unused-privates.scala:97: warning: local var x in method f2 is never set - it could be a val +warn-unused-privates.scala:97: warning: local var x in method f2 is never set: consider using immutable val var x = 100 // warn about it being a var ^ warn-unused-privates.scala:104: warning: private class Bar1 in object Types is never used @@ -97,6 +97,12 @@ warn-unused-privates.scala:118: warning: local class DingDongDoobie is never use warn-unused-privates.scala:121: warning: local type OtherThing is never used type OtherThing = String // warn ^ +warn-unused-privates.scala:201: warning: pattern var z in method f is never used; `z@_' suppresses this warning + case z => "warn" + ^ +warn-unused-privates.scala:208: warning: pattern var z in method f is never used; `z@_' suppresses this warning + case Some(z) => "warn" + ^ error: No warnings can be incurred under -Xfatal-warnings. -33 warnings found +35 warnings found one error found diff --git a/test/files/neg/warn-unused-privates.scala b/test/files/neg/warn-unused-privates.scala index f0580f02d5..777e0f1579 100644 --- a/test/files/neg/warn-unused-privates.scala +++ b/test/files/neg/warn-unused-privates.scala @@ -193,3 +193,19 @@ trait Forever { trait Ignorance { private val readResolve = 42 // ignore } + +trait CaseyKasem { + def f = 42 match { + case x if x < 25 => "no warn" + case y if toString.nonEmpty => "no warn" + y + case z => "warn" + } +} +trait CaseyAtTheBat { + def f = Option(42) match { + case Some(x) if x < 25 => "no warn" + case Some(y @ _) if toString.nonEmpty => "no warn" + case Some(z) => "warn" + case None => "no warn" + } +} -- cgit v1.2.3