scala-library-next
scala-library-next copied to clipboard
Proposal: Structure of extension methods
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:
- Provide users with an easy way to access the extended functionality; which leaves the door open for cross-compiling with
3.1in the future. - Ensure names that will not be part of a future Scala release to be on a
nextsubpackage (as required by #24). - 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>Extensionsplaced in anextsubpackage. - 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
nextsubdirectory. - Finally, the implicit conversion from
NameintoNext<Name>ExtensionsMUST be declared in the package objectnext; and MUST be namedscalaNextSyntaxFor<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:
- One single
nextpackage for all extensions:scala.next._ - Multiple
nextpackages 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. (๐)
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
NameintoNext<Name>Extensions[...] MUST be namedscalaNextSyntaxFor<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.
One small thing I'm curious about: do you think Next<Name>CompanionExtensions value classes should share a file with Next<Name>Extensions?
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 Next<Name>CompanionExtensions value classes should share a file with Next<Name>Extensions?
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:
:+1: 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 :)
But the first letter of the
defmay 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.
@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?
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.
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).
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.
can you have an object that shares the name of a package?
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.
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:
- Everything in a single (
package)object- Like #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. - Each on its own
object- Like how #23 looks now - The downside is that then users would need one import per type; likeimport scala.next.collection.immutable.LazyList._(or something like that). - Each on its own
traitand then a singleobjectthat 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 granularobjects. (this again would be a simplified version of what libraries like cats do, the difference is that in those libraries thetraitonly contains the implicit conversion and the value class is outside)
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!
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).
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.