Deprecate package object with extends clause #7662

adriaanm · 2019-01-17T13:44:15Z

UPDATE: the deprecation ended up being put behind -Xsource:2.14 (see #7857)

Workaround: use a regular object and import it. I consider the explicit import an advantage: it makes it more clear where implicits are coming from. In general, I would not recommend defining implicits in package objects.

Longer term, package objects will be replaced by top-level definitions.

To enable this migration to top-level definitions as a replacement
for package objects, we must first remove the main aspect of
package objects that complicates their implementation and
that would not longer be supported by top-level definitions.

Once that's done, the migration would mean rewriting
package object foo { ... } to package foo { ... },
assuming the ... are exclusively definitions, and not statements.

Losing inheritance for package objects means that some code duplication
will be required (you'll have to add val/type aliases for members,
instead of inheriting the members directly). You also can't use inheritance
to prioritize implicits, but, implicits defined in packages are also
being phased out (i.e., package prefixe don't contribute to the implicit scope).

The main complication with supporting full-on package objects is
that name resolution needs to know all definitions in a package
in order to resolve the parents of said package, and when there's
a cycle there (a package object inherits a class defined in the package),
the implementation becomes challenging due to the open nature of packages.

While we can easily rule out the cycle in the following snippet that
uses plain objects, with package objects it's not so easy:

object Dogs extends Dogs.Animal { // cycle!
  class Animal
}

The spec currently has similar awkwardness in place to deal with this:

The package object should not define a member with the same name as
one of the top-level objects or classes defined in package $p$. If
there is a name conflict, the behavior of the program is currently
undefined. It is expected that this restriction will be lifted in a
future version of Scala.

Finally, package objects are objects in name only -- they already are
not first-class values, and now they also can't extend anything. This
leaves them as simply syntactic baggage. Top-level definitions are
intuitive, require no syntax, and can be implemented in the same way
as package objects behind the scenes, albeit without the issue of cycles.

package object foo

class D {
  println(foo) // error: package foo is not a value
}

smarter · 2019-01-17T13:50:42Z

Based on the discussion in scala/scala-dev#441 this seems to be a pretty controversial change, are you intending to go ahead with this anyway ?

adriaanm · 2019-01-17T14:07:59Z

Yep

adriaanm · 2019-01-17T14:10:16Z

Seriously, though, I read that discussion as having valid pros and cons for deprecation, though with the pros outnumbering the cons.

smarter

I read that discussion as having valid pros and cons for deprecation, though with the pros outnumbering the cons.

Please list these pros/cons in the commit message :).

adriaanm · 2019-01-17T14:44:51Z

will do -- in the mean time, here's a beauty: 357905c

adriaanm · 2019-01-17T14:48:13Z

tests to fix for tickets

dwijnand · 2019-01-17T14:57:07Z

How does

object `package` extends C

behave, instead?

som-snytt · 2019-01-20T08:29:36Z

The best part is that it blows the future wide open to re-introduce package objects as a module system.

som-snytt

OK. People want to take away our cycles, but where would we be without cycle sheds?

som-snytt · 2019-01-20T08:43:55Z

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

      val tstart = in.offset
      atPos(start, if (name == nme.ERROR) start else nameOffset) {
-        val template = templateOpt(mods, name, NoMods, Nil, tstart)
+        val template = templateOpt(mods, if  (isPackageObject) nme.PACKAGEkw else name, NoMods, Nil, tstart)


scalafmt where are you? SIP-12 where are you? There's always either too many spaces or not enough spaces around a paren. if ( isPO ) ofc. Also ofc we need API for name.orElsePackageIf(b). But I'd be satisfied with deleting one space, if my new computer glasses worked.

but! who doesn't like more space! (hint: markdown)

som-snytt · 2019-01-20T08:49:47Z

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala


+      // we can't easily check this later, because `gen.mkParents` adds the default AnyRef parent, and we need to warn based on what the user wrote
+      if (name == nme.PACKAGEkw && parents.nonEmpty) deprecationWarning(tstart, s"package object inheritance is deprecated", "2.13.0")
+


It's a pity it's quite pithy. I wonder if it should say another word, like, "And we're taking package objects away from you entirely in the next release." First they took away my inheritance, then my objects, next there will be no packages at all under the tree. No fun living at the orphanage.

I blame the boomers. Anyway, I added some links and verbiage.

To enable migration to top-level definitions as a replacement for package objects, we must first remove the main aspect of package objects that complicates their implementation and that would not longer be supported by top-level definitions. Once that's done, the migration would mean rewriting `package object foo { ... }` to `package foo { ... }`, assuming the `...` are exclusively definitions, and not statements. Losing inheritance for package objects means that some code duplication will be required (you'll have to add val/type aliases for members, instead of inheriting the members directly). You also can't use inheritance to prioritize implicits, but, implicits defined in packages are also being phased out (i.e., package prefixe don't contribute to the implicit scope). The main complication with supporting full-on package objects is that name resolution needs to know all definitions in a package in order to resolve the parents of said package, and when there's a cycle there (a package object inherits a class defined in the package), the implementation becomes challenging due to the open nature of packages. While we can easily rule out the cycle in the following snippet that uses plain objects, with package objects it's not so easy: ``` object Dogs extends Dogs.Animal { // cycle! class Animal } ``` The spec currently has similar awkwardness in place to deal with this: > The package object should not define a member with the same name as > one of the top-level objects or classes defined in package $p$. If > there is a name conflict, the behavior of the program is currently > undefined. It is expected that this restriction will be lifted in a > future version of Scala. Finally, package objects are objects in name only -- they already are not first-class values, and now they also can't extend anything. This leaves them as simply syntactic baggage. Top-level definitions are intuitive, require no syntax, and can be implemented in the same way as package objects behind the scenes, albeit without the issue of cycles. ``` package object foo class D { println(foo) // error: package foo is not a value } ```

