ExplicitImports.jl icon indicating copy to clipboard operation
ExplicitImports.jl copied to clipboard

Doesn't work in package tests when Base.@deprecate_binding is used on Julia 1.10

Open danielmatz opened this issue 1 year ago • 3 comments

I am calling check_no_implicit_imports in my package tests. I had to deprecate something recently and used Base.@deprecate_binding. This seems to work fine on Julia 1.11, but on 1.10 it seems that check_no_implicit_imports tries to access the deprecated binding, and since the package tests run with --depwarn=yes, the tests error out.

danielmatz avatar Nov 09 '24 19:11 danielmatz

Oh, interesting. I think we try to access every binding currently. I wonder if there’s some way we can check ahead of time if it’s deprecated and/or turn off the error temporarily.

I believe we use a try/catch, I wonder why that doesn’t work. Do you have a stacktrace handy?

ericphanson avatar Nov 09 '24 19:11 ericphanson

Here's a stack trace:

     Testing Running tests...
Rotations.UnitQuaternionTimeDerivative is deprecated, use Quaternion instead.
Imports: Error During Test at /nobackup2/gitlab_runner/zDyhf_w4/0/dmatz/rotations.jl/test/runtests.jl:21
  Got exception outside of a @test
  use of deprecated variable: Rotations.UnitQuaternionTimeDerivative
  Stacktrace:
    [1] _find_submodules(mod::Module)
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/ExplicitImports.jl:312
    [2] find_submodules(mod::Module, file::String)
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/ExplicitImports.jl:358
    [3] explicit_imports(mod::Module, file::String; skip::Tuple{Module, Module, Module}, strict::Bool, warn_stale::Nothing, file_analysis::Dict{Any, Any})
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/ExplicitImports.jl:126
    [4] explicit_imports
      @ /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/ExplicitImports.jl:115 [inlined]
    [5] check_no_implicit_imports(mod::Module, file::String; skip::Tuple{Module, Module, Module}, ignore::Tuple{}, allow_unanalyzable::Tuple{})
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/checks.jl:226
    [6] check_no_implicit_imports
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/checks.jl:223 [inlined]
    [7] check_no_implicit_imports(mod::Module)
      @ ExplicitImports /nobackup2/gitlab-runner/zDyhf_w4/0/dmatz/rotations.jl/.julia/packages/ExplicitImports/8BXjx/src/checks.jl:223
    [8] macro expansion
      @ /nobackup2/gitlab_runner/zDyhf_w4/0/dmatz/rotations.jl/test/runtests.jl:22 [inlined]
    [9] macro expansion
      @ /software/julia/julia-1.10.0/share/julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined]
   [10] macro expansion
      @ /nobackup2/gitlab_runner/zDyhf_w4/0/dmatz/rotations.jl/test/runtests.jl:22 [inlined]
   [11] macro expansion
      @ /software/julia/julia-1.10.0/share/julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined]
   [12] macro expansion
      @ /nobackup2/gitlab_runner/zDyhf_w4/0/dmatz/rotations.jl/test/runtests.jl:18 [inlined]
   [13] macro expansion
      @ /software/julia/julia-1.10.0/share/julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined]
   [14] top-level scope
      @ /nobackup2/gitlab_runner/zDyhf_w4/0/dmatz/rotations.jl/test/runtests.jl:11
   [15] include(fname::String)
      @ Base.MainInclude ./client.jl:489
   [16] top-level scope
      @ none:6
   [17] eval
      @ Core ./boot.jl:385 [inlined]
   [18] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:291
   [19] _start()
      @ Base ./client.jl:552

danielmatz avatar Nov 09 '24 22:11 danielmatz

https://github.com/ericphanson/ExplicitImports.jl/blob/f4d64b0853f9f59befa219d2b52745ee65a76053/src/ExplicitImports.jl#L311-L320

Looks like we only catch UndefVarError. If I start julia with --depwarn=error, I get

julia> f() = 1
f (generic function with 1 method)

julia> Base.@deprecate f() println()
f (generic function with 1 method)

julia> e = try f() catch e; e end
ErrorException("`f()` is deprecated, use `println()` instead.")

julia> e.msg
"`f()` is deprecated, use `println()` instead."

So I guess it's just an ErrorException, not a special type. So I guess we should catch those too there.

ericphanson avatar Nov 10 '24 12:11 ericphanson