From fcbd3616c1dbe636d22d2b164aee3f1c32c52a42 Mon Sep 17 00:00:00 2001
From: Nate Bosch <nbosch@google.com>
Date: Tue, 21 Nov 2023 13:51:47 -0800
Subject: [PATCH] Add more advice away from predicate (#233)

Add a best practice entry and expand the dartdoc for `predicate` to
mention that the failure messages are not as detailed as `TypeMatcher`,
and that the latter is preferred when the value can fail in more than
one way.

Add a generic argument to the example usage - most cases should prefer
to include a generic.

Cleanup an unnecessary private `typedef` with a single usage.
---
 README.md                  | 11 +++++++++++
 lib/src/core_matchers.dart | 21 +++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index 9f44237..3044238 100644
--- a/README.md
+++ b/README.md
@@ -253,3 +253,14 @@ why the test failed. For instance compare the failures between
      Which: has length of <2>
 
 ```
+
+### Prefer TypeMatcher to predicate if the match can fail in multiple ways
+
+The `predicate` utility is a convenient shortcut for testing an arbitrary
+(synchronous) property of a value, but it discards context and failures are
+opaque. Different failure modes cannot be distinguished in the output which is
+determined by a single "description" argument. Using `isA<SomeType>()` and the
+`TypeMatcher.having` API to extract and test derived properties in a structured
+way brings the context of that structure through to failure messages, so
+failures for different reasons will have distinguishable and actionable failure
+messages.
diff --git a/lib/src/core_matchers.dart b/lib/src/core_matchers.dart
index 45d1f27..70e3449 100644
--- a/lib/src/core_matchers.dart
+++ b/lib/src/core_matchers.dart
@@ -290,20 +290,29 @@ class _In<T> extends FeatureMatcher<T> {
       description.add('is in ').addDescriptionOf(_source);
 }
 
-/// Returns a matcher that uses an arbitrary function that returns
-/// true or false for the actual value.
+/// Returns a matcher that uses an arbitrary function that returns whether the
+/// value is considered a match.
 ///
 /// For example:
 ///
-///     expect(v, predicate((x) => ((x % 2) == 0), "is even"))
+///     expect(actual, predicate<num>((v) => (v % 2) == 0, 'is even'));
+///
+/// Use this method when a value is checked for one conceptual property
+/// described by [description].
+///
+/// If the value can be rejected for more than one reason prefer using [isA] and
+/// the [TypeMatcher.having] API to build up a matcher with output that can
+/// distinquish between them.
+///
+/// Using an explicit generict argument allows a passed function literal to have
+/// an inferred argument type of [T], and values of the wrong type will be
+/// rejected with an informative message.
 Matcher predicate<T>(bool Function(T) f,
         [String description = 'satisfies function']) =>
     _Predicate(f, description);
 
-typedef _PredicateFunction<T> = bool Function(T value);
-
 class _Predicate<T> extends FeatureMatcher<T> {
-  final _PredicateFunction<T> _matcher;
+  final bool Function(T) _matcher;
   final String _description;
 
   _Predicate(this._matcher, this._description);