Strong mode allows noSuchMethod to suppress unimplemented member warnings

[leafpetersen]

This code passes strong mode, but the second call to eatFood will fail at runtime since there is no actual eatFood method on MockCat. It's not clear that we want to just disallow this pattern entirely, but unconditionally disabling the unimplemented member warnings without doing something to make it sound is clearly the wrong thing.

class Cat {
  bool eatFood(String food) => true;
}

class MockCat implements Cat {
  dynamic noSuchMethod(Invocation invocation) {
    return 3;
  }
}

void main () {
  print((new MockCat() as dynamic).eatFood(""));
  print(new MockCat().eatFood(""));
}

[leafpetersen]

We could just continue to accept this, but treat MockCat as dynamic for codegen. Alternatively we could automatically fill in the missing methods with calls that delegate to the noSuchMethod method. @jmesserly points out that the former doesn't work if you want to be able implement a non-dynamic interface, so that would leave the second approach.

A question is whether this should happen automatically, or whether you should get an error unless you explicitly ask for the delegation.

[lrhn]

Personally, I think noSuchMethod is too fragile, and it should be removed and replaced by something else. It's too easy to receive method calls that you are not prepared for, and you have to specify the interfaces anyway, the noSuchMethod methods doesn't help you with your dynamic type.

I would prefer something like Java's dynamicProxy where you create an object to implement specific interfaces, and only the methods of those interfaces are forwarded to the handler.
That sounds like what you are saying about filling in the missing methods and forwarding to noSuchMethod, except that it would make the actual noSuchMethod private and only call it from those added methods.

For strong mode, I do think it would be a reasonable approach to restrict noSuchMethod calls to the unimplemented methods of the actual interfaces of the object. Since noSuchMethod is virtual, you can still override it in subclasses.

[leafpetersen]

Yes, what I was thinking was that you could ask the compiler to fill in the missing methods with stubs of the right types, each of which just forwards to noSuchMethod. This is probably not that hard to implement, and should be essentially backwards compatible. Making the actual noSuchMethod private would be breaking, but we could postpone that change.

[srawlins]

This works for Mockito as well, as it expects to only receive calls to methods that the mocked class actually has. It doesn't need to respond to or behave well for method calls that are totally bogus.

[jmesserly]

The compiler would basically fill in something like this?

class Cat {
  bool eatFood(String food) => true;
}

class MockCat implements Cat {
  // compiler generated
  bool eatFood(String food) {
    return this.noSuchMethod(new _internal.Invocation(#eatFood, [food])) as bool;
  }

  dynamic noSuchMethod(Invocation invocation) {
    return 3;
  }
}

At the moment, we'd have to do this to be sound in DDC for all types with user-defined nSM? Unless it can be restricted somehow (via an annotation?)

(edit: slight changes for clarity)

[leafpetersen]

Yes, that's what I have in mind. Either we do this for all types with user-defined nSM (and keep the existing analyzer warning suppression), or we make it opt-in, and get rid of the warning suppression. For the latter, we would probably have to fix up some existing code, but it probably wouldn't be too hard to just write a tool to mark existing uses of nSM as necessary. If we were to eventually make the nSM private though, then there would be no reason to add a nSM method and not opt-in.

There's an interesting question as to what to do about fields. I suppose we could just have the compiler add the field, but that's probably surprising behavior. I don't immediately see how to do better though, given the strong mode restriction on overriding fields with getter/setters.

[jmesserly]

"implements" fields is okay though, right?

class C {
  int field;
}
// okay in strong mode
class D implements C {
  int get field => 42;
  set field(x) { print('set $x'); }
}
main() {
  new D().field = new D().field;
}

[jmesserly]

Note, DDC supports this now. Abstract members on a class that implements noSuchMethod induces compiler-generated members that forward to noSuchMethod.

[jmesserly]

(marking as language issue as I don't think we need any code changes in Analyzer/strong mode, until we have a language decision)

[leafpetersen]

Closing this, as the issue is resolved.