Check unused explicit imports, e.g. using `ExplicitImports.jl`

[jonas-schulze]

One of my packages broke because of

using Base.Iterators: countfrom, repeat

As of Julia 1.9, repeat is no longer available that way, and I didn't notice that it was in Base all along.
While checking the other imported identifiers, I noticed some other candidates that didn't even need anymore.
It would be awesome if Aqua could detect those.

[lgoettgens]

Even though this is not a feature of Aqua, there already is a dedicated tool for exactly this: Check out https://github.com/ericphanson/ExplicitImports.jl

[jonas-schulze]

I only skimmed the README of ExplicitImports and did not notice this feature. Thanks for pointing that out! I will use ExplicitImports then for my use case.

Should such a test be added to Aqua? If not, feel free to close this issue.

[j-fu]

I think this would make much sense to have Aqua as a one-stop solution. However, ExplicitImports requires Julia 1.7, not sure if this fits with the Aqua policy.

May be they can make the test a no-op on earlier versions.

[ericphanson]

FYI ExplicitImports.jl v1.5 has expanded scope a bit, and can detect some issues related to qualified names, like using LinearAlgebra.promote_op instead of Base.promote_op (they are the same function but the name comes from Base), which I think could be a good fit for Aqua.

All the testing-oriented checks it does are here: https://ericphanson.github.io/ExplicitImports.jl/stable/api/#Checks-to-use-in-testing.

However, I don't think it would yet catch the case from the OP. If it were qualified like Iterators.repeat, then the new checks in 1.5 could catch it. I filed JuliaTesting/ExplicitImports.jl#52 for the original issue.

[ericphanson]

Just to say ExplicitImports v1.6 should now be able to handle the case from the OP via the new check_all_explicit_imports_via_owners , since it is an explicit import of a name from a module which doesn’t own that name (and the name isn't declared public or exported in that module).

There’s a table of all the checks here: https://github.com/ericphanson/ExplicitImports.jl/tree/v1.6.0?tab=readme-ov-file#summary

[ericphanson]

ExplicitImports.jl 1.7 now supports Julia 1.6, so if Aqua was willing to require 1.6 instead of 1.0, it would be addable.

It is harder to support older Julias like 1.0 in ExplicitImports.jl since some of the tests involve newer syntax (on purpose, to make sure we are parsing tricky cases correctly) so they can't run on older Julia's. I had to make some tests conditional on 1.7+ and to support below 1.5 I would have to make even more tests not run on older Julia's, which at some point may compromise correctness there.

[KeithWM]

What is the exact worth of supporting Julia 1.0?

[gdalle]

Just popping by to say that the minimum Julia version is now 1.6. @ericphanson would you still be interested in giving this a shot? Perhaps along with #348 since you did the same for ExplicitImports?

[ericphanson]

I think the way to do this is add an Aqua-style API to ExplicitImports (e.g. JuliaTesting/ExplicitImports.jl#143), then add ExplicitImports as a dependency to Aqua and integrate it via that API.

re-precommit: I'm not really interested at this time, I am not happy with current pre-commit integration and find it more annoying than useful when I try to use it. However I think an app / CLI interface would be nice for Aqua independently of pre-commit, and could take some lessons from ExplictImport's app functionality which similarly does dynamic analysis.

[gdalle]

I'll offer you a deal: if you merge and release JuliaTesting/ExplicitImports.jl#143,
I'll take care of the corresponding PR to Aqua

[ericphanson]

Deal! 😄. v1.15.0 is being registered with an all-in-one test_explicit_imports function as well as individual test_no_implicit_imports, test_no_stale_explicit_imports,
test_all_explicit_imports_via_owners, test_all_explicit_imports_are_public,
test_all_qualified_accesses_via_owners, test_all_qualified_accesses_are_public,
test_no_self_qualified_accesses functions.