-
Notifications
You must be signed in to change notification settings - Fork 87
Rewrite collection.{Map, Set}.+ and collection.Set.- #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
related: scala/bug#10957 |
import scala.collection | ||
import scala.collection.immutable | ||
import scala.collection.mutable.{Map, Set} // Challenge to make shure the scoping is correct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: “make sure”
(iset + 2 + 3).toString | ||
iset + 2 + 3 - 4 | ||
iset + 1 - 2 | ||
cset ++ _root_.scala.collection.Set(1) -- _root_.scala.collection.Set(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be working. Can you elaborate on what’s missing?
imap + (2 -> 3) + (3 -> 4) | ||
(iset + 2 + 3).toString | ||
iset + 2 + 3 - 4 | ||
iset + 1 - 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julienrf this test fails ^
5b8efb3
to
0d63ea8
Compare
} | ||
|
||
final class TypeMatcher(symbol: Symbol)(implicit index: SemanticdbIndex) { | ||
def unapply(tree: Tree): Option[Unit] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI you can make it return a Boolean
rather than an Option[Unit]
. Then at use site just write case CollectionSet() =>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
6b5b7fb
to
13d6566
Compare
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
13d6566
to
037561a
Compare
@julienrf ready for review |
cset + 1 | ||
cset - 2 | ||
cmap + (2 -> 3) + ((4, 5)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add the following test case?
- imap + (2 -> 3) + ((4, 5))
+ imap + (2 -> 3) + ((4, 5))
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