https://pantsbuild.org/ logo
Join Slack
Powered by
Ok, there are more issues in Scala 3 dependency in...
# development
l
Ok, there are more issues in Scala 3 dependency inference. I have added two failing tests here: https://github.com/pantsbuild/pants/compare/main...mberndt123:pants:broken-scala-tests Pants’ current dependency inference doesn't know about extension methods and doesn't understand type classes (given/using). Both problems also affect implicits.
The problem here is that Scala 3 allows us to declare defs and givens on the package level, which means they're accessible without an import. And that means that a dependency should be declared between all files in the same package 😒
I think the only way to fix this would be to add some special rules. For example we could simply say that package-level 
given
, 
extension
and 
implicit
declarations are allowed only in files named 
package.scala
. If such declarations are found elsewhere, we throw an error and say that dependency inference doesn't support that. To make migration of existing codebases easier, we add an option, like 
allow_package_level_implicits
that allows such declarations but adds all these dependencies between files in the same package. It would be switched off by default, and when such a problematic declaration is found, we print an error telling the user that they can either switch that option on (and get crappy dependency inference) or change their code.
Or better yet, we could add an option to allow users to specify which files they want to allow package-level `implicit`s in, maybe with a glob pattern, and disallow them everywhere else. And the default would be 
**/package.scala
c
I think there sortof already are flags for that? There's one to add a dep on the package object and another to add dependencies on sibling files. https://www.pantsbuild.org/docs/reference-scala-infer#force_add_siblings_as_dependencies
l
Well declaring dependencies on all files generated by the same 
scala_sources
is excessive as they don't necessarily live in the same package. I think the "sibling" bit is actually misleading here. And it says that this option can be helpful when compilation fails due to missing dependencies, however this bug can also lead to cases where compilation succeeds even though it ought to fail.
And yes, adding a dependency on the package object is correct in Scala 2 but it's insufficient for Scala 3. These existing options aren't enough, they're doing both too much and too little
They also lack enforcement. Adding these dependencies fixes the problem, but unless you forbid 
given
, 
extension
and 
implicit
at the package level, people might not even realize they have a problem.
c
That makes sense, thanks for explaining. I guess, then, that it's not really possible to do dependency inference for a file without analysing potentially all the files (or all the files with any of the implicit machinery)? And that essentially we'd either have to reimplement a lot of the Scala compiler machinery or constrain the Scala language use we support?
l
Well, all the files in the same package I would say.
And maybe even more… I'm not sure right now what happens when you e. g. start a file with 
package foo; package bar;
rather than 
package foo.bar
.
And you're right, afaics you'd have to constrain the language to improve on that.
I'm not even sure how scalac/zinc handle this case…
I'd have to do some experimentation
c
I wonder if they hook into the compiler itself to do the symbol resolution? Or the Metals language server or similar? Given that dependency inference needs to happen for all files, it might be fine to just call out to one of these tools if they provide that information.
l
Now that I think about it I'm not even sure it's restricted to `given`/`extension`/`implicit`. What about method overloading? A normal method call might become ambiguous if you add a new overload for a method…
@happy-kitchen-89482, can you comment on this?
Haha, sbt also gets this wrong 🤣
h
This is all outside my area of expertise. I was looking into this like 10 years ago...
l
@happy-kitchen-89482 my point is: there's an issue here, and I'd be willing to implement some kind of solution, but I’m going to need some buy-in from the pants team before I get to work. I don't want to spend time working on this only to find my pull request being rejected. So if you're not the person to ask, then who is? @fast-nail-55400 perhaps?
h
Ah, gotcha. Would you be able to write up your proposal briefly in a github issue? That would be a great place to focus the conversation, then I'll chime in as well as bringing in a couple of greater experts than. myself.
f
I'm willing to be consulted here including a call if you want some buy-in.
The existing dependency inference was written with Scala 2 in mind. I never worked with Scala 3 so that code is definitely in for a rethink.
l
Ok cool, I'll see if I can put something together over the next days
👍 1
2 Views
Open in Slack
PreviousNext
Powered by