Extend -Xlint:nullary-override for the reverse (nilary) case

[som-snytt]

If def m is overriding a def m() -Xlint:nullary-override now reports it as well, unless the def m() is defined in Java (such as Object's toString or hashCode) which are exempt.

[som-snytt]

The limited isJavaDefined test doesn't help here, and also the early warning beats the error; a late warning would be suppressed; maybe issue them after typer?

+t6534.scala:9: warning: nullary method assumes an empty parameter list from the overridden definition
+class Bippy4(val x: Int) extends AnyVal { override def hashCode = -1 }          // error
+                                                       ^
 t6534.scala:9: error: redefinition of hashCode method. See SIP-15, criterion 4. is not allowed in value class
 class Bippy4(val x: Int) extends AnyVal { override def hashCode = -1 }          // error

[lrytz]

I think we should postpone any nullary/nilary changes to 2.13.3 (will do that for the other related PRs as well)

[som-snytt]

This just enhances the optional warning and encourages alignment with Scala 3.

[dwijnand]

LGTM. It could be that this needs to be pushed further into -Xsource:2.14 or even on-by-default-land, but we can tweak that post-merge.

[som-snytt]

Added a commit (not prematurely squashed) to use the warning category; the test tests that the warning is nowarnable; and the doc for nowarn is fixed.

[dwijnand]

That's a great addition. Thanks, Som.

[SethTisue]

I think we should postpone any nullary/nilary changes to 2.13.3

repeating this just to make sure nobody hits “merge” for now

[som-snytt]

This just enhances the optional warning and encourages alignment with Scala 3.

repeating this in case @lrytz retracts his objection because it uses his warning framework. Or his nowarning framework.

[lrytz]

in case @lrytz retracts his objection

I was swimming in some confusion about the entrie situation when I made that comment :-) We should still merge improvements in this area that are non-controversial.

For the issue here, maybe we can first investigate the viability of an alternative approach: allow f to override f() without synthetically inserting the parameter list. It might be less risky than it sounds, because f() can override f today.

$> ~/Applications/bin/scala-runners/scala -213x -Xsource:3
Welcome to Scala 2.13.2-bin-b4428c8 (Java HotSpot(TM) 64-Bit GraalVM EE 19.2.1, Java 1.8.0_231).
Type in expressions for evaluation. Or try :help.

scala> class C { def f = 0 }; class D extends C { override def f() = 1 }
class C
class D

scala> (new D).f
               ^
       warning: Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method f,
       or remove the empty argument list from its definition (Java-defined methods are exempt).
       In Scala 3, an unapplied method like this will be eta-expanded into a function.
val res0: Int = 1

[lrytz]

(by "allow" i mean the implementation; for users, any mismatch in overriding should trigger a warning)

[som-snytt]

Since this PR doesn't introduce any options or cruft, I hear lrytz saying we can merge this and get the doc fix, and then further experiments with parens may update the check files.

The current spec is surely all about Java parens: implementing a trait in Java. But maybe there is no downside to letting overrides add or remove empty parens willy-nilly. Which is an apt pun for nillary.

The advance warning for eta-expansion is true only when expected type:

Starting dotty REPL...
scala> def f() = 42
def f(): Int

scala> f                                                                                                    
1 |f
  |^
  |method f must be called with () argument

scala> f: (() => Int)
val res0: () => Int = Lambda$1273/0x0000000802c5fc40@547aaa32

scala> def f = 42
def f: Int

scala> f: (() => Int)
1 |f: (() => Int)
  |^
  |Found:    Int
  |Required: () => Int

[dwijnand]

The next change is to enable override warnings under -Xsource:2.14 (aka -Xsource:3).

[dwijnand]

@som-snytt I took the liberty of updating the PR title and description for the current state of the PR. But please check as I might've made mistakes (or killed your PR description "vibe" - edit away 😄).

Also, looking back again, I'm not sure about the tests: asides from the @warn addition, isn't the rest just duplications? Any reason for that?

Lastly, should this addition be under the same lint warning? It's simpler, but do we think users might want to enable one but not the other? -Xlint:nilary-override?

[som-snytt]

@dwijnand thanks for the improved doc factor.

I think having both checks exercised in the nullary-override test shows that both warnings are shown and also that they are reported in an unexpected order.

It looks like I added the nowarn test when I added warning category support.

As to having a separate lint, the mechanics of that are just heavy enough to weigh against it. Also they are really the same, a mismatch in scala 3. Also too many options, and what would you name it.

[dwijnand]

Also too many options, and what would you name it.

Xlint:nilary-override, but I'm fine with leaving it together.

[som-snytt]

Right, except for a possible pun on "nilary clinton", it's not meaningful to folks not accustomed to paramss.

[lrytz]

investigate the viability of an alternative approach: allow f to override f() without synthetically inserting the parameter list.

For the record, previous discussion about this is here: #8445 (comment).

The adaptation leaks, that's why I'd prefer to change the implementation:

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

scala> (new C).name(0)
res0: Char = C

scala> (new C).toString(0)
                        ^
       error: no arguments allowed for nullary method toString: ()String

That said, I'm fine merging this as a step in the right direction.

[som-snytt]

I agree it will be nice to respect the parens as written.

There is an analogy in constructors, although the different syntax ruins it:

scala> class C { def apply(i: Int) = 42+i }
defined class C

scala> new C
res0: C = C@52a33c3f

scala> new C(1)
             ^
       error: no arguments allowed for nullary constructor C: ()C

scala> (new C)(1)
res2: Int = 43

scala> (res2.toString)(0)
                       ^
       error: no arguments allowed for nullary method toString: ()String

[lrytz]

The suggested change would likely also fix scala/bug#11158 (and be an alternative to #7625)