scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.89k stars 1.06k forks source link

Enum values method should be defined with empty param list #11248

Open ekrich opened 3 years ago

ekrich commented 3 years ago

Compiler version

Scala 3.0.0-M3 and 2.13.4

When cross compiling java.time for Scala.js the following call site needs to be changed to Month.values to compile with Scala 3. https://github.com/ekrich/sjavatime/blob/master/sjavatime/shared/src/main/scala/java/time/LocalDate.scala#L408

When the code is compiled with 2.13.4 then it causes a warning because our Scala 2 implementation includes ():

[warn] /Users/eric/workspace/sjavatime/sjavatime/shared/src/main/scala/java/time/LocalDate.scala:408:23: Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method values,
[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]     val month = Month.values.takeWhile(_.firstDayOfYear(leap) <= dayOfYear).last
[warn]                       ^
[warn] two warnings found

Expectation

When defining Java APIs in Scala.js and Scala Native, we define empty parameter list methods with (). Since enum can be used to write Java APIs and the values() method is a Java API method, it should be defined with ()s. Other applicable Java methods should also be changed to match.

As per Sébastien Doeraene @sjrd 07:22 on the Dotty gitter channel.

I agree. It would make sense to define with (). Please file an issue on GitHub.

som-snytt commented 3 years ago

If MyEnum.values is (to be taken as) Java API, then it should benefit from all the usual paren-exemptions. It should not warn in Scala 2.

odersky commented 3 years ago

I disagree. Scala has the conventions that you should not use () if the method is immutable, i.e. could be a property. This differs from Java, but nobody would seriously propose to use length() on Iterables just because Java uses it. And nobody would want to go back to toString(). I don't see a difference with values here. And I do see the problem that it would set a bad precedent and dilute our hard-gained conventions when to drop ().

abgruszecki commented 3 years ago

@som-snytt's argument makes sense. It seems like there's a migration problem here, and the correct solution is to make the values method exempt from missing-paren warnings.