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.

git-svn-id: http://lampsvn.epfl.ch/svn-repos/scala/scala/trunk@25608 5e8d7ff9-d8ef-0310-90f0-a4852d11357a

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

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, _) =>

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

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) {

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
               }
           }

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) = 

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")
+          }
         }
       }
     }

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 {

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

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

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

test/files/neg/stmt-expr-discard.flags

@@ -0,0 +1 @@
+-Xfatal-warnings

test/files/neg/stmt-expr-discard.scala

@@ -0,0 +1,5 @@
+class A {
+  def f = 1
+    + 2
+    - 4
+}

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

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)
 }

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

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]

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) => ()
       ^

test/files/neg/t278.scala

@@ -2,5 +2,5 @@ class C {
   class A
   def a = () => ()
   def a = (p:A) => ()
-  a[A]
+  println(a[A])
 }

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

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

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

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 = ()
 }
 

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

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) }
 }

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

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] {

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

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 }