Is there tooling to mark unused imports?

Tooling
1

Is there a tool (that I can run as part of CI for example) that detects unused namespace imports within a module (or the corresponding package)?

Eg in

module Foo

import Statistics: mean

awesome_functionality() = 1 # does not use mean at all

end

I would expect it to tell me that I imported Statistics.mean and yet it is unused.

A more advanced form would check all symbols and error when nothing at all is used from a module, eg

module Bar

using LinearAlgebra # but nothing it exports is used
import Random # never used

awesome_functionality() = 1

end
3 Likes
2

I think that’s the “stale imports” feature of

?

11 Likes
3

Thanks! I was not aware of the package. ExplicitImports.check_no_stale_explicit_imports catches the first case (using Statistics: mean) but not the using / import of packages that end up unused.

Is there a functionality in the package that does the latter?

4

This would be tricky to properly detect. For example, the checking system would have to make sure that a package extension wasn’t triggered by the import which you relied on indirectly.

A classic example would be

using DifferentiationInterface: AutoForwardDiff, derivative
import ForwardDiff

derivative(sin, AutoForwardDiff(), 1.0)

This looks like it doesn’t use ForwardDiff at all, but it would fail if ForwardDiff wasn’t loaded.

5 Likes
5

You can use print_explicit_imports, when I tried it, it just left out the packages that weren’t used at all.

2 Likes
6

Aqua.jl also does some of that

4 Likes
7

Yeah, ExplicitImports can’t tell if you’ve loaded a package for the “side-effects” (e.g. new methods) vs to get new names in your namespace. It can only say “PkgX has provided such and such names into your namespace and you aren’t using them” (though it isn’t 100% accurate at this currently and I don’t think the check_ functions use this).

However, I think a good syntax here is: if you are only loading ForwardDiff for the “side-effects”, you could indicate this with

import ForwardDiff as _

i.e., I want to load ForwardDiff but I don’t want it to bring any names into my namespace, not even the module itself, since I’m not using them. I checked Julia 1.11 and this syntax does seem to be supported already!

I could imagine a future ExplicitImports version suggesting rewrites of import ForwardDiff into import ForwardDiff as _ if there are no names being used (thereby assuming it is there for the side-effects). Then the dev can delete it if they know they aren’t in fact using the side-effects.

14 Likes
8

Worth adding to the documentation?

4 Likes
9

ExplicitImports docs or Julia docs?

10

Probably both:

  1. first in the Julia docs. It is a useful trick for conditional loading without bringing anything into the namespace, regardless of other usage
  2. into ExplicitImports.jl for the purpose discussed in this topic.
4 Likes
11

Is Aqua.jl able to detect that? If not, perhaps create a feature request for this package?

12

I for one think Aqua.jl should absorb ExplicitImports.jl, or at least offer to call it in a nice way

7 Likes
13

I also think that would be good. Some notes:

  • Currently Aqua supports Julia 1.6, but ExplicitImports needs 1.7+; the easy fix is to just make Aqua more restrictive and add the dep
  • Currently ExplicitImports has some false positives and false negatives (see README/issues). Many of those could be fixed once JuliaLowering is stable by using it instead of my ad-hoc buggy lowering. But maybe it would be better to just rewrite the functionality on top of JuliaLowering rather than adapting the existing code; I’m not sure. I do hope to have a version of this in ExplicitImports.jl at some point, whether as a rewrite or an upgrade.
  • Currently ExplicitImports does not yet support JuliaSyntax v1 (its work-in-progress to upgrade it). So it could cause downgrades.
  • adding ExplicitImports would add a few deps like AbstractTrees and Markdown (not too many though, I’ve tried to be careful there).
  • There is an issue here: Check unused explicit imports, e.g. using `ExplicitImports.jl` · Issue #268 · JuliaTesting/Aqua.jl · GitHub
  • I’d prefer to keep them as separate packages and expose any API Aqua needs from ExplicitImports rather than move the code into a submodule of Aqua, since the styles/implementations are fairly different and it’s a big chunk of code. But I’d be happy to move ExplicitImports into the JuliaTesting GitHub org if that would be preferable.
4 Likes
14

Maybe we could add ExplicitImports.jl to Aqua.jl as an extension? There could be a function like

function Aqua.test_explicit_imports(
    package,
    check_no_implicit_imports=true,
    check_no_stale_explicit_imports=true,
    check_all_explicit_imports_via_owners=true,
    check_all_explicit_imports_are_public=true,
    check_all_qualified_accesses_via_owners=true,
    check_all_qualified_accesses_are_public=true,
    check_no_self_qualified_accesses=true
)

which does nothing unless the weakdep is loaded. It would also solve my main gripe with ExplicitImports.jl, namely the need to use these six functions separately.

1 Like
15

Sure, well if you just want that function we could just add it to ExplicitImports. To me it seems like the advantage of integration with Aqua would be Aqua.test_all – but I don’t think that should be via an extension since it would give different results if ExplicitImports was loaded or not.

1 Like
16

Yeah, I was thinking Aqua.test_all could also have a test_explicit_imports option like for the other types of tests. The fact that results differ based on whether or not ExplicitImports.jl is loaded does suck a little, but in that case we could either

  • print a warning
  • straight up error (and then set the default to test_explicit_imports=false)

The last option would enable reliable but voluntary integration. We did a variant of that for test_undocumented_names too because it is not available on Julia 1.10.

3 Likes