Hint about forbidden combination of implicit values and conversions

[ysthakur]

For fixing #16453.

When an implicit argument of some type T is missing, this looks for all Conversions in scope resulting in type T and includes a note that you can't combine implicit values and conversions. However, if it does find a Conversion[A, T], it doesn't check if there's actually an implicit A in scope. The desired example message in the original issue did also find an implicit value to give as input to the Conversion, but I figured that's extra work for the compiler to do and might not be desired.

Also, the error message doesn't show the actual source of the found conversions, it's showing generated mangled names. I'll have to fix that.

It's my first time making a change to the dotc code, so please tell me if I've done something wrong.

[ysthakur]

Would it be worth it to filter out Conversions for which there aren't any implicits to feed to? For example, in the code below, there's no given Int to feed into the conversion. Should it also check for that or just include any Conversions found that result in Option[Int]s?

given Conversion[Int, Option[Int]] = ???
summon[Option[Int]]

[prolativ]

In general I would say that any hints from the compiler on how to get rid of implicitNotFound error might be useful to users, but the deeper we try to go with this, the more corner cases will appear, and these improvements can get in later in separate PRs.

[prolativ]

Some tests in the CI are failing. You'll have to update the *.check files accordingly to make the expected error messages match what the compiler outputs now. This also shows that printing all available conversions might not be a good idea

[info] Test dotty.tools.dotc.CompilationTests.negAll started
Output from 'tests/neg/i4986c.scala' did not match check file. Actual output:
-- [E172] Type Error: tests/neg/i4986c.scala:38:8 ----------------------------------------------------------------------
38 |  test.f // error
        ^
        Missing X$Y for Test[Char]
-- [E172] Type Error: tests/neg/i4986c.scala:39:13 ---------------------------------------------------------------------
39 |  test.g[Int] // error
             ^
             Missing Outer[Int] with OuterMember = pkg.Outer[Int]#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:40:13 ---------------------------------------------------------------------
40 |  test.h[X$Y] // error
             ^
             Missing Outer[pkg.X$Y] with OuterMember = pkg.Outer[pkg.X$Y]#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:41:24 ---------------------------------------------------------------------
41 |  test.i[Option[String]] // error
                        ^
                        Missing implicit outer param of type Outer[Option[String]] for Test[Char]
-- [E172] Type Error: tests/neg/i4986c.scala:42:43 ---------------------------------------------------------------------
42 |  test.j[(Long, Long), Int | String, Array] // error
                                           ^
Missing Inner[Int | String, Array] with InnerMember = pkg.Outer[(Long, Long)]#Inner[Int | String, Array]#InnerMember from Outer[(Long, Long)] with OuterMember = pkg.Outer[(Long, Long)]#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:43:53 ---------------------------------------------------------------------
43 |  test.k[Either[String, Any], Seq[Seq[Char]], Vector] // error
                                                     ^
    Missing implicit inner param of type Outer[Either[String, Any]]#Inner[Seq[Seq[Char]], Vector] for Test[Char]
-- [E172] Type Error: tests/neg/i4986c.scala:45:87 ---------------------------------------------------------------------
45 |  implicitly[Outer[Option[String] | List[Iterable[Char]]] { type MyType = BigDecimal }] // error
                                                                                       ^
