Change extension method syntax.

[odersky]

Use (this: T) parameter instead of prefix parameter list.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

1. Separate subject from body with a blank line
2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
3. Limit the subject line to 72 characters
4. Capitalize the subject line
5. Do not end the subject line with a period
6. Use the imperative mood in the subject line ("Add" instead of "Added")
7. Wrap the body at 80 characters
8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

[jducoeur]

Opinion: having chewed on it for a few months (and I recognize that I objected to it originally), I like the prefix syntax more than this. The this format is too subtle, IMO, whereas the prefix format really jumps out and says "this isn't just an ordinary function" -- I think it promotes much better code glanceability...

[propensive]

What's the motivation for changing this back? I taught the new syntax to a dozen people at Scala Days, and once they had understood that they no longer had to mentally rearrange the parameters and method name at the use site, they preferred it. Having been using it myself for a month now, I much prefer it.

[odersky]

@jducoeur @propensive
At present it's a trial balloon. Having tried it out myself, notably on Flags.scala in the compiler, I had the impression it did not scale so well. My main issue was that it was hard to see at a glance what was being defined since the defined name was hidden after the first parameter list. Did you try it out in a larger setting (i.e. more than 10 extension methods)? What was your impression?

[propensive]

No, I've only used it in smallish prototypes so far, but I can see how that scenario might be less clear if the "this" parameter is given a name with a different length each time. (Though in Flags.scala it's always called x, so that's not an issue here.)

Just glancing at Flags.scala, though, it's really easy for me to see how methods like & and is should be used at the call-site, and I can also see immediately that they're extension methods. (That is also true, if the parameter is called this, but maybe less obvious.)

[propensive]

👍 for it being just a "trial balloon". ;)

[odersky]

Maybe writing the extension method name on a new line would help. I.e.

def (x: FlagSet) 
    isOneOf (flags: FlagSet): Boolean = ...

instead of

def (x: FlagSet) isOneOf (flags: FlagSet): Boolean = ...

Right now that's not allowed, but I made a PR with a fix. Will experiment with both possibilities for a bit more.

[propensive]

That's great - I'd like to have the option of doing that, moreso for alphanumeric method names than symbolic ones. It would fit well with some of the whitespace formatting I'm still experimenting with... fifteen years into using this language. ;)

[milessabin]

@odersky I'm guessing this will break shapeless in the community build ... what's the usual sequencing of things when breakage of this sort happens? Let the community build break first and fix it up later?

[odersky]

@milessabin The idea in general is to support both syntaxes for one release cycle, which gives us time to move the community build over.

[odersky]

I am thinking about allowing both new and old syntax (the latter fixed wrt newlines) during one release cycle. That gives us time to experiment at large with it before deciding.

[lrytz]

I always thought the backwards scope of type parameters is weird

def (xs: List[A]) flatMap [A, B](f: A => List[B]): List[B]

so 👍

[He-Pin]

I more like

def < (that: T) on (this: T) = this.compareTo(that) < 0

[He-Pin]

I always thought the backwards scope of type parameters is weird

def (xs: List[A]) flatMap [A, B](f: A => List[B]): List[B]

so 👍

So do I. but I would like to see

def flatMap [A, B](f: A => List[B]) on (xs: List[A]) : List[B]

alias with the given clause.

def max[T](x: T, y: T) given (ord: Ord[T]): T =

[floating-cat]

Maybe a dumb idea:
I see you write def (x: FlagSet) 23 times in Flag.scala. Would it better to introduce some syntax like this:

delegate FlagOps {
  x: FlagSet =>
}

for the default receiver to reduce these verbosity?

[mumutu66]

much more prefer new line as it can make both the target type and method clear at a glance

[odersky]

delegate FlagOps {
  x: FlagSet =>
}

That already means something else (a self type). I believe the repetition is not so bad. Let's see how many of these things appear over a large codebase before we decide it needs fixing.

[dwijnand]

@odersky given it's provided good insight, perhaps it's a good idea to include a trimmed down, but still representational, Flags.scala as a pos test, so the impact of different ideas can be judged.

[odersky]

I have added Flags.scala as a test twice, once using infix syntax, the other using this parameters

Observations:

