ericphanson / ExplicitImports.jl

Developer tooling for Julia namespace management: detecting implicit imports, improper explicit imports, and improper qualified accesses
https://ericphanson.github.io/ExplicitImports.jl/
MIT License
62 stars 2 forks source link

Suggest replacing `using Foo` by `using Foo: Foo` #2

Closed carstenbauer closed 4 months ago

carstenbauer commented 4 months ago

I had using Libdl in ThreadPinning.jl but in the code I was always referencing functions explicitly as Libdl.dllist(...). It would be great if the package could recommend in this case - where no export is actually used implicitly - to change using Libdl to using Libdl: Libdl. This way, there are no (unused) functions flying around that could be used implicitly in the future.

(Of course, I can't speak to how difficult that would be, because I haven't looked at the implementation of the package.)

ericphanson commented 4 months ago

The way the implementation currently works is that it sees using Foo in the code, says "hey Foo, that's name that's in global scope, surely it's coming from another module", and then it queries Base.which(ThreadPinning, :Foo), and sees: oh Foo is coming from Foo great, let's tell them to import that. And then I cruelly filter it out! 😂. So it would be quite trivial to allow. We can definitely turn that on if folks prefer; I thought perhaps it would be annoying.

ericphanson commented 4 months ago

I'd welcome a PR to do this; removing that bit of code is trivial, but what would be slightly more work is to update the sorting so that using Foo: Foo comes before using Foo: Abc, since we probably want the package import to come first in the list. I think to do that, we'd want to change this line. That's where we sort the names within a given module, so it probably should check if either key equals nameof(v1) (or v2, they should be the same I think) and sort that one first. (Then we'd also need to update the tests!).

ericphanson commented 4 months ago

I think removing the filtering wasn't quite enough bc in some cases we don't add the using Foo: Foo still. However I expect https://github.com/ericphanson/ExplicitImports.jl/pull/8 will fix it, as I've reworked the logic there and am careful to add using statements to the list of names