In order to fix this I think we'd have to introduce a way to identify the part of the classpath that is the project's output classes, then it would be possible to distinguish if:

1. a class is being recompiled (i.e. it's present on the classpath, as a part of the project's output classes, which is all good; just (incremental) recompilation); or
2. a class is being redefined, by also being present on the classpath from a dependency.

I sympathise with the issue and the scaling aspect, but I think this isn't a critical problem because good project/package practices avoids it and/or good testing practices catch it (albeit indirectly, such as your example).

I feel like we allow this partly to support shadowing (monkey patching) of other libraries, including scala-library.
If the compiler or Zinc adds this check, and if someone has 1000 JAR files on the classpath, it would create an untenable situation. I feel like this could be a post-compilation linting performed on the CI similar to Mima where JARs can be treated as a set of classes, the tool can check to make sure the sets do not overlap (and with user-defined opt-outs for monkey patchers out there).

I sympathise with the issue and the scaling aspect, but I think this isn't a critical problem because good project/package practices avoids it and/or good testing practices catch it (albeit indirectly, such as your example).

I agree - this isn't a very critical issue for most projects, but the resulting error message isn't straightforward and I spent about 2 hours trying to figure out why my project wouldn't compile. If Scala were more like Java in that you have class-per-file type of pattern, then it might be easier to spot. With Scala it is more challenging because classes can be defined irrelevant to the file name which means tracking down a clash is more difficult.

I think changing the behavior would go too far since it seems that this allows monkey patching of libraries. Mabye something in the error message that could hint at where the class is located or the jar would make the error message more be useful? e.g. instead of value amount is not a member of a.b.c.Memory add value amount is not a member of a.b.c.Memory @ /path/to/repo/module/fileB.scala? I don't know how difficult implementing that type of solution would be, or if it is even desirable. I do realize the example I provide here is ambiguous because it could reference the location at which the error occurs or the location of the class but it should get my point across about the additional information being provided in the message.

One thing that it isn't clear to me is how this would go about being added for the general case when the particular class doesn't lie within the repository e.g. if Memory was defined by an external dependency, would the path to the dependency jar suffice, or maybe just leave the additional path out altogether?

This is reminiscent of scala/scala#8753 where the question is when do you see the symbol of a companion.

The example is that an object is introduced, not a class that hides another class on the class path.

It would be nice if (as in REPL) you always got a warning when a class and object of same name are not companions, because that has to be a user error, even if the language allows it.

(This feels like zinc's problem to me rather than scalac's problem. But perhaps I'd feel differently if I had a deeper understanding.)

I don't think this has anything to do with Zinc (incremental compilation). From Scala compiler's perspective subproject compilation is analogous to having a JAR library on the classpath.

So here's a more contrived (but sbt-less) example to illustrate what's happening.

$ cs fetch org.scala-lang.modules:scala-parser-combinators_2.13:1.1.2
$ cat Test.scala
package scala
package util.parsing.combinator

object RegexParsers

object Calculator extends RegexParsers
$ scalac Test.scala -cp $HOME/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-parser-combinators_2.13/1.1.2/scala-parser-combinators_2.13-1.1.2.jar
Test.scala:6: error: not found: type RegexParsers
object Calculator extends RegexParsers
                          ^
one error found

@ZacBlanco wrote expectation (thank you for this):

At compile time, I would expect an error to be thrown about the name clash, rather than telling me that amount is not a member of Memory.

Following Zac's logic, the compiler should throw an error that RegexParsers was shadowed. I disagree with this expectation.

@eed3si9n if not zinc, then sbt?

If I understand correctly, Zac is saying Scala compiler should forbid shadowing of classes in -cp.

My claim is that:

1. it's neither sbt nor Zinc's place to call the shot on this issue. (We can reproduce this with just scalac as I've shown above)
2. it's okay to shadow classpath in Scala 2.x, and if we want to forbid it then Zac should start a discussion to require override in Scala 3.x etc.

Okay. It sounds like we should close this since

1. we agree that scalac (version 2) cannot forbid this
2. scalac doesn't have enough information to give a more informative error

?

Per my previous comment, this is just an artifact of separate compilation. There is no reason not to emit the same (useful) error in both of the following cases:

➜  t12289 cat t.scala

package p {
  trait T
}
➜  t12289 cat c.scala

package p {
  object T
  class C extends T
}
➜  t12289 scalac -d /tmp *la
t.scala:3: error: Companions 'trait T' and 'object T' must be defined in same file:
  Found in .../t12289/t.scala and .../t12289/c.scala
  trait T
        ^
1 error
➜  t12289 rm -rf /tmp/p
➜  t12289 scalac -d /tmp t.scala
➜  t12289 scalac -d /tmp -classpath /tmp c.scala
c.scala:4: error: not found: type T
  class C extends T
                  ^
1 error
➜  t12289

Cf this more shadowy example

➜  t12289 cat c.scala

package p {
  object Main {
    object T
    class C extends T
  }
}
➜  t12289 rm -rf /tmp/p
➜  t12289 scalac -d /tmp t.scala
➜  t12289 scalac -d /tmp -classpath /tmp c.scala

If you expect a class artifact to induce classpath shadowing, then you might also expect it to shadow lexically.

But the more important principle is that the error message should be informative.

The ticket says: "the current error provides no useful information and is confusing to the user."

Thanks for the interesting discussion guys...following up some of these statements:

Following Zac's logic, the compiler should throw an error that RegexParsers was shadowed. I disagree with this expectation

I agree that the ability to create shadow types is valuable and should be allowed - the problem is when shadow types cause compilation errors that don't provide useful error messages.

If I understand correctly, Zac is saying Scala compiler should forbid shadowing of classes in -cp.

Similar to above, I don't think it should be forbidden, but compiler errors introduced by shadowing should be more helpful, even if just providing context about where the type came from. i.e. my suggestion to add the path to the jar+package that the type came from on the error. I'm not saying that's the best solution, just an example of how it could be improved.

@ZacBlanco can scalac know, for any particular error, whether this is the root cause or not? I don't think we want to add paths to every "...is not a member..." error, plus I assume there are actually other errors this might manifest as

@ZacBlanco can scalac know, for any particular error, whether this is the root cause or not?

I'm not sure if this is rhetoric. I'm not a scalac developer, just a scala language user so I don't really know the answer this question

I don't think we want to add paths to every "...is not a member..." error,

Similar to what I mentioned above - my suggestion is a possible simple solution, but not one I would actually recommend implementing plainly. I recognize there is probably going to be more nuance to a solution than just adding that more context/information to that whole class of error messages. I'm just trying to provide some insight to what a user might want to see (not sure of whether it's even possible to implement given the compiler architecture that I honestly know nothing about). I agree there are probably other errors that could manifest as a result of the shadowing behavior

The PR tries to warn for "package p has a (stale) member T, let me check if that's actually on the class path".

The line from Goldilocks is, "Someone's been sleeping in my bed, and here she is!"

I think the user action is natural: I'm using some p.T and I want to define a factory p.T.apply so I add an object.

In a normal world, I would get the error that companions must be co-defined.

On the "project management" level, see the previous comment for "good practices", it would nice to have tooling support. This has something to do with not splitting packages, but I'm not sure offhand (without research) whether that means Java modules, or output artifacts (sealed jars), or sbt projects. It's natural to have a test project with test classes in the package under test.

This also falls under "the compiler has information about the types in a diagnostic and it's not sharing with me."