• The scannability problems I had with infix syntax went away once I put the infix method name on a new line. Now it is quite clear I think.
• There's still the annoyance that you can't find an extension method foo by searching for def foo.
• The problem with type parameters also goes away if you insert a newline. In fact an extension
  method signature would be formatted in exactly the same way as a method in an implicit class,
  except that there is the leading parameter and a newline.
• In summary, I am not sure about using this as a parameter name. It looks like we are mixing up two features here that are better kept separate.
• this as a parameter is still a leaky abstraction in that one cannot do all the things one expects to do with class-based this. Even if we import all this members silently (which we do), there's still a problem:

class B {
  def f = ???
}
class A {
  def g(this: B)(that: C) = ???
  def h(this: B)(that: C) = {
    this.f // OK
    f // OK, expands to this.f
    this.g(that) // OK
    g(that) // type error: expected: B, found: C
  }
}

There are good reasons why things are as they are, but there is then the problem that, if one sees
a this.g(that) one cannot simply drop the this., which is what one would be used to, normally.

[odersky]

Another area where infix is clearly superior are right-associative infix operators.

Compare

def (x: A) 
    :: (y: List[A]) = ...

with

def ::(this: A)(y: List[A]) = ...

This is what's currently implemented, but it looks wrong. Probably we should do instead:

def :: (x: A)(this: List[A]) = ...

But that would complicate the parsing rules quite a bit.

In fact the infix syntax is so clean that we might want keep this as the only way to define a right-associative operator.

[julienrf]

* `this` as a parameter is still a leaky abstraction in that one cannot do all the things one expects to do with class-based `this`. Even if we import all `this` members silently (which we do), there's still a problem:

class B {
  def f = ???
}
class A {
  def g(this: B)(that: C) = ???
  def h(this: B)(that: C) = {
    this.f // OK
    f // OK, expands to this.f
    this.g(that) // OK
    g(that) // type error: expected: B, found: C
  }
}

There are good reasons why things are as they are, but there is then the problem that, if one sees
a this.g(that) one cannot simply drop the this., which is what one would be used to, normally.

I believe that this.g should not compile: this should only refer to the left operand, not the enclosing class. If one wants to refer to the enclosing class he can use an alias:

class B {
  def f = ???
}
class A { a =>
  def g(this: B)(that: C) = ???
  def h(this: B)(that: C) = {
    this.f // OK
    f // error: f is not a member of type A
    this.g(that) // error: g is not a member of type B
    a.g(that) // OK
    g(that) // type error: expected: B, found: C (maybe infix operations should always be called with an explicit lhs?)
    // Should we allow the following?
    a.g(this)(that)
  }
}

[odersky]

@julienrf

this.g(that) compiles since g is an extension method on B. It's a good demonstration why mixing the two meanings of this is confusing.

[dwijnand]

    def (x: FlagSet)
        isEmpty: Boolean = (x.bits & ~KINDFLAGS) == 0

    /** Is a given flag set a subset of another flag set? */
    def (x: FlagSet)
        <= (y: FlagSet): Boolean = (x.bits & y.bits) == x.bits

    /** Does the given flag set apply to terms? */
    def (x: FlagSet)
        isTermFlags: Boolean = (x.bits & TERMS) != 0

    /** Does the given flag set apply to terms? */
    def (x: FlagSet)
        isTypeFlags: Boolean = (x.bits & TYPES) != 0

is just dying for x: FlagSet to be defined somewhere else, once and for all.

What if we stayed with infix syntax, available as top-level extension methods and elsewhere but, in addition, you can define the shared LHS within the context. Basically like infix classes. Something like:

