Skip to content

Commit 84864eb

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 29aca34 commit 84864eb

File tree

6 files changed

+160
-123
lines changed

6 files changed

+160
-123
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-77
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,6 @@ 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 CollectionMap: TypeMatcher = TypeMatcher(
28-
Symbol("_root_.scala.collection.immutable.Map#"),
29-
Symbol("_root_.scala.collection.mutable.Map#"),
30-
Symbol("_root_.scala.Predef.Map#")
31-
)
32-
33-
val CollectionSet: TypeMatcher = TypeMatcher(Symbol("_root_.scala.collection.Set#"))
34-
3511
def replaceSymbols(ctx: RuleCtx): Patch = {
3612
ctx.replaceSymbols(
3713
"scala.collection.LinearSeq" -> "scala.collection.immutable.List",
@@ -54,18 +30,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
5430
Symbol("_root_.scala.runtime.Tuple2Zipped.Ops.zipped."),
5531
Symbol("_root_.scala.runtime.Tuple3Zipped.Ops.zipped.")
5632
)
57-
val setPlus =
58-
SymbolMatcher.exact(
59-
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;)Lscala/collection/Set;.")
60-
)
61-
val setMinus =
62-
SymbolMatcher.exact(
63-
Symbol("_root_.scala.collection.SetLike#`-`(Ljava/lang/Object;)Lscala/collection/Set;.")
64-
)
65-
val mapPlus =
66-
SymbolMatcher.exact(
67-
Symbol("_root_.scala.collection.MapLike#`+`(Lscala/Tuple2;)Lscala/collection/Map;.")
68-
)
6933
val setPlus2 = SymbolMatcher.exact(
7034
Symbol("_root_.scala.collection.SetLike#`+`(Ljava/lang/Object;Ljava/lang/Object;Lscala/collection/Seq;)Lscala/collection/Set;.")
7135
)
@@ -119,11 +83,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
11983
Symbol("_root_.scala.collection.mutable.ArrayBuilder.make(Lscala/reflect/ClassTag;)Lscala/collection/mutable/ArrayBuilder;.")
12084
)
12185

122-
val mapZip =
123-
SymbolMatcher.exact(
124-
Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.")
125-
)
126-
12786
def startsWithParens(tree: Tree): Boolean =
12887
tree.tokens.headOption.map(_.is[Token.LeftParen]).getOrElse(false)
12988

@@ -262,34 +221,6 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
262221
}.asPatch
263222
}
264223

265-
def replaceSetMapPlusMinus(ctx: RuleCtx): Patch = {
266-
def rewriteOp(op: Tree, rhs: Tree, doubleOp: String, col0: String): Patch = {
267-
val col = "_root_.scala.collection." + col0
268-
val callSite =
269-
if (startsWithParens(rhs)) {
270-
ctx.addLeft(rhs, col)
271-
}
272-
else {
273-
ctx.addLeft(rhs, col + "(") +
274-
ctx.addRight(rhs, ")")
275-
}
276-
277-
ctx.addRight(op, doubleOp) + callSite
278-
}
279-
280-
ctx.tree.collect {
281-
case Term.ApplyInfix(CollectionSet(), op @ setPlus(_), Nil, List(rhs)) =>
282-
rewriteOp(op, rhs, "+", "Set")
283-
284-
case Term.ApplyInfix(CollectionSet(), op @ setMinus(_), Nil, List(rhs)) =>
285-
rewriteOp(op, rhs, "-", "Set")
286-
287-
case Term.ApplyInfix(_, op @ mapPlus(_), Nil, List(rhs)) =>
288-
rewriteOp(op, rhs, "+", "Map")
289-
}.asPatch
290-
}
291-
292-
293224
def replaceMutSetMapPlus(ctx: RuleCtx): Patch = {
294225
def rewriteMutPlus(lhs: Term, op: Term.Name): Patch = {
295226
ctx.addRight(lhs, ".clone()") +
@@ -331,12 +262,7 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
331262
}.asPatch
332263
}
333264

334-
def replaceMapZip(ctx: RuleCtx): Patch = {
335-
ctx.tree.collect {
336-
case ap @ Term.Apply(Term.Select(CollectionMap(), mapZip(_)), List(_)) =>
337-
ctx.addRight(ap, ".toMap")
338-
}.asPatch
339-
}
265+
340266

341267
def replaceMapMapValues(ctx: RuleCtx): Patch = {
342268
ctx.tree.collect {
@@ -355,12 +281,10 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex)
355281
replaceMutableSet(ctx) +
356282
replaceSymbolicFold(ctx) +
357283
replaceSetMapPlus2(ctx) +
358-
replaceSetMapPlusMinus(ctx) +
359284
replaceMutSetMapPlus(ctx) +
360285
replaceMutMapUpdated(ctx) +
361286
replaceIterableSameElements(ctx) +
362287
replaceArrayBuilderMake(ctx) +
363-
replaceMapZip(ctx) +
364288
replaceMapMapValues(ctx)
365289
}
366290
}

0 commit comments

Comments
 (0)