-
Notifications
You must be signed in to change notification settings - Fork 337
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
bugfix: completions for implicit methods #6156
Conversation
Hey @kasiaMarek, thank you for this, after the #5904 I really wanted to have better implicit completions in Scala 2 too. 👍 Does this PR replace the work done by @tgodzik in #6002, or is it something related, but not exactly the same? Asking just out of curiosity, to gauge how far the support for the aging Scala 2 will improve. 😉 |
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.
LGTM
} | ||
} | ||
|
||
case class TypeWithParams(tpe: Type, typeParams: List[Symbol]) |
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.
I don't think this case class is helpful, but its just a nit.
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.
That's a good point actually. I changed this and it should much cleaner now.
tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala
Outdated
Show resolved
Hide resolved
@@ -219,7 +219,7 @@ class CompletionSnippetSuite extends BaseCompletionSuite { | |||
"2.13" -> | |||
"""|Iterable | |||
|Iterable[$0] {} | |||
|IterableOnce[$0] {} | |||
|IterableOnce[$0] |
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.
why the change here?
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.
That's caused by presentationCompiler.restart()
. This test only ever worked when the whole suite was run, not just this test. I'll create an issue to investigate this.
tpe <:< tpeWithParams.tpe || tpe <:< adjustedType | ||
} | ||
|
||
private def performSubstitutions( |
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.
Could you add some explanations here?
Not sure to be honest, I was thinking about "indirect" (chained) implicit completions, as well as completions for type class instances, but maybe those are either already covered by the changes or out of scope (no pun intended). 🙂 Some quick code to visualize, sorry if it's not clear enough: trait Format[T] {
def format(input: T): String
}
object Format {
def apply[T](implicit ev: Format[T]): Format[T] = ev
}
object Foo {
implicit class IntOps(private val i: Int) extends AnyVal {
def inc: Int = i + 1
def dec: Int = i - 1
def print: String = s"($i)"
}
implicit class StringOps(private val s: String) extends AnyVal {
def quote: String = s""""$s""""
def howMany: Int = s.size
}
implicit val formatForInt: Format[Int] = new Format[Int] {
override def format(input: Int): String = s"int = $input"
}
implicit def formatForOther[T]: Format[T] = new Format[T] {
override def format(input: T): String = input.toString()
}
implicit class FormatOps[T](private val f: Format[T]) extends AnyVal {
def formatBetter(input: T): String = s">>>${f.format(input)}<<<"
}
}
object Bar {
//import Foo.* <-- not in scope
val a = 1.inc.dec.print.quote.howMany
val b = Format[Int].format(1)
val c = Format[String].formatBetter("test")
} EDIT: addendum, perhaps the completions shouldn't even care whether there is or isn't any valid typeclass instance present in the workspace for the given type (compiler will catch it later anyway), only "understand" the trait with |
There isn't much of a difference if it's chained or not. But it will suggest the completions with an import one by one ( EDIT: Except for the fact that this particular case doesn't work at all. It's some sort of workspace method search bug.
That's a whole different story. If I understand this correctly, it's more of a search implicit values by type. I think it might be better to get something like this using a code action on implicit value missing error. |
resolves: #5893