Skip to content

Commit

Permalink
Rewrite: Move Set/Map.{+,-} and Map.zip to Experimental rules.
Browse files Browse the repository at this point in the history
Terms don't have type in scalameta yet: scalameta/scalameta#1212

The SymbolMatcher is not enought for those rules since we need to check the type of the lhs.
One workarround is to jump to the symbol if we see an identifier on the lhs. for example:

iset: immutable.Set[Int]
// ...
iset + 1

we can jump to the symbol of iset and get it's type.

This is really fragile, since it's not going to work if we have aliases/subtyping.

However, it's useful to create those rules, since when we are unblock by scalameta#1212 it's
trivial to add the remaining piece
  • Loading branch information
MasseGuillaume committed Jun 27, 2018
1 parent 29aca34 commit 84864eb
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 123 deletions.
27 changes: 27 additions & 0 deletions scalafix/input/src/main/scala/fix/ExperimentalSrc.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
rule = "scala:fix.Experimental"
*/
package fix

import scala.collection
import scala.collection.immutable
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct

class ExperimentalSrc(iset: immutable.Set[Int],
cset: collection.Set[Int],
imap: immutable.Map[Int, Int],
cmap: collection.Map[Int, Int]) {
iset + 1
iset - 2
cset + 1
cset - 2

cmap + (2 -> 3)
cmap + ((4, 5))
imap + (2 -> 3)
imap + ((4, 5))

// Map.zip
imap.zip(List())
List().zip(List())
}
29 changes: 6 additions & 23 deletions scalafix/input/src/main/scala/fix/SetMapSrc.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,10 @@ rule = "scala:fix.Scalacollectioncompat_newcollections"
*/
package fix

import scala.collection
import scala.collection.immutable
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct

class SetMapSrc(iset: immutable.Set[Int],
cset: collection.Set[Int],
imap: immutable.Map[Int, Int],
cmap: collection.Map[Int, Int]) {
iset + (2, 3)
imap + (2 -> 3, 3 -> 4)
(iset + (2, 3)).toString
iset + (2, 3) - 4
imap.mapValues(_ + 1)
iset + 1
iset - 2
cset + 1
cset - 2
cmap + (2 -> 3)
cmap + ((4, 5))
imap + (2 -> 3)
imap + ((4, 5))
imap.zip(List())
List().zip(List())
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
set + (2, 3)
map + (2 -> 3, 3 -> 4)
(set + (2, 3)).toString
set + (2, 3) - 4
map.mapValues(_ + 1)
}
27 changes: 27 additions & 0 deletions scalafix/output/src/main/scala/fix/ExperimentalSrc.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@



package fix

import scala.collection
import scala.collection.immutable
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct

class ExperimentalSrc(iset: immutable.Set[Int],
cset: collection.Set[Int],
imap: immutable.Map[Int, Int],
cmap: collection.Map[Int, Int]) {
iset + 1
iset - 2
cset ++ _root_.scala.collection.Set(1)
cset -- _root_.scala.collection.Set(2)

cmap ++ _root_.scala.collection.Map(2 -> 3)
cmap ++ _root_.scala.collection.Map((4, 5))
imap + (2 -> 3)
imap + ((4, 5))

// Map.zip
imap.zip(List()).toMap
List().zip(List())
}
29 changes: 6 additions & 23 deletions scalafix/output/src/main/scala/fix/SetMapSrc.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,10 @@

package fix

import scala.collection
import scala.collection.immutable
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct

class SetMapSrc(iset: immutable.Set[Int],
cset: collection.Set[Int],
imap: immutable.Map[Int, Int],
cmap: collection.Map[Int, Int]) {
iset + 2 + 3
imap + (2 -> 3) + (3 -> 4)
(iset + 2 + 3).toString
iset + 2 + 3 - 4
imap.mapValues(_ + 1).toMap
iset + 1
iset - 2
cset ++ _root_.scala.collection.Set(1)
cset -- _root_.scala.collection.Set(2)
cmap ++ _root_.scala.collection.Map(2 -> 3)
cmap ++ _root_.scala.collection.Map((4, 5))
imap + (2 -> 3)
imap + ((4, 5))
imap.zip(List()).toMap
List().zip(List())
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
set + 2 + 3
map + (2 -> 3) + (3 -> 4)
(set + 2 + 3).toString
set + 2 + 3 - 4
map.mapValues(_ + 1).toMap
}
93 changes: 93 additions & 0 deletions scalafix/rules/src/main/scala/fix/Experimental.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package fix

import scalafix._
import scalafix.syntax._
import scalafix.util._
import scala.meta._

case class Experimental(index: SemanticdbIndex) extends SemanticRule(index, "Experimental") {
// WARNING: TOTAL HACK
// this is only to unblock us until Term.tpe is available: https://github.com/scalameta/scalameta/issues/1212
// if we have a simple identifier, we can look at his definition at query it's type
// this should be improved in future version of scalameta
object TypeMatcher {
def apply(symbols: Symbol*)(implicit index: SemanticdbIndex): TypeMatcher =
new TypeMatcher(symbols: _*)(index)
}

final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) {
def unapply(tree: Tree): Boolean = {
index.denotation(tree)
.exists(_.names.headOption.exists(n => symbols.exists(_ == n.symbol)))
}
}

