Skip to content

Commit

Permalink
Enable avoid_dynamic_calls lint (#2410)
Browse files Browse the repository at this point in the history
Add argument types to `typedMatches` arguments which were unnecessarily
widening it back to `dynamic`. 

Add some missing `as dynamic` and `as Function` which is the pattern the
lint uses to allow intentional dynamic calls.

Ignore some dynamic calls in a test for a legacy API.

Ignore a couple false positives for calling methods (as opposed to
getters) on a cast expression. The false positive was fixed in
https://dart-review.googlesource.com/c/sdk/+/390640
but not yet published.

Remove the deprecated lint `package_api_docs`.

Replaces dart-archive/matcher#205
  • Loading branch information
natebosch authored Oct 29, 2024
1 parent 9a267de commit 42495c2
Show file tree
Hide file tree
Showing 14 changed files with 34 additions and 28 deletions.
4 changes: 4 additions & 0 deletions pkgs/matcher/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.12.18-wip

* Remove some dynamic invocations.

## 0.12.17

* Require Dart 3.4
Expand Down
1 change: 1 addition & 0 deletions pkgs/matcher/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ include: package:lints/recommended.yaml
linter:
rules:
- always_declare_return_types
- avoid_dynamic_calls
- avoid_private_typedef_functions
- avoid_unused_constructor_parameters
- cancel_subscriptions
Expand Down
3 changes: 2 additions & 1 deletion pkgs/matcher/lib/src/core_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ class _ReturnsNormally extends FeatureMatcher<Function> {
@override
bool typedMatches(Function f, Map matchState) {
try {
f();
// ignore: unnecessary_cast
(f as Function)();
return true;
} catch (e, s) {
addStateInfo(matchState, {'exception': e, 'stack': s});
Expand Down
3 changes: 1 addition & 2 deletions pkgs/matcher/lib/src/expect/throws_matcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ class Throws extends AsyncMatcher {
}

try {
item as Function;
var value = item();
var value = (item as Function)();
if (value is Future) {
return _matchFuture(value, 'returned a Future that emitted ');
}
Expand Down
2 changes: 1 addition & 1 deletion pkgs/matcher/lib/src/iterable_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class _UnorderedMatches extends _IterableMatcher {
.add(' unordered');

@override
Description describeTypedMismatch(dynamic item,
Description describeTypedMismatch(Iterable item,
Description mismatchDescription, Map matchState, bool verbose) =>
mismatchDescription.add(_test(item.toList())!);

Expand Down
19 changes: 9 additions & 10 deletions pkgs/matcher/lib/src/map_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'feature_matcher.dart';
import 'interfaces.dart';
import 'util.dart';

/// Returns a matcher which matches maps containing the given [value].
Matcher containsValue(Object? value) => _ContainsValue(value);

class _ContainsValue extends Matcher {
class _ContainsValue extends FeatureMatcher<Map> {
final Object? _value;

const _ContainsValue(this._value);

@override
bool matches(Object? item, Map matchState) =>
(item as dynamic).containsValue(_value);
bool typedMatches(Map item, Map matchState) => item.containsValue(_value);
@override
Description describe(Description description) =>
description.add('contains value ').addDescriptionOf(_value);
Expand All @@ -26,16 +26,15 @@ class _ContainsValue extends Matcher {
Matcher containsPair(Object? key, Object? valueOrMatcher) =>
_ContainsMapping(key, wrapMatcher(valueOrMatcher));

class _ContainsMapping extends Matcher {
class _ContainsMapping extends FeatureMatcher<Map> {
final Object? _key;
final Matcher _valueMatcher;

const _ContainsMapping(this._key, this._valueMatcher);

@override
bool matches(Object? item, Map matchState) =>
(item as dynamic).containsKey(_key) &&
_valueMatcher.matches(item[_key], matchState);
bool typedMatches(Map item, Map matchState) =>
item.containsKey(_key) && _valueMatcher.matches(item[_key], matchState);

@override
Description describe(Description description) {
Expand All @@ -47,9 +46,9 @@ class _ContainsMapping extends Matcher {
}

@override
Description describeMismatch(Object? item, Description mismatchDescription,
Map matchState, bool verbose) {
if (!(item as dynamic).containsKey(_key)) {
Description describeTypedMismatch(
Map item, Description mismatchDescription, Map matchState, bool verbose) {
if (!item.containsKey(_key)) {
return mismatchDescription
.add(" doesn't contain key ")
.addDescriptionOf(_key);
Expand Down
8 changes: 4 additions & 4 deletions pkgs/matcher/lib/src/numeric_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class _IsCloseTo extends FeatureMatcher<num> {
const _IsCloseTo(this._value, this._delta);

@override
bool typedMatches(dynamic item, Map matchState) {
bool typedMatches(num item, Map matchState) {
var diff = item - _value;
if (diff < 0) diff = -diff;
return diff <= _delta;
Expand All @@ -32,8 +32,8 @@ class _IsCloseTo extends FeatureMatcher<num> {
.addDescriptionOf(_value);

@override
Description describeTypedMismatch(dynamic item,
Description mismatchDescription, Map matchState, bool verbose) {
Description describeTypedMismatch(
num item, Description mismatchDescription, Map matchState, bool verbose) {
var diff = item - _value;
if (diff < 0) diff = -diff;
return mismatchDescription.add(' differs by ').addDescriptionOf(diff);
Expand Down Expand Up @@ -67,7 +67,7 @@ class _InRange extends FeatureMatcher<num> {
this._low, this._high, this._lowMatchValue, this._highMatchValue);

@override
bool typedMatches(dynamic value, Map matchState) {
bool typedMatches(num value, Map matchState) {
if (value < _low || value > _high) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion pkgs/matcher/lib/src/operator_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class _AllOf extends Matcher {
@override
Description describeMismatch(dynamic item, Description mismatchDescription,
Map matchState, bool verbose) {
var matcher = matchState['matcher'];
var matcher = matchState['matcher'] as Matcher;
matcher.describeMismatch(
item, mismatchDescription, matchState['state'], verbose);
return mismatchDescription;
Expand Down
2 changes: 1 addition & 1 deletion pkgs/matcher/lib/src/order_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class _OrderingMatcher extends Matcher {
return _equalValue;
} else if ((item as dynamic) < _value) {
return _lessThanValue;
} else if (item > _value) {
} else if ((item as dynamic) > _value) {
return _greaterThanValue;
} else {
return false;
Expand Down
6 changes: 3 additions & 3 deletions pkgs/matcher/lib/src/string_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class _StringStartsWith extends FeatureMatcher<String> {
const _StringStartsWith(this._prefix);

@override
bool typedMatches(dynamic item, Map matchState) => item.startsWith(_prefix);
bool typedMatches(String item, Map matchState) => item.startsWith(_prefix);

@override
Description describe(Description description) =>
Expand All @@ -97,7 +97,7 @@ class _StringEndsWith extends FeatureMatcher<String> {
const _StringEndsWith(this._suffix);

@override
bool typedMatches(dynamic item, Map matchState) => item.endsWith(_suffix);
bool typedMatches(String item, Map matchState) => item.endsWith(_suffix);

@override
Description describe(Description description) =>
Expand All @@ -119,7 +119,7 @@ class _StringContainsInOrder extends FeatureMatcher<String> {
const _StringContainsInOrder(this._substrings);

@override
bool typedMatches(dynamic item, Map matchState) {
bool typedMatches(String item, Map matchState) {
var fromIndex = 0;
for (var s in _substrings) {
var index = item.indexOf(s, fromIndex);
Expand Down
2 changes: 1 addition & 1 deletion pkgs/matcher/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: matcher
version: 0.12.17
version: 0.12.18-wip
description: >-
Support for specifying test expectations via an extensible Matcher class.
Also includes a number of built-in Matcher implementations for common cases.
Expand Down
8 changes: 4 additions & 4 deletions pkgs/matcher/test/expect_async_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ void main() {
test('works with no arguments', () async {
var callbackRun = false;
var monitor = await TestCaseMonitor.run(() {
// ignore: deprecated_member_use_from_same_package
// ignore: deprecated_member_use_from_same_package, avoid_dynamic_calls
expectAsync(() {
callbackRun = true;
})();
Expand All @@ -349,7 +349,7 @@ void main() {
test('works with dynamic arguments', () async {
var callbackRun = false;
var monitor = await TestCaseMonitor.run(() {
// ignore: deprecated_member_use_from_same_package
// ignore: deprecated_member_use_from_same_package, avoid_dynamic_calls
expectAsync((arg1, arg2) {
callbackRun = true;
})(1, 2);
Expand All @@ -362,7 +362,7 @@ void main() {
test('works with non-nullable arguments', () async {
var callbackRun = false;
var monitor = await TestCaseMonitor.run(() {
// ignore: deprecated_member_use_from_same_package
// ignore: deprecated_member_use_from_same_package, avoid_dynamic_calls
expectAsync((int arg1, int arg2) {
callbackRun = true;
})(1, 2);
Expand All @@ -375,7 +375,7 @@ void main() {
test('works with 6 arguments', () async {
var callbackRun = false;
var monitor = await TestCaseMonitor.run(() {
// ignore: deprecated_member_use_from_same_package
// ignore: deprecated_member_use_from_same_package, avoid_dynamic_calls
expectAsync((arg1, arg2, arg3, arg4, arg5, arg6) {
callbackRun = true;
})(1, 2, 3, 4, 5, 6);
Expand Down
1 change: 1 addition & 0 deletions pkgs/matcher/test/matcher/throws_type_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ void main() {
group('[throwsNoSuchMethodError]', () {
test('passes when a NoSuchMethodError is thrown', () {
expect(() {
// ignore: avoid_dynamic_calls
(1 as dynamic).notAMethodOnInt();
}, throwsNoSuchMethodError);
});
Expand Down
1 change: 1 addition & 0 deletions pkgs/matcher/test/mirror_matchers_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// ignore_for_file: deprecated_member_use_from_same_package

@TestOn('vm')
library;

import 'package:matcher/mirror_matchers.dart';
import 'package:test/test.dart';
Expand Down

0 comments on commit 42495c2

Please sign in to comment.