Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Alternatives for code generation with NNBD #286

Open
pedromassango opened this issue Oct 11, 2020 · 13 comments
Open

[proposal] Alternatives for code generation with NNBD #286

pedromassango opened this issue Oct 11, 2020 · 13 comments

Comments

@pedromassango
Copy link

Based on NULL_SAFETY_README.md looks like Mockito will be using code gen solution due to limitations introduced by NNBD explained in the same file. I'm creating this issue to discuss some alternative solutions since code_gen is clearly a big step back for Mockito.

Proposal

The file metioned above says that the problem is that Mockito need a default value that can be used for every parameter and with NNBD we can't use null because some parameters might be non-nullable. I think one option would be to make everything nullable and leave the developer make sure the API is used properly.

Note: share your alternative solutions in the comments.

@srawlins
Copy link
Member

I think one option would be to make everything nullable and leave the developer make sure the API is used properly.

The developer is free to do this. Code generation is not required when using Mockito under null safety. If the developer wishes to mock or verify methods with non-nullable parameters or return types, then code generation is one option. Manual mock implementation is another.

@pedromassango
Copy link
Author

The problem is that mocking manually requires a lot of boilerplate code.

Argument matchers

Mockito's helpful argument matchers like any, argThat, captureAny, etc. all return null. In the code above, null is being passed to start. null is used because, before null safety, it is the only value that is legally assignable to any type. Under null safety, however, it is illegal to pass null where a non-nullable int is expected.

What if we create alternative typed getters/methods that does not allow nullable values? We can even have it as extensions of the existing matchers.

Here in the Sample 1 bellow I've created a getter to allow me to pass in any argument of type String, we can build one for each Dart's data type:

  • String get anyString => any as String;
    • We can use this in non-nullable arguments of type String.
Sample 1
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';

enum LoginResult { success, invalidEmail, invalidPwd }

class MockAPIService extends Mock implements APIService {}

class APIService {
  LoginResult login(String email, String pwd) {
    return email.isEmpty
        ? LoginResult.invalidEmail
        : pwd.isEmpty ? LoginResult.invalidPwd
        : LoginResult.success;
  }
}

void main() {
  test('RegEx matcher', () {
    final data = [
      '[email protected]',
      '[email protected]',
      '[email protected]'
    ];
    final pattern = r'(\w+)(?:@)(\w+)';

    data.forEach((element) {
      final r = RegExp(pattern);
      r.allMatches(element).forEach((resultItem) {
        final username = resultItem.group(0);
        final emailDomain = resultItem.group(1);
        //final emailDomain2 = resultItem.group(2);
        print('$username : $emailDomain Cc: ${resultItem.groupCount}');
      });
    });
  });

  test('NNBD', () {
    var server = MockAPIService();
    when(server.login(anyString, '')).thenReturn(LoginResult.invalidPwd);

    final result = server.login('[email protected]', '');
    expect(result, LoginResult.invalidPwd);
  });
}

String get anyString => any as String;

Sample 2

Here I just did the same one getter for integers and one function for any type T:

  • int get anyInt => any as int;
  • T anything<T>() => any as T;
    • We can pass this as a non-nullable argument for parameters of type T.
Sample 2
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';

class MockHttpServer extends Mock implements HttpServer {}
class HttpServer {
  Uri start(int port)  {
    return Uri(port: port);
  }

  Uri doNothing(Uri uri)  {
    return uri;
  }
}


void main() {
  test('RegEx matcher', () {
    final data = [
      '[email protected]',
      '[email protected]',
      '[email protected]'
    ];
    final pattern = r'(\w+)(?:@)(\w+)';

    data.forEach((element) {
      final r = RegExp(pattern);
      r.allMatches(element).forEach((resultItem) {
        final username = resultItem.group(0);
        final emailDomain = resultItem.group(1);
        //final emailDomain2 = resultItem.group(2);
        print('$username : $emailDomain Cc: ${resultItem.groupCount}');
      });
    });
  });

  test('NNBD', () {
    var server = MockHttpServer();
    var uri = Uri.parse('http://localhost:8080');
    // server.start
    when(server.start(anyInt)).thenReturn(uri);
    final result1 = server.start(0000);
    expect(result1.port, 8080);
    // server.doNothing
    when(server.doNothing(anything())).thenReturn(Uri(port: 9999));
    final result2 = server.doNothing(Uri());
    expect(result2.port, 9999);
  });
}

int get anyInt => any as int;
T anything<T>() => any as T;

@srawlins is there any problem with the suggesttions above?

@srawlins
Copy link
Member

These examples result in runtime errors:

String get anyString => any as String;

The cast (as) will fail, as any returns null (not a non-nullable String).

@pedromassango
Copy link
Author

pedromassango commented Oct 12, 2020

These examples result in runtime errors:

String get anyString => any as String;

The cast (as) will fail, as any returns null (not a non-nullable String).

Have you tried it? I have NNBD enabled and the tests pass!

Edit:
Without this the only problem I faced was from the analyzer saying "can't pass Null into String".

@srawlins
Copy link
Member

If I write this file:

void main() {
  String s = anyString;
}

String get anyString => null as String;

into a package with this SDK constraint:

environment:
  sdk: '>=2.10.0 <3.0.0'

and run the script with the non-nullable experiment:

$ dart --enable-experiment=non-nullable a.dart

then I get the following error:

Unhandled exception:
type 'Null' is not a subtype of type 'String' in type cast
#0      anyString (file:///Users/srawlins/code/dart-markdown/a.dart:5:30)
#1      main (file:///Users/srawlins/code/dart-markdown/a.dart:2:14)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:301:19)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

@maks
Copy link

maks commented Oct 12, 2020

Pretty much same thing on NS Dartpad too...

Screenshot 2020-10-13 at 08 27 52

@pedromassango
Copy link
Author

Hm, looks live I've just enabled NNBD for the analyzer and not actually running the code with it. Just tried on Dartpad and clearly don’t work.

@letsar
Copy link

letsar commented Oct 15, 2020

This is maybe a silly question, but since Never will be the new bottom type, why cannot your use Never instead of Null in these cases?

@srawlins
Copy link
Member

One reason is that you cannot instantiate Never. Mockito would still need a placeholder object, a replacement for null which represents Never.

I think there are other reasons, involving how Never works. Like if Mock's noSuchMethod returned Never...

@pedromassango
Copy link
Author

@srawlins
It is possible to overcome this by using reflections? Just asking since I don't have knowledge in this field.

@srawlins
Copy link
Member

Hmm good question. Dart's runtime-reflection library is called dart:mirrors. The question is how to preserve the current API (when(obj.m(any))) and support methods with non-nullable parameters (and return types).

You could conceive of any API where any is a generic function like T any<T>() {...}, so you would write: when(obj.m(any<Foo>())) and the any function would maybe create a ClassMirror of T, and create a new instance of it, and return that. Perhaps similarly for the return type. I'm not certain you actually can do this, but it sounds like it might lead to something.

I see two problems with reflection though:

  • This requires an API change, converting all of the current argument matchers (any, anyNamed, argThat, etc) to something new. We're not currently looking to pay the price of changing the API.
  • Runtime reflection with dart:mirrors is very slow and creates huge binaries. As in, unacceptably slow and huge.

@knaeckeKami
Copy link

for anyone looking into reflection, just a heads up, so you don't waste your time:
dart-lang/sdk#44310 (comment)

@felangel
Copy link

felangel commented Jan 5, 2021

fwiw I’ve been messing around with an alternative mocking API to address some of these issues at https://pub.dev/packages/mocktail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants