MockBuilder: Fix a few nullability bugs.
Methods should not be overridden unless they have a non-nullable return type, or at least one non-nullable parameter type. Previously, there was a complicated, pre-non-nullability logic to determine this for methods. This has been fixed to simply ask the type system.
Type variables are now correctly suffixed with a `?` in order to match the source type annotation; the _typeReference logic now handles TypeParameterTypes.
Stop using NullabilitySuffix; the TypeSystem is just a better way to handle question marks. dynamic can be handled separately.
PiperOrigin-RevId: 314792190
diff --git a/lib/src/builder.dart b/lib/src/builder.dart
index ff878f3..99a2a3f 100644
--- a/lib/src/builder.dart
+++ b/lib/src/builder.dart
@@ -14,7 +14,6 @@
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart' as analyzer;
import 'package:analyzer/dart/element/type_system.dart';
import 'package:build/build.dart';
@@ -167,6 +166,12 @@
..url = _typeImport(dartType)
..types.addAll(typeArguments);
}));
+
+ // Only override members of a class declared in a library which uses the
+ // non-nullable type system.
+ if (!sourceLibIsNonNullable) {
+ return;
+ }
for (final field in classToMock.fields) {
if (field.isPrivate || field.isStatic) {
continue;
@@ -191,22 +196,8 @@
});
}
- // TODO(srawlins): Update this logic to correctly handle non-nullable return
- // types. Right now this logic does not seem to be available on DartType.
- bool _returnTypeIsNonNullable(MethodElement method) {
- var type = method.returnType;
- if (type.isDynamic || type.isVoid) return false;
- if (method.isAsynchronous && type.isDartAsyncFuture ||
- type.isDartAsyncFutureOr) {
- var typeArgument = (type as analyzer.InterfaceType).typeArguments.first;
- if (typeArgument.isDynamic || typeArgument.isVoid) {
- // An asynchronous method which returns `Future<void>`, for example,
- // does not need a dummy return value.
- return false;
- }
- }
- return true;
- }
+ bool _returnTypeIsNonNullable(MethodElement method) =>
+ typeSystem.isPotentiallyNonNullable(method.returnType);
// Returns whether [method] has at least one parameter whose type is
// potentially non-nullable.
@@ -220,7 +211,6 @@
// final c1 = C<int?>(); // m's parameter's type is nullable.
// final c2 = C<int>(); // m's parameter's type is non-nullable.
bool _hasNonNullableParameter(MethodElement method) =>
- sourceLibIsNonNullable &&
method.parameters.any((p) => typeSystem.isPotentiallyNonNullable(p.type));
/// Build a method which overrides [method], with all non-nullable
@@ -274,8 +264,7 @@
// TODO(srawlins): Optionally pass a non-null return value to `noSuchMethod`
// which `Mock.noSuchMethod` will simply return, in order to satisfy runtime
// type checks.
- // TODO(srawlins): Handle getter invocations with `Invocation.getter`,
- // and operators???
+ // TODO(srawlins): Handle getter invocations with `Invocation.getter`.
final invocation = refer('Invocation').property('method').call([
refer('#${method.displayName}'),
literalList(invocationPositionalArgs),
@@ -453,17 +442,16 @@
/// * InterfaceTypes (classes, generic classes),
/// * FunctionType parameters (like `void callback(int i)`),
/// * type aliases (typedefs), both new- and old-style,
- /// * enums.
+ /// * enums,
+ /// * type variables.
// TODO(srawlins): Contribute this back to a common location, like
// package:source_gen?
Reference _typeReference(analyzer.DartType type) {
if (type is analyzer.InterfaceType) {
- return TypeReference((TypeReferenceBuilder b) {
+ return TypeReference((b) {
b
..symbol = type.name
- // Using the `nullabilitySuffix` rather than `TypeSystem.isNullable`
- // is more correct for types like `dynamic`.
- ..isNullable = type.nullabilitySuffix == NullabilitySuffix.question
+ ..isNullable = typeSystem.isPotentiallyNullable(type)
..url = _typeImport(type)
..types.addAll(type.typeArguments.map(_typeReference));
});
@@ -483,15 +471,21 @@
}
});
}
- return TypeReference((TypeReferenceBuilder trBuilder) {
+ return TypeReference((b) {
var typedef = element.enclosingElement;
- trBuilder
+ b
..symbol = typedef.name
..url = _typeImport(type);
for (var typeArgument in type.typeArguments) {
- trBuilder.types.add(_typeReference(typeArgument));
+ b.types.add(_typeReference(typeArgument));
}
});
+ } else if (type is analyzer.TypeParameterType) {
+ return TypeReference((b) {
+ b
+ ..symbol = type.name
+ ..isNullable = typeSystem.isNullable(type);
+ });
} else {
return refer(type.displayName, _typeImport(type));
}
diff --git a/test/builder_test.dart b/test/builder_test.dart
index a5d5894..aa79a52 100644
--- a/test/builder_test.dart
+++ b/test/builder_test.dart
@@ -133,7 +133,7 @@
);
});
- test('generates a mock class and overrides methods parameters', () async {
+ test('generates a mock class and overrides method parameters', () async {
await _testWithNonNullable(
{
...annotationsAsset,
@@ -171,7 +171,7 @@
void d(String one, {String two, String three = ""}) => super
.noSuchMethod(Invocation.method(#d, [one], {#two: two, #three: three}));
_i3.Future<void> e(String s) async =>
- super.noSuchMethod(Invocation.method(#e, [s]));
+ super.noSuchMethod(Invocation.method(#e, [s]), Future.value(null));
}
'''),
},
@@ -450,11 +450,11 @@
void j(int? a(), int b());
void k(void a(int? x), void b(int x));
void l<T>(T? a, T b);
+ void m(dynamic a, int b);
}
'''),
},
outputs: {
- // TODO(srawlins): The type of l's first parameter should be `T?`.
'foo|test/foo_test.mocks.dart': dedent(r'''
import 'package:mockito/mockito.dart' as _i1;
import 'package:foo/foo.dart' as _i2;
@@ -474,7 +474,8 @@
super.noSuchMethod(Invocation.method(#j, [a, b]));
void k(void Function(int?) a, void Function(int) b) =>
super.noSuchMethod(Invocation.method(#k, [a, b]));
- void l<T>(T a, T b) => super.noSuchMethod(Invocation.method(#l, [a, b]));
+ void l<T>(T? a, T b) => super.noSuchMethod(Invocation.method(#l, [a, b]));
+ void m(dynamic a, int b) => super.noSuchMethod(Invocation.method(#m, [a, b]));
}
'''),
},
@@ -488,10 +489,12 @@
...simpleTestAsset,
'foo|lib/foo.dart': dedent(r'''
abstract class Foo {
- int f();
- int? g();
- List<int?> h();
- List<int> i();
+ int f(int a);
+ int? g(int a);
+ List<int?> h(int a);
+ List<int> i(int a);
+ j(int a);
+ T? k<T extends int>(int a);
}
'''),
},
@@ -504,10 +507,12 @@
///
/// See the documentation for Mockito's code generation for more information.
class MockFoo extends _i1.Mock implements _i2.Foo {
- int f() => super.noSuchMethod(Invocation.method(#f, []), 0);
- int? g() => super.noSuchMethod(Invocation.method(#g, []), 0);
- List<int?> h() => super.noSuchMethod(Invocation.method(#h, []), []);
- List<int> i() => super.noSuchMethod(Invocation.method(#i, []), []);
+ int f(int a) => super.noSuchMethod(Invocation.method(#f, [a]), 0);
+ int? g(int a) => super.noSuchMethod(Invocation.method(#g, [a]));
+ List<int?> h(int a) => super.noSuchMethod(Invocation.method(#h, [a]), []);
+ List<int> i(int a) => super.noSuchMethod(Invocation.method(#i, [a]), []);
+ dynamic j(int a) => super.noSuchMethod(Invocation.method(#j, [a]));
+ T? k<T extends int>(int a) => super.noSuchMethod(Invocation.method(#k, [a]));
}
'''),
},
@@ -549,9 +554,40 @@
...simpleTestAsset,
'foo|lib/foo.dart': dedent(r'''
class Foo {
- void a(int? m) {}
- void b(dynamic n) {}
- void c(int Function()? o) {}
+ void a(int? p) {}
+ void b(dynamic p) {}
+ void c(var p) {}
+ void d(final p) {}
+ void e(int Function()? p) {}
+ }
+ '''),
+ },
+ outputs: {
+ 'foo|test/foo_test.mocks.dart': dedent(r'''
+ import 'package:mockito/mockito.dart' as _i1;
+ import 'package:foo/foo.dart' as _i2;
+
+ /// A class which mocks [Foo].
+ ///
+ /// See the documentation for Mockito's code generation for more information.
+ class MockFoo extends _i1.Mock implements _i2.Foo {}
+ '''),
+ },
+ );
+ });
+
+ test('does not override methods with a nullable return type', () async {
+ await _testWithNonNullable(
+ {
+ ...annotationsAsset,
+ ...simpleTestAsset,
+ 'foo|lib/foo.dart': dedent(r'''
+ abstract class Foo {
+ void a();
+ b();
+ dynamic c();
+ void d();
+ int? f();
}
'''),
},
@@ -965,7 +1001,7 @@
...simpleTestAsset,
'foo|lib/foo.dart': dedent(r'''
abstract class Foo {
- dynamic f(int a) {}
+ int f(int a);
}
'''),
},
