Closed SethTisue closed 10 months ago
@som-snytt @lrytz You two want to race to see who fixes it first...?
History: the warning originally landed in https://github.com/scala/scala/pull/8846 (Scala 2.13.3) and then it was moved from namers to refchecks in https://github.com/scala/scala/pull/10345 (Scala 2.13.11).
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.
I'm glad I fixed crasher handling in partest at some point.
?! 1 - neg/t12851 [caught UnsupportedOperationException - Position.point on NoPosition]
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 🤔
s/spurious/scurrilous
➜ 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 https://github.com/scala/scala/pull/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.
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.
Scala version: 2.13.11
T.scala
C.scala
Reproduction steps
Result
compiling
T.scala
produces a correct warning:but then compiling
C.scala
produces this: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 https://github.com/scala/scala/pull/10484 , where after a recent change for 2.13.12 the attempt to issue the warning actually crashes the compiler.