delegate FlagOps(x: FlagSet) { // or some other way
  def bits: Long = opaques.toBits(x)

...

  def isEmpty: Boolean         = (x.bits & ~KINDFLAGS) == 0
  def <= (y: FlagSet): Boolean = (x.bits & y.bits) == x.bits
  def isTermFlags: Boolean     = (x.bits & TERMS) != 0
  def isTypeFlags: Boolean     = (x.bits & TYPES) != 0
  def toTypeFlags: FlagSet     = if (x.bits == 0) x else FlagSet(x.bits & ~KINDFLAGS | TYPES)
  def toTermFlags: FlagSet     = if (x.bits == 0) x else FlagSet(x.bits & ~KINDFLAGS | TERMS)
  def toCommonFlags: FlagSet   = if (x.bits == 0) x else FlagSet(x.bits | KINDFLAGS)
  def numFlags: Int            = java.lang.Long.bitCount(x.bits & ~KINDFLAGS)
  def firstBit: Int            = java.lang.Long.numberOfTrailingZeros(x.bits & ~KINDFLAGS)

...

[soronpo]

What if we stayed with infix syntax, available as top-level extension methods and elsewhere but, in addition, you can define the shared LHS within the context. Basically like infix classes. Something like:

delegate FlagOps(x: FlagSet) { // or some other way
  def bits: Long = opaques.toBits(x)

...

This is just bringing back implicit classes. Indeed I see the benefit, but I don't think we should use the delegate keyword for that.

[soronpo]

I actually don't see any problem with implicit classes other than the irrelevant use of the word implicit and that we need to name the class. I think we should just add another keyword that truly describes our intent.

extension class (x: FlagSet) { //No need to name it!
  def bits: Long = opaques.toBits(x)

...

  def isEmpty: Boolean         = (x.bits & ~KINDFLAGS) == 0
  def <= (y: FlagSet): Boolean = (x.bits & y.bits) == x.bits
  def isTermFlags: Boolean     = (x.bits & TERMS) != 0
  def isTypeFlags: Boolean     = (x.bits & TYPES) != 0
  def toTypeFlags: FlagSet     = if (x.bits == 0) x else FlagSet(x.bits & ~KINDFLAGS | TYPES)
  def toTermFlags: FlagSet     = if (x.bits == 0) x else FlagSet(x.bits & ~KINDFLAGS | TERMS)
  def toCommonFlags: FlagSet   = if (x.bits == 0) x else FlagSet(x.bits | KINDFLAGS)
  def numFlags: Int            = java.lang.Long.bitCount(x.bits & ~KINDFLAGS)
  def firstBit: Int            = java.lang.Long.numberOfTrailingZeros(x.bits & ~KINDFLAGS)

...

• Extension classes can inherit traits and classes and have their own scope.
• Extension classes can be anonymous
• Extension classes can co-exist with extension methods. I don't see a reason to favor one over the other. They both have merit.

Alternatively, we can use the for keyword to describe extension classes:

class for (x: FlagSet) { 
  def bits: Long = opaques.toBits(x)

...

[onekirne]

I want to suggest something else to do with def (this:T) method, namely to handle that uncomfortable but extremely useful thing still sitting between delegates and implicit conversions. It goes without saying that implicit conversions have issues; but implementing Iterable[T] as a delegate is something many users will want to do.

Please remember that scala.collection.Iterable is not a typeclass.

The problem in current Dotty is as follows:

  delegate Example for Iterable[String] given (can_be_anything_but_this:Nothing){
    override def (that:StrangeArtifact) iterator:Iterator[String] = ??? //does not refer to any abstract method
    override def iterator:Iterator[String] = ??? //has no useful reference to "this"
    def concat = this.fold("")(_+_) //"this" refers to the delegate itself, it goes nowhere!
  }

The proposed solution is easiest to explain with an example of the required code transformation.

Given this hypothetical user code:

  delegate Example for Iterable[String] {
    override def (this:StrangeArtifact) iterator:Iterator[String] = this.read
    override def (this:StringStream) iterator:Iterator[String] = ???
    def (this) concat = this.fold("")(_+_)
  }

And given either one of these are understood by the compiler:

//only converts for extension methods:
  abstract class Delegated[-T, +U] extends Conversion[T,U]
//or encode that constraint with a marker trait:
  trait DoNotImplicitlyConvert

In current Dotty, the above snippet could be rewritten as follows:

  abstract class Example extends Iterable[String] with DoNotImplicitlyConvert {
    def concat = this.fold("")(_+_) //prefix "this" is simply removed
  }
  delegate for Delegated[StringStream,Example] {
    def apply(from:StringStream) = new Example {
      override def iterator:Iterator[String] = ??? //prefix "this" again simply removed
    }
  }
  delegate for Delegated[StrangeArtifact, Example] {
    def apply(from:StrangeArtifact) = new Example {
      override def iterator:Iterator[String] = from.read //inside "this" needs to be renamed to "from"
    }
  }

The following already works using that rewrite:

  StrangeArtifact.map { string => ???  }
  StrangeArtifact.concat

But this also compiles, which is undesirable:

  def foo(bar:Iterable[String]) = ???
  foo(StrangeArtifact) //should detect trait DoNotImplicitlyConvert

In summary, usage of "this" keyword in prefix position implies that all methods from Iterable[String] would be made accessible as extension methods; without allowing any non- extension method implicit conversions. Delegate syntax would allow multiple types of conversions to be bundled into a single delegate, reducing boilerplate and allowing analogous implementations to be kept in close proximity.

The rules are that only abstract members of Iterable[String]/ect can be implemented via "this" syntax, and all abstract members must be implemented for every "this" type that gets mentioned. The elided type in def (this) concat still refers to the delegate itself which still extends Iterable[String], but now that also makes sense.

Implementing a typeclass would look almost identical, but strictly without using the keyword "this".

(Some important technical considerations: allowing only abstract methods to be overridden would allow inlining of the method call, without creating any Example instance at all. It works precisely because delegate-"this" is being shadowed by parameter-"this", that is a feature not a bug! Since def apply(this:T):Iterator[String] from Conversions is not implemented by the user, it makes the overhead of doing a complete conversion just to implement one method call much more explicit.)

I think my suggestion covers most of the desired functionality, without introducing any additional keywords. And it provides beginners with a smooth transition from simple extension methods, to extending with traditional traits, to implementing typeclasses; all with nearly identical syntax, namely def (x:T) extensionMethod, override def (this:T) interfaceMethod, override def (x:T) typeclassMethod.

[liufengyun]

Ideally, the meaning of indexicals like this should come from its context, always. And ideally, there should be only one way how to derive the meaning for this from a context.

Using this as a formal parameter is weird, it's not in line with how indexicals are used in natural languages nor programming languages, despite the fact that in a formal system we are able to give semantics to it.

I can imagine the this syntax will cause problems in learning and teaching Scala.

The ad-hoc this syntax also breaks the integrity of the syntax: if it can be used as name for method parameter, why not for class parameters?

[odersky]

Ideally, the meaning of indexicals like this should come from its context, always. And ideally, there should be only one way how to derive the meaning for this from a context.

The reasoning resonates with me.

[odersky]

In the end I believe that using this as a parameter name has too many downsides. It forces a "fake" OO thinking where you pretend that this is something which it is not. There's too much magic in that this implicitly opens up the scope of its referent. The magic is leaky because it does not work for extension methods itself. And there is no real gain from doing so. The cleanest approach is to treat the receiver parameter as nothing more than a parameter.

[odersky]

That leaves three plausible candidates for extension methods

• infix, i.e. what we have now
• use this as a modifier (what we used to have)
• use a modifier such as extension on the method.

I.e.

def (x: FlagSet)
    is(y: Flag): Boolean = ...

vs

def is(this x: FlagSet)(y: Flag): Boolean = ...

vs

extension def is(x: FlagSet)(y: Flag): Boolean

I think the infix syntax reads well, and makes obvious what is defined. this as a modifier is considerably more confusing and overloads the meaning of this. extension def is possible but wordy. The one remaining downside with infix syntax is that I can't grep for definitions easily. But on any decent editor or IDE I'd do a "find definition" instead.

[anatoliykmetyuk]

Personally, I like the infix notation most since it conveys the intent in the most concise manner. If we want to make the newline mandatory, I'd add braces:

def (x: FlagSet) {
  is (y: Flag): Boolean = ...
  | (that: FlagSet) = ...
  & (that: FlagSet) = ...
}

I'd also make the braces optional if we want to define only one method. I think there is a lot to gain flexibility wise, so I'd not consider the difficulties grepping as a significant obstacle (most of the people are using IntelliJ anyway).

The extensions modifier also works, essentially conveying the same intent as the infix position. We just move the semantic information from the positioning to the modifier.

I don't like this as a modifier. this has a long history of being an object, just doesn't read well as a modifier.

[milessabin]

Has @anatoliykmetyuk's "grouped" infix syntax been floated before?

def (x: FlagSet) {
  is (y: Flag): Boolean = ...
  | (that: FlagSet) = ...
  & (that: FlagSet) = ...
}

Presumably this defines three extension methods? If we can make this work, I like it a lot.

[sjrd]

I've not seen it before. It's the first one I see that is not completely wrong.

[liufengyun]

Ideally, we should have a syntax that talks explicitly about the type to be extended (something similar to #4085 ):

extension StringOps for (str: String) {
  def * (x: Int): String = e
}

• it's clear that it's an extension for some type
• it will be easy to grep for all extensions in a project
• it's more efficient for implicit search to filter from extension groups than individual extension methods
• only one way of doing things: extension group

The question that remains is how to support extension method in an implicit instance, like the following?

trait Ord[T] {
  def compare(x: T, y: T): Int
  def (x: T) < (y: T) = compare(x, y) < 0
  def (x: T) > (y: T) = compare(x, y) > 0
}

Can something like the following work?

trait Ord[T] {
  def compare(x: T, y: T): Int
}

extension OrdOps[T] for (x: T) given Ord[T] {
  def < (y: T) = the[Ord[T]].compare(x, y) < 0
  def > (y: T) = the[Ord[T]].compare(x, y) > 0
}

Previously, the implicit search rule for extension methods is twisted --- an extension method qualifies if either it is visible in scope, or an object containing the extension method is implicit.

With the new syntax, the rule is more regular: an extension method qualifies if the extension group qualifies. It seems we need a new concept extension group, which might require more engineering efforts.

[anatoliykmetyuk]

I don't think it is necessary to name extension groups, as the containing methods are supposed to be available at the same level where the group is defined. Also, I don't see the motivation for making a new keyword, extension, if def can be used.

It is a good point though that we may want to allow implicits and type params in the group definition site. So maybe:

def [T] (x: T) given Ord[T] {
  < (y: T) = the[Ord[T]].compare(x, y) < 0
  > (y: T) = the[Ord[T]].compare(x, y) > 0
}

a > b  // use right away, no imports needed

Having def before the extension methods in the group is a good solution for grep-friendliness. But I like the idea of dropping def, as it makes for a DRY code. Indeed: if there is supposed to be no other statements at all in groups besides defs, why have def as a modifier before them?

The question that remains is how to support extension method in an implicit instance, like the following?

We can do:

trait Ord[T] {
  def compare(x: T, y: T): Int
  def [T] (x: T) given Ord[T] {
    < (y: T) = the[Ord[T]].compare(x, y) < 0
    > (y: T) = the[Ord[T]].compare(x, y) > 0
  }
}

And have a rule that extension methods (groups) are looked up in the implicit instances available in scope for the type. I recall we had such a rule anyway?

[neko-kai]

How would overrides work with this syntax? One of the big differences wrt extension methods vs. implicit classes is the abiilty to override extension methods.

[anatoliykmetyuk]

Extension methods are supposed to be ordinary methods with an added ability to be called in an OOP manner. In this spirit:

<modifiers> def (x: String) {
  op1 = ...
  op2(other: String) = ...
  op3
}

Should desugar to

<modifiers> def (x: String) op1 = ...
<modifiers> def (x: String) op2(other: String) = ...
<modifiers> def op3

This way, we get override (and all the other possible mods).

[anatoliykmetyuk]

We can also allow the following:

<modifiers> def (x: String) {
  op1 = ...
  <more_mods> op2(other: String) = ...
  op3
}

Desugar to:

<modifiers> def (x: String) op1 = ...
<modifiers> <more_modes> def (x: String) op2(other: String) = ...
<modifiers> def op3

[onekirne]

Personally dislike that mixing of method signatures/types, it makes code harder to read.
So maybe take that idea but push it further in another direction:

def (a:A) _ (b:B):C {
  & = ...
  + = ...
  - = ...
}

then generalize:

def _:A {
  head = ...
  last = ...
}

similar abstract forms:

def (a:A) _ (b:B):C { &,+,- }
def _:A { head, last }

also var a, b, c = 1 as identical to:

var _:Int {
  a, b, c = 1
}

finally defining synonyms:

def (m:M[A]) _ (b:B)(f:(A,B)⇒C):C {
  foldRight, :\ = ...
}