package-level lint: unused, unexposed public members

[srawlins]

I'd like a lint rule that flags public library members (classes, top-level variables, typedefs, functions) and public class members (all the usual suspects) that are unused within their package, and not exposed from a library in the lib/ directory.

// lib/foo.dart

export 'src/foo.dart' show usedConstant;

// lib/src/foo.dart

// BAD
final unusedConstant = "CONSTANT";

final usedConstant = "CONSTANT";

[matanlurey]

I don't see how this is possible for the analyzer, it won't necessarily know the entire scope of imports.

[srawlins]

I thought the linter project had some aims to do package-level analysis? @pq is this done/feasible/planned?

[bwilkerson]

It isn't currently implemented. I'm not sure whether we're going to get there. But I do think we want to do this kind of analysis (and I have a prototype implementation that I've been using on some of our packages).

[pq]

Thanks for chiming in, @bwilkerson! Was just about to ping you. 😄

[chrisnorman7]

I'd like to add my support for this request too. I've been doing a lot of work with FFI bindings recently, and it would be nice to know what new functions ETC ffigen has created that I'm not yet using. Otherwise you have to go out and catalogue every function, and work out if you've already used it yet.

Unless I'm completely mis-writing bindings of course. That is more than possible.

[srawlins]

I had forgotten about my linter request here 😆 and in fact, I think this is beyond what the linter does these days...

I have an internal proposal for Whole World analysis, which would enfold this. I think there are some great benefits to Whole World analysis, and would like to prioritize the feature.

[rrousselGit]

👋 Just dropping by to gently bump this.

It'd help detect a lot of dead code. Lots of members end-up public when splitting members across different libraries.
It's common that after a refactoring, some public class/function isn't removed even though it's now unused.

The same logic could be applied to dependencies too. It's common that after a refactoring, a package isn't used anymore yet isn't removed from the pubspec.yaml.

[incendial]

It'd help detect a lot of dead code. Lots of members end-up public when splitting members across different libraries.
It's common that after a refactoring, some public class/function isn't removed even though it's now unused.

Have you tried DCM check-unused-code?

[incendial]

@rrousselGit I see the reaction, but don't understand it though. Do you really need a real-time IDE highlight? If not, DCM can already show unused code from the CLI.

[rrousselGit]

Yes, real-time IDE highlight matters.

And you're asking to use a package. This is an important feature. I would expect this to be official, not community-based.

[incendial]

Yes, real-time IDE highlight matters.

The only question is how it will affect the analyzer's performance.

And you're asking to use a package.

The package that already solves the problem with unused code (and other) while the Dart team can focus on more important (and mostly importantly unsolved) things. And yeah, in that terms analyzer is also a package.

I would expect this to be official, not community-based.

It kinda destroys the whole idea of having the community and community based packages if some features "are expected to be official". Don't see any particular reason for that.

[rrousselGit]

I don't think this is the right place to debate the validity of packages.
But anyway there's nothing wrong with you working on DCM, supporting this, and wanting to promote it. But that's not what's being discussed here.

I believe that this feature is important enough that it should be core to the SDK, like how many other lints are also core to the SDK.
This is in the end an extension of unused_element, one of the most used lint rules, to catch some false negatives.

[incendial]

But that's not what's being discussed here.

I mean, you've pointed out to a specific problem giving impression of importance like it's not solved anyhow else which is not true (with the only argument why you can't use an existing solution is that it's not official). It has nothing to do with promoting DCM.

This is in the end an extension of unused_element

It might look like that from a user standpoint, but implementation wise it's not. Keeping all the references and updating them properly (taking into consideration multiple packages and that multiple drivers should not store same references multiple times, etc.) is a whole another level of problems. Checking usage for private variables is easy.