dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

proposal: `do_not_use_mockito` #58791

Open matanlurey opened 2 years ago

matanlurey commented 2 years ago

do_not_use_mockito

Description

Do not use package:mockito.

Details

Mocking, or creating short-lived proxy objects that pretend to implement a type can be useful when testing certain contracts between related objects; for example, you may want to ensure that pet.eat(bowl) only calls the bowl.grab(...) method once with specific arguments (the pet's mouth size?).

However, mocking is generally extremely over-used. Typical apps and libraries usually have no need for mocking, and would be better served either by using the real object (especially for value-type immutable objects), and/or using or contributing to fakes - intentional sub-types intended for testing.

This lint would let teams avoid (or start avoiding additional) uses of mockito.

Kind

Style

Good Examples

N/A

Bad Examples

// LINT
import 'package:mockito/mockito.dart';

Discussion checklist

/cc @srawlins

srawlins commented 2 years ago

I don't see us ever implementing this, but I like the thought. :)

matanlurey commented 2 years ago

What if I implemented it :)?

bwilkerson commented 2 years ago

I appreciate the offer, but it's not really about the cost of the initial implementation at this point. Unfortunately, the cost of maintaining the lints has increased as the language has grown, so we find ourselves needing to be more cautious about adding new lints.

We're in the process of defining the criteria we want to use going forward for allowing a lint to be added to the linter. We don't have a concrete set yet, but when we do we'll open it up for wider discussion and eventually publish them. As we start to use those criteria, the question will change from "is there any good reason to not add this lint" to "does it meet the criteria for new lints".

I agree with Sam that we likely won't add this lint at this point. The reason I say that is because I don't think it meets the bar in terms of bringing enough value to enough users. For most users it's enough to just not allow a package to have a dependency on mockito, and it isn't clear that users need a rule to enforce that given how often dependencies are typically added.

But I'm open to hearing if there's a criteria that you think we could apply uniformly when making these decisions that would justify the cost of adding this rule.

incendial commented 2 years ago

@matanlurey DCM provides a configurable rule for banning imports https://dartcodemetrics.dev/docs/rules/common/avoid-banned-imports, please take a look if you're not limited by the standard analyzer / linter 🙂