improvement-proposals icon indicating copy to clipboard operation
improvement-proposals copied to clipboard

SIP-68: Reference-able Package Objects

Open lihaoyi opened this issue 1 year ago • 6 comments

lihaoyi avatar Dec 14 '24 01:12 lihaoyi

I have no strong feeling on this proposal. It's a bit sad that we could not actually get rid of package objects, but oh well.

One comment: the proposal argues that it improves regularity: objects and package objects become more the same. While that is true, it also degrades another regularity aspect. Now, a package that has a package object does not behave the same as a package that doesn't have one. If there is a package object, one can use a.b as a value; but if there isn't, we cannot.

I don't think this is a deal breaker, but it should be clearly acknowledged in the proposal, IMO.

sjrd avatar Jan 24 '25 14:01 sjrd

Another inconsistency that this proposal introduces: a.b.c (where a.b is a package) can now sometimes be refactored to val x = a.b; x.c. Not if c is defined in the package (a top-level class / object / term definition).

IIUC, the motivation for this proposal is to improve the ergonomics when package objects are deliberately used for building the API of a library. Like package object requests extends requests.BaseSession, which is useful for import requests._ but also a useful first-class value. That seems fine with me, like others said it cements the status of package objects with inheritance in the language. (sbt uses it too).

The Mill use case I don't understand so well as there's additional magic going on in the build tool. I looked a bit at

  • https://mill-build.org/mill/fundamentals/modules.html#_rootmodule
  • https://mill-build.org/mill/large/multi-file-builds.html

Again, the SIP seems fine and helpful if we keep package objects. (I wonder why in the examples they are declared as package x; object `package` instead of package object x, and what the magic import $packages._ is.)

lrytz avatar Jan 24 '25 15:01 lrytz

I have removed the unnecessary limitations, added text mentioning the irregularity brought up by @sjrd and @lrytz, and added open questions related to our discussion about package objects participating in .apply and .type syntax. I think this proposal is good to go now

lihaoyi avatar Jan 25 '25 01:01 lihaoyi

See https://github.com/scala/scala3/pull/22011 for the implementation

odersky avatar Feb 16 '25 09:02 odersky

I tried integrating this into Mill as part of 3.7.0-RC1 in https://github.com/com-lihaoyi/mill/pull/4745 and taking advantage of the feature to remove the code-gen hacks we were previously relying on.

So far it seems to work great and does what it says on the box, all unit and integration tests pass, so we're quite confident it does what it's meant to do and does so correctly.

There are some random oversights (https://github.com/scala/scala3/issues/22830) and incidental problems around the -experimental flag (https://github.com/scala/scala3/issues/22831).

lihaoyi avatar Mar 18 '25 08:03 lihaoyi

Also opened a PR to IntelliJ, which also shows some nice simplifications in their reference following code https://github.com/JetBrains/intellij-scala/pull/676

lihaoyi avatar Mar 19 '25 01:03 lihaoyi

@lihaoyi I was given the wrong info and hence thought this document was in a final state. But the text doesn't look final. Could you finalize the document when you find the time? Thanks!

bracevac avatar Nov 20 '25 12:11 bracevac

@lihaoyi I was given the wrong info and hence thought this document was in a final state. But the text doesn't look final. Could you finalize the document when you find the time? Thanks!

Sorry for the noise. (Not only) I was confused what the semantics of "status:shipped" and "stage:completed" are. I'm going to make some changes to the SIP page and this repository that should be more sensible.

bracevac avatar Nov 27 '25 10:11 bracevac