summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSom Snytt <som.snytt@gmail.com>2016-09-14 23:55:16 -0700
committerSom Snytt <som.snytt@gmail.com>2017-03-11 23:38:08 -0800
commitf672aff3f3b92506741b62d8f7eae6d1e0dc36a7 (patch)
tree7bfacf173c601643e83a91bb5eb53fce9e0caf5e
parent9d9abffc94b28785e54bc2179b495d81f29b1e7f (diff)
downloadscala-f672aff3f3b92506741b62d8f7eae6d1e0dc36a7.tar.gz
scala-f672aff3f3b92506741b62d8f7eae6d1e0dc36a7.tar.bz2
scala-f672aff3f3b92506741b62d8f7eae6d1e0dc36a7.zip
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.
-rw-r--r--src/compiler/scala/tools/nsc/ast/parser/Parsers.scala9
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala17
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Typers.scala3
-rw-r--r--src/reflect/scala/reflect/internal/StdAttachments.scala6
-rw-r--r--src/reflect/scala/reflect/runtime/JavaUniverseForce.scala1
-rw-r--r--src/repl/scala/tools/nsc/interpreter/ILoop.scala6
-rw-r--r--test/files/neg/warn-unused-privates.check20
-rw-r--r--test/files/neg/warn-unused-privates.scala32
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
+ }
+
+}