Offer warning when demonstrably non-side-effect...

Offer warning when demonstrably non-side-effecting expressions appear in
statement position, which should be unintentional by definition. Threw
in removal of six places with useless discarded expressions which the
warning informed me about. No review.

29 files changed, 123 insertions, 44 deletions

diff --git a/src/compiler/scala/reflect/internal/Constants.scala b/src/compiler/scala/reflect/internal/Constants.scala
index 55bb7c2ceb..2edb0d1fe6 100644
--- a/src/compiler/scala/reflect/internal/Constants.scala
+++ b/src/compiler/scala/reflect/internal/Constants.scala
@@ -55,6 +55,8 @@ trait Constants extends api.Constants {
     def isLongRange: Boolean  = ByteTag <= tag && tag <= LongTag
     def isFloatRange: Boolean = ByteTag <= tag && tag <= FloatTag
     def isNumeric: Boolean    = ByteTag <= tag && tag <= DoubleTag
+    def isNonUnitAnyVal       = BooleanTag <= tag && tag <= DoubleTag
+    def isAnyVal              = UnitTag <= tag && tag <= DoubleTag
 
     def tpe: Type = tag match {
       case UnitTag    => UnitClass.tpe
diff --git a/src/compiler/scala/reflect/internal/TreeInfo.scala b/src/compiler/scala/reflect/internal/TreeInfo.scala
index a8ec7e34f5..de46069ee2 100644
--- a/src/compiler/scala/reflect/internal/TreeInfo.scala
+++ b/src/compiler/scala/reflect/internal/TreeInfo.scala
@@ -68,6 +68,10 @@ abstract class TreeInfo {
   }
 
   /** Is tree a stable and pure expression?
+   *  !!! Clarification on what is meant by "pure" here would be appreciated.
+   *  This implementation allows both modules and lazy vals, which are pure in
+   *  the sense that they always return the same result, but which are also
+   *  side effecting.  So for now, "pure" != "not side effecting".
    */
   def isPureExpr(tree: Tree): Boolean = tree match {
     case EmptyTree
@@ -77,6 +81,10 @@ abstract class TreeInfo {
       true
     case Ident(_) =>
       tree.symbol.isStable
+    // this case is mostly to allow expressions like -5 and +7, but any
+    // member of an anyval should be safely pure
+    case Select(Literal(const), name) =>
+      const.isAnyVal && (const.tpe.member(name) != NoSymbol)
     case Select(qual, _) =>
       tree.symbol.isStable && isPureExpr(qual)
     case TypeApply(fn, _) =>
diff --git a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
index 6c1245dd46..9eda2e4097 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/MarkupParsers.scala
@@ -397,11 +397,9 @@ trait MarkupParsers {
     def xScalaPatterns: List[Tree] = escapeToScala(parser.seqPatterns(), "pattern")
 
     def reportSyntaxError(pos: Int, str: String) = parser.syntaxError(pos, str)
-    def reportSyntaxError(str: String) = {
+    def reportSyntaxError(str: String) {
       reportSyntaxError(curOffset, "in XML literal: " + str)
-      val result = ch
-      nextch
-      result
+      nextch()
     }
 
     /** '<' xPattern  ::= Name [S] { xmlPattern | '{' pattern3 '}' } ETag
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index 7d68abccf0..9adb7dce20 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -928,7 +928,7 @@ trait Namers { self: Analyzer =>
         // is more compact than: def foo[T, T2](a: T, x: T2)(implicit w: ComputeT2[T, T2])
         // moreover, the latter is not an encoding of the former, which hides type inference of T2, so you can specify T while T2 is purely computed
         val checkDependencies: TypeTraverser = new TypeTraverser {
-          def traverse(tp: Type) = {
+          def traverse(tp: Type) {
             tp match {
               case SingleType(_, sym) =>
                 if (sym.owner == meth && sym.isValueParameter && !(okParams contains sym))
@@ -941,7 +941,6 @@ trait Namers { self: Analyzer =>
               case _ =>
                 mapOver(tp)
             }
-            this
           }
         }
         for(vps <- vparamSymss) {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 725bf90af3..3b6b44316b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -2165,6 +2165,20 @@ trait Typers extends Modes with Adaptations {
                   if (treeInfo.isSelfConstrCall(result) && result.symbol.pos.pointOrElse(0) >= exprOwner.enclMethod.pos.pointOrElse(0))
                     error(stat.pos, "called constructor's definition must precede calling constructor's definition")
                 }
+                result match {
+                  case EmptyTree | Literal(Constant(()))  => ()
+                  case _ =>
+                    if (treeInfo isPureExpr result) {
+                      val sym = result.symbol
+                      if (sym != null && (sym.isModule || sym.isLazy)) {
+                        debuglog("'Pure' but side-effecting expression in statement position: " + result)
+                      }
+                      else context.warning(stat.pos,
+                        "a pure expression does nothing in statement position; " +
+                        "you may be omitting necessary parentheses"
+                      )
+                    }
+                }
                 result
               }
           }
diff --git a/src/compiler/scala/tools/nsc/util/Statistics.scala b/src/compiler/scala/tools/nsc/util/Statistics.scala
index 95cdc77a09..27239b9b9f 100644
--- a/src/compiler/scala/tools/nsc/util/Statistics.scala
+++ b/src/compiler/scala/tools/nsc/util/Statistics.scala
@@ -71,7 +71,6 @@ abstract class StatisticsInfo {
 
   def countNodes(tree: Tree, counts: ClassCounts) {
     for (t <- tree) counts(t.getClass) += 1
-    counts
   }
 
   def showRelative(base: Long)(value: Long) =
diff --git a/src/library/scala/collection/JavaConversions.scala b/src/library/scala/collection/JavaConversions.scala
index f67fd47708..fad990e140 100644
--- a/src/library/scala/collection/JavaConversions.scala
+++ b/src/library/scala/collection/JavaConversions.scala
@@ -678,18 +678,19 @@ object JavaConversions {
           }
         }
 
-        def remove() = prev match {
-          case Some(k) =>
-            underlying match {
-              case mm: mutable.Map[A, _] =>
-                val v = mm remove k.asInstanceOf[A]
-                prev = None
-                v
-              case _ =>
-                throw new UnsupportedOperationException("remove")
-            }
-          case _ =>
-            throw new IllegalStateException("next must be called at least once before remove")
+        def remove() {
+          prev match {
+            case Some(k) =>
+              underlying match {
+                case mm: mutable.Map[A, _] =>
+                  mm remove k.asInstanceOf[A]
+                  prev = None
+                case _ =>
+                  throw new UnsupportedOperationException("remove")
+              }
+            case _ =>
+              throw new IllegalStateException("next must be called at least once before remove")
+          }
         }
       }
     }
diff --git a/src/library/scala/xml/dtd/ContentModel.scala b/src/library/scala/xml/dtd/ContentModel.scala
index cae2236346..1e9a3a4b58 100644
--- a/src/library/scala/xml/dtd/ContentModel.scala
+++ b/src/library/scala/xml/dtd/ContentModel.scala
@@ -52,7 +52,6 @@ object ContentModel extends WordExp {
       sb append sep
       buildString(z, sb)
     }
-    sb
   }
 
   def buildString(c: ContentModel, sb: StringBuilder): StringBuilder = c match {
diff --git a/src/swing/scala/swing/LayoutContainer.scala b/src/swing/scala/swing/LayoutContainer.scala
index da1c766437..0570e7bb57 100644
--- a/src/swing/scala/swing/LayoutContainer.scala
+++ b/src/swing/scala/swing/LayoutContainer.scala
@@ -61,7 +61,6 @@ trait LayoutContainer extends Container.Wrapper {
       val (v, msg) = areValid(l)
       if (!v) throw new IllegalArgumentException(msg)
       add(c, l)
-      this
     }
     def get(c: Component) = Option(constraintsFor(c))
     override def size = peer.getComponentCount
diff --git a/test/files/neg/scopes.check b/test/files/neg/scopes.check
index 2f2eaa758f..f8e8c3758a 100644
--- a/test/files/neg/scopes.check
+++ b/test/files/neg/scopes.check
@@ -7,6 +7,9 @@ scopes.scala:5: error: x is already defined as value x
 scopes.scala:8: error: y is already defined as value y
     val y: Float = .0f
         ^
+scopes.scala:6: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+  {
+  ^
 scopes.scala:11: error: x is already defined as value x
   def f1(x: Int, x: Float) = x
                  ^
@@ -19,4 +22,5 @@ scopes.scala:13: error: x is already defined as value x
 scopes.scala:15: error: x is already defined as value x
     case x::x => x
             ^
+one warning found
 7 errors found
diff --git a/test/files/neg/stmt-expr-discard.check b/test/files/neg/stmt-expr-discard.check
new file mode 100644
index 0000000000..2d6420a61d
--- /dev/null
+++ b/test/files/neg/stmt-expr-discard.check
@@ -0,0 +1,7 @@
+stmt-expr-discard.scala:3: error: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+    + 2
+    ^
+stmt-expr-discard.scala:4: error: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+    - 4
+      ^
+two errors found
diff --git a/test/files/neg/stmt-expr-discard.flags b/test/files/neg/stmt-expr-discard.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/stmt-expr-discard.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/stmt-expr-discard.scala b/test/files/neg/stmt-expr-discard.scala
new file mode 100644
index 0000000000..e60c854625
--- /dev/null
+++ b/test/files/neg/stmt-expr-discard.scala
@@ -0,0 +1,5 @@
+class A {
+  def f = 1
+    + 2
+    - 4
+}
diff --git a/test/files/neg/t1181.check b/test/files/neg/t1181.check
index 2d7205c61f..3724752a85 100644
--- a/test/files/neg/t1181.check
+++ b/test/files/neg/t1181.check
@@ -1,4 +1,8 @@
+t1181.scala:8: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+     case (Nil, Nil) => map
+                        ^
 t1181.scala:9: error: missing parameter type
      _ => buildMap(map.updated(keyList.head, valueList.head), keyList.tail, valueList.tail)
      ^
+one warning found
 one error found
diff --git a/test/files/neg/t2078.scala b/test/files/neg/t2078.scala
index 03eaa7ed0b..342ba088c7 100644
--- a/test/files/neg/t2078.scala
+++ b/test/files/neg/t2078.scala
@@ -5,5 +5,5 @@ class A[-S](y : S) {
 object Test extends App {
   val a = new A(1)
   val b = a : A[Nothing]
-  b.f.x
+  println(b.f.x)
 }
diff --git a/test/files/neg/t2641.check b/test/files/neg/t2641.check
index 771624e8d9..2056a1b9ab 100644
--- a/test/files/neg/t2641.check
+++ b/test/files/neg/t2641.check
@@ -1,35 +1,35 @@
-t2641.scala:19: error: illegal cyclic reference involving trait ManagedSeq
+t2641.scala:18: error: illegal cyclic reference involving trait ManagedSeq
         with TraversableViewLike[A, ManagedSeqStrict[A], ManagedSeq[A]]
                                                          ^
-t2641.scala:17: error: illegal inheritance;
+t2641.scala:16: error: illegal inheritance;
  self-type ManagedSeq does not conform to ManagedSeqStrict[A]'s selftype ManagedSeqStrict[A]
         extends ManagedSeqStrict[A]
                 ^
-t2641.scala:18: error: illegal inheritance;
+t2641.scala:17: error: illegal inheritance;
  self-type ManagedSeq does not conform to scala.collection.TraversableView[A,ManagedSeqStrict[A]]'s selftype scala.collection.TraversableView[A,ManagedSeqStrict[A]]
         with TraversableView[A, ManagedSeqStrict[A]]
              ^
-t2641.scala:17: error: illegal inheritance;
+t2641.scala:16: error: illegal inheritance;
  self-type ManagedSeq does not conform to ScalaObject's selftype ScalaObject
         extends ManagedSeqStrict[A]
         ^
-t2641.scala:25: error: something is wrong (wrong class file?): trait ManagedSeq with type parameters [A,Coll] gets applied to arguments [], phase = typer
+t2641.scala:24: error: something is wrong (wrong class file?): trait ManagedSeq with type parameters [A,Coll] gets applied to arguments [], phase = typer
   trait Transformed[+B] extends ManagedSeq[B, Coll] with super.Transformed[B]
         ^
-t2641.scala:27: error: something is wrong (wrong class file?): trait ManagedSeq with type parameters [A,Coll] gets applied to arguments [], phase = namer
+t2641.scala:26: error: something is wrong (wrong class file?): trait ManagedSeq with type parameters [A,Coll] gets applied to arguments [], phase = namer
   trait Sliced extends Transformed[A] with super.Sliced {
                ^
-t2641.scala:27: error: illegal inheritance; superclass Any
+t2641.scala:26: error: illegal inheritance; superclass Any
  is not a subclass of the superclass ManagedSeqStrict
  of the mixin trait Transformed
   trait Sliced extends Transformed[A] with super.Sliced {
                        ^
-t2641.scala:27: error: illegal inheritance; superclass Any
+t2641.scala:26: error: illegal inheritance; superclass Any
  is not a subclass of the superclass Object
  of the mixin trait Sliced
   trait Sliced extends Transformed[A] with super.Sliced {
                                                  ^
-t2641.scala:28: error: value managedIterator is not a member of ManagedSeq
+t2641.scala:27: error: value managedIterator is not a member of ManagedSeq
     override def managedIterator = self.managedIterator slice (from, until)
                                         ^
 9 errors found
diff --git a/test/files/neg/t2641.scala b/test/files/neg/t2641.scala
index 5529035f79..bc048e039e 100644
--- a/test/files/neg/t2641.scala
+++ b/test/files/neg/t2641.scala
@@ -9,8 +9,7 @@ abstract class ManagedSeqStrict[+A]
 {
     override def companion: GenericCompanion[ManagedSeqStrict] = null
 
-    override def foreach[U](f: A => U): Unit =
-        null
+    override def foreach[U](f: A => U): Unit = ()
 }
 
 trait ManagedSeq[+A, +Coll]
diff --git a/test/files/neg/t278.check b/test/files/neg/t278.check
index ad1078f897..675ef910ee 100644
--- a/test/files/neg/t278.check
+++ b/test/files/neg/t278.check
@@ -2,8 +2,8 @@ t278.scala:5: error: overloaded method value a with alternatives:
   => C.this.A => Unit <and>
   => () => Unit
  does not take type parameters
-  a[A]
-  ^
+  println(a[A])
+          ^
 t278.scala:4: error: method a is defined twice
   def a = (p:A) => ()
       ^
diff --git a/test/files/neg/t278.scala b/test/files/neg/t278.scala
index 16ffe10595..39a711bb09 100644
--- a/test/files/neg/t278.scala
+++ b/test/files/neg/t278.scala
@@ -2,5 +2,5 @@ class C {
   class A
   def a = () => ()
   def a = (p:A) => ()
-  a[A]
+  println(a[A])
 }
diff --git a/test/files/neg/t2910.check b/test/files/neg/t2910.check
index ff190122d6..44bf1993db 100644
--- a/test/files/neg/t2910.check
+++ b/test/files/neg/t2910.check
@@ -10,7 +10,7 @@ t2910.scala:16: error: forward reference extends over definition of value z
 t2910.scala:30: error: forward reference extends over definition of value x
     lazy val f: Int = x
                       ^
-t2910.scala:34: error: forward reference extends over definition of variable x
+t2910.scala:35: error: forward reference extends over definition of variable x
     lazy val f: Int = g
                       ^
 5 errors found
diff --git a/test/files/neg/t2910.scala b/test/files/neg/t2910.scala
index b772ee4d43..d9a781032c 100644
--- a/test/files/neg/t2910.scala
+++ b/test/files/neg/t2910.scala
@@ -29,6 +29,7 @@ object Test {
   {
     lazy val f: Int = x
     val x: Int = f
+    println(x)
   }
   {
     lazy val f: Int = g
diff --git a/test/files/neg/t4166.check b/test/files/neg/t4166.check
index 24129c54ad..10b77d841a 100644
--- a/test/files/neg/t4166.check
+++ b/test/files/neg/t4166.check
@@ -1,4 +1,4 @@
 t4166.scala:3: error: super constructor arguments cannot reference unconstructed `this`
-class Demo extends Base(new { Demo.this }) {
+class Demo extends Base(new { Demo.this.toString }) {
                               ^
 one error found
diff --git a/test/files/neg/t4166.scala b/test/files/neg/t4166.scala
index c20796c43c..a2ee0671ab 100644
--- a/test/files/neg/t4166.scala
+++ b/test/files/neg/t4166.scala
@@ -1,11 +1,11 @@
 class Base(a: Any)
 
-class Demo extends Base(new { Demo.this }) {
+class Demo extends Base(new { Demo.this.toString }) {
   val x: Any = ()
 }
 
 
-class Demo2 extends Base(new { this }) {
+class Demo2 extends Base(new { this.toString }) {
   val x: Any = ()
 }
 
diff --git a/test/files/neg/t4419.check b/test/files/neg/t4419.check
index 8a5d95ca4e..a53e0c95da 100644
--- a/test/files/neg/t4419.check
+++ b/test/files/neg/t4419.check
@@ -1,4 +1,4 @@
 t4419.scala:2: error: forward reference extends over definition of value b
-  { val b = a; val a = 1 }
+  { val b = a; val a = 1 ; println(a) }
             ^
 one error found
diff --git a/test/files/neg/t4419.scala b/test/files/neg/t4419.scala
index 38a34be489..5dc86d354e 100644
--- a/test/files/neg/t4419.scala
+++ b/test/files/neg/t4419.scala
@@ -1,3 +1,3 @@
 class A {
-  { val b = a; val a = 1 }
+  { val b = a; val a = 1 ; println(a) }
 }
\ No newline at end of file
diff --git a/test/files/neg/unit-returns-value.check b/test/files/neg/unit-returns-value.check
index 18368f45ab..ab458a350b 100644
--- a/test/files/neg/unit-returns-value.check
+++ b/test/files/neg/unit-returns-value.check
@@ -1,4 +1,7 @@
+unit-returns-value.scala:4: error: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+    if (b) return 5
+                  ^
 unit-returns-value.scala:4: error: enclosing method f has result type Unit: return value discarded
     if (b) return 5
            ^
-one error found
+two errors found
diff --git a/test/files/neg/variances.scala b/test/files/neg/variances.scala
index 181783f48a..f693480d1d 100644
--- a/test/files/neg/variances.scala
+++ b/test/files/neg/variances.scala
@@ -29,7 +29,7 @@ object Covariant {
         def b2a(b : B) : A
         def doit(b : B) = setA(b2a(b))
       }
-      ()
+      println("")
     }
   }
   class Foo3[+A] {
diff --git a/test/files/run/repl-bare-expr.check b/test/files/run/repl-bare-expr.check
index 04daa48232..8b6434e986 100644
--- a/test/files/run/repl-bare-expr.check
+++ b/test/files/run/repl-bare-expr.check
@@ -4,15 +4,33 @@ Type :help for more information.
 scala> 
 
 scala> 2 ; 3
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              2 ;;
+              ^
 res0: Int = 3
 
 scala> { 2 ; 3 }
+<console>:8: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              { 2 ; 3 }
+                ^
 res1: Int = 3
 
 scala> 5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
   1 +
   2 +
   3 } ; bippy+88+11
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
+              ^
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
+                  ^
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
+                                         ^
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ; 10 ; case object Cow ; 20 ; class Moo { override def toString = "Moooooo" } ; 30 ; def bippy = {
+                                                                                                ^
 defined module Cow
 defined class Moo
 bippy: Int
diff --git a/test/files/run/repl-parens.check b/test/files/run/repl-parens.check
index 2f56e5ddd4..a3b0fd1939 100644
--- a/test/files/run/repl-parens.check
+++ b/test/files/run/repl-parens.check
@@ -20,6 +20,12 @@ scala>   (  (2 + 2 )  )
 res5: Int = 4
 
 scala> 5 ;   (  (2 + 2 )  ) ; ((5))
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ;   (  (2 + 2 )  ) ;;
+              ^
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              5 ;   (  (2 + 2 )  ) ;;
+                          ^
 res6: Int = 5
 
 scala> (((2 + 2)), ((2 + 2)))
@@ -34,9 +40,18 @@ res9: String = 4423
 scala> 
 
 scala> 55 ; ((2 + 2)) ; (1, 2, 3)
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              55 ; ((2 + 2)) ;;
+              ^
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              55 ; ((2 + 2)) ;;
+                       ^
 res10: (Int, Int, Int) = (1,2,3)
 
 scala> 55 ; (x: Int) => x + 1 ; () => ((5))
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              55 ; (x: Int) => x + 1 ;;
+              ^
 res11: () => Int = <function0>
 
 scala> 
@@ -45,6 +60,9 @@ scala> () => 5
 res12: () => Int = <function0>
 
 scala> 55 ; () => 5
+<console>:7: warning: a pure expression does nothing in statement position; you may be omitting necessary parentheses
+              55 ;;
+              ^
 res13: () => Int = <function0>
 
 scala> () => { class X ; new X }