Skip to content

Commit 037561a

Browse files
Rewrite collection.{Map, Set}.+ and collection.Set.-
This is incomplete since, it's blocked by scalameta/scalameta#1212 (Add support to query type of a term) iset: immutable.Set[Int] cset: collecion.Set[Int] both have +/- implemented via SetLike given iset + 1 cset + 1 we know that + is from SetLike, but it's not possible to get the type of iset/cset Rewrite collection.Set.+ restrict type of lhs scalafix allow us to match on SetLike.+ but we cannot check the type of the expression on the lhs We can work arround this limitation if the lhs is a identifier (simple case). This allow us to document the expected type on the lhs and prepare the implementation once scalameta/scalameta#1212 is resolved
1 parent 6a1d39c commit 037561a

File tree

3 files changed

+101
-15
lines changed

3 files changed

+101
-15
lines changed

scalafix/input/src/main/scala/fix/SetMapSrc.scala

+19-7
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,22 @@ rule = "scala:fix.Scalacollectioncompat_newcollections"
33
*/
44
package fix
55

6-
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
7-
set + (2, 3)
8-
map + (2 -> 3, 3 -> 4)
9-
(set + (2, 3)).map(x => x)
10-
set + (2, 3) - 4
11-
map.mapValues(_ + 1)
12-
}
6+
import scala.collection
7+
import scala.collection.immutable
8+
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct
9+
10+
class SetMapSrc(iset: immutable.Set[Int],
11+
cset: collection.Set[Int],
12+
imap: immutable.Map[Int, Int],
13+
cmap: collection.Map[Int, Int]) {
14+
iset + (2, 3)
15+
imap + (2 -> 3, 3 -> 4)
16+
(iset + (2, 3)).toString
17+
iset + (2, 3) - 4
18+
imap.mapValues(_ + 1)
19+
iset + 1
20+
iset - 2
21+
cset + 1
22+
cset - 2
23+
cmap + (2 -> 3) + ((4, 5))
24+
}

scalafix/output/src/main/scala/fix/SetMapSrc.scala

+18-6
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,22 @@
33

44
package fix
55

6-
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
7-
set + 2 + 3
8-
map + (2 -> 3) + (3 -> 4)
9-
(set + 2 + 3).map(x => x)
10-
set + 2 + 3 - 4
11-
map.mapValues(_ + 1).toMap
6+
import scala.collection
7+
import scala.collection.immutable
8+
import scala.collection.mutable.{Map, Set} // Challenge to make sure the scoping is correct
9+
10+
class SetMapSrc(iset: immutable.Set[Int],
11+
cset: collection.Set[Int],
12+
imap: immutable.Map[Int, Int],
13+
cmap: collection.Map[Int, Int]) {
14+
iset + 2 + 3
15+
imap + (2 -> 3) + (3 -> 4)
16+
(iset + 2 + 3).toString
17+
iset + 2 + 3 - 4
18+
imap.mapValues(_ + 1).toMap
19+
iset + 1
20+
iset - 2
21+
cset ++ _root_.scala.collection.Set(1)
22+
cset -- _root_.scala.collection.Set(2)
23+
cmap ++ _root_.scala.collection.Map(2 -> 3) ++ _root_.scala.collection.Map((4, 5))
1224
}

scalafix/rules/src/main/scala/fix/Scalacollectioncompat_newcollections.scala

