Lint top level members in tests libraries if they are not used in the library itself

[alexeieleusis]

Right now the analyzer won't warn if a top level member is not used within the library, there is no reason for such members to live in the library.

It could either be extended to all libraries that have a main method or somehow determine what a tests library is, probably something that imports package test, has a main, group and test invocation.

[yjbanov]

To clarify, my feature request is to lint public top-level members in test code. The analyzer already warns about unused private top-level members. The other condition is that the library being linted is itself a test, and not a test utility library. It is perfectly fine to keep your test-only utility libraries under the test/ directory. So the rule is, if all of the following conditions are satisfied, issue a warning:

• the file is in test/ directory
• the file has a top-level main() function
• the file contains a public top-level member (variable, function, getter, setter)
• the said top-level member is not used by the library itself

[matanlurey]

Is there a particular reason not to just make your top-level members private?

You'd get this benefit "for free".

[yjbanov]

@matanlurey if developers did the right thing on their own you wouldn't need the linter or the analyzer :) I expect the lint to trigger mostly when a developer unknowingly defines a public member in a _test.dart file but ends up not actually using it. The analyzer does not warn about this case, despite it almost always being a bug in the code. I see this a lot in real world code.

Speaking of which, perhaps the _test.dart prefix should be one of the conditions.

[matanlurey]

I'm just worried we are entering territory where we are trying to "auto code review" - I don't know if I'd want to slow down my IDE and analyzer for this ;)

[alexeieleusis]

It might be a different problem then, there has been some discussion on several lints on when you want to run them. Roughly I think there are 3 categories: the ones you want in the IDE, the ones you run before requesting a code review and the ones before submitting.

[yjbanov]

I don't know if I'd want to slow down my IDE and analyzer for this

That's a valid concern. If this lint is computationally intensive, it probably should not be part of the dev cycle.

[yjbanov]

3 categories

At a glance, I like this idea.

[goderbauer]

As a data point: Recently did some clean-up in the framework with the help of this tool: https://pub.dev/packages/dart_code_metrics. We had accumulated quite a bit of dead code in tests: flutter/flutter#104550.

This proposal (or #1258 or #2395) probably could have prevented that build-up of dead code.

[srawlins]

Solved with #3513