Missing Outer[Option[String] | List[Iterable[Char]]] with OuterMember = pkg.Outer[Option[String] | List[Iterable[Char]]]{type MyType = BigDecimal}#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:46:106 --------------------------------------------------------------------
46 |  implicitly[(Outer[Option[String] | List[Iterable[Char]]] { type MyType = BigDecimal })#Inner[Byte, Seq]] // error
                                                                                                          ^
Missing Inner[Byte, Seq] with InnerMember = pkg.Outer[Option[String] | List[Iterable[Char]]]{type MyType = BigDecimal}#Inner[Byte, Seq]#InnerMember from Outer[Option[String] | List[Iterable[Char]]] with OuterMember = pkg.Outer[Option[String] | List[Iterable[Char]]]{type MyType = BigDecimal}#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:47:33 ---------------------------------------------------------------------
47 |  implicitly[Outer[Int] @myAnnot] // error
                                 ^
                                 Missing Outer[Int] with OuterMember = pkg.Outer[Int] @myAnnot#OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:52:52 ---------------------------------------------------------------------
52 |  implicitly[Outer[Int] { type OuterMember = Long }] // error
                                                    ^
                                                    Missing Outer[Int] with OuterMember = Long
-- [E172] Type Error: tests/neg/i4986c.scala:53:24 ---------------------------------------------------------------------
53 |  implicitly[outer.type] // error
                        ^
                        Missing Outer[Int] with OuterMember = pkg.Test.outer.OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:54:104 --------------------------------------------------------------------
54 |  implicitly[(Outer[Int] { type OuterMember = Long })#Inner[Long, Iterator] { type InnerMember = Byte }] // error
                                                                                                        ^
                   Missing Inner[Long, Iterator] with InnerMember = Byte from Outer[Int] with OuterMember = Long
-- [E172] Type Error: tests/neg/i4986c.scala:55:69 ---------------------------------------------------------------------
55 |  implicitly[outer.Inner[Long, Iterator] { type InnerMember = Byte }] // error
                                                                     ^
Missing Inner[Long, Iterator] with InnerMember = Byte from Outer[Int] with OuterMember = pkg.Test.outer.OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:56:24 ---------------------------------------------------------------------
56 |  implicitly[inner.type] // error
                        ^
Missing Inner[Long, Iterator] with InnerMember = pkg.Test.inner.InnerMember from Outer[Int] with OuterMember = pkg.Test.outer.OuterMember
-- [E172] Type Error: tests/neg/i4986c.scala:58:33 ---------------------------------------------------------------------
58 |  implicitly[U[Int, Option, Map]] // error
                                 ^
                                 There's no U[Int, Option, Map]
-- [E172] Type Error: tests/neg/i4986c.scala:62:19 ---------------------------------------------------------------------
62 |  i.m[Option[Long]] // error
                   ^
String; List; [A, _] =>> List[Option[?]]; Int; Option[Long]; 

Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
The following conversions in scope result in Int: 
  - implicit def Short2short(x: Short): Short
  - implicit def Integer2int(x: Integer): Int
  - implicit def Character2char(x: Character): Char
  - implicit def Byte2byte(x: Byte): Byte


Test output dumped in: tests/neg/i4986c.check.out
  See diff of the checkfile (`brew install icdiff` for colored diff)
    > diff tests/neg/i4986c.check tests/neg/i4986c.check.out
  Replace checkfile with current output
    > mv tests/neg/i4986c.check.out tests/neg/i4986c.check
      
Output from 'tests/neg/i9958.scala' did not match check file. Actual output:
-- [E172] Type Error: tests/neg/i9958.scala:1:30 -----------------------------------------------------------------------
1 |val x = summon[[X] =>> (X, X)] // error
                              ^
No given instance of type [X] =>> (X, X) was found for parameter x of method summon in object Predef

Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
The following conversions in scope result in [X] =>> (X, X): 
  - implicit def unitArrayOps(xs: Array[Unit]): scala.collection.ArrayOps[Unit]
  - implicit def float2Float(x: Float): Float
  - implicit def wrapShortArray(xs: Array[Short]): scala.collection.mutable.ArraySeq.ofShort
  - implicit def booleanArrayOps(xs: Array[Boolean]): scala.collection.ArrayOps[Boolean]
  - implicit def wrapRefArray[T <: AnyRef](xs: Array[T]): scala.collection.mutable.ArraySeq.ofRef[T]
  - final implicit def StringFormat[A](self: A): StringFormat[A]
  - implicit def shortArrayOps(xs: Array[Short]): scala.collection.ArrayOps[Short]
  - implicit def doubleArrayOps(xs: Array[Double]): scala.collection.ArrayOps[Double]
  - implicit def refArrayOps[T <: AnyRef](xs: Array[T]): scala.collection.ArrayOps[T]
  - implicit def short2Short(x: Short): Short
  - implicit def genericWrapArray[T](xs: Array[T]): scala.collection.mutable.ArraySeq[T]
  - implicit def genericArrayOps[T](xs: Array[T]): scala.collection.ArrayOps[T]
  - implicit def Long2long(x: Long): Long
  - implicit def char2Character(x: Char): Character
  - implicit def double2Double(x: Double): Double
  - implicit def intArrayOps(xs: Array[Int]): scala.collection.ArrayOps[Int]
  - implicit def longWrapper(x: Long): scala.runtime.RichLong
  - implicit def wrapBooleanArray(xs: Array[Boolean]): scala.collection.mutable.ArraySeq.ofBoolean
  - implicit def augmentString(x: String): scala.collection.StringOps
  - implicit def longArrayOps(xs: Array[Long]): scala.collection.ArrayOps[Long]
  - implicit def Float2float(x: Float): Float
  - implicit def wrapByteArray(xs: Array[Byte]): scala.collection.mutable.ArraySeq.ofByte
  - implicit def wrapUnitArray(xs: Array[Unit]): scala.collection.mutable.ArraySeq.ofUnit
  - implicit def Short2short(x: Short): Short
  - implicit def tuple2ToZippedOps[T1, T2](x: (T1, T2)): runtime.Tuple2Zipped.Ops[T1, T2]
  - implicit def Double2double(x: Double): Double
  - implicit def int2Integer(x: Int): Integer
  - final implicit def Ensuring[A](self: A): Ensuring[A]
  - implicit def copyArrayToImmutableIndexedSeq[T](xs: Array[T]): IndexedSeq[T]
  - final implicit def ArrowAssoc[A](self: A): ArrowAssoc[A]
  - implicit def byteWrapper(x: Byte): scala.runtime.RichByte
  - implicit def wrapLongArray(xs: Array[Long]): scala.collection.mutable.ArraySeq.ofLong
  - implicit def floatArrayOps(xs: Array[Float]): scala.collection.ArrayOps[Float]
  - implicit def Boolean2boolean(x: Boolean): Boolean
  - implicit def charWrapper(c: Char): scala.runtime.RichChar
  - implicit def wrapString(s: String): scala.collection.immutable.WrappedString
  - implicit def boolean2Boolean(x: Boolean): Boolean
  - implicit def intWrapper(x: Int): scala.runtime.RichInt
  - implicit def Integer2int(x: Integer): Int
  - implicit def floatWrapper(x: Float): scala.runtime.RichFloat
  - implicit def wrapFloatArray(xs: Array[Float]): scala.collection.mutable.ArraySeq.ofFloat
  - implicit def Character2char(x: Character): Char
  - implicit def doubleWrapper(x: Double): scala.runtime.RichDouble
  - implicit def byteArrayOps(xs: Array[Byte]): scala.collection.ArrayOps[Byte]
  - implicit def long2Long(x: Long): Long
  - implicit def tuple3ToZippedOps[T1, T2, T3](x: (T1, T2, T3)): runtime.Tuple3Zipped.Ops[T1, T2, T3]
  - implicit def byte2Byte(x: Byte): Byte
  - implicit def wrapDoubleArray(xs: Array[Double]): scala.collection.mutable.ArraySeq.ofDouble
  - implicit def wrapCharArray(xs: Array[Char]): scala.collection.mutable.ArraySeq.ofChar
  - implicit def wrapIntArray(xs: Array[Int]): scala.collection.mutable.ArraySeq.ofInt
  - implicit def Byte2byte(x: Byte): Byte
  - implicit def shortWrapper(x: Short): scala.runtime.RichShort
  - implicit def charArrayOps(xs: Array[Char]): scala.collection.ArrayOps[Char]
  - implicit def booleanWrapper(x: Boolean): scala.runtime.RichBoolean
-- [E007] Type Mismatch Error: tests/neg/i9958.scala:8:10 --------------------------------------------------------------
8 |def b = f(a) // error
          ^
          Found:    G[[A <: Int] =>> List[A]]
          Required: G[List]

 longer explanation available when compiling with `-explain`


Test output dumped in: tests/neg/i9958.check.out
  See diff of the checkfile (`brew install icdiff` for colored diff)
    > diff tests/neg/i9958.check tests/neg/i9958.check.out
  Replace checkfile with current output
    > mv tests/neg/i9958.check.out tests/neg/i9958.check
      
Output from 'tests/neg/summon-function.scala' did not match check file. Actual output:
-- [E172] Type Error: tests/neg/summon-function.scala:2:23 -------------------------------------------------------------
2 |  summon[Int => String] // error
                       ^
No given instance of type Int => String was found for parameter x of method summon in object Predef

Note: implicit conversions are not automatically applied to arguments of using clauses. You will have to pass the argument explicitly.
The following conversions in scope result in Int => String: 
  - implicit def wrapRefArray[T <: AnyRef](xs: Array[T]): scala.collection.mutable.ArraySeq.ofRef[T]
  - implicit def genericWrapArray[T](xs: Array[T]): scala.collection.mutable.ArraySeq[T]
  - implicit def copyArrayToImmutableIndexedSeq[T](xs: Array[T]): IndexedSeq[T]


Test output dumped in: tests/neg/summon-function.check.out
  See diff of the checkfile (`brew install icdiff` for colored diff)
    > diff tests/neg/summon-function.check tests/neg/summon-function.check.out
  Replace checkfile with current output
    > mv tests/neg/summon-function.check.out tests/neg/summon-function.check
      
Error:  Test dotty.tools.dotc.CompilationTests.negAll failed: java.lang.AssertionError: Neg test should have failed, but did not, took 95.038 sec
Error:      at dotty.tools.vulpix.ParallelTesting$CompilationTest.checkExpectedErrors(ParallelTesting.scala:1031)
Error:      at dotty.tools.dotc.CompilationTests.negAll(CompilationTests.scala:191)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
Error:      at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Error:      at java.lang.reflect.Method.invoke(Method.java:567)
Error:      ...

[ysthakur]

Yeah, filtering the Conversions should hopefully fix most of them.

[ysthakur]

I've modified it now to get all implicits in context and then get suitable conversions for each of those implicits, but I'm not sure how to check if an implicit can actually be passed to a conversion. The tryConversion method inside the typedImplicit method in Implicits.scala looks promising.

It also needs to filter out implicit methods that take normal parameters (not just type parameters and implicit parameters) so that implicit conversions like char2Character don't get picked up. That one should be easier, though.

[ysthakur]

@prolativ Tests are passing now! I had to change it to get all implicits in scope and attempt to convert each implicit into the expected type using typed, so now it only reports the relevant implicits, which I think is more useful than reporting the implicit conversions, since those won't be used directly.

There are a few things I'm not sure on, however. One is whether getting all implicits in scope and trying each out would be a good idea when someone is working with a typeclass-heavy library or something with lots of implicits in scope.

Another is about the helper I made to check if the tree returned from typed has errors. It only checks whether the whole tree (implicit conversion applied to found implicit) is an ErrorType or, if it's a Scala 2-style implicit parameter, whether it's an Apply whose only arg is of type ErrorType. Are there any other cases to look for?

Also, I'm not entirely sure if the helper I made for checking if a found implicit is a Scala 2-style implicit conversion covers all cases, but it doesn't seem like a normal parameter list can follow an implicit parameter list, so I assume it's fine.

[prolativ]

Actually, what was the problem with using ctx.implicits.eligible(ViewProto(...)) to find out if an implicit value can be converted to the type that we're searching for?

[ysthakur]

I was thinking that in case the implicit value was something like given [T <: Bar]: Foo[T], it would've needed wildApprox to fill in the type argument to Foo and Foo[?] would've been too permissive, but after your comment, I just realized wildApprox takes bounds into account. It'd be clearer than the current approach and would give us the conversions that would apply to the implicits. I'll try that later.

[ysthakur]

So I tried the following to check if there are applicable conversions:

ctx.implicits.eligible(
  ViewProto(
    wildApprox(imp.underlying.widen.finalResultType),
    fail.expectedType
  )
).nonEmpty

This works fine when the conversion doesn't have any type parameters (summon[String] and implicitly[String] work fine in the test), but when it does have type parameters, all the implicits in scope are listed (summon[Option[Int]] and implicitly[Option[Int]] don't work). Gonna have to do some more investigating there. I made another branch to test this out. Here's the first run.

[prolativ]

I tried to dig into that and it seems that we were trying to reinvent the wheel. There's already the method viewExists available in this context, which seems to does what canBeConverted was supposed to do, and it even has the same signature

[ysthakur]

Nice find!

[ysthakur]

Thanks for all the help prolativ!

[prolativ]

Well done, thanks for the contribution