val CollectionMap: TypeMatcher = TypeMatcher(
Symbol("_root_.scala.collection.immutable.Map#"),
Symbol("_root_.scala.collection.mutable.Map#"),
Symbol("_root_.scala.Predef.Map#")
)

val CollectionSet: TypeMatcher = TypeMatcher(Symbol("_root_.scala.collection.Set#"))

val mapZip =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.")
)

val mapPlus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
)

val setPlus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
)

val setMinus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
)

def startsWithParens(tree: Tree): Boolean =
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)

def replaceMapZip(ctx: RuleCtx): Patch = {
ctx.tree.collect {
case ap @ Term.Apply(Term.Select(CollectionMap(), mapZip(_)), List(_)) =>
ctx.addRight(ap, ".toMap")
}.asPatch
}

def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
val col = "_root_.scala.collection." + col0
val callSite =
if (startsWithParens(rhs)) {
ctx.addLeft(rhs, col)
}
else {
ctx.addLeft(rhs, col + "(") +
ctx.addRight(rhs, ")")
}

ctx.addRight(op, doubleOp) + callSite
}

ctx.tree.collect {
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "+", "Set")

case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "-", "Set")

case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "+", "Map")
}.asPatch
}

override def fix(ctx: RuleCtx): Patch =
replaceSetMapPlusMinus(ctx) +
replaceMapZip(ctx)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,6 @@ import scala.meta._
case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
extends SemanticRule(index, "Scalacollectioncompat_newcollections") {

// WARNING: TOTAL HACK
// this is only to unblock us until Term.tpe is available: https://github.com/scalameta/scalameta/issues/1212
// if we have a simple identifier, we can look at his definition at query it's type
// this should be improved in future version of scalameta
object TypeMatcher {
def apply(symbols: Symbol*)(implicit index: SemanticdbIndex): TypeMatcher =
new TypeMatcher(symbols: _*)(index)
}

final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) {
def unapply(tree: Tree): Boolean = {
index.denotation(tree)
.exists(_.names.headOption.exists(n => symbols.exists(_ == n.symbol)))
}
}

val CollectionMap: TypeMatcher = TypeMatcher(
Symbol("_root_.scala.collection.immutable.Map#"),
Symbol("_root_.scala.collection.mutable.Map#"),
Symbol("_root_.scala.Predef.Map#")
)

val CollectionSet: TypeMatcher = TypeMatcher(Symbol("_root_.scala.collection.Set#"))

def replaceSymbols(ctx: RuleCtx): Patch = {
ctx.replaceSymbols(
"scala.collection.LinearSeq" -> "scala.collection.immutable.List",
Expand All @@ -54,18 +30,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
Symbol("_root_.scala.runtime.Tuple2Zipped.Ops.zipped."),
Symbol("_root_.scala.runtime.Tuple3Zipped.Ops.zipped.")
)
val setPlus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
)
val setMinus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
)
val mapPlus =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
)
val setPlus2 = SymbolMatcher.exact(
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;Ljava/lang/Object;Lscala/collection/Seq;)Lscala/collection/Set;.")
)
Expand Down Expand Up @@ -119,11 +83,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
Symbol("_root_.scala.collection.mutable.ArrayBuilder.make(Lscala/reflect/ClassTag;)Lscala/collection/mutable/ArrayBuilder;.")
)

val mapZip =
SymbolMatcher.exact(
Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.")
)

def startsWithParens(tree: Tree): Boolean =
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)

Expand Down Expand Up @@ -262,34 +221,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
}.asPatch
}

def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
val col = "_root_.scala.collection." + col0
val callSite =
if (startsWithParens(rhs)) {
ctx.addLeft(rhs, col)
}
else {
ctx.addLeft(rhs, col + "(") +
ctx.addRight(rhs, ")")
}

ctx.addRight(op, doubleOp) + callSite
}

ctx.tree.collect {
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "+", "Set")

case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "-", "Set")

case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
rewriteOp(op, rhs, "+", "Map")
}.asPatch
}


def replaceMutSetMapPlus(ctx: RuleCtx): Patch = {
def rewriteMutPlus(lhs: Term, op: Term.Name): Patch = {
ctx.addRight(lhs, ".clone()") +
Expand Down Expand Up @@ -331,12 +262,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
}.asPatch
}

def replaceMapZip(ctx: RuleCtx): Patch = {
ctx.tree.collect {
case ap @ Term.Apply(Term.Select(CollectionMap(), mapZip(_)), List(_)) =>
ctx.addRight(ap, ".toMap")
}.asPatch
}


def replaceMapMapValues(ctx: RuleCtx): Patch = {
ctx.tree.collect {
Expand All @@ -355,12 +281,10 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
replaceMutableSet(ctx) +
replaceSymbolicFold(ctx) +
replaceSetMapPlus2(ctx) +
replaceSetMapPlusMinus(ctx) +
replaceMutSetMapPlus(ctx) +
replaceMutMapUpdated(ctx) +
replaceIterableSameElements(ctx) +
replaceArrayBuilderMake(ctx) +
replaceMapZip(ctx) +
replaceMapMapValues(ctx)
}
}

0 comments on commit 84864eb

Please sign in to comment.