AllOf() matcher crashes if inner matchers throw

[bobjackman]

trafficstars

I don't have time to create a full PR with tests, but here's a repro and a patch if someone has the time to integrate them.

I believe this is closely related to, but still distinct from #174

Repro

import 'package:test/test.dart';

void main() {
  test('foo', () {
    var myVal = 'somestring';
    expect(myVal, allOf(containsPair('foo', 'bar')), reason: 'oh noes!');
  });
}

Which outputs:

dart:core                                         Object.noSuchMethod
package:matcher/src/operator_matchers.dart 61:13  _AllOf.describeMismatch
package:test_api                                  expect
test\demo.dart 6:5                                main.<fn>

NoSuchMethodError: The method 'describeMismatch' was called on null.
Receiver: null
Tried calling: describeMismatch("somestring", Instance of 'StringDescription', null, false)

Fix

--- a/lib/src/operator_matchers.dart
+++ b/lib/src/operator_matchers.dart
@@ -46,10 +46,15 @@ class _AllOf extends Matcher {
   @override
   bool matches(dynamic item, Map matchState) {
     for (var matcher in _matchers) {
+      try {
         if (!matcher.matches(item, matchState)) {
           addStateInfo(matchState, {'matcher': matcher});
           return false;
         }
+      } catch(_) {
+        addStateInfo(matchState, {'matcher': matcher});
+        rethrow;
+      }
     }
     return true;
   }

Alternate Fix

Since the original code didn't only called addStateInfo() for a failed match, the above assumes that's important and as such, preserves the same logic (again, I didn't have time to explore in more depth). If this is not the case and it's indeed acceptable to call addStateInfo() in all cases, then the fix becomes simpler:

--- a/lib/src/operator_matchers.dart
+++ b/lib/src/operator_matchers.dart
@@ -46,10 +46,10 @@ class _AllOf extends Matcher {
   @override
   bool matches(dynamic item, Map matchState) {
     for (var matcher in _matchers) {
+        addStateInfo(matchState, {'matcher': matcher});
         if (!matcher.matches(item, matchState)) {
-          addStateInfo(matchState, {'matcher': matcher});
           return false;
         }
     }
     return true;
   }

Workaround

For the time being, this crash can be avoided by rewriting the test like so:

-expect(myVal, allOf(containsPair('foo', 'bar')), reason: 'oh noes!');
+expect(myVal, isA<Map>().having((x) => x, 'that', allOf(containsPair('foo', 'bar'))), reason: 'on noes!');

[bobjackman]

In a similar vein:

var expectedValues = generateValues()
  ..renameKey('someKey', 'someOtherExistingKey'); // -- throws `Bad state: Existing element would be overwrittenexisting value`, but gets supressed due to below

expect(actualValues, expectedValues);

which outputs:

package:matcher/src/equals_matcher.dart 272:43  _DeepMatcher.describeMismatch
package:test_api                                expect
test\models\User_test.dart 59:7                 main.<fn>.<fn>

type 'Null' is not a subtype of type '_Mismatch' in type cast

Similarly, the workaround for this one is also:

-expect(actualValues, equals(expectedValues));
+expect(actualValues, isA<Map>().having((x) => x, 'that', equals(expectedValues)));