+64-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,24 @@ import scala.meta._
88
case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
99
extends SemanticRule(index, "Scalacollectioncompat_newcollections") {
1010

11+
// WARNING: TOTAL HACK
12+
// this is only to unblock us until Term.tpe is available: https://github.com/scalameta/scalameta/issues/1212
13+
// if we have a simple identifier, we can look at his definition at query it's type
14+
// this should be improved in future version of scalameta
15+
object TypeMatcher {
16+
def apply(symbols: Symbol*)(implicit index: SemanticdbIndex): TypeMatcher =
17+
new TypeMatcher(symbols: _*)(index)
18+
}
19+
20+
final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) {
21+
def unapply(tree: Tree): Boolean = {
22+
index.denotation(tree)
23+
.exists(_.names.headOption.exists(n => symbols.exists(_ == n.symbol)))
24+
}
25+
}
26+
27+
val CollectionSet: TypeMatcher = TypeMatcher(Symbol("_root_.scala.collection.Set#"))
28+
1129
def replaceSymbols(ctx: RuleCtx): Patch = {
1230
ctx.replaceSymbols(
1331
"scala.collection.LinearSeq" -> "scala.collection.immutable.List",
@@ -30,6 +48,18 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
3048
Symbol("_root_.scala.runtime.Tuple2Zipped.Ops.zipped."),
3149
Symbol("_root_.scala.runtime.Tuple3Zipped.Ops.zipped.")
3250
)
51+
val setPlus =
52+
SymbolMatcher.exact(
53+
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
54+
)
55+
val setMinus =
56+
SymbolMatcher.exact(
57+
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
58+
)
59+
val mapPlus =
60+
SymbolMatcher.exact(
61+
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
62+
)
3363
val setPlus2 = SymbolMatcher.exact(
3464
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;Ljava/lang/Object;Lscala/collection/Seq;)Lscala/collection/Set;.")
3565
)
@@ -82,6 +112,9 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
82112
Symbol("_root_.scala.collection.mutable.ArrayBuilder.make(Lscala/reflect/ClassTag;)Lscala/collection/mutable/ArrayBuilder;.")
83113
)
84114

115+
def startsWithParens(tree: Tree): Boolean =
116+
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)
117+
85118
def replaceMutableSet(ctx: RuleCtx) =
86119
ctx.tree.collect {
87120
case retainSet(n: Name) =>
@@ -192,7 +225,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
192225
def replaceSetMapPlus2(ctx: RuleCtx): Patch = {
193226
def rewritePlus(ap: Term.ApplyInfix, lhs: Term, op: Term.Name, rhs1: Term, rhs2: Term): Patch = {
194227
val tokensToReplace =
195-
if(ap.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)) {
228+
if(startsWithParens(ap)) {
196229
// don't drop surrounding parens
197230
ap.tokens.slice(1, ap.tokens.size - 1)
198231
} else ap.tokens
@@ -217,6 +250,34 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
217250
}.asPatch
218251
}
219252

253+
def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
254+
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
255+
val col = "_root_.scala.collection." + col0
256+
val callSite =
257+
if (startsWithParens(rhs)) {
258+
ctx.addLeft(rhs, col)
259+
}
260+
else {
261+
ctx.addLeft(rhs, col + "(") +
262+
ctx.addRight(rhs, ")")
263+
}
264+
265+
ctx.addRight(op, doubleOp) + callSite
266+
}
267+
268+
ctx.tree.collect {
269+
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
270+
rewriteOp(op, rhs, "+", "Set")
271+
272+
case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
273+
rewriteOp(op, rhs, "-", "Set")
274+
275+
case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
276+
rewriteOp(op, rhs, "+", "Map")
277+
}.asPatch
278+
}
279+
280+
220281
def replaceMutSetMapPlus(ctx: RuleCtx): Patch = {
221282
def rewriteMutPlus(lhs: Term, op: Term.Name): Patch = {
222283
ctx.addRight(lhs, ".clone()") +
@@ -265,7 +326,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
265326
ctx.addRight(ap, ".toMap")
266327
}.asPatch
267328
}
268-
329+
269330
override def fix(ctx: RuleCtx): Patch = {
270331
replaceToList(ctx) +
271332
replaceSymbols(ctx) +
@@ -276,6 +337,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
276337
replaceMutableSet(ctx) +
277338
replaceSymbolicFold(ctx) +
278339
replaceSetMapPlus2(ctx) +
340+
replaceSetMapPlusMinus(ctx) +
279341
replaceMutSetMapPlus(ctx) +
280342
replaceMutMapUpdated(ctx) +
281343
replaceIterableSameElements(ctx) +

0 commit comments

Comments
 (0)