neko-kai · 2019-01-25T15:29:34Z

It's a big change that will break a lot of libraries. Shouldn't this go through a SIP?

adriaanm · 2019-01-25T16:42:48Z

The actual change will. Deprecation won't.

neko-kai · 2019-01-25T19:31:24Z

It's nearly the same thing, though – I doubt it would be 'undeprecated' if the SIP doesn't go through. Seems like a very one-sided decision to me.

soronpo · 2019-01-25T20:02:56Z

It's nearly the same thing, though – I doubt it would be 'undeprecated' if the SIP doesn't go through. Seems like a very one-sided decision to me.

I tend to agree. A mitigation can be if there was a warning suppression mechanism for the library to use so it can use fatal warnings. Deprecating is a way to say "please don't do this/this is not recommended". That is fine, but we need away to reply to the compiler "I agree, but shut up please".

adriaanm · 2019-01-25T20:06:24Z

Sure, I started working on that here
https://github.com/adriaanm/scala/tree/deprconf. Help finishing the job very welcome

soronpo · 2019-02-04T13:01:05Z

@adriaanm Can you please schedule a 2.13 community-build for this PR?

adriaanm · 2019-02-04T13:20:54Z

Since it's merged, the community build will pick it up. I assume you're worried about its effect on warnings, but those won't show up in the community build because we already disable -Xfatal-warnings there in most projects.

godenji · 2019-02-17T02:33:07Z

@adriaanm from your reply on the corresponding issue

I think it's actually desirable to have a least an explicit entry point (an import in this case) to understanding where implicits come from

But it's not just implicits, you may have non-implicit methods, vals, objects, etc. that you want in scope at the package level. Having to convert package object extends ... to plain object extends ... and then import that in every source file is honestly a massive pain compared to the wonderous status quo that obviates all this (IMO) needless extra work.

This is going to wreak serious havok, every code base built with -Xfatal-warnings is going to blow up. What the heck, this is just merged into 2.13, no SIP??

dwijnand · 2019-02-17T06:11:39Z

They can drop fatal warnings or resolve the deprecation warnings. The compiler development can't be bound by the constraints of fatal warnings.

som-snytt · 2019-02-17T06:47:10Z

Besides using -Yimports to add an "auto-imported predef", I'm following up Adriaan's "filter ALL the things" idea at #7728, so users can keep fatal warnings by either moving a package object to root import position (which might work well for top packages of a library or app) or just whitelisting the warnings.

Note that -Xfatal-warnings will become -Werror, but the old name will still work without itself emitting a deprecation.

(Edit: now I wish it were called, -Xmortal-admonitions.)

godenji · 2019-02-17T11:57:48Z

They can drop fatal warnings or resolve the deprecation warnings. The compiler development can't be bound by the constraints of fatal warnings.

Well, the issue is that there's no viable replacement. Top-level definitions aren't yet supported; custom predef is globally scoped (IDE support is probably amazing); plain objects require an import in every source file.

I can appreciate that this change will make life easier for the compiler team, but it's strictly worse for users that have benefited for years from package scoped auto-imports.

dwijnand · 2019-02-17T16:58:29Z

Another alternative is writing the forwarders in the package object. But I agree with you those are the tradeoffs for users making use of this feature which is being deprecated.

