Skip to content
This repository was archived by the owner on Sep 3, 2020. It is now read-only.

T1002281 class cast ex #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
package scala.tools.refactoring
package common

import collection.mutable.ListBuffer
import tools.nsc.Global
import scala.collection.mutable.ListBuffer
import scala.reflect.internal.util.OffsetPosition
import scala.reflect.internal.util.RangePosition
import scala.reflect.internal.Flags

trait Selections extends TreeTraverser with common.PimpedTrees {

Expand Down Expand Up @@ -73,12 +72,12 @@ trait Selections extends TreeTraverser with common.PimpedTrees {
/**
* Returns true if the given Tree is fully contained in the selection.
*/
def contains(t: Tree) = isPosContainedIn(t.pos, pos)
def contains(t: Tree): Boolean = isPosContainedIn(t.pos, pos)

/**
* Returns true if the given Tree fully contains this selection.
*/
def isContainedIn(t: Tree) = isPosContainedIn(pos, t.pos)
def isContainedIn(t: Tree): Boolean = isPosContainedIn(pos, t.pos)

/**
* Tries to find the selected SymTree: first it is checked if the selection
Expand Down Expand Up @@ -233,7 +232,7 @@ trait Selections extends TreeTraverser with common.PimpedTrees {

// some trees have to be selected as a whole if more than one child
// is selected. For example if a method parameter and the body is selected,
// we select the DefDef as a whole to get a complete selection.
// we select the DefDef as a whole to get a complete selection.
def expandToParentIfRequired(s: Selection) =
s.enclosingTree match {
case t @ (_: DefDef | _: Function | _: If | _: Match | _: Try | _: CaseDef) =>
Expand Down Expand Up @@ -276,12 +275,21 @@ trait Selections extends TreeTraverser with common.PimpedTrees {
else
None

/**
* An [[scala.reflect.internal.util.OffsetPosition]] is needed as a new
* position. Throws an exception if another position type is passed.
*/
def withPos(newPos: Position): Selection = {
val p = newPos match {
case p: RangePosition => p
case p: OffsetPosition => new RangePosition(p.source, p.start, p.start, p.start)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, you really need a RangePosition there. Why not throw as well on OffsetPosition, rather than giving a RangePosition which has high chances of being false ( I expect callers will want the word at point) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, and I wonder why we get an OffsetPosition in the first place...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess SI-8807 is one reason...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What could go wrong if OffsetPosition is treated as RangePosition? This way we need to ensure that no OffsetPosition is passed into scala-refactoring otherwise it blows up. Maybe that is the right thing to do, then I just need to fix the ticket on the IDE side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There just shouldn't be a reason to use an OffsetPosition. Remember, they're only used for compiler generated trees, so if a refactoring is started from the editor, there should always be a better suited RangePosition available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, and having an exception on OffsetPositions works as a canary.

case p => throw new IllegalArgumentException(s"An offset position is required, but was: $p")
}
val outer = this
new Selection {
val root = outer.root
val file = outer.file
val pos = newPos.asInstanceOf[RangePosition]
override val root = outer.root
override val file = outer.file
override val pos = p
}
}

Expand Down Expand Up @@ -388,36 +396,36 @@ trait Selections extends TreeTraverser with common.PimpedTrees {
}
}

def skipForExpressionTrees(t: Tree) = t match {
def skipForExpressionTrees(t: Tree): Tree = t match {
case t @ TypeApply(fun: Select, args) if fun.pos.eq(t.pos) && fun.pos.eq(fun.qualifier.pos) =>
fun.qualifier
case t @ Select(qualifier, nme) if t.pos.eq(qualifier.pos) && nme.toTermName.toString == "withFilter" =>
qualifier
case t => t
}

case class FileSelection(file: tools.nsc.io.AbstractFile, root: Tree, from: Int, to: Int) extends Selection {
case class FileSelection(override val file: tools.nsc.io.AbstractFile, override val root: Tree, from: Int, to: Int) extends Selection {

@deprecated("Please use the primary constructor.", "0.4.0")
def this(file: tools.nsc.io.AbstractFile, from: Int, to: Int) = {
this(file, compilationUnitOfFile(file).get.body, from, to)
}

lazy val pos = new RangePosition(root.pos.source, from, from, to)
override lazy val pos = new RangePosition(root.pos.source, from, from, to)
}

object FileSelection {
@deprecated("Please use the primary constructor.", "0.4.0")
def apply(file: tools.nsc.io.AbstractFile, from: Int, to: Int) = new FileSelection(file: tools.nsc.io.AbstractFile, from: Int, to: Int)
def apply(file: tools.nsc.io.AbstractFile, from: Int, to: Int): FileSelection = new FileSelection(file: tools.nsc.io.AbstractFile, from: Int, to: Int)
}

case class TreeSelection(root: Tree) extends Selection {
case class TreeSelection(override val root: Tree) extends Selection {

if (!root.pos.isRange)
error("Position not a range.")

val pos = root.pos.asInstanceOf[RangePosition]
override val pos: RangePosition = root.pos.asInstanceOf[RangePosition]

val file = pos.source.file
override val file: tools.nsc.io.AbstractFile = pos.source.file
}
}