Language: Do extension methods take priority over methods which are inaccessible because of nullability?

Created on 20 Dec 2019  路  11Comments  路  Source: dart-lang/language

Consider the following example (from @scheglov):

class A {
  int get foo => 0;
}

extension E on A? {
  int get foo => 0;
}

main(A? a) {
  a.foo;
}

Should the invocation of foo resolve to A.foo be an error, because A has a getter named foo but a has a nullable type, or should it resolve to E.foo since foo is not accessible on type A??

Either approach seems consistent.

  • Resolving to the extension method only takes an error case and turns it into a non-error case.
  • On the other hand, one could argue that a user may be trying to call the instance method without realizing that the receiver is nullable, and hence resolving to the extension method may be surprising
  • It does seem potentially useful to be able to define custom null aware versions of instance methods.

@munificent @lrhn @eernstg thoughts?

nnbd

Most helpful comment

I am in agreement with @lrhn here. Unless there are further thoughts, I'd like to propose that we resolve this in favor of saying that for the purposes of extension method resolution, there is no special treatment of nullable types with respect to what members are considered accessible, and hence that in my original example, foo resolves to the extension method E.foo.

All 11 comments

I think it's essentially just a (disguised) variant of

void main() {
  1.isEven;  
  (1 as num).isEven; // prints "extension called"
}

extension on num {
  bool get isEven { print("extension called"); return this is int ? isEven: false; }
}

When the compiler knows the type as "num", it invokes the extension method.
Unfortunately, the extension method causes stack overflow because the compiler is not yet capable of correctly inferring the type, but I think it's a temporary phenomenon. (Another temporary phenomenon is the spurious message about "unnecessary cast" at 1 as num.)

By analogy, the same thing is supposed to happen with nullable types, so the only behavior consistent with the above example would be for a.foo to call an extension method IMO.

Edit Dec 21: I'd like to point out a certain inconsistency in the treatment of dynamic variables WRT extension methods. Currently, if the variable is dynamic, it doesn't match extension on dynamic, and the reasoning leading to this decision was something like this: "if we know the variable just as dynamic, every method can be potentially defined in the actual class, and because we have to prioritize instance methods over the extension methods, no extension will match dynamic".

However, the same reasoning applies to any T (e.g. num in the above example) - indeed, there can be many subclasses of T, and for any xxx, one of the subclasses might have xxx defined. Under this perspective, the justification for the current treatment of dynamic looks less convincing.

The root cause of the problem is the decision to prioritize instance methods over extension methods. I'm not questioning the wisdom of the idea, but I just want to point out that implementing it consistently is difficult, for a simple reason: the compiler generally cannot know what methods are defined in the actual class.

Maybe we need another form of reasoning - e.g. based on the estimate of statistical probabilities of different use cases and the chances of surprising results in each of them.

EDIT: turns out, Kotlin treats extensions exactly like dart does today, and this treatment logically extends to nullable types. Example from Kotlin docs:

fun Any?.toString(): String {
    if (this == null) return "null"
    // after the null check, 'this' is autocast to a non-null type, so the toString() below
    // resolves to the member function of the Any class
    return toString()
}

I free this confusing, and against a principal of Dart that it doesn't over load methods with same name and different signature (arity and / or types of parameters).

A note in passing that for our other unionish type ('FutureOr') we already take the interpretation that the methods of the underlying are not accessible and hence resolve to the extension method. Example:

import 'dart:async';

extension<T> on FutureOr<T> {
  String get isEven => "Hello";
}

void main() {
  FutureOr<int> x = 3;
  print(x.isEven);  // Prints "Hello";
}

Consistency would suggest doing the same for ?.

It could be argued that members of union type operands shouldn't be _accessible_, but they are recognized as such. For instance, a receiver of type A | B would allow for invocations of foo if foo were a member of C where A <: C and B <: C, but if bar is a member of A and not B then an access to bar on a receiver of type A | B is an error, even in the case where there is an applicable extension method.

The rationale would be that it is confusing to allow accesses to a member of one or more of "the types being unioned", because it's so easy to assume that this member is actually common for them all.

In the case with the receiver a of type A? it would then be an error to invoke a.foo because it is an attempt to invoke an instance member of A. It doesn't help that there is an extension getter named foo as well (that is: we refuse to call it, unless it's resolved explicitly as E(a).foo), because it's _more important to prevent the misunderstanding_ that this will invoke the instance member than it is to offer the convenience of being able to invoke the extension member.

