Fixed SI-5603. Early definitions now get transparent positions.

This includes a fix for a minor problem described in #594 -
ensureNonOverlapping still focuses on default position when outside of early defs.
Review by @dragos, @odersky.

5 files changed, 107 insertions, 39 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala
index 1d29e33c50..9a6d32be01 100644
--- a/src/compiler/scala/tools/nsc/ast/Trees.scala
+++ b/src/compiler/scala/tools/nsc/ast/Trees.scala
@@ -95,11 +95,12 @@ trait Trees extends reflect.internal.Trees { self: Global =>
     val (edefs, rest) = body span treeInfo.isEarlyDef
     val (evdefs, etdefs) = edefs partition treeInfo.isEarlyValDef
     val gvdefs = evdefs map {
-      case vdef @ ValDef(_, _, tpt, _) => copyValDef(vdef)(
-        // !!! I know "atPos in case" wasn't intentionally planted to
-        // add an air of mystery to this file, but it is the sort of
-        // comment which only its author could love.
-        tpt = atPos(vdef.pos.focus)(TypeTree() setOriginal tpt setPos tpt.pos.focus), // atPos in case
+      case vdef @ ValDef(_, _, tpt, _) =>
+        copyValDef(vdef)(
+        // atPos for the new tpt is necessary, since the original tpt might have no position
+        // (when missing type annotation for ValDef for example), so even though setOriginal modifies the
+        // position of TypeTree, it would still be NoPosition. That's what the author meant.
+        tpt = atPos(vdef.pos.focus)(TypeTree() setOriginal tpt setPos tpt.pos.focus),
         rhs = EmptyTree
       )
     }
@@ -125,7 +126,7 @@ trait Trees extends reflect.internal.Trees { self: Global =>
             DefDef(constrMods, nme.CONSTRUCTOR, List(), vparamss1, TypeTree(), Block(lvdefs ::: List(superCall), Literal(Constant())))))
       }
     }
-    constrs foreach (ensureNonOverlapping(_, parents ::: gvdefs))
+    constrs foreach (ensureNonOverlapping(_, parents ::: gvdefs, focus=false))
     // Field definitions for the class - remove defaults.
     val fieldDefs = vparamss.flatten map (vd => copyValDef(vd)(mods = vd.mods &~ DEFAULTPARAM, rhs = EmptyTree))
 
diff --git a/src/compiler/scala/tools/nsc/interactive/RangePositions.scala b/src/compiler/scala/tools/nsc/interactive/RangePositions.scala
index 06828f3a3a..b702d2787c 100644
--- a/src/compiler/scala/tools/nsc/interactive/RangePositions.scala
+++ b/src/compiler/scala/tools/nsc/interactive/RangePositions.scala
@@ -41,11 +41,11 @@ self: scala.tools.nsc.Global =>
   /** A position that wraps a set of trees.
    *  The point of the wrapping position is the point of the default position.
    *  If some of the trees are ranges, returns a range position enclosing all ranges
-   *  Otherwise returns default position.
+   *  Otherwise returns default position that is either focused or not.
    */
-  override def wrappingPos(default: Position, trees: List[Tree]): Position = {
+  override def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = {
     val ranged = trees filter (_.pos.isRange)
-    if (ranged.isEmpty) default.focus
+    if (ranged.isEmpty) if (focus) default.focus else default
     else new RangePosition(default.source, (ranged map (_.pos.start)).min, default.point, (ranged map (_.pos.end)).max)
   }
 
@@ -59,13 +59,25 @@ self: scala.tools.nsc.Global =>
     if (headpos.isDefined) wrappingPos(headpos, trees) else headpos
   }
 
-/*
-  override def integratePos(tree: Tree, pos: Position) =
-    if (pos.isSynthetic && !tree.pos.isSynthetic) tree.syntheticDuplicate
-    else tree
-*/
-
   // -------------- ensuring no overlaps -------------------------------
+  
+  /** Ensure that given tree has no positions that overlap with
+   *  any of the positions of `others`. This is done by
+   *  shortening the range, assigning TransparentPositions
+   *  to some of the nodes in `tree` or focusing on the position.
+   */
+  override def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean) {
+    def isOverlapping(pos: Position) =
+      pos.isRange && (others exists (pos overlaps _.pos))
+    if (isOverlapping(tree.pos)) {
+      val children = tree.children
+      children foreach (ensureNonOverlapping(_, others, focus))
+      if (tree.pos.isOpaqueRange) {
+        val wpos = wrappingPos(tree.pos, children, focus)
+        tree setPos (if (isOverlapping(wpos)) tree.pos.makeTransparent else wpos)
+      }
+    }
+  }
 
   def solidDescendants(tree: Tree): List[Tree] =
     if (tree.pos.isTransparent) tree.children flatMap solidDescendants
@@ -106,24 +118,6 @@ self: scala.tools.nsc.Global =>
     if (ts.head == t) replacement ::: ts.tail
     else ts.head :: replace(ts.tail, t, replacement)
 