SethTisue · 2019-02-20T20:25:25Z

@godenji thanks for speaking up with your concerns about this, I think the questions you are asking and the points you're bringing up will be of concern to others as well.

no SIP??

well, I think everyone on the SIP committee is aware of these changes, and IIRC this has come up in several meetings...

...but on the other hand, I was a bit surprised not to find top level definitions on the list at scala/scala3#5221, and I've now said so at scala/scala3#5221 (comment)

every code base built with -Xfatal-warnings is going to blow up.

Adriaan and I both agree with @dwijnand that compiler development and the progress of the language can't be bound by the constraints of fatal warnings.

https://github.com/ghik/silencer provides a escape hatch. another possible escape hatch is to enable fatal warnings only on the main Scala version you are targeting (e.g., 2.12)

trying to cross-compile to multiple Scala versions without any warnings at all in any of the versions you are targeting may sometimes be possible for some projects, but making it possible in general isn't really a goal for us, it's far too constraining.

Well, the issue is that there's no viable replacement. Top-level definitions aren't yet supported

good point. @adriaanm is there a reason this deprecation couldn't wait for 2.14? are we deprecating in 2.13 because we want to remove it in 2.14 because there's something in the 2.14 roadmap that conflicts?I'd like to understand this as well.

SethTisue · 2019-02-20T20:35:35Z

I think everyone on the SIP committee is aware of these changes, and IIRC this has come up in several meetings...

ah, just found scala/scala-dev#441 (comment) where I wrote:

this was discussed by the SIP committee at the SIP meeting this week

unfortunately it seems that the minutes from that meeting were never published; at least, they're missing from https://github.com/scala/docs.scala-lang/tree/master/_sips/minutes (probably a casualty of the changeover in the SIP process lead position, I suppose), but there is a video at https://www.youtube.com/watch?v=dFEkS71rbW8 and the agenda includes "Remove package objects", with a link to scala/scala-dev#441

so essentially scala/scala-dev#441 is the SIP, so it was up for public discussion, and the committee was made aware as well

so process-wise, I think once this is added to the list at
scala/scala3#5221 we can say the process was followed. especially since this is, for now, only a deprecation

adriaanm · 2019-02-20T20:35:44Z

Top level definitions provide no means for inheritance, which is why we’re deprecating this aspect of package objects. That’s why we’re deprecating now so that we can hopefully add top level definitions in 2.14

…

On Wed, 20 Feb 2019 at 12:25, Seth Tisue ***@***.***> wrote: @godenji <https://github.com/godenji> thanks for speaking up with your concerns about this, I think the questions you are asking and the points you're bringing up will be of concern to others as well. no SIP?? well, I think everyone on the SIP committee is aware of these changes, and IIRC this has come up in several meetings... ...but on the other hand, I was a bit surprised not to find top level definitions on the list at scala/scala3#5221 <scala/scala3#5221>, and I've now said so at scala/scala3#5221 (comment) <scala/scala3#5221 (comment)> every code base built with -Xfatal-warnings is going to blow up. Adriaan and I both agree with @dwijnand <https://github.com/dwijnand> that compiler development and the progress of the language can't be bound by the constraints of fatal warnings. https://github.com/ghik/silencer provides a escape hatch. another possible escape hatch is to enable fatal warnings only on the main Scala version you are targeting (e.g., 2.12) trying to cross-compile to multiple Scala versions without any warnings at all in any of the versions you are targeting may sometimes be possible for some projects, but making it possible in general isn't really a goal for us, it's far too constraining. Well, the issue is that there's no viable replacement. Top-level definitions aren't yet supported good point. @adriaanm <https://github.com/adriaanm> is there a reason this deprecation couldn't wait for 2.14? are we deprecating in 2.13 because we want to remove it in 2.14 because there's something in the 2.14 roadmap that conflicts?I'd like to understand this as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7662 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjy3S7r7QYAj6p_la5mBNgvIGjKPEOks5vPa84gaJpZM4aFOW-> .

smarter · 2019-02-20T20:57:44Z

Dotty right now has both package objects with inheritance and top-level objects, so from what I can see there's no urgency to deprecate this.

Ichoran · 2019-02-20T21:11:58Z

There doesn't seem to be any logical reason why package objects' inheritance has to be deprecated in order for top-level definitions to go in in 2.14--or now, for that matter. Even if you use the package object mechanism for TLDs, you just need to mangle names slightly differently and the two can coexist. Yes, there's weird stuff about how the two can look into each other, but you just make some ad-hoc rules to simplify the logic during the period where both coexist. (E.g. allow the visibility to go only one way.)

