Emit warning when overriding method changes parameter names?

Please consider the following code:

trait A {
  def foo(i: Int, j: String): Unit
}

class B extends A {
  def foo(i0: Int, j0: String): Unit = ()
}

the implementing class B has extended A so it must implement foo, but the parameter names are not the same as the parent (i -> i0, j -> j0). The result of this is that this now yields an error:

val b: B = new B
b.foo(i = 1, k = "") // ERROR!
b.foo(i0 = 1, k0 = "") // ok

In order to call the method with the original parameter names, B's must be upcast to A:

val b: B = new B
(b: A).foo(i = 1, k = "") // ok

I would think that in virtually every instance where an extending method changes the parameter names in an overriding method, this is done by accident, and probably most Scala users have never thought about happens when you change a parameter name in an override, and would have to test it out to see what happens (I know I did!), but when it does happen, it is impossible to correct without breaking source compatibility.

So I would propose that code like the above should emit a warning. Though, in some cases like for example when a method is overriding two super methods, it would be up for debate whether a warning should emitted, because the two super methods may disagree in parameter names:

trait A {
  def foo(i: Int): Unit
}
trait B {
  def foo(j: Int): Unit
}

class C extends A with B {
  // up for debate if parameter name should be prescribed as `j` 
  // or if no warning is emitted
  def foo(ij: Int): Unit 
}

Precedence in Kotlin

Kotlin, having named parameters, does also emit warnings in this situation:

interface A {
    fun foo(i: Int): Unit
}
class B: A {
    override fun foo(j: Int) {}
    //Warning:(86, 22) Kotlin: The corresponding parameter in the supertype 'A' is named 'i'. 
    //This may cause problems when calling this function with named arguments.
}

Example violations in the standard library:

Just to show that this corner case has caused problems, here are instances in the stdlib where parameters have been (probably accidentally) misaligned with their parent. Here’s a couple:


  HashSet(1).contains(element = 0)
  (HashSet(1): Set[Int]).contains(elem = 0)

  // list.filter is actually a double whammy!
  // List.filter takes p, which overrides 
  // collection.Iterable.filter which takes pred, which overrides
  // collection.IterableOnceOps.filter, which takes p again
  List(1).filter(p = _ > 0)
  (List(1): collection.Iterable[Int]).filter(pred = _ > 0)
  (List(1): collection.IterableOnceOps[Int, collection.Iterable, collection.Iterable[Int]]).filter(p = _ > 0)
11 Likes

For concrete types this makes a lot of sense. But for abstract ones, I feel like changing the name is useful in some situations.

For example, when implementing type classes for custom types, I like to replace names like fa with list, future, etc.
Also, when using type members.

So, what about introducing also a way to signal that for some methods the names could be changed?

1 Like

Some may disagree but I think that this is really a very very minor use-case, not really worth complecting the language for.

1 Like

It really depends on the code you work on.

I end up implementing typeclass instances on a fairly regular basis, so having to ensure the names match exactly would be a minor hassle.

For whatever it’s worth, despite this issue being something of an infrequent corner-case, it seems like a common-sense suggestion

2 Likes

There’s an informal convention in Scala that parameter names that are not recognizable words can be changed freely. In particular, single letter names such as i or i0. It’s generally too much to require that these are kept stable across versions.

3 Likes

Alongside @deprecatedOverriding and @deprecatedName, it seems quite natural to add @deprecatedOverridingName.

Noting that “impossible to correct without breaking source compatibility” means you use (@deprecatedName("i") i0: Int) to permit both names, whichever is preferred.

I am reminded that you can omit the name, which means omit the pleonastic name. I am sure someone suggested pleonasm as the behavior at issue when I added the feature, as I am sure that no one has ever exploited it.

scala> def f(@deprecatedName i: Int) = i * 2
f: (i: Int)Int

scala> f(i = 42)
           ^
       warning: naming parameter i is deprecated.
res0: Int = 84

One use case was: Don’t name the parameter, its name may change.

The other use case was: I couldn’t think of a good name for this parameter, so don’t name it.

Not an override, but a counter-example by analogy is constructor parameters, where it’s desirable to avoid aliasing.

class C(var theC: Int)
class D(c0: Int) extends C(c0)

as opposed to

class C(var c: Int)
class D(c: Int) extends C(c) { def f = c }

which warns.

TIL there’s a conventional reason names like foldFun were deprecated in favor of op. To avoid lock-in!

Glancing at Iterator, I wonder if size, len, etc are locked-in, or are considered aliases for n, or elem for x.

withPartial takes a boolean x which I am required by style mores to name:

Iterator(1,2,3).grouped(2).withPartial(x = true)

I would have avoided that conundrum by replacing withPartial with partially and adding its complement, impartially.

I don’t know why they say naming is hard, I think it’s great fun.

1 Like

It would be really nice to have this as an opt-in compiler feature (flag), but I also don’t mind if a linter introduces this feature instead.

In any case, it’s important to keep the option to make exceptions for cases where it is intended to have a different parameter name (like @BalmungSan mentioned); probably via some annotation.

2 Likes