Spurious, positionless overrides-nullary-method warning when extending under separate compilation

[SethTisue]

Scala version: 2.13.11

T.scala

trait T1 {
  def method: Int
}
trait T2 extends T1 {
  def method() = 0
}

C.scala

class C extends T2

Reproduction steps

scalac T.scala
scalac -classpath . C.scala

Result

compiling T.scala produces a correct warning:

T.scala:5: warning: method with a single empty parameter list overrides method without any parameter list
  def method() = 0
      ^
1 warning

but then compiling C.scala produces this:

warning: method with a single empty parameter list overrides method without any parameter list
1 warning

we shouldn't be issuing this warning at all IMO, but also note the lack of position information, which means it's difficult to figure out where it's even coming from

Context

This came up over at scala/scala#10484 , where after a recent change for 2.13.12 the attempt to issue the warning actually crashes the compiler.

[SethTisue]

@som-snytt @lrytz You two want to race to see who fixes it first...?

History: the warning originally landed in scala/scala#8846 (Scala 2.13.3) and then it was moved from namers to refchecks in scala/scala#10345 (Scala 2.13.11).

[som-snytt]

Thanks for the repro! I'll take a look, as I must refresh my memory for namePos of valdef and the Giants aren't playing today.

[som-snytt]

I'm glad I fixed crasher handling in partest at some point.

?! 1 - neg/t12851                                [caught UnsupportedOperationException - Position.point on NoPosition]

[SethTisue]

I thought the same problem might exist for nullary-overrides-nilary ("method without a parameter list overrides a method with a single empty one"), but it doesn't reproduce that way 'round 🤔

[som-snytt]

s/spurious/scurrilous

[lrytz]

➜ sandbox scv 2.13.12-bin-fa1c9eb A.scala B.scala
A.scala:5: warning: method with a single empty parameter list overrides method without any parameter list
  def method() = 0
      ^
1 warning


➜ sandbox scv 2.13.12-bin-910c59d A.scala B.scala
A.scala:5: warning: method with a single empty parameter list overrides method without any parameter list [quickfixable]
  def method() = 0
      ^
A.scala:5: warning: method with a single empty parameter list overrides method without any parameter list
  def method() = 0
      ^
2 warnings

so the second warning started showing up with scala/scala#10484.

I think before we only got a single warning because the second one was filtered by the FilteringReporter. After that PR, the two warnings no longer have the same message because one has [quickfixable], but not the other. So they are both shown.

[som-snytt]

The forthcoming PR addresses both when to warn for the override and the position to warn at. (The symbol position should be tree.namePos.)

The reason the inverse case (def m overrides def m()) does not warn for separate compilation is that the parens are injected by namer; a tree attachment enables warning. In separate compilation, there is no attachment and both signatures have parens.

I haven't looked at the double-warning question yet, but I remember that the "extended explanation" feature (which I remove in a current PR for 2.13.13) had the same problem. That is where a message text could have a "fold line":

Bad thing!
----
Let me explain more...

where you'd see the long form only once, and only the top half after that. The short form was registered with the filtering reporter. (I think that feature is nicer than re-run with -explain-more for more explanation, but it was not adopted.)

If actionable messages may/may not have an addendum, maybe the addendum should be stripped before registration.