Rewrite Map.zip -> Map.zip.toMap #62
Conversation
| @@ -66,6 +66,11 @@ case class Scalacollectioncompat_newcollections(index: SemanticdbIndex) | |||
| Symbol("_root_.scala.collection.mutable.SetLike.retain.") | |||
| ) | |||
|
|
|||
| val mapZip = | |||
| SymbolMatcher.exact( | |||
| Symbol("_root_.scala.collection.IterableLike#zip(Lscala/collection/GenIterable;Lscala/collection/generic/CanBuildFrom;)Ljava/lang/Object;.") | |||
There was a problem hiding this comment.
It looks like it’s going to match any call to zip, even if the receiver is not of type Map[_, _].
There was a problem hiding this comment.
then we are blocked by scalameta/scalameta#1212
MasseGuillaume
force-pushed
the
map-zip
branch
3 times, most recently
from
1e993e1 to
eec6b8a
Compare
| final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) { | ||
| def unapply(tree: Tree): Boolean = { | ||
| index.denotation(tree) | ||
| .map(_.names.headOption.exists(n => symbols.exists(_ == n.symbol))) |
There was a problem hiding this comment.
What are names? Why take the headOption of it?
There was a problem hiding this comment.
I’m sorry but I’d like to have more information about what it does.
There was a problem hiding this comment.
It's in the commit message: ae843d4
There was a problem hiding this comment.
That does not really answer my question. Why do we use names.headOption.exists rather than names.exists?
There was a problem hiding this comment.
for Set[Int] it would be:
1) scala.collection.immutable.Set
2) Int
we take the head.
There was a problem hiding this comment.
OK, so if I write val xs = HashMap(1 -> "foo") and then xs.zip(…), the denotation for xs will contain Seq("scala.collection.immutable.HashMap", "Int", "String")? And then it won’t match with the types you wrote when you defined CollectionMap.
Is it possible to know all the ancestors of the type symbol?
There was a problem hiding this comment.
Please leave a comment in the source code that this is totally hacky and is only used to unblock writing further tests as a temporary workaround while Term.tpe is not supported.
There was a problem hiding this comment.
Is it possible to know all the ancestors of the type symbol?
v0.6 has support for that but not v0.5
| def unapply(tree: Tree): Boolean = { | ||
| index.denotation(tree) | ||
| .map(_.names.headOption.exists(n => symbols.exists(_ == n.symbol))) | ||
| .getOrElse(false) |
There was a problem hiding this comment.
.map(p).getOrElse(false) can be replaced with .exists(p)
| new TypeMatcher(symbols: _*)(index) | ||
| } | ||
|
|
||
| final class TypeMatcher(symbols: Symbol*)(implicit index: SemanticdbIndex) { |
There was a problem hiding this comment.
Do you mind adding some documentation?
There was a problem hiding this comment.
Yes please, in particular highlight in caps lock that this is not The Real Deal. This is only to unblock tests, I would in fact prefer if this was not merged into master because users should not run this.
|
@julienrf ready for review |
| val CollectionMap: TypeMatcher = TypeMatcher( | ||
| Symbol("_root_.scala.collection.immutable.Map#"), | ||
| Symbol("_root_.scala.collection.mutable.Map#"), | ||
| Symbol("_root_.scala.Predef.Map#") |
There was a problem hiding this comment.
Can you add HashMap and TreeMap as well?
There was a problem hiding this comment.
I would rather not dig that hole too deep. The idea is to document what is the expected type, not to have a working rule.
|
What do you think of merging this as an “Experimental” rule separate from “NewCollections”? |
|
Yes, let's do this. |
No description provided.