fix: Improve the output of many matchers that expect specific types

- Add a new TypedMatcher class to generalize `matches` type checking
- Use TypeMatcher across many of the existing Matcher implementations
- Update tests validate new, more consistent failure messages
- Add a few new tests for `isIn`

Author: kevmoo

CHANGELOG.md

@@ -7,6 +7,8 @@
 
 - Deprecated the `isInstanceOf` class. Use `TypeMatcher` instead.
 
+- Improved the output of `Matcher` instances that fail due to type errors.
+
 ## 0.12.2+1
 
 - Updated SDK version to 2.0.0-dev.17.0

lib/src/core_matchers.dart

@@ -2,6 +2,7 @@
 // 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';
 
@@ -69,15 +70,17 @@ const Matcher isNaN = const _IsNaN();
 /// A matcher that matches any non-NaN value.
 const Matcher isNotNaN = const _IsNotNaN();
 
-class _IsNaN extends Matcher {
+class _IsNaN extends FeatureMatcher<num> {
   const _IsNaN();
-  bool matches(item, Map matchState) => double.nan.compareTo(item) == 0;
+  bool typedMatches(num item, Map matchState) =>
+      double.nan.compareTo(item) == 0;
   Description describe(Description description) => description.add('NaN');
 }
 
-class _IsNotNaN extends Matcher {
+class _IsNotNaN extends FeatureMatcher<num> {
   const _IsNotNaN();
-  bool matches(item, Map matchState) => double.nan.compareTo(item) != 0;
+  bool typedMatches(num item, Map matchState) =>
+      double.nan.compareTo(item) != 0;
   Description describe(Description description) => description.add('not NaN');
 }
 
@@ -127,10 +130,10 @@ class isInstanceOf<T> extends TypeMatcher<T> {
 /// a wrapper will have to be created.
 const Matcher returnsNormally = const _ReturnsNormally();
 
-class _ReturnsNormally extends Matcher {
+class _ReturnsNormally extends FeatureMatcher<Function> {
   const _ReturnsNormally();
 
-  bool matches(f, Map matchState) {
+  bool typedMatches(Function f, Map matchState) {
     try {
       f();
       return true;
@@ -143,8 +146,8 @@ class _ReturnsNormally extends Matcher {
   Description describe(Description description) =>
       description.add("return normally");
 
-  Description describeMismatch(
-      item, Description mismatchDescription, Map matchState, bool verbose) {
+  Description describeTypedMismatch(Function item,
+      Description mismatchDescription, Map matchState, bool verbose) {
     mismatchDescription.add('threw ').addDescriptionOf(matchState['exception']);
     if (verbose) {
       mismatchDescription.add(' at ').add(matchState['stack'].toString());
@@ -257,11 +260,12 @@ class _Contains extends Matcher {
   const _Contains(this._expected);
 
   bool matches(item, Map matchState) {
+    var expected = _expected;
     if (item is String) {
-      return item.contains((_expected as Pattern));
+      return expected is Pattern && item.contains(expected);
     } else if (item is Iterable) {
-      if (_expected is Matcher) {
-        return item.any((e) => (_expected as Matcher).matches(e, matchState));
+      if (expected is Matcher) {
+        return item.any((e) => expected.matches(e, matchState));
       } else {
         return item.contains(_expected);
       }
@@ -287,27 +291,29 @@ class _Contains extends Matcher {
 
 /// Returns a matcher that matches if the match argument is in
 /// the expected value. This is the converse of [contains].
-Matcher isIn(expected) => new _In(expected);
+Matcher isIn(expected) {
+  if (expected is Iterable) {
+    return new _In(expected, expected.contains);
+  } else if (expected is String) {
+    return new _In<Pattern>(expected, expected.contains);
+  } else if (expected is Map) {
+    return new _In(expected, expected.containsKey);
+  }
 
-class _In extends Matcher {
-  final Object _expected;
+  throw new ArgumentError.value(
+      expected, 'expected', 'Only Iterable, Map, and String are supported.');
+}
 
-  const _In(this._expected);
+class _In<T> extends FeatureMatcher<T> {
+  final Object _source;
+  final bool Function(T) _containsFunction;
 
-  bool matches(item, Map matchState) {
-    var expected = _expected;
-    if (expected is String) {
-      return expected.contains(item as Pattern);
-    } else if (expected is Iterable) {
-      return expected.contains(item);
-    } else if (expected is Map) {
-      return expected.containsKey(item);
-    }
-    return false;
-  }
+  const _In(this._source, this._containsFunction);
+
+  bool typedMatches(T item, Map matchState) => _containsFunction(item);
 
   Description describe(Description description) =>
-      description.add('is in ').addDescriptionOf(_expected);
+      description.add('is in ').addDescriptionOf(_source);
 }
 
 /// Returns a matcher that uses an arbitrary function that returns
@@ -322,13 +328,13 @@ Matcher predicate<T>(bool f(T value),
 
 typedef bool _PredicateFunction<T>(T value);
 
-class _Predicate<T> extends Matcher {
+class _Predicate<T> extends FeatureMatcher<T> {
   final _PredicateFunction<T> _matcher;
   final String _description;
 
   _Predicate(this._matcher, this._description);
 
-  bool matches(item, Map matchState) => _matcher(item as T);
+  bool typedMatches(T item, Map matchState) => _matcher(item);
 
   Description describe(Description description) =>
       description.add(_description);

lib/src/equals_matcher.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'description.dart';
+import 'feature_matcher.dart';
 import 'interfaces.dart';
 import 'util.dart';
 
@@ -16,67 +17,61 @@ import 'util.dart';
 /// handle cyclic structures a recursion depth [limit] can be provided. The
 /// default limit is 100. [Set]s will be compared order-independently.
 Matcher equals(expected, [int limit = 100]) => expected is String
-    ? new _StringEqualsMatcher(expected) as Matcher
+    ? new _StringEqualsMatcher(expected)
     : new _DeepMatcher(expected, limit);
 
 typedef _RecursiveMatcher = List<String> Function(
     dynamic, dynamic, String, int);
 
 /// A special equality matcher for strings.
-class _StringEqualsMatcher extends Matcher {
+class _StringEqualsMatcher extends FeatureMatcher<String> {
   final String _value;
 
   _StringEqualsMatcher(this._value);
 
-  bool get showActualValue => true;
-
-  bool matches(item, Map matchState) => _value == item;
+  bool typedMatches(String item, Map matchState) => _value == item;
 
   Description describe(Description description) =>
       description.addDescriptionOf(_value);
 
-  Description describeMismatch(
-      item, Description mismatchDescription, Map matchState, bool verbose) {
-    if (item is! String) {
-      return mismatchDescription.addDescriptionOf(item).add('is not a string');
-    } else {
-      var buff = new StringBuffer();
-      buff.write('is different.');
-      var escapedItem = escape(item);
-      var escapedValue = escape(_value);
-      var minLength = escapedItem.length < escapedValue.length
-          ? escapedItem.length
-          : escapedValue.length;
-      var start = 0;
-      for (; start < minLength; start++) {
-        if (escapedValue.codeUnitAt(start) != escapedItem.codeUnitAt(start)) {
-          break;
-        }
+  Description describeTypedMismatch(String item,
+      Description mismatchDescription, Map matchState, bool verbose) {
+    var buff = new StringBuffer();
+    buff.write('is different.');
+    var escapedItem = escape(item);
+    var escapedValue = escape(_value);
+    var minLength = escapedItem.length < escapedValue.length
+        ? escapedItem.length
+        : escapedValue.length;
+    var start = 0;
+    for (; start < minLength; start++) {
+      if (escapedValue.codeUnitAt(start) != escapedItem.codeUnitAt(start)) {
+        break;
       }
-      if (start == minLength) {
-        if (escapedValue.length < escapedItem.length) {
-          buff.write(' Both strings start the same, but the actual value also'
-              ' has the following trailing characters: ');
-          _writeTrailing(buff, escapedItem, escapedValue.length);
-        } else {
-          buff.write(' Both strings start the same, but the actual value is'
-              ' missing the following trailing characters: ');
-          _writeTrailing(buff, escapedValue, escapedItem.length);
-        }
+    }
+    if (start == minLength) {
+      if (escapedValue.length < escapedItem.length) {
+        buff.write(' Both strings start the same, but the actual value also'
+            ' has the following trailing characters: ');
+        _writeTrailing(buff, escapedItem, escapedValue.length);
       } else {
-        buff.write('\nExpected: ');
-        _writeLeading(buff, escapedValue, start);
-        _writeTrailing(buff, escapedValue, start);
-        buff.write('\n  Actual: ');
-        _writeLeading(buff, escapedItem, start);
-        _writeTrailing(buff, escapedItem, start);
-        buff.write('\n          ');
-        for (var i = (start > 10 ? 14 : start); i > 0; i--) buff.write(' ');
-        buff.write('^\n Differ at offset $start');
+        buff.write(' Both strings start the same, but the actual value is'
+            ' missing the following trailing characters: ');
+        _writeTrailing(buff, escapedValue, escapedItem.length);
       }
-
-      return mismatchDescription.add(buff.toString());
+    } else {
+      buff.write('\nExpected: ');
+      _writeLeading(buff, escapedValue, start);
+      _writeTrailing(buff, escapedValue, start);
+      buff.write('\n  Actual: ');
+      _writeLeading(buff, escapedItem, start);
+      _writeTrailing(buff, escapedItem, start);
+      buff.write('\n          ');
+      for (var i = (start > 10 ? 14 : start); i > 0; i--) buff.write(' ');
+      buff.write('^\n Differ at offset $start');
     }
+
+    return mismatchDescription.add(buff.toString());
   }
 
   static void _writeLeading(StringBuffer buff, String s, int start) {

lib/src/feature_matcher.dart

@@ -0,0 +1,32 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// 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 'core_matchers.dart';
+import 'interfaces.dart';
+
+// TODO: export this from the root 'matcher.dart' library
+/// A [Matcher] implementation that validates the type of `item` passed to
+/// [matches].
+abstract class FeatureMatcher<T> extends TypeMatcher<T> {
+  const FeatureMatcher();
+
+  bool matches(item, Map matchState) =>
+      super.matches(item, matchState) && typedMatches(item, matchState);
+
+  bool typedMatches(T item, Map matchState);
+
+  Description describeMismatch(
+      item, Description mismatchDescription, Map matchState, bool verbose) {
+    if (item is T) {
+      return describeTypedMismatch(
+          item, mismatchDescription, matchState, verbose);
+    }
+
+    return super.describe(mismatchDescription.add('not an '));
+  }
+
+  Description describeTypedMismatch(T item, Description mismatchDescription,
+          Map matchState, bool verbose) =>
+      mismatchDescription;
+}