test icon indicating copy to clipboard operation
test copied to clipboard

Change test function to detect when it is called with a Matcher Function() instead of a Matcher

Open whesse opened this issue 3 years ago • 3 comments

With instantiated constructor tearoffs, the dynamic argument to test(actual, dynamic matcher) can accidentally be IsA<T> instead of IsA<T>().

It would be nice to detect that, and give a useful error, or automatically generate the Matcher in that case.

Reported as issue https://github.com/dart-lang/sdk/issues/48842

@lrhn

whesse avatar Apr 20 '22 12:04 whesse

Hmm, that does seem like something that is easy to do accidentally. But at the same time I am worried about false positives (similar to the concern raised in the original issue).

It seems to me that the experience we really want is that of a lint, it shows up early (in the IDE), and you can ignore false positives fairly easily. But you also have to explicitly enable it, which is less good, and the code for it ends up very disconnected from this package, which is also not ideal.

I don't particularly like the idea of a runtime error or warning here (we actually have no notion of warnings at all in this package). Many matchers themselves take matchers or objects, and it would probably be difficult to make sure we had good consistent coverage everywhere.

jakemac53 avatar Apr 20 '22 14:04 jakemac53

One thing that might avoid false positives is that the case here (forgetting the parentheses after a matcher constructor) would return a function that takes no arguments. Can that be checked in a type-check? I think no valid use of matcher would actually want to match a concrete object of that function type.

whesse avatar Apr 20 '22 14:04 whesse

I think no valid use of matcher would actually want to match a concrete object of that function type.

Tests of matchers (or utility code that returns matchers) are an example potentially valid use case :).

jakemac53 avatar Apr 20 '22 14:04 jakemac53