That the deprecation is going in before the replacement is ready is rather disappointing. That the envisioned replacement isn't actually a full replacement because it doesn't cleanly solve the implicit priority thing (as far as I can tell) is also disappointing.

I understand the inherent problems that package objects have, and look forward to them going away. I haven't liked to have to use that mechanism, and I haven't enjoyed the bugs. But they're here now and work and aid usability. So without a good alternative for inheritance use cases, the deprecation doesn't feel ready to me.

neko-kai · 2019-02-20T21:25:03Z

I, for one, am NOT looking forward to package objects going away. The fact that top-level is first-class and composable by mixins like any other object is one of the beauties of Scala to me.

dwijnand · 2019-02-20T21:42:48Z

Even if you use the package object mechanism, you just need to mangle names slightly differently and the two can coexist. Yes, there's weird stuff about how the two can look into each other, but you just make some ad-hoc rules to simplify the logic during the period where both coexist. (E.g. allow the visibility to go only one way.)

There's lots of wonderfully written detail about that "weird stuff" in gkossakowski/kentuckymule#6 (comment).

SethTisue · 2019-02-20T22:07:13Z

I, for one, am NOT looking forward to package objects going away

please use scala/scala-dev#441 for any general discussion, so we can keep the discussion here on the pull request focused on 1) the implementation, and 2) the timing of the deprecation

adriaanm · 2019-02-20T22:45:54Z

Again, please note that we’re only talking about deprecating the EXTENDS clause of a package object. Not package objects themselves. Once your code base is free of package objects that use inheritance, migrating to toplevel definitions is trivial.

…

On Wed, 20 Feb 2019 at 14:07, Seth Tisue ***@***.***> wrote: I, for one, am NOT looking forward to package objects going away please use scala/scala-dev#441 <scala/scala-dev#441> for any general discussion, so we can keep the discussion here focused on 1) the implementation, and 2) the timing of the deprecation — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7662 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjy84d22To57SXwJKtPzVd_pz_vR1lks5vPccUgaJpZM4aFOW-> .

soronpo · 2019-02-20T22:51:31Z

Since it's merged, the community build will pick it up. I assume you're worried about its effect on warnings, but those won't show up in the community build because we already disable -Xfatal-warnings there in most projects.

@SethTisue is it possible to run a community build that does not disable -Xfatal-warnings to get a feel of how exposed we are to this deprecation?

SethTisue · 2019-02-20T22:57:00Z

is it possible to run a community build that does not disable -Xfatal-warnings to get a feel of how exposed we are to this deprecation?

I think that would give less information than the current setup gives, not more. if we enable -Xfatal-warnings, then almost certainly some low-level project will fail and knock out the rest of the build

I think what you want is just to analyze the log of the existing community build and see how often a specific warning we are interested in (such as this one) crops up. I guess we'd want to make sure -deprecation is enabled build-wide so that we'd always see the specific form of the warning and not just a "there were 37 deprecation warnings" type message. dbuild supports that.

Ichoran · 2019-02-21T00:00:42Z

@dwijnand - Despite the problems, people who have used inheritance with package objects have already made as much peace as there is to be made with those issues. So it still seems plausible that the deprecation could wait until there was something with fewer rough edges to migrate to.

@SethTisue - Getting that deprecation log would be great. I know I use the package object extends pattern a number of times in my code (entirely for implicits, not API exporting), but I can't tell whether I'm an anomaly in that regard.

smarter · 2019-03-13T08:30:46Z

New development: #7857

scala-jenkins added this to the 2.13.0-RC1 milestone Jan 17, 2019

adriaanm requested a review from som-snytt January 17, 2019 13:44

smarter mentioned this pull request Jan 17, 2019

Drop package objects with inheritance scala/scala-dev#441

Closed

adriaanm requested a review from smarter January 17, 2019 14:13

smarter reviewed Jan 17, 2019

View reviewed changes

adriaanm force-pushed the depr-pkg-extends branch from 7973505 to ae1c98f Compare January 17, 2019 14:48

adriaanm changed the title ~~Deprecate package object with extends clause~~ Deprecate package object with extends clause [ci: last-only] Jan 17, 2019

adriaanm force-pushed the depr-pkg-extends branch from ae1c98f to 67dd905 Compare January 17, 2019 17:11

SethTisue added the release-notes worth highlighting in next release notes label Jan 17, 2019

adriaanm force-pushed the depr-pkg-extends branch 2 times, most recently from 5e9acc1 to 9ee9469 Compare January 18, 2019 10:43