-  /** Ensure that given tree has no positions that overlap with
-   *  any of the positions of `others`. This is done by
-   *  shortening the range or assigning TransparentPositions
-   *  to some of the nodes in `tree`.
-   */
-  override def ensureNonOverlapping(tree: Tree, others: List[Tree]) {
-    def isOverlapping(pos: Position) =
-      pos.isRange && (others exists (pos overlaps _.pos))
-    if (isOverlapping(tree.pos)) {
-      val children = tree.children
-      children foreach (ensureNonOverlapping(_, others))
-      if (tree.pos.isOpaqueRange) {
-        val wpos = wrappingPos(tree.pos.focus, children)
-        tree setPos (if (isOverlapping(wpos)) tree.pos.makeTransparent else wpos)
-      }
-    }
-  }
-
   /** Does given list of trees have mutually non-overlapping positions?
    *  pre: None of the trees is transparent
    */
diff --git a/src/reflect/scala/reflect/internal/Positions.scala b/src/reflect/scala/reflect/internal/Positions.scala
index 6ae9b40fcb..faa161d6b1 100644
--- a/src/reflect/scala/reflect/internal/Positions.scala
+++ b/src/reflect/scala/reflect/internal/Positions.scala
@@ -10,23 +10,25 @@ trait Positions extends api.Positions { self: SymbolTable =>
   /** A position that wraps a set of trees.
    *  The point of the wrapping position is the point of the default position.
    *  If some of the trees are ranges, returns a range position enclosing all ranges
-   *  Otherwise returns default position.
+   *  Otherwise returns default position that is either focused or not.
    */
-  def wrappingPos(default: Position, trees: List[Tree]): Position = default
+  def wrappingPos(default: Position, trees: List[Tree]) = wrappingPos(default, trees, true)
+  def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = default
 
   /** A position that wraps the non-empty set of trees.
    *  The point of the wrapping position is the point of the first trees' position.
-   *  If all some the trees are non-synthetic, returns a range position enclosing the non-synthetic trees
+   *  If some of the trees are non-synthetic, returns a range position enclosing the non-synthetic trees
    *  Otherwise returns a synthetic offset position to point.
    */
   def wrappingPos(trees: List[Tree]): Position = trees.head.pos
 
   /** Ensure that given tree has no positions that overlap with
    *  any of the positions of `others`. This is done by
-   *  shortening the range or assigning TransparentPositions
-   *  to some of the nodes in `tree`.
+   *  shortening the range, assigning TransparentPositions
+   *  to some of the nodes in `tree` or focusing on the position.
    */
-  def ensureNonOverlapping(tree: Tree, others: List[Tree]) {}
+  def ensureNonOverlapping(tree: Tree, others: List[Tree]){ ensureNonOverlapping(tree, others, true) }
+  def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean) {}
 
   trait PosAssigner extends Traverser {
     var pos: Position
diff --git a/test/files/run/t5603.check b/test/files/run/t5603.check
new file mode 100644
index 0000000000..5127d3c1c7
--- /dev/null
+++ b/test/files/run/t5603.check
@@ -0,0 +1,29 @@
+[[syntax trees at end of                    parser]] // newSource1
+[0:241]package [0:0]<empty> {
+  [0:82]abstract trait Greeting extends [15:82][83]scala.AnyRef {
+    [15]def $init$() = [15]{
+      [15]()
+    };
+    [23:39]val name: [33:39]String;
+    [46:76]val msg = [56:76][56:72][56:71]"How are you, ".$plus([72:76]name)
+  };
+  [87:209]class C extends [94:209][151:159]Greeting {
+    [119:139]val nameElse = _;
+    [95:101]<paramaccessor> private[this] val i: [98:101]Int = _;
+    <119:139>def <init>([95]i: [98]Int) = <119:139>{
+      <119:139>val nameElse = <134:139>"Bob";
+      [94][94][94]super.<init>();
+      [94]()
+    };
+    [168:184]val name = [179:184]"avc";
+    [191:203][191:198]println([199:202]msg)
+  };
+  [215:241]object Test extends [227:241][235:238]App {
+    [227]def <init>() = [227]{
+      [227][227][227]super.<init>();
+      [227]()
+    };
+    [NoPosition]<empty>
+  }
+}
+
diff --git a/test/files/run/t5603.scala b/test/files/run/t5603.scala
new file mode 100644
index 0000000000..60dfd01fee
--- /dev/null
+++ b/test/files/run/t5603.scala
@@ -0,0 +1,42 @@
+import scala.tools.partest._
+import java.io._
+import scala.tools.nsc._
+import scala.tools.nsc.util.CommandLineParser
+import scala.tools.nsc.{Global, Settings, CompilerCommand}
+import scala.tools.nsc.reporters.ConsoleReporter
+
+object Test extends DirectTest {
+
+  override def extraSettings: String = "-usejavacp -Xprint:parser -Ystop-after:parser -d " + testOutput.path
+
+  override def code = """
+    trait Greeting {
+      val name: String
+      val msg = "How are you, "+name
+    }
+    class C(i: Int) extends {
+      val nameElse = "Bob"
+    } with Greeting {
+      val name = "avc"
+      println(msg)
+    }
+
+    object Test extends App {}
+  """.trim
+
+  override def show(): Unit = {
+    // redirect err to out, for logging
+    val prevErr = System.err
+    System.setErr(System.out)
+    compile()
+    System.setErr(prevErr)
+  }
+
+  override def newCompiler(args: String*): Global = {
+
+    val settings = new Settings()
+    settings.Xprintpos.value = true
+    val command = new CompilerCommand((CommandLineParser tokenize extraSettings) ++ args.toList, settings)
+    new Global(command.settings, new ConsoleReporter(settings)) with interactive.RangePositions
+  }
+}