From f672aff3f3b92506741b62d8f7eae6d1e0dc36a7 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 14 Sep 2016 23:55:16 -0700 Subject: SI-8040 Warn unused pattern vars Warn for unused `case X(x) =>` but, as an escape hatch, not for `case X(x @ _) =>`. The latter form is deemed documentary. (Named args could serve a similar purpose, `case X(x = _) =>`.) An attachment is used to mark the bound var, and the symbol position is used to correlate the identifier with the variable that is introduced. This mechanism doesn't work yet when only a single var is defined. --- .../scala/tools/nsc/ast/parser/Parsers.scala | 9 +++++- .../tools/nsc/typechecker/TypeDiagnostics.scala | 17 ++++++++---- .../scala/tools/nsc/typechecker/Typers.scala | 3 +- .../scala/reflect/internal/StdAttachments.scala | 6 ++++ .../scala/reflect/runtime/JavaUniverseForce.scala | 1 + src/repl/scala/tools/nsc/interpreter/ILoop.scala | 6 ++-- test/files/neg/warn-unused-privates.check | 20 +++++++++++++- test/files/neg/warn-unused-privates.scala | 32 ++++++++++++++++++++++ 8 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 0cdba861a5..dabce69b2f 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1957,7 +1957,14 @@ self => pattern3() case Ident(name) => in.nextToken() - atPos(p.pos.start) { Bind(name, pattern3()) } + 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 _ => t + } + } case _ => p } } diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 3cc896f3bd..f344364a75 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -33,7 +33,7 @@ import scala.annotation.tailrec * @version 1.0 */ trait TypeDiagnostics { - self: Analyzer => + self: Analyzer with StdAttachments => import global._ import definitions._ @@ -79,6 +79,8 @@ trait TypeDiagnostics { prefix + name.decode } + private def atBounded(t: Tree) = t.hasAttachment[AtBoundIdentifierAttachment.type] + /** Does the positioned line assigned to t1 precede that of t2? */ def posPrecedes(p1: Position, p2: Position) = p1.isDefined && p2.isDefined && p1.line < p2.line @@ -473,6 +475,7 @@ trait TypeDiagnostics { val targets = mutable.Set[Symbol]() val setVars = mutable.Set[Symbol]() val treeTypes = mutable.Set[Type]() + val atBounds = mutable.Set[Symbol]() def defnSymbols = defnTrees.toList map (_.symbol) def localVars = defnSymbols filter (t => t.isLocalToBlock && t.isVar) @@ -496,6 +499,7 @@ trait TypeDiagnostics { case t: MemberDef if qualifies(t.symbol) => defnTrees += t case t: 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 _ => } // Only record type references which don't originate within the @@ -541,10 +545,13 @@ trait TypeDiagnostics { def unusedTypes = defnTrees.toList filter (t => isUnusedType(t.symbol)) def unusedTerms = { val all = defnTrees.toList.filter(v => isUnusedTerm(v.symbol)) - // filter out setters if already warning for getter, indicated by position - if (all.exists(_.symbol.isSetter)) - all.filterNot(v => v.symbol.isSetter && all.exists(g => g.symbol.isGetter && g.symbol.pos.point == v.symbol.pos.point)) - else all + + // filter out setters if already warning for getter, indicated by position. + // also documentary names in patterns. + 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) + ) } // local vars which are never set, except those already returned in unused def unsetVars = localVars filter (v => !setVars(v) && !isUnusedTerm(v)) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 8333d5d295..b0a8b5d4c6 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -4266,7 +4266,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper val name = tree.name val body = tree.body name match { - case name: TypeName => assert(body == EmptyTree, context.unit + " typedBind: " + name.debugString + " " + body + " " + body.getClass) + case name: TypeName => + assert(body == EmptyTree, s"${context.unit} typedBind: ${name.debugString} ${body} ${body.getClass}") val sym = if (tree.symbol != NoSymbol) tree.symbol else { diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index fc49de1cf6..f72c1eb1b3 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -57,6 +57,12 @@ trait StdAttachments { */ case object BackquotedIdentifierAttachment extends PlainAttachment + /** Indicates that the host `Ident` has been created from a pattern2 binding, `case x @ p`. + * In the absence of named parameters in patterns, allows nuanced warnings for unused variables. + * Hence, `case X(x = _) =>` would not warn; for now, `case X(x @ _) =>` is documentary if x is unused. + */ + case object AtBoundIdentifierAttachment extends PlainAttachment + /** Identifies trees are either result or intermediate value of for loop desugaring. */ case object ForAttachment extends PlainAttachment diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 72e21f67fe..7aa3b113dd 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -40,6 +40,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.SAMFunction this.DelambdafyTarget this.BackquotedIdentifierAttachment + this.AtBoundIdentifierAttachment this.ForAttachment this.SyntheticUnitAttachment this.SubpatternsAttachment diff --git a/src/repl/scala/tools/nsc/interpreter/ILoop.scala b/src/repl/scala/tools/nsc/interpreter/ILoop.scala index 9635f320fe..cd263a26f6 100644 --- a/src/repl/scala/tools/nsc/interpreter/ILoop.scala +++ b/src/repl/scala/tools/nsc/interpreter/ILoop.scala @@ -958,17 +958,19 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter) def withSuppressedSettings[A](body: => A): A = { val ss = this.settings import ss._ - val noisy = List(Xprint, Ytyperdebug) + val noisy = List(Xprint, Ytyperdebug, browse) val noisesome = noisy.exists(!_.isDefault) - val current = (Xprint.value, Ytyperdebug.value) + val current = (Xprint.value, Ytyperdebug.value, browse.value) if (isReplDebug || !noisesome) body else { this.settings.Xprint.value = List.empty + this.settings.browse.value = List.empty this.settings.Ytyperdebug.value = false try body finally { Xprint.value = current._1 Ytyperdebug.value = current._2 + browse.value = current._3 intp.global.printTypings = current._2 } } diff --git a/test/files/neg/warn-unused-privates.check b/test/files/neg/warn-unused-privates.check index 6e1511d0e5..d273aa40f4 100644 --- a/test/files/neg/warn-unused-privates.check +++ b/test/files/neg/warn-unused-privates.check @@ -61,6 +61,24 @@ warn-unused-privates.scala:137: warning: private method x in class OtherNames is warn-unused-privates.scala:138: warning: private method y_= in class OtherNames is never used private def y_=(i: Int): Unit = ??? ^ +warn-unused-privates.scala:153: warning: local val x in method f is never used + val C(x, y, Some(z)) = c // warn + ^ +warn-unused-privates.scala:153: warning: local val y in method f is never used + val C(x, y, Some(z)) = c // warn + ^ +warn-unused-privates.scala:153: warning: local val z in method f is never used + val C(x, y, Some(z)) = c // warn + ^ +warn-unused-privates.scala:161: warning: local val z in method h is never used + val C(x @ _, y @ _, z @ Some(_)) = c // warn? + ^ +warn-unused-privates.scala:166: warning: local val x in method v is never used + val D(x) = d // warn + ^ +warn-unused-privates.scala:170: warning: local val x in method w is never used + val D(x @ _) = d // fixme + ^ warn-unused-privates.scala:97: warning: local var x in method f2 is never set - it could be a val var x = 100 // warn about it being a var ^ @@ -80,5 +98,5 @@ warn-unused-privates.scala:121: warning: local type OtherThing is never used type OtherThing = String // warn ^ error: No warnings can be incurred under -Xfatal-warnings. -27 warnings found +33 warnings found one error found diff --git a/test/files/neg/warn-unused-privates.scala b/test/files/neg/warn-unused-privates.scala index 2f67882632..1b702c7555 100644 --- a/test/files/neg/warn-unused-privates.scala +++ b/test/files/neg/warn-unused-privates.scala @@ -140,3 +140,35 @@ class OtherNames { def f = y } + +case class C(a: Int, b: String, c: Option[String]) +case class D(a: Int) + +trait Boundings { + + def c = C(42, "hello", Some("world")) + def d = D(42) + + def f() = { + val C(x, y, Some(z)) = c // warn + 17 + } + def g() = { + val C(x @ _, y @ _, Some(z @ _)) = c // no warn + 17 + } + def h() = { + val C(x @ _, y @ _, z @ Some(_)) = c // warn? + 17 + } + + def v() = { + val D(x) = d // warn + 17 + } + def w() = { + val D(x @ _) = d // fixme + 17 + } + +} -- cgit v1.2.3