adriaanm changed the title ~~Deprecate package object with extends clause [ci: last-only]~~ Deprecate package object with extends clause Jan 18, 2019

adriaanm force-pushed the depr-pkg-extends branch from 9ee9469 to acc9997 Compare January 18, 2019 17:22

som-snytt approved these changes Jan 20, 2019

View reviewed changes

adriaanm force-pushed the depr-pkg-extends branch from acc9997 to 3e18e9f Compare January 25, 2019 09:50

soronpo mentioned this pull request Jan 25, 2019

Allow toplevel definitions scala/scala3#5754

Merged

sjrd mentioned this pull request Jan 30, 2019

Release 2.13.0-RC1 scala/scala-dev#599

Closed

51 tasks

adriaanm merged commit 9deac55 into scala:2.13.x Feb 4, 2019

sjrd mentioned this pull request Feb 4, 2019

[no-master] Adapt a neg test in JSInteropTest for Scala 2.13.x. scala-js/scala-js#3559

Merged

dwijnand mentioned this pull request Feb 17, 2019

type aliases breaking implicit resolution scala/bug#10582

Closed

SethTisue mentioned this pull request Feb 20, 2019

Scala 3 SIPs, meta-issue for tracking scala/scala3#5221

Closed

25 tasks

adriaanm mentioned this pull request Mar 12, 2019

Delay deprecating package object inheritance until 2.14 #7857

Merged

SethTisue removed the release-notes worth highlighting in next release notes label Mar 18, 2019

SethTisue changed the title ~~Deprecate package object with extends clause~~ [REVERTED] Deprecate package object with extends clause Apr 4, 2019

som-snytt mentioned this pull request Oct 5, 2023

compile error Scala 2.13.12 with -Xsource:3 eed3si9n/scalaxb#606

Open

som-snytt changed the title ~~[REVERTED] Deprecate package object with extends clause~~ Deprecate package object with extends clause Oct 5, 2023


		// we can't easily check this later, because `gen.mkParents` adds the default AnyRef parent, and we need to warn based on what the user wrote
		if (name == nme.PACKAGEkw && parents.nonEmpty) deprecationWarning(tstart, s"package object inheritance is deprecated", "2.13.0")

Deprecate package object with extends clause #7662

Deprecate package object with extends clause #7662

Uh oh!

Conversation

adriaanm commented Jan 17, 2019 • edited by SethTisue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarter commented Jan 17, 2019

Uh oh!

adriaanm commented Jan 17, 2019

Uh oh!

adriaanm commented Jan 17, 2019

Uh oh!

smarter left a comment

Choose a reason for hiding this comment

Uh oh!

adriaanm commented Jan 17, 2019

Uh oh!

adriaanm commented Jan 17, 2019

Uh oh!

dwijnand commented Jan 17, 2019

Uh oh!

som-snytt commented Jan 20, 2019

Uh oh!

som-snytt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

som-snytt Jan 20, 2019

Choose a reason for hiding this comment

Uh oh!

adriaanm Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

som-snytt Jan 20, 2019

Choose a reason for hiding this comment

Uh oh!

adriaanm Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

neko-kai commented Jan 25, 2019

Uh oh!

adriaanm commented Jan 25, 2019

Uh oh!

neko-kai commented Jan 25, 2019

Uh oh!

soronpo commented Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriaanm commented Jan 25, 2019

Uh oh!

soronpo commented Feb 4, 2019

Uh oh!

adriaanm commented Feb 4, 2019

Uh oh!

godenji commented Feb 17, 2019

Uh oh!

dwijnand commented Feb 17, 2019

Uh oh!

som-snytt commented Feb 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

godenji commented Feb 17, 2019

Uh oh!

dwijnand commented Feb 17, 2019

Uh oh!

SethTisue commented Feb 20, 2019

Uh oh!

SethTisue commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriaanm commented Feb 20, 2019 via email

Uh oh!

smarter commented Feb 20, 2019

Uh oh!

Ichoran commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neko-kai commented Feb 20, 2019

adriaanm commented Jan 17, 2019 •

edited by SethTisue

Loading

som-snytt left a comment •

edited

Loading

adriaanm Jan 25, 2019 •

edited

Loading

soronpo commented Jan 25, 2019 •

edited

Loading

som-snytt commented Feb 17, 2019 •

edited

Loading

SethTisue commented Feb 20, 2019 •

edited

Loading

Ichoran commented Feb 20, 2019 •

edited

Loading

SethTisue commented Feb 20, 2019 •

edited

Loading

SethTisue commented Feb 20, 2019 •

edited

Loading