Mocking a class with @Deprecated methods should include the same Deprecation warnings

[tony-ditchlabs]

When a class with fields or methods marked as @Deprecated("someMessage") are Mocked, the @Deprecated is stripped away from the resulting Mock class.

It's very helpful for Dart Analysis to point out deprecated code with warnings, but with Mocked classes, the warnings all disappear - methods that are or aren't deprecated are indistinguishable from each other. Updating tests becomes quite the chore.

It would be helpful to include a way to toggle including / excluding @Deprecated annotations when generating Mocks, though to prevent a breaking change, I suspect it would have to be set to false by default.

[srawlins]

I agree, I think there are a few annotations for which it is idiomatic for subclasses to be similarly annotated: @deprecated, @experimental, @internal, @Protected, @visibleForX`.

[yanok]

Seems quite doable, but I likely won't have time to implement this anytime soon.

And we probably want to start looking at macros, instead of investing more time into improving the codegen.

[natebosch]

I don't understand the value here. When I invoke a method on a *Mock in a test file, it's because I must stub a call the library code will make. The place I want to see a deprecation warning in the library code - I cannot change my mock until I change the library code that uses the mock in that way.

What are the use cases for removing calls to deprecated APIs on *Mock instances under test that isn't covered by the deprecation warnings at the usage sites under lib/?

[tony-ditchlabs]

If I mark a method as @Deprecated, and then remove all references to it in my lib folder, then there is nothing in Dart Analysis warning me that I've used the deprecated method, making it safe to delete. If I'm deprecating a few dozen classes and methods, then clearing out the Dart Analysis panel entirely gives me the assurance that I've caught all instances of the deprecated code, and it's safe to be deleted.

But if I've mocked it, that's false. If I delete the deprecated code, then when I run build runner my tests are littered with errors, without the hints on how to fix it that I wrote in all those @Deprecated annotations.

When you're doing something huge, like deprecating and replacing an entire database, having the deprecation notices in the mocks would save a massive amount of time.

[natebosch]

It sounds like the information you're directly interested in is which methods are stubbed but not called. A deprecated mock is a good proxy in the specific case where lib/ has no more violations, but it's not going to catch any of the other reasons you might remove a call in the library code without removing the stub.

If we can solve the deprecation forwarding we may as well do it to catch this subset where it's easy. I do wonder if folks would be interested in being strict about only stubbing calls that do get made to help catch dead stubs.

[srawlins]

If we can solve the deprecation forwarding we may as well do it to catch this subset where it's easy. I do wonder if folks would be interested in being strict about only stubbing calls that do get made to help catch dead stubs.

+1 I think this is a real cause of bloat and dead code that slowly grows in tests with mocks. I've never come up with a good solution for "at the end of a test case / all test cases, check which stubs were unnecessary." Other than a mutations testing framework, which are typically very expensive and would not be a built-in check.