Skip to content
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

Spurious auto-application warnings with java methods with `-Xsource:2.14` #11639

Open
neko-kai opened this issue Jul 17, 2019 · 7 comments · May be fixed by scala/scala#8296

Comments

@neko-kai
Copy link

commented Jul 17, 2019

With -Xsource:2.14 my build produces warnings when calling java methods without parentheses, which is the currently recommended Scala style:

Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method toString,
[warn] or remove the empty argument list from its definition (Java-defined methods are exempt).
[warn] In Scala 3, an unapplied method like this will be eta-expanded into a function.
[warn]         case o => o.toString

However, toString is a Java method, which as the warning itself suggests, is excluded from deprecation!

@eed3si9n

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

@kaishh Thanks for the report.

I agree with exempting toString. It's curious whether the compiler thinks of toString as Java-defined or its own via scala.Any.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

For a potential contributor, a good place to start would be looking into the changeset in scala/scala#7660.

@eed3si9n

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

See also @adriaanm's description in scala/scala#7660:

(4.2) condition for auto-application by -Xsource level:

  • until 2.14: none (assuming condition for (4.3) was not met)

  • in 3.0: meth.isJavaDefined
    TODO decide -- currently the condition is more involved to give slack to
    Scala methods overriding Java-defined ones; I think we should resolve that
    by introducing slack in overriding e.g. a Java-defined def toString()
    by a Scala-defined def toString. This also works better for dealing
    with accessors overriding Java-defined methods.

    The current strategy in methodSig is problematic:

    // Add a () parameter section if this overrides some method with () parameters
    val vparamSymssOrEmptyParamsFromOverride =

    This means an accessor that overrides a Java-defined method gets a
    MethodType instead of a NullaryMethodType, which breaks lots of
    assumptions about accessors

@som-snytt

This comment has been minimized.

Copy link

commented Jul 17, 2019

scala> typeOf[String].member(TermName("toString")).asMethod.isJavaDefined
res1: Boolean = true

scala> case class K(k: String)
defined class K

scala> typeOf[K].member(TermName("toString")).asMethod.isJavaDefined
res2: Boolean = false

scala> class C
defined class C

scala> typeOf[C].member(TermName("toString")).asMethod.isJavaDefined
res3: Boolean = true
@joroKr21

This comment has been minimized.

Copy link

commented Jul 18, 2019

scala> case class Foo(override val toString: String)
defined class Foo

scala> typeOf[Foo].member(TermName("toString")).typeSignature
res1: $r.intp.global.Type = => String

scala> case class Bar(b: String) { override def toString: String = "" }
defined class Bar

scala> typeOf[Bar].member(TermName("toString")).typeSignature
res2: $r.intp.global.Type = ()String

🤔

@som-snytt

This comment has been minimized.

Copy link

commented Jul 18, 2019

Thanks @joroKr21

scala> class C { override val toString = "hi" }
defined class C

scala> new C().toString()
                       ^
       error: not enough arguments for method apply: (i: Int)Char in class StringOps.
       Unspecified value parameter i.

scala> class C { override def toString = "hi" }
defined class C

scala> new C().toString()
res1: String = hi

Both cases are covered by 5.1.4 on overriding, except that defs cannot eliminate a param list, by the special rule mentioned there.

Did I say that correctly? It's so late here, I'm going to sleep. Probably a good spec is legible at bedtime.

@eed3si9n eed3si9n self-assigned this Aug 1, 2019
eed3si9n added a commit to eed3si9n/scala that referenced this issue Aug 1, 2019
Fixes scala/bug#11639

The compiler gets confused about the lineage of `toString` apparently, so instead of trying to look for the Java flag, I am just going to see if the name exists under `AnyTpe`.
eed3si9n added a commit to eed3si9n/scala that referenced this issue Aug 1, 2019
Fixes scala/bug#11639
Fixes scala/bug#11657

The compiler gets confused about the lineage of `toString` apparently, so instead of trying to look for the Java flag, I am just going to see if the name exists under `AnyTpe`.
@eed3si9n

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Here's my PR for this - scala/scala#8296

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.