-
Notifications
You must be signed in to change notification settings - Fork 37
Remove dynamic invocation #205
Changes from 8 commits
d0e1317
2fa0d2e
3ce0574
df9e5af
7fd4375
f277c91
7823bb2
dd59642
b362392
1fa085a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,9 @@ class _Empty extends Matcher { | |
const _Empty(); | ||
|
||
@override | ||
bool matches(Object? item, Map matchState) => (item as dynamic).isEmpty; | ||
bool matches(Object? item, Map matchState) => | ||
// ignore: avoid_dynamic_calls | ||
(item as dynamic).isEmpty; | ||
|
||
@override | ||
Description describe(Description description) => description.add('empty'); | ||
|
@@ -27,7 +29,9 @@ class _NotEmpty extends Matcher { | |
const _NotEmpty(); | ||
|
||
@override | ||
bool matches(Object? item, Map matchState) => (item as dynamic).isNotEmpty; | ||
bool matches(Object? item, Map matchState) => | ||
// ignore: avoid_dynamic_calls | ||
(item as dynamic).isNotEmpty; | ||
|
||
@override | ||
Description describe(Description description) => description.add('non-empty'); | ||
|
@@ -144,11 +148,11 @@ class isInstanceOf<T> extends TypeMatcher<T> { | |
/// a wrapper will have to be created. | ||
const Matcher returnsNormally = _ReturnsNormally(); | ||
|
||
class _ReturnsNormally extends FeatureMatcher<Function> { | ||
class _ReturnsNormally extends FeatureMatcher<Function()> { | ||
const _ReturnsNormally(); | ||
|
||
@override | ||
bool typedMatches(Function f, Map matchState) { | ||
bool typedMatches(Function() f, Map matchState) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is likely a behavior change that we don't have tests for... |
||
try { | ||
f(); | ||
return true; | ||
|
@@ -190,6 +194,7 @@ class _HasLength extends Matcher { | |
@override | ||
bool matches(Object? item, Map matchState) { | ||
try { | ||
// ignore: avoid_dynamic_calls | ||
final length = (item as dynamic).length; | ||
return _matcher.matches(length, matchState); | ||
} catch (e) { | ||
|
@@ -205,6 +210,7 @@ class _HasLength extends Matcher { | |
Description describeMismatch(Object? item, Description mismatchDescription, | ||
Map matchState, bool verbose) { | ||
try { | ||
// ignore: avoid_dynamic_calls | ||
final length = (item as dynamic).length; | ||
return mismatchDescription.add('has length of ').addDescriptionOf(length); | ||
} catch (e) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ class Throws extends AsyncMatcher { | |
// function. | ||
@override | ||
dynamic /*FutureOr<String>*/ matchAsync(Object? item) { | ||
if (item is! Function && item is! Future) { | ||
if (item is! Function() && item is! Future) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This stopped accepting values which are a cc @jakemac53 - I have completely overlooked the fact that functions are not subtypes if they have different numbers of generic type arguments... I wonder if there are any other places we have been more strict than we should and are unintentionally blocking generic functions used as callbacks. |
||
return 'was not a Function or Future'; | ||
} | ||
|
||
|
@@ -82,7 +82,7 @@ class Throws extends AsyncMatcher { | |
} | ||
|
||
try { | ||
item as Function; | ||
item as Function(); | ||
var value = item(); | ||
if (value is Future) { | ||
return _matchFuture(value, 'returned a Future that emitted '); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's too bad we don't have a better way to suppress these than the ignore. I filed https://github.com/dart-lang/linter/issues/4806
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be replaced with
if (item is Iterable) return item.isEmpty; else if (item is String) return item.isEmpty;
?That would technically be breaking if someone defined an unknown object with an
isEmpty
getter. But the fact that it currently works is surprising to me. I thought the implementation relied on pattern match, not dynamic invocationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think supporting an
isEmpty
property on an arbitrary user defined class was an intentional feature and I'm not eager to try to remove it. It's not a design we'd carry forward tochecks
.