Lint concrete class with deceptive member signature caused by inherited covariant parameter

[eernstg]

Consider the following example:

class A {
  void m(covariant int i) {}
}

abstract class I {
  void m(num n);
}

class B extends A implements I {
  // Has member signature `void m(covariant num)`.
}

void main() {
  B().m(3.1); // OK statically, but throws at run time.
}

This situation is unfortunate, because it is known statically that an instance of C has the member signature void m(covariant int); but because the method implementation is inherited and a superinterface says otherwise, the static analysis reports the member signature void m(covariant num).

This means that we will have a run-time type error, and it is known statically, but there is no indication that there is a problem at compile-time.

As a matter of style, we could eliminate this problem by always declaring the actual member signature of the inherited method in the case where there is a covariant parameter whose type is a subtype of the one in the class interface:

// ...

class B extends A implements I {
  void m(covariant int i);
}

void main() {
  B().m(3.1); // A compile-time error, as it should be.
}

The situation may not arise very often, but this is just one more reason why developers shouldn't have to think about it, when they could have it flagged by a lint:

LINT declare_covariant_parameter_type: A parameter p in the interface of a concrete class C is covariant-by-declaration and has type T, but the implementation in an instance of C of the enclosing method is inherited, and the corresponding parameter has type S such that S <: T and S != T.

The lint advice would be to add an abstract declaration with the same signature as the inherited method. There could be a quick fix.

[bwilkerson]

My first thought was that this seemed like a check that should be enabled by default, and should potentially even be made an error in the language spec., but then I realized that it doesn't lead to a runtime error if the arguments passed to m are always assignable to the type of i (int in this case), so technically that would be a false positive in some situations.

But if this construct is, then it seems like the better place to catch the error might be at the call site, with an indication that the type of the argument isn't compatible with the type of the parameter in the concrete implementation of the method.

That leads me right back to the question of making it an error in the language spec. We statically know what the signature of the concrete method inherited by B actually is, so we know that any invocation of m on an instance of B has to conform to that signature. So, in a situation in which the inherited implementation (A.m) is narrower than the signature in the interface (of B), should we specify that invocation sites need to conform to the inherited implementation (or more accurately the signature that would be required if we were to introduce an abstract method in B, though in this case the addition of covariant doesn't impact the type checking at the call site)? I think that would also catch this situation and wouldn't have any false positives, but I'm probably missing some subtle case.

[eernstg]

should potentially even be made an error in the language spec

I proposed in the language team a long time ago that in this case we should compute the member signature for m in the interface of the class B based on A.m, which would have exactly the same effect as specifying the abstract B.m (where all names refer to the example here).

However, considering that proposal a bit more closely, it is an anomaly in that it uses a different algorithm than the one used to compute the member signature for any other member that isn't implemented in the given class (B in the example), and that applies to unimplemented methods as well as methods where an implementation is inherited.

For instance, if some other parameter has type dynamic in a superinterface, but type Object? in the inherited method implementation, developers would be justified in being highly surprised that they can't trust the "nearest" declaration to determine the type of that other parameter, just because this is a concrete class and that method has a covariant-by-decl parameter:

class A2 {
  void m(covariant int i, Object? o) {}
}

class B2 extends A2 {
  void m(num n, dynamic d);
}

class C2 extends B2 {
  // Has member signature `void m(covariant num, dynamic)`,
  // and `void m(covariant int, Object?)` would be a surprise!
}

We would then dive down into more specialized rules like "we only take the type of the parameters that are covariant-by-declaration, and only the ones whose type is a proper subtype of the one in the "normal" member signature", etc. ad nauseam ;-).

So I think it's a better strategy to simply make developers aware of this situation (presumably it's quite rare), and then leave it to them to explicitly write an abstract declaration, manually choosing a signature that resolves those conflicts in a way that fits the situation.