dotty-feature-requests icon indicating copy to clipboard operation
dotty-feature-requests copied to clipboard

Scala Wart: Extraneous extension methods on Any

Open lihaoyi opened this issue 7 years ago • 9 comments

Opening this issue, as suggested by Martin, to provide a place to discuss the individual warts brought up in the blog post Warts of the Scala Programming Language and the possibility of mitigating/fixing them in Dotty (and perhaps later in Scala 2.x). These are based on Scala 2.x behavior, which I understand Dotty follows closely, apologies in advance if it has already been fixed


Scala adds a bunch of extension methods on every value in your codebase:

@ 1.ensuring(_ > 2)
java.lang.AssertionError: assertion failed
  scala.Predef$.assert(Predef.scala:204)
  scala.Predef$Ensuring$.ensuring$extension2(Predef.scala:316)
  $sess.cmd25$.<init>(cmd25.sc:1)
  $sess.cmd25$.<clinit>(cmd25.sc:-1)

@ 1.formatted("hello %s")
res26: String = "hello 1"

@ 1.synchronized(println("yo"))
yo

It really shouldn't. In my experience, there extension methods are rarely ever used. If someone wants to use them, they can import the functions themselves or write their own extensions. It doesn't make any sense to pollute the method-namespace of every value in existance to add some unused functionality. Perhaps ## could also be replaced by a library function ##(...) or hash(...) or similar.

Apart from these probably-uncontroversial nobody-will-care-if-we-remove-them extension methods, there is of course the + operator added by any2StringAdd, but that one is probably significant enough to warrant it's own discussion separate from these miscellaneous bits and pieces

There are also all the java.lang.Object methods: finalize, notify, wait, etc.. These could in theory be removed from Scala's Anys and shifted into library functions as well, since they are infrequently used (and don't work at all in Scala.js). That however could be a different discussion from removing ensuring an formatted (and ##)

PostScript: as @danarmak and @DarkDimius have noted below, the various java.lang.Object methods do not exist on Any, unless you have your own stray implicit conversions lying around (e.g. an implicit class that doesn't extend AnyVal)

lihaoyi avatar May 29 '17 14:05 lihaoyi

There are also all the java.lang.Object methods: finalize, notify, wait, etc.

These are not present on Any, only on AnyRef. They are only available on builtin value types like Int due to implicit conversion to java.lang.Integer, which is provided by implicit methods in Predef (int2Integer, etc).

danarmak avatar May 31 '17 16:05 danarmak

@danarmak even on a bare Any it seems I can call those fellas:

@ val x: Any = null
x: Any = null
@ x.notify()
java.lang.IllegalMonitorStateException
  java.lang.Object.notify(Native Method)
  $sess.cmd1$.<init>(cmd1.sc:1)
  $sess.cmd1$.<clinit>(cmd1.sc:-1)

It seems to automatically delegate to java.lang.Object#notify. Not sure if it's an implicit conversion somewhere or compiler magic that does that, but happens on bar Anys too and not just on specific subclasses like Int and friends

lihaoyi avatar May 31 '17 16:05 lihaoyi

@lihaoyi

Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_45).
Type in expressions for evaluation. Or try :help.

scala> class AA { def foo(a: Any) = a.notify()}
<console>:11: error: value notify is not a member of Any
       class AA { def foo(a: Any) = a.notify()}
                                      ^

You have some strange implicits in your scope

DarkDimius avatar May 31 '17 16:05 DarkDimius

Dotty agrees with Scala in this regard:

Starting dotty REPL...
Welcome to Scala.next (pre-alpha, git-hash: adf6962)  (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_45).
Type in expressions to have them evaluated.
Type :help for more information.
scala> class AA { def foo(a: Any) = a.notify()}
-- [E008] Member Not Found Error: <console>:4:31 -------------------------------
4 |class AA { def foo(a: Any) = a.notify()}
  |                             ^^^^^^^^
  |                             value `notify` is not a member of Any(a)
scala>

DarkDimius avatar May 31 '17 16:05 DarkDimius

@DarkDimius huh you are right... it doesn't work in the Scala REPL. Ammonite must be doing something funny

lihaoyi avatar May 31 '17 16:05 lihaoyi

I am kind of surprised that ensuring (alongside with require) are not used more often. People use assert, so why not ensuring? It's arguably just as useful.

I definitely think that formatted should be moved out, along with String +. The main issue is that it's tedious to manage the migration. Maybe scalafix can help (that seems to be our magic wand for everything these days :-).

synchronized is technically not an extension method; it's treated by the Scala compiler as a regular method on Object. That's why you can write

def foo() = synchronized { ... }

Ideally, synchronized would only exist on a special trait, like Monitor. The problem with that is that it would invalidate a lot of concurrent code (where e.g. you do synchronized on an array) and there's no obvious way to fix such code.

odersky avatar Jun 19 '17 15:06 odersky

I am kind of surprised that ensuring (alongside with require) are not used more often. People use assert, so why not ensuring? It's arguably just as useful.

I'm not so much complaining about it's existence, just it's namespace pollution as an extension method on all expressions. If it was a function scala.Predef.ensuring, I think that would be a big improvement. And it would better match up with assert, which after all is scala.Predef.assert, not some extension method on Any that takes a lambda. (I wouldn't mind if assert took a value and a lambda too, but that's a separate discussion)

I definitely think that formatted should be moved out, along with String +. The main issue is that it's tedious to manage the migration. Maybe scalafix can help (that seems to be our magic wand for everything these days :-).

If that's the only blocker, then that's great! Presumably there are lots of other backwards-incompatible changes in dotty that will need to be fixed up, so either this'll get solved together with them, or this whole Dotty thing is dead on arrival anyway 😛

Ideally, synchronized would only exist on a special trait, like Monitor. The problem with that is that it would invalidate a lot of concurrent code (where e.g. you do synchronized on an array) and there's no obvious way to fix such code.

Could a stopgap measure be to move synchronized into a intrinsic/magic predef function, so you had to call e.g.

scala.concurrent.synchronized(myArray){ 
  ... 
}

And have it turn into the bytecode that gets generated now? Maybe it's a bit magic, but not any more magic than it is now. At least as a static method it's no longer polluting the namespace of all objects' methods, and you only get synchronized polluting your local namespace if you import scala.concurrent.synchronized or something

Again, I don't so much care about the underlying data-model of "synchronized being available on anything" (Living on the JVM, I've made peace with the "everyone and their dog has an inbuilt mutex") I'd just like to avoid namespace pollution, and the method namespace on Any is very precious real estate...

lihaoyi avatar Jun 19 '17 15:06 lihaoyi

But it's much nicer to write

def square(x: Double) = x * x ensuring (_ >= 0)

than

def square(x: Double) = ensuring(x * x)(_ >= 0)

The second syntax is so awkward that it would surely put people off. [In fact, people who do run-time or compile-time contracts are very fond of ensuring, it's just that the community at large seems not to know much about the feature, and how it is used].

Regarding, synchronized, actually, synchronized is on Object not Any, but that's almost as bad. I would not want to force every program to replace

synchronized { ... }

with

synchronized(this) { ... }

On the other hand, it would be possible to move synchronized into a decorator. That way we could demand an import to use it, which would be altogether a good thing.

odersky avatar Jun 19 '17 16:06 odersky

The second syntax is so awkward that it would surely put people off. [In fact, people who do run-time or compile-time contracts are very fond of ensuring, it's just that the community at large seems not to know much about the feature, and how it is used].

We could preserve the original syntax as an extension method, and move it behind an import scala.Predef.EnsuringSyntax._ import. "A method that some sub-group in the community uses heavily and other people much less" seems like the archetypical candidate for an optional extension-method.

I don't care much about changing the syntax, I just want it to be optional so it's not polluting everyone's namespaces by default

On the other hand, it would be possible to move synchronized into a decorator. That way we could demand an import to use it, which would be altogether a good thing.

That would be good enough for me; I don't care as much about changing the syntax as making it optional.

On the other hand, If I understand "decorator" to mean implicit-class-extension-method, that won't work calling it without an explicit this e.g. this.synchronized. At least not in Scala 2:

scala> implicit class FooOps(t: Any){def bar = 1}
defined class FooOps

scala> bar
<console>:13: error: not found: value bar
       bar
       ^

scala> this.bar
res2: Int = 1

If you really-truly wanted to preserve the syntax, using a user-land implementation, you could use a (trivial) macro that provides an implicit Caller[T] and take it as an implicit parameter, as described here. However, that would make the explicit syntax more awkward scala.concurrent.synchronized{...}(target).

Or, since synchronized is already a compiler intrinsic (I think?) you could also have it not take the implicit Caller[T], have both free-function and extension-method versions imported from some namespace to make the typechecker happy, and just have the following compiler phases special case it to do the right thing automagically.

lihaoyi avatar Jun 25 '17 01:06 lihaoyi