slackhq / slack-lints

A collection of custom Android/Kotlin lint checks we use in our Android and Kotlin code bases at Slack.
Apache License 2.0
225 stars 11 forks source link

[Feature Request] Add support for other mocking frameworks #99

Closed SimonMarquis closed 11 months ago

SimonMarquis commented 1 year ago

Would you consider adding support for another popular mocking framework? We currently have developped our own ad-hoc Lint detector to report wrong usages of mocks using the Mockk.io library. But I think we could benefit from running this detection only once our codebase.

It could be done either with a lint config parameter, or maybe simpler by adding the relevant classes/methods in these variables: https://github.com/slackhq/slack-lints/blob/fd01ddf324d748686c7e2b853e13add739270cab/slack-lint-checks/src/main/java/slack/lint/mocking/MockDetector.kt#L34-L41

SimonMarquis commented 1 year ago

For reference, I tried to build of version of our internal Detector over here:

ZacSweers commented 1 year ago

I'd be open to making these mock factories configurable, similar to what we do in compose lints for view model injection - https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/ViewModelInjectionDetector.kt#L25-L31

SimonMarquis commented 1 year ago

@ZacSweers I started a draft PR #126 and for now I extracted the previous values as defaults. I'm not sure what your thoughts are about this (empty defaults, vs existing mockito ones).

Also, regarding the potential tests to be added, what would make more sense here?

ZacSweers commented 1 year ago

Keeping the defaults sounds good. I think we may also want to unify classes and methods together, i.e. "org.mockito.Mockito.mock" rather than split them.

For tests, keeping the existing defaults while added some basic extra tests to test that custom mocking factories are working I think should be enough.

ZacSweers commented 11 months ago

Fixed in https://github.com/slackhq/slack-lints/pull/126