scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Scaladoc should hide implicitly "overridden" extension methods. #6110

Open scabug opened 12 years ago

scabug commented 12 years ago

The Scala compiler resolves unbound method applications by searching for implicit conversions in companion objects along a type's linearization. Leveraging this you can create statically dispatched polymorphic methods by placing definitions in an implicit class in the companion object of each template in a hierarchy. You might rightly discourage such a design, but doing so allows for aggressive inlining and closure elimination in abstract interfaces without overly constraining implementations. It also lets you override methods with incompatible type signatures.

Scaladoc's new support for documenting implicits provides discoverability and clarity for such designs. Unfortunately, Scaladoc doesn't accurately reflect the compiler's interpretation of extension methods. The following code gives an example of this pattern and notes where Scaladoc misinterprets the definitions.

package nonvirtual

class A[X] { override def toString = "A" }
object A {
  implicit class AOps[X](val __ : A[X]) extends AnyVal {
    def f1(x: Int) = "A"
    def f2(x: Int) = new A[X]
    def f3[Y](x: Int) = new A[Y]
    def f4[Y](f: X => Y) = "A"
  }
}

class B[X] extends A[X] { override def toString = "B" }
object B {
  implicit class BOps[X](val __ : B[X]) extends AnyVal {
    // Scaladoc lists f1, f2, and f3 as shadowed in B rather than overriden
    def f1(x: Int) = "B"
    def f2(x: Int) = new B[X]
    def f3[Y](x: Int) = new B[Y]
    // Scaladoc redundantly lists AOps.f4 in B when only BOps.f4 should appear
    def f4[Y](f: X => Y) = "B"
  }
}

object Test extends App {
  // no implicit resolution conflicts
  val a = new A[String]
  val b = new B[String]
  println("a.f1: " + a.f1(0))
  println("b.f1: " + b.f1(0))
  println("a.f2: " + a.f2(0))
  println("b.f2: " + b.f2(0))
  println("a.f3: " + a.f3(0))
  println("b.f3: " + b.f3(0))
  println("a.f4: " + a.f4(identity))
  println("b.f4: " + b.f4(identity))
}

The test routine has no ambiguities and statically resolves method implementations as you would expect:

a.f1: A
b.f1: B
a.f2: A
b.f2: B
a.f3: A
b.f3: B
a.f4: A
b.f4: B

I appreciate all the effort that's gone into making Scaladoc such a great tool. Keep up the good work!

scabug commented 12 years ago

Imported From: https://issues.scala-lang.org/browse/SI-6110?orig=1 Reporter: Chris Sachs (c9r) Affected Versions: 2.10.0-M5

scabug commented 12 years ago

@VladUreche said (edited on Jul 20, 2012 1:49:56 PM UTC): Hi Chris,

I see what you mean, declaring a member ambiguous or shadowed is done too eagerly. This was a conscious decision to clean up the collections list of members, but I agree shadowing and ambiguity are grossly overestimated. We should add a flag to use a more accurate shadowing and ambiguity check, but since that depends on the order of the conversions, it's a pretty big change. The current flag, -implicits-sound-shadowing, only changes the shadowing but not ambiguity resolution, so it won't fix the pattern above.

scabug commented 12 years ago

Chris Sachs (c9r) said: Hi Vlad. I think you picked the right way to present implicits to users; separating shadowed and ambiguous members works well, and already does the right thing in the important cases. I love how the new diagrams show implicit conversions as well. I hope eventually you're able to work implicit overriding into Scaladoc's model. But for now I'm ecstatic to see implicits show up at all. Thanks.