-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clashing type names across SBT modules doesn't result in an error #12289
Comments
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:
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. |
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 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 |
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):
Following Zac's logic, the compiler should throw an error that |
@eed3si9n if not zinc, then sbt? |
If I understand correctly, Zac is saying Scala compiler should forbid shadowing of classes in My claim is that:
|
Okay. It sounds like we should close this since
? |
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:
Cf this more shadowy example
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:
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.
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 |
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
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 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." |
reproduction steps
scala version:
2.12.10
I have two SBT modules
A
andB
Module A has
fileA.scala
which defines some classesThen in module
B
I define a dependency on moduleA
. I also havefileB.scala
that uses the same package and has a trait with the same name as theMemory
class from moduleA
.Then in another file within module B I have
When I try to compile all of the modules I get the compilation error
problem
Note we have a clash in names within the same package
a.b.c
Two separate files across separate SBT modules define theMemory
type. One is a trait, the other a class. At compile time, I would expect an error to be thrown about the name clash, rather than telling me thatamount
is not a member ofMemory
. This would indicate to me there is no issue with the naming of the class (or trait).If I am unaware of the
Memory
trait the current error provides no useful information and is confusing to the user. This is particularly a problem for larger code bases where a developer might not know the existence of every single type.The text was updated successfully, but these errors were encountered: