bug icon indicating copy to clipboard operation
bug copied to clipboard

Spurious SAM Eta expansion warnings for non-Java interfaces

Open neko-kai opened this issue 6 years ago • 5 comments

I'm getting spurious warnings for the following example code with Scala 2.13 & -Xsource:2.13:

object example {

  trait ModuleBase
  trait Binding

  trait ModuleMake[T <: ModuleBase] {
    def make(bindings: Set[Binding]): T

    final def empty: T = make(Set.empty)
  }

  final class X(s: Set[Binding]) extends ModuleBase
  object X {
    def make(s: Set[Binding]): X = new X(s)

    implicit val moduleApi: ModuleMake[X] = X.make
  }

}
[warn] example.scala:16:47: Eta-expansion performed to meet expected type example.ModuleMake[example.X], which is SAM-equivalent to Set[example.Binding] => example.X,
[warn] even though trait ModuleMake is not annotated with `@FunctionalInterface`;
[warn] to suppress warning, add the annotation or write out the equivalent function literal.

According to scala -W, Xlint:eta-sam warning must fire only for Java-defined interfaces non-annotated , but ModuleMake is a native Scala-defined trait and probably shouldn't trigger the warning.

-Xlint:eta-sam                 Warn on eta-expansion to meet a Java-defined functional interface that is not explicitly annotated with @FunctionalInterface.

neko-kai avatar Jul 20 '19 16:07 neko-kai

I am getting this in 3.2.0-RC2:

@js.native
trait Row extends js.Object {
  def getValue(columnId: String): ReactElement = js.native
}

If I pass function handle: row.getValue as a parameter

[warn] |method getValue is eta-expanded even though getValue does not have the @FunctionalInterface annotation.

evbo avatar Aug 17 '22 20:08 evbo

@evbo it may be useful here for visitors to this ticket to know that Scala 3 has the same problem (if it really is the same!). but if you're interested in seeing it fixed in Scala 3 you'd need to open a ticket at https://github.com/lampepfl/dotty/issues

SethTisue avatar Aug 17 '22 20:08 SethTisue

Strong opinionated statement from the source at https://github.com/lampepfl/dotty/issues/9135#issuecomment-641119858

I believe converting unapplied functions to SAM types is dicey anyway, in the sense that it can easily give something unexpected. I personally would have preferred to only convert lambdas to SAMs, not method handles. That ship has sailed, but I am against introducing even more flexibility here.

Implementing comment at https://github.com/lampepfl/dotty/pull/5717

Also, issue a warning when eta-expanding to a SAM type without a @FunctionalInterface annotation.

Further view on utility of the warning: https://github.com/scala/scala/pull/8941#issuecomment-628076277

That ticket also links to previous discussion https://github.com/lampepfl/dotty/issues/4364.

Worth noting that the warning is under -Xlint and can be disabled as of 2.13.8.

➜  snips scalac -d /tmp -Xlint t8941.scala
t8941.scala:6: warning: Eta-expansion performed to meet expected type Sam1S, which is SAM-equivalent to Any => Any,
even though trait Sam1S is not annotated with `@FunctionalInterface`;
to suppress warning, add the annotation or write out the equivalent function literal.
  val t5b: Sam1S    = meth1                   // ok, but warning
                      ^
1 warning
➜  snips scalac -d /tmp -Xlint:-eta-sam,_ t8941.scala
➜  snips

I expected a warning under -Xsource:3 because Dotty warns; then the warning could be suppressed by -Wconf with cat=lint-eta-sam as reported by -Wconf:help.

It would be great if the warning included the warning "category"; this should be easy in Dotty (where it's not yet fully implemented) because of error numbers.

The text for the -Xlint:eta-sam help is probably an early draft. The test targets a Scala trait explicitly.

som-snytt avatar Aug 18 '22 09:08 som-snytt

What is the root cause of the issue? What is wrong with passing method handle? Would be nice to get a layman's explanation as this is a design pattern I think is concise for scalajs, where you're often passing function handles around (lovingly callback hell).

evbo avatar Aug 18 '22 12:08 evbo

@evbo I suggest following Seth's advice and just opening a ticket on dotty for your issue, which I don't know about.

som-snytt avatar Aug 18 '22 17:08 som-snytt