@eernstg : here's a potential counter-argument to your reasoning about union types: T? is not just a union type of T and Null, but more than that: T is a subclass of T?. After all, given a function foo(int? x), we can always call it as foo(5), and it's never an error. But if we had genuine union types, then A would (probably) not treated as a subclass of A | B (please correct me if I'm wrong). So, because of this "more intimate" relationship between T? and T, the fact that T? is also a union of T and Null becomes accidental in the context of the discussion, and the case naturally gravitates to this one:

void main() {
  1.isEven;  
  (1 as num).isEven; // prints "extension called"
}

extension on num {
  bool get isEven { print("extension called"); return this is int ? isEven: false; }
}

Now, if you substitute int? for num in this example, it should work similarly because the relationship between num and int is the same as between int? and int. WDYT?

It should resolve to E.foo.

The type A? does not have a foo member. It should be irrelevant that A has one because no-one is invoking anything on the A type here.

I assume this is NNBD code because there is a ? on the type, and in NNBD code A? x; x.foo; is not valid. As such, there is room for E.foo to be called without conflicting with anything.

On the other hand, if you had written a?.foo or a!.foo, then the A member would be invoked.

This indeed allows you to create extensions on the nullable extension of a type:

extension on A? {
   int? get foo => this?.foo;
}

if that is what you want.

The on type of E is not particularly important here, it could be Object? and the answer shouldn't change. Then in the same way FutureOr<A> x; x.foo should also invoke E.foo.
If we generalize to (hypothetical) general union types, the union type should not be considered as having any method that isn't on a common supertype of all the types of the union (if it has any at all).

extension on FutureOr<A> {
  FutureOr<int> get foo => this is Future<A> ? this.then((e) => e.foo) : this.foo;
}

@Tatumizer
If we had general union types, then most likely A would be a subtype of A | B. It would even be a proper subtype if B is not already a subtype of A. The union type A | B is defined as a "least supertype" of A and B meaning that it is a supertype, and if any other type is a supertype of both A and B, then it's a supertype of A | B too.

I am in agreement with @lrhn here. Unless there are further thoughts, I'd like to propose that we resolve this in favor of saying that for the purposes of extension method resolution, there is no special treatment of nullable types with respect to what members are considered accessible, and hence that in my original example, foo resolves to the extension method E.foo.

I agree with both @lrhn and @leafpetersen that the simple and direct reasoning is "receiver type A? does not have an instance method foo, hence a.foo invokes the extension getter". That's consistent with all the existing rules, so we could certainly do that.

When I mentioned that we might still want to make a.foo an error in this situation it was because the distinction is delicate, and it might give rise to some confusion when a.foo invokes the extension getter just because its type is nullable:

class A {
  bool get foo => true;
}

extension E on A? {
  bool get foo => false;
}

void main() {
  A? a;
  if (!a.foo) print("Extension getter said no!");
  a = A();
  if (a.foo) print("Instance getter said yes!");
}

In this case I mentioned the possibility that the first a.foo should be an error, because it's too easy to forget that it can't be the instance member when the receiver is nullable.

The underlying rationale is that instance members and extension members are not designed to blend in with each other, they are designed to be kept separate. So we should nudge developers in the direction of _avoiding name clashes_ between instance and extension members, rather than trying to exploit name clashes to get some sort of overloading relationship between them.

The rule can be generalized: With a receiver x of type A | B, if A has an instance member m which is not inherited from a common superinterface of both A and B, x.m is an error. In particular, we do not exploit the ineligibility of the instance member to allow an expression like x.m() to invoke an extension method.

This generalized form is actually meaningful. For example, with a receiver x of type A | B | ... | K, if all of A .. J all have a foo member inherited from a common superinterface, but K doesn't, then x.foo would be an error. But if we don't make it an error then x.foo can be an extension member invocation.

I envision that it won't be very easy to read or maintain code using union types if it becomes a common technique to write a bunch of extension methods with the same names as instance members, in order to use them on unions for such members m where "at least one of the union operands _doesn't_ have an m".

There is also a non-zero chance that the first a.foo is very much deliberate. At least if the extension method had been written bool get foo => this?.foo ?? false;, because that kind of "null inclusion" for instance methods is a use-case that I have seen for extension methods.

There is also a non-zero chance that the first a.foo is very much deliberate.

The situation could be similar to async functions returning void: There is a delicate balance between "that's cool, I can now do this extra thing" and "that's error prone, get me a lint!". But I think I'll drop my worries and support allowing the extension method invocation. ;-)

I still believe that the dangers that I've described are real (especially for the general case of A | B | ... | K), but we can promote the notion that it is _only_ good design to use this setup _when_ the purpose is to _widen_ the applicability of a specific instance member _to a union_ of types (e.g., to perform "null inclusion" on the instance member, or to perform "non-future inclusion" on a receiver of type FutureOr<T> for some T).

The point is that the instance member and the extension member are carefully synchronized with each other, which means that it is much less important for developers to understand whether any given call site will call the extension member or the instance member: They are guaranteed to have the same meaning and purpose.

Ok, let's resolve this as proposed. For the purposes of extension method resolution, there is no special treatment of nullable types with respect to what members are considered accessible, and hence that in my original example, foo resolves to the extension method E.foo.

Was this page helpful?
0 / 5 - 0 ratings