Skip to content

Commit e1ca895

Browse files
MasseGuillaumeMasseGuillaume
committed
Rewrite: Move Set/Map.{+,-} and Map.zip to Experimental rules.
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
1 parent 2aef942 commit e1ca895

File tree

6 files changed

+160
-99
lines changed

6 files changed

+160
-99
lines changed

‎scalafix/input/src/main/scala/fix/ExperimentalSrc.scala

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
rule = "scala:fix.Experimental"
3+
*/
4+
package fix
5+
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 ExperimentalSrc(iset: immutable.Set[Int],
11+
cset: collection.Set[Int],
12+
imap: immutable.Map[Int, Int],
13+
cmap: collection.Map[Int, Int]) {
14+
iset + 1
15+
iset - 2
16+
cset + 1
17+
cset - 2
18+
19+
cmap + (2 -> 3)
20+
cmap + ((4, 5))
21+
imap + (2 -> 3)
22+
imap + ((4, 5))
23+
24+
// Map.zip
25+
imap.zip(List())
26+
List().zip(List())
27+
}

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

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

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)
24-
cmap + ((4, 5))
25-
imap + (2 -> 3)
26-
imap + ((4, 5))
27-
imap.zip(List())
28-
List().zip(List())
6+
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
7+
set + (2, 3)
8+
map + (2 -> 3, 3 -> 4)
9+
(set + (2, 3)).toString
10+
set + (2, 3) - 4
11+
map.mapValues(_ + 1)
2912
}

‎scalafix/output/src/main/scala/fix/ExperimentalSrc.scala

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
2+
3+
4+
package fix
5+
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 ExperimentalSrc(iset: immutable.Set[Int],
11+
cset: collection.Set[Int],
12+
imap: immutable.Map[Int, Int],
13+
cmap: collection.Map[Int, Int]) {
14+
iset + 1
15+
iset - 2
16+
cset ++ _root_.scala.collection.Set(1)
17+
cset -- _root_.scala.collection.Set(2)
18+
19+
cmap ++ _root_.scala.collection.Map(2 -> 3)
20+
cmap ++ _root_.scala.collection.Map((4, 5))
21+
imap + (2 -> 3)
22+
imap + ((4, 5))
23+
24+
// Map.zip
25+
imap.zip(List()).toMap
26+
List().zip(List())
27+
}

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

+6-23
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,10 @@
33

44
package fix
55

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)
24-
cmap ++ _root_.scala.collection.Map((4, 5))
25-
imap + (2 -> 3)
26-
imap + ((4, 5))
27-
imap.zip(List()).toMap
28-
List().zip(List())
6+
class SetMapSrc(set: Set[Int], map: Map[Int, Int]) {
7+
set + 2 + 3
8+
map + (2 -> 3) + (3 -> 4)
9+
(set + 2 + 3).toString
10+
set + 2 + 3 - 4
11+
map.mapValues(_ + 1).toMap
2912
}

‎scalafix/rules/src/main/scala/fix/Experimental.scala

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package fix
2+
3+
import scalafix._
4+
import scalafix.syntax._
5+
import scalafix.util._
6+
import scala.meta._
7+
8+
case class Experimental(index: SemanticdbIndex) extends SemanticRule(index, "Experimental") {
9+
// WARNING: TOTAL HACK
10+
// this is only to unblock us until Term.tpe is available: https://github.com/scalameta/scalameta/issues/1212
11+
// if we have a simple identifier, we can look at his definition at query it's type
12+
// this should be improved in future version of scalameta
13+
object TypeMatcher {
14+
def apply(symbols: Symbol*)(implicit index: SemanticdbIndex): TypeMatcher =
15+
new TypeMatcher(symbols: _*)(index)
16+
}
17+
18+
final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) {
19+
def unapply(tree: Tree): Boolean = {
20+
index.denotation(tree)
21+
.exists(_.names.headOption.exists(n => symbols.exists(_ == n.symbol)))
22+
}
23+
}
24+
25+
val CollectionMap: TypeMatcher = TypeMatcher(
26+
Symbol("_root_.scala.collection.immutable.Map#"),
27+
Symbol("_root_.scala.collection.mutable.Map#"),
28+
Symbol("_root_.scala.Predef.Map#")
29+
)
30+
31+
val CollectionSet: TypeMatcher = TypeMatcher(Symbol("_root_.scala.collection.Set#"))
32+
33+
val mapZip =
34+
SymbolMatcher.exact(
35+
Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.")
36+
)
37+
38+
val mapPlus =
39+
SymbolMatcher.exact(
40+
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
41+
)
42+
43+
val setPlus =
44+
SymbolMatcher.exact(
45+
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
46+
)
47+
48+
val setMinus =
49+
SymbolMatcher.exact(
50+
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
51+
)
52+
53+
def startsWithParens(tree: Tree): Boolean =
54+
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)
55+
56+
def replaceMapZip(ctx: RuleCtx): Patch = {
57+
ctx.tree.collect {
58+
case ap @ Term.Apply(Term.Select(CollectionMap(), mapZip(_)), List(_)) =>
59+
ctx.addRight(ap, ".toMap")
60+
}.asPatch
61+
}
62+
63+
def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
64+
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
65+
val col = "_root_.scala.collection." + col0
66+
val callSite =
67+
if (startsWithParens(rhs)) {
68+
ctx.addLeft(rhs, col)
69+
}
70+
else {
71+
ctx.addLeft(rhs, col + "(") +
72+
ctx.addRight(rhs, ")")
73+
}
74+
75+
ctx.addRight(op, doubleOp) + callSite
76+
}
77+
78+
ctx.tree.collect {
79+
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
80+
rewriteOp(op, rhs, "+", "Set")
81+
82+
case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
83+
rewriteOp(op, rhs, "-", "Set")
84+
85+
case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
86+
rewriteOp(op, rhs, "+", "Map")
87+
}.asPatch
88+
}
89+
90+
override def fix(ctx: RuleCtx): Patch =
91+
replaceSetMapPlusMinus(ctx) +
92+
replaceMapZip(ctx)
93+
}

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

+1-53
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
8888
Symbol("_root_.scala.runtime.Tuple2Zipped.Ops.zipped."),
8989
Symbol("_root_.scala.runtime.Tuple3Zipped.Ops.zipped.")
9090
)
91-
val setPlus =
92-
SymbolMatcher.exact(
93-
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
94-
)
95-
val setMinus =
96-
SymbolMatcher.exact(
97-
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
98-
)
99-
val mapPlus =
100-
SymbolMatcher.exact(
101-
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
102-
)
10391
val setPlus2 = SymbolMatcher.exact(
10492
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;Ljava/lang/Object;Lscala/collection/Seq;)Lscala/collection/Set;.")
10593
)
@@ -153,11 +141,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
153141
Symbol("_root_.scala.collection.mutable.ArrayBuilder.make(Lscala/reflect/ClassTag;)Lscala/collection/mutable/ArrayBuilder;.")
154142
)
155143

156-
val mapZip =
157-
SymbolMatcher.exact(
158-
Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.")
159-
)
160-
161144
def startsWithParens(tree: Tree): Boolean =
162145
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)
163146

@@ -287,34 +270,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
287270
}.asPatch
288271
}
289272

290-
def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
291-
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
292-
val col = "_root_.scala.collection." + col0
293-
val callSite =
294-
if (startsWithParens(rhs)) {
295-
ctx.addLeft(rhs, col)
296-
}
297-
else {
298-
ctx.addLeft(rhs, col + "(") +
299-
ctx.addRight(rhs, ")")
300-
}
301-
302-
ctx.addRight(op, doubleOp) + callSite
303-
}
304-
305-
ctx.tree.collect {
306-
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
307-
rewriteOp(op, rhs, "+", "Set")
308-
309-
case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
310-
rewriteOp(op, rhs, "-", "Set")
311-
312-
case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
313-
rewriteOp(op, rhs, "+", "Map")
314-
}.asPatch
315-
}
316-
317-
318273
def replaceMutSetMapPlus(ctx: RuleCtx): Patch = {
319274
def rewriteMutPlus(lhs: Term, op: Term.Name): Patch = {
320275
ctx.addRight(lhs, ".clone()") +
@@ -356,12 +311,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
356311
}.asPatch
357312
}
358313

359-
def replaceMapZip(ctx: RuleCtx): Patch = {
360-
ctx.tree.collect {
361-
case ap @ Term.Apply(Term.Select(CollectionMap(), mapZip(_)), List(_)) =>
362-
ctx.addRight(ap, ".toMap")
363-
}.asPatch
364-
}
314+
365315

366316
def replaceMapMapValues(ctx: RuleCtx): Patch = {
367317
ctx.tree.collect {
@@ -562,12 +512,10 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
562512
replaceMutableSet(ctx) +
563513
replaceSymbolicFold(ctx) +
564514
replaceSetMapPlus2(ctx) +
565-
replaceSetMapPlusMinus(ctx) +
566515
replaceMutSetMapPlus(ctx) +
567516
replaceMutMapUpdated(ctx) +
568517
replaceArrayBuilderMake(ctx) +
569518
replaceIterableSameElements(ctx) +
570-
replaceMapZip(ctx) +
571519
replaceMapMapValues(ctx)
572520
}
573521
}

0 commit comments

Comments
 (0)