Proposal: Structure of extension methods

[BalmungSan]

This proposal is based upon #24 for naming and package structure (here we are assuming that both proposals were chosen, at least partially) and further specifies where should the extension methods be placed.

This proposal has three end goals:

1. Provide users with an easy way to access the extended functionality; which leaves the door open for cross-compiling with 3.1 in the future.
2. Ensure names that will not be part of a future Scala release to be on a next subpackage (as required by Proposals for naming and package structure #24).
3. Strive for a codebase that is easy to maintain.

As such, I propose the following:

• All extension methods for the same type MUST be defined in a single class named Next<Name>Extensions placed in a next subpackage.
• Such class MUST be final & private[next]
• Such class MUST be a value class; and its single constructor argument MUST be private.
• Such class MUST be placed in a file called Next<Name>Extensions.scala.
• Such file MUST only contain the previous defined class.
• Such file must exist inside a next subdirectory.
• Finally, the implicit conversion from Name into Next<Name>Extensions MUST be declared in the package object next; and MUST be named scalaNextSyntaxFor<Name>.

Example:

// file: src/main/scala/scala/next/Next<Name>Extensions.scala

package scala.next

private[next] final class Next<Name>Extensions[A](private val col: <Name>[A]) extends AnyVal {
  // Extension methods go here.
}

// file: src/main/scala/scala/next/package.scala

package scala

import scala.language.implicitConversions

package object next {
  implicit final def scalaNextSyntaxFor<Name>[A](coll: <Name>[A]): Next<Name>Extensions[A] =
    new Next<Name>Extensions(col)
}

One open discussion for this proposal would be:

1. One single next package for all extensions: scala.next._
2. Multiple next packages for each package of the stdlib: scala.collection.next._, scala.collection.immutable.next._, etc.

---

Vote with:
In favour of this proposal going with option 1. (👍)
In favour of this proposal going with option 2. (🚀)
Completely against this proposal. (👎)

[NthPortal]

I think there has been previous discussion about whether to have a single next package or to have them "more localised" (e.g. scala.collection.immutable.next, though I can't recall where off the top of my head.

---

I would like to make one counter-proposal and ask one clarifying question.

the implicit conversion from Name into Next<Name>Extensions [...] MUST be named scalaNextSyntaxFor<Name>

The reasoning behind my proposal to call the value classes Next<Name>Extensions was based on the assumption that the value class itself would be implicit, to avoid shadowing of implicits. If we're going to separate the value class definition from the implicit conversion to it, I'd like to propose that the conversion method simply share the name of the value class (i.e. implicit def Next<Name>Extensions(<name>: <Name>): Next<Name>Extensions = new Next<Name>Extensions(<name>)).

Such class MUST be final & private[next]

Do you mean that the class itself or the constructor should be private[next]? My intuition says that the former should not work (i.e. should not give you access to the extension methods), though I don't actually know.

---

I like this proposal a lot, and I think it will help a lot with maintainability.

[NthPortal]

One small thing I'm curious about: do you think Next<Name>CompanionExtensions value classes should share a file with Next<Name>Extensions?

[BalmungSan]

the conversion method simply share the name of the value class (i.e. implicit def Next<Name>Extensions(<name>: <Name>): Next<Name>Extensions = new Next<Name>Extensions(<name>)).

I just did a quick experiment and it seems that if the implicit conversion has the same name as the value class, even if the code compiles it never finds the extension methods. But the first letter of the def may be lowercase and then it works.
Being honest I just used the same names I used in neotypes which I just borrowed from cats, so I really not care if the implicit conversion shares the name (but with an initial lowercase letter) as the value class instead of being called "Syntax".

Do you mean that the class itself or the constructor should be private[next]? My intuition says that the former should not work (i.e. should not give you access to the extension methods), though I don't actually know.

The class itself, which should automatically make the constructor also private AFAIK.
It does work, again this is a simplified approach of what I did in neotypes which is a simplified approach of what cats does.
It seems that you can leak private objects if you call a method immediately on them, this trick is also commonly used in the partially applied trick where the partially applied class is private.

One small thing I'm curious about: do you think NextCompanionExtensions value classes should share a file with NextExtensions?

Since both value classes will not be companions of each other, I do not see any benefit on having them on the same file.
Whereas I do see a benefit, in terms of maintainability, discoverability and incremental compilations; in having both on different classes. Of course, a valid counter-argument is that some people may try to find the extension methods of a companion object on the same file as the extension methods of the companion class; but I personally would not.
However, I think this should also be open to votation.

Vote with:

👍 to keep the extension methods of the companion object on its own file.
🚀 to keep the extension methods of the companion object in the same file.

---

I like this proposal a lot, and I think it will help a lot with maintainability.

That was the idea :)

[sjrd]

But the first letter of the def may be lowercase and then it works.

That would be very dangerous for case insensitive file systems, for example on Windows. It might crash on such systems.

[BalmungSan]

@sjrd not sure if I am missing something but I was talking about the name of the implicit def, how that would be affected by the filesystem?

[sjrd]

In general on Windows you can't have a class and an object whose names differ only in case. The object will also generate a class so it will clash with the class. The class may also generate an object (notably it will if it is a value class), which will also clash.

If it's a def, there's a chance it would work, because maybe the def doesn't clash with the generated object because it doesn't need a file. But I wouldn't take that chance.

[BalmungSan]

Effectively the compiler is way more complex than what I thought.

That is probably a good reason why cats uses a different name for the class and the def (or it may have been just a causality).

[som-snytt]

I haven't read all the words yet, but I'm wondering why there is an embrace of package object next rather than plain old object next?

Using package objects feels so 2016. I guess that would be pre-election 2016, from a US perspective, and definitely pre-October-surprise.

[NthPortal]

can you have an object that shares the name of a package?

[som-snytt]

Is it desirable to have a next package?

OK I just read the top of this ticket, after previously getting scared off by the MUST legalese.

I don't see the reason to avoid leveraging implicit classes and hand-implement the def conversions.

I would vote for "localized" name space such as scala.util.matching.next. If I wanted to propose renaming the package to scala.util.regex, would I put it under scala.util.next.regex, etc? I guess that would argue for a next package.

Otherwise I might put some private[collection.immutable] classes in that package and add conversions to immutable.next, if my new classes were so gnarly that I couldn't just drop them in the next object.

[BalmungSan]

I don't see the reason to avoid leveraging implicit classes and hand-implement the def conversions.

Because implicit classes can't be top-level so we can not just define them in their own files.
This leaves us with three options:

1. Everything in a single (package) object - Like add cycle method to LazyList #17 looks now - The downside of that, which is the point of this proposal is that as this repo grows those files would become somewhat big and with many symbols on them, which will make incremental compilation slow and would difficult navigating in the source code.
2. Each on its own object - Like how Add groupMapReduce method to IterableOnceOps #23 looks now - The downside is that then users would need one import per type; like import scala.next.collection.immutable.LazyList._ (or something like that).
3. Each on its own trait and then a single object that extends all traits - The question with this one would be where to place those traits and if those should be public (so a user can inherit them to have the syntax instead of importing from the object) or keep private. Another question would be if we should then provide granular objects. (this again would be a simplified version of what libraries like cats do, the difference is that in those libraries the trait only contains the implicit conversion and the value class is outside)

[BalmungSan]

My intuition says that the former should not work (i.e. should not give you access to the extension methods), though I don't actually know.

@NthPortal just to be sure I just did this quick experiment with a local publish of #23

// Welcome to the Ammonite Repl 2.3.8 (Scala 2.13.3 Java 11.0.9)
@ import $ivy.`org.scala-lang.modules::scalalibrarynext:0.0.1+27-8f9fd9cb-SNAPSHOT` 
// import $ivy.$  

@ import scala.collection.next._ 
// import scala.collection.next._

@ "Hello World Goodbye World".split(' ').iterator.groupMapReduce(identity)(_ => 1)(_ + _)
// res: Map[String, Int] = Map("Hello" -> 1, "World" -> 2, "Goodbye" -> 1)

🙂

Also, if I ask Ammonite what can I import from scala.collection.next._ it only shows the package (and its member, i.e. the implicit conversions). Thus, the extension class is effectively private for users!

[NthPortal]

I'm largely in favour of this proposal, though I'm not a huge fan of the value classes being private—that seems confusing and unintuitive to me, regardless of it working (possibly a bug?). despite saying this, I don't think it's worth holding the proposal up over that, and my feelings on it are not super strong.

I feel much more strongly about getting this finalised and getting a few PRs merged soon (although we can also merge PRs without waiting and then just tweak the code later once we finalise this).

[NthPortal]

I propose that we don't wait on finalizing this and #24 to merge PRs, and we can just refactor the library once we've decided on those.