Conflicting outer definition in same file: ensure source exists

[som-snytt]

Follow-up for #17033, fixes #17433

[odersky]

Otherwise LGTM

[som-snytt]

@lrytz I pushed a fix for when the inherited member is overloaded, so it's necessary to consult alternatives for source.

I see I was confused by overloaded def Value in Enumeration. The neg test is where the inherited overload is in the current file and we want to check that the conflicting definition is also in the current file.

I see alternatives incurs an allocation as in Scala 2. The test could be predicated on isOverloaded, but maybe it happens rarely that an inherited symbol conflicts with a top-level. Unless toString is deemed to conflict with top-level toString from package objects.

[som-snytt]

The fix exposes the overloaded retry. But that is not intended: it should check that both symbols are the conflicting def is from the current compilation unit. It is weird but not wrong to mix in utility methods. I swear never again to say, Nobody codes like that.

 [error] -- [E049] Reference Error: /__w/dotty/dotty/community-build/community-projects/akka/akka-actor-tests/src/test/scala/akka/pattern/RetrySpec.scala:21:20 
[error] 21 |      val retried = retry(() => Future.successful(5), 5, 1 second)
[error]    |                    ^^^^^
[error]    |                    Reference to retry is ambiguous.
[error]    |                    It is both defined in package akka.pattern
[error]    |                    and inherited subsequently in class RetrySpec

Indeed,

package object pattern
    extends PipeToSupport
    with AskSupport
    with GracefulStopSupport
    with FutureTimeoutSupport
    with RetrySupport

and

class RetrySpec extends AkkaSpec with RetrySupport {

also just to cover all the bases

object RetrySupport extends RetrySupport

Edit: note sameTermOrType test fails for overload. Not sure why it passes otherwise, unless "package objects are deprecated, let alone package object mixins."

[som-snytt]

[error] -- [E049] Reference Error: /__w/dotty/dotty/community-build/community-projects/specs2/core/shared/src/main/scala/specs2/run.scala:27:4 
[error] 27 |    run(args, exit = true)
[error]    |    ^^^
[error]    |    Reference to run is ambiguous.
[error]    |    It is both defined in package specs2
[error]    |    and inherited subsequently in object run
[error]    |
[error]    | longer explanation available when compiling with `-explain`

that is

object run extends ClassRunnerMain:
  def main(args: Array[String]) =
    run(args, exit = true)

where it inherits an overloaded run from ClassRunnerMain.

[lrytz]

I think the ambiguity error on

object run extends ClassRunnerMain:
  def main(args: Array[String]) =
    run(args, exit = true)

is not bad, this is the sort of thing that can confuse people.

[odersky]

This currently fails for specs2, so it seems we need to improve the logic.

[som-snytt]

@odersky I improved specs2 instead.

I think this is exactly the ambiguity that was intended originally. (See Lukas #17439 (comment))

[lrytz]

Updated specs2. Locally (community-build/testOnly -- *specs2) I got this failure:

[error]  16 != 13 (LocationSpec.scala:18)
[error]
[error]  23 != 18 (LocationSpec.scala:27)
[error]
[error]  20 != 19 (LocationSpec.scala:30)
[error]
[error]  22 != 21 (LocationSpec.scala:33)
[error]
[error] Failed: Total 697, Failed 4, Errors 0, Passed 693, Pending 2
[error] Failed tests:
[error] 	org.specs2.io.LocationSpec

related to etorreborre/specs2@947e823. Not sure what to make of it.

[lrytz]

Yeah, same error here. Should I revert that commit on dotty-staging?

[som-snytt]

@lrytz that was related to #17428

I mentioned the PR (on specs2 etorreborre/specs2#1152) and that one could not tell if or when it might land. There is a request to improve debugging, but there is also the dotty PR to improve debugging inline code, which will touch line numbers.

[lrytz]

Ready to review (@odersky). Short history

• this fixes a corner-case of Include top-level symbols from same file in outer ambiguity error #17033
• it was half approved ("otherwise LGTM" above)
• specs2 needed a fix (Specify this.run in run etorreborre/specs2#1153), not a bad example of how the new warning enforces cleaner code
• the specs2 update was delayed for other reasons, but is now done
• the original PR (17003) was reverted in the meantime Revert "Include top-level symbols from same file in outer ambiguity error" #17438

So this PR reverts the revert, and adds the fix.

[lrytz]

Added a forward-port of scala/bug#12816 (package object members are not conflicting with inherited definitions).