From 7d6c264f66b1851102bfe4d529024d86bf849ece Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 5 Jun 2025 01:15:39 +0200 Subject: [PATCH 01/19] wip: upgrade to JuliaSyntax v1 --- Project.toml | 4 +- src/get_names_used.jl | 34 +- src/parse_utilities.jl | 12 +- test/runtests.jl | 1912 ++++++++++++++++++++-------------------- 4 files changed, 1007 insertions(+), 955 deletions(-) diff --git a/Project.toml b/Project.toml index 23ac81b..9c440d0 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "ExplicitImports" uuid = "7d51a73a-1435-4ff3-83d9-f097790105c7" -version = "1.11.2" +version = "1.11.3" authors = ["Eric P. Hanson"] [deps] @@ -17,7 +17,7 @@ AbstractTrees = "0.4.5" Aqua = "0.8.4" Compat = "4.15" DataFrames = "1.6" -JuliaSyntax = "0.4.8" +JuliaSyntax = "1" LinearAlgebra = "<0.0.1, 1" Logging = "<0.0.1, 1" Markdown = "<0.0.1, 1" diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 3049a89..2373a84 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -270,9 +270,7 @@ function in_for_argument_position(node) # We must be on the LHS of a `for` `equal`. if !has_parent(node, 2) return false - elseif parents_match(node, (K"=", K"for")) - return child_index(node) == 1 - elseif parents_match(node, (K"=", K"cartesian_iterator", K"for")) + elseif parents_match(node, (K"iteration", K"for")) return child_index(node) == 1 elseif kind(parent(node)) in (K"tuple", K"parameters") return in_for_argument_position(get_parent(node)) @@ -293,13 +291,11 @@ end function in_generator_arg_position(node) # We must be on the LHS of a `=` inside a generator - # (possibly inside a filter, possibly inside a `cartesian_iterator`) + # (possibly inside a filter, possibly inside a `iteration`) if !has_parent(node, 2) return false - elseif parents_match(node, (K"=", K"generator")) || - parents_match(node, (K"=", K"cartesian_iterator", K"generator")) || - parents_match(node, (K"=", K"filter")) || - parents_match(node, (K"=", K"cartesian_iterator", K"filter")) + elseif parents_match(node, (K"iteration", K"generator")) || + parents_match(node, (K"iteration", K"filter")) return child_index(node) == 1 elseif kind(parent(node)) in (K"tuple", K"parameters") return in_generator_arg_position(get_parent(node)) @@ -356,7 +352,7 @@ function analyze_name(leaf; debug=false) # update our state val = get_val(node) k = kind(node) - args = nodevalue(node).node.raw.args + args = nodevalue(node).node.raw.children debug && println(val, ": ", k) # Constructs that start a new local scope. Note `let` & `macro` *arguments* are not explicitly supported/tested yet, @@ -494,6 +490,11 @@ function analyze_all_names(file) return analyze_per_usage_info(per_usage_info), untainted_modules end +# this would ideally be identity, but hashing SyntaxNode's is slow on v1.0.2 +# https://github.com/JuliaLang/JuliaSyntax.jl/issues/558 +# so we will settle for some unlikely chance at collisions and just check the string rep of the values +_simplify_hashing(scope_path) = map(string ∘ get_val, scope_path) + function is_name_internal_in_higher_local_scope(name, scope_path, seen) # We will recurse up the `scope_path`. Note the order is "reversed", # so the first entry of `scope_path` is deepest. @@ -506,7 +507,7 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) end # Ok, now pop off the first scope and check. scope_path = scope_path[2:end] - ret = get(seen, (; name, scope_path), nothing) + ret = get(seen, (; name, scope_path=_simplify_hashing(scope_path)), nothing) if ret === nothing # Not introduced here yet, trying recursing further continue @@ -527,9 +528,9 @@ function analyze_per_usage_info(per_usage_info) # Otherwise, we are in local scope: # 1. Next, if the name is a function arg, then this is not a global name (essentially first usage is assignment) # 2. Otherwise, if first usage is assignment, then it is local, otherwise it is global - seen = Dict{@NamedTuple{name::Symbol,scope_path::Vector{JuliaSyntax.SyntaxNode}},Bool}() + seen = Dict{@NamedTuple{name::Symbol,scope_path::Vector{String}},Bool}() return map(per_usage_info) do nt - @compat if (; nt.name, nt.scope_path) in keys(seen) + @compat if (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) in keys(seen) return PerUsageInfo(; nt..., first_usage_in_scope=false, external_global_name=missing, analysis_code=IgnoredNonFirst) @@ -561,7 +562,8 @@ function analyze_per_usage_info(per_usage_info) (nt.is_assignment, InternalAssignment)) if is_local external_global_name = false - push!(seen, (; nt.name, nt.scope_path) => external_global_name) + push!(seen, + (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=reason) @@ -572,13 +574,15 @@ function analyze_per_usage_info(per_usage_info) nt.scope_path, seen) external_global_name = false - push!(seen, (; nt.name, nt.scope_path) => external_global_name) + push!(seen, + (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=InternalHigherScope) end external_global_name = true - push!(seen, (; nt.name, nt.scope_path) => external_global_name) + push!(seen, + (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=External) end diff --git a/src/parse_utilities.jl b/src/parse_utilities.jl index fbfdece..cfab037 100644 --- a/src/parse_utilities.jl +++ b/src/parse_utilities.jl @@ -51,7 +51,7 @@ AbstractTrees.children(::SkippedFile) = () function AbstractTrees.children(wrapper::SyntaxNodeWrapper) node = wrapper.node if JuliaSyntax.kind(node) == K"call" - children = JuliaSyntax.children(node) + children = js_children(node) if length(children) == 2 f, arg = children::Vector{JuliaSyntax.SyntaxNode} # make JET happy if f.val === :include @@ -60,7 +60,7 @@ function AbstractTrees.children(wrapper::SyntaxNodeWrapper) return [SkippedFile(location)] end if JuliaSyntax.kind(arg) == K"string" - children = JuliaSyntax.children(arg) + children = js_children(arg) # if we have interpolation, there may be >1 child length(children) == 1 || @goto dynamic c = only(children) @@ -92,10 +92,14 @@ function AbstractTrees.children(wrapper::SyntaxNodeWrapper) end end return map(n -> SyntaxNodeWrapper(n, wrapper.file, wrapper.bad_locations), - JuliaSyntax.children(node)) + js_children(node)) end -js_children(n::Union{TreeCursor,SyntaxNodeWrapper}) = JuliaSyntax.children(js_node(n)) +js_children(n::Union{TreeCursor,SyntaxNodeWrapper}) = js_children(js_node(n)) + +# https://github.com/JuliaLang/JuliaSyntax.jl/issues/557 +js_children(n::Union{JuliaSyntax.SyntaxNode}) = something(JuliaSyntax.children(n), ()) + js_node(n::SyntaxNodeWrapper) = n.node js_node(n::TreeCursor) = js_node(nodevalue(n)) diff --git a/test/runtests.jl b/test/runtests.jl index 5ef9731..45707fa 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -74,1026 +74,1070 @@ include("test_explicit_imports.jl") include("main.jl") include("Test_Mod_Underscores.jl") -# For deprecations, we are using `maxlog`, which -# the TestLogger only respects in Julia 1.8+. -# (https://github.com/JuliaLang/julia/commit/02f7332027bd542b0701956a0f838bc75fa2eebd) -if VERSION >= v"1.8-" - @testset "deprecations" begin - include("deprecated.jl") +@testset "ExplicitImports" begin + # For deprecations, we are using `maxlog`, which + # the TestLogger only respects in Julia 1.8+. + # (https://github.com/JuliaLang/julia/commit/02f7332027bd542b0701956a0f838bc75fa2eebd) + if VERSION >= v"1.8-" + @testset "deprecations" begin + include("deprecated.jl") + end end -end -# package extension support needs Julia 1.9+ -if VERSION > v"1.9-" - @testset "Extensions" begin - submods = ExplicitImports.find_submodules(TestPkg) - @test length(submods) == 2 - DataFramesExt = Base.get_extension(TestPkg, :DataFramesExt) - @test haskey(Dict(submods), DataFramesExt) - - ext_imports = Dict(only_name_source(explicit_imports(TestPkg)))[DataFramesExt] - @test ext_imports == [(; name=:DataFrames, source=DataFrames), - (; name=:DataFrame, source=DataFrames), - (; name=:groupby, source=DataFrames)] || - ext_imports == [(; name=:DataFrames, source=DataFrames), - (; name=:DataFrame, source=DataFrames), - (; name=:groupby, source=DataFrames.DataAPI)] + # package extension support needs Julia 1.9+ + if VERSION > v"1.9-" + @testset "Extensions" begin + submods = ExplicitImports.find_submodules(TestPkg) + @test length(submods) == 2 + DataFramesExt = Base.get_extension(TestPkg, :DataFramesExt) + @test haskey(Dict(submods), DataFramesExt) + + ext_imports = Dict(only_name_source(explicit_imports(TestPkg)))[DataFramesExt] + @test ext_imports == [(; name=:DataFrames, source=DataFrames), + (; name=:DataFrame, source=DataFrames), + (; name=:groupby, source=DataFrames)] || + ext_imports == [(; name=:DataFrames, source=DataFrames), + (; name=:DataFrame, source=DataFrames), + (; name=:groupby, source=DataFrames.DataAPI)] + end end -end -@testset "function arg bug" begin - # https://github.com/ericphanson/ExplicitImports.jl/issues/62 - df = DataFrame(get_names_used("test_mods.jl").per_usage_info) - subset!(df, :name => ByRow(==(:norm)), :module_path => ByRow(==([:TestMod13]))) + @testset "function arg bug" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/62 + df = DataFrame(get_names_used("test_mods.jl").per_usage_info) + subset!(df, :name => ByRow(==(:norm)), :module_path => ByRow(==([:TestMod13]))) - @test_broken check_no_stale_explicit_imports(TestMod13, "test_mods.jl") === nothing -end + @test_broken check_no_stale_explicit_imports(TestMod13, "test_mods.jl") === nothing + end -@testset "owner_mod_for_printing" begin - @test owner_mod_for_printing(Core, :throw, Core.throw) == Base - @test owner_mod_for_printing(Core, :println, Core.println) == Core -end + @testset "owner_mod_for_printing" begin + @test owner_mod_for_printing(Core, :throw, Core.throw) == Base + @test owner_mod_for_printing(Core, :println, Core.println) == Core + end -# https://github.com/ericphanson/ExplicitImports.jl/issues/69 -@testset "Reexport support" begin - @test check_no_stale_explicit_imports(TestMod15, "test_mods.jl") === nothing - @test isempty(improper_explicit_imports_nonrecursive(TestMod15, "test_mods.jl")) - @test isempty(improper_explicit_imports(TestMod15, "test_mods.jl")[1][2]) -end + # https://github.com/ericphanson/ExplicitImports.jl/issues/69 + @testset "Reexport support" begin + @test check_no_stale_explicit_imports(TestMod15, "test_mods.jl") === nothing + @test isempty(improper_explicit_imports_nonrecursive(TestMod15, "test_mods.jl")) + @test isempty(improper_explicit_imports(TestMod15, "test_mods.jl")[1][2]) + end -if VERSION >= v"1.7-" - # https://github.com/ericphanson/ExplicitImports.jl/issues/70 - @testset "Compat skipping" begin - @test check_all_explicit_imports_via_owners(TestMod14, "test_mods.jl") === nothing - @test check_all_qualified_accesses_via_owners(TestMod14, "test_mods.jl") === nothing + if VERSION >= v"1.7-" + # https://github.com/ericphanson/ExplicitImports.jl/issues/70 + @testset "Compat skipping" begin + @test check_all_explicit_imports_via_owners(TestMod14, "test_mods.jl") === + nothing + @test check_all_qualified_accesses_via_owners(TestMod14, "test_mods.jl") === + nothing - @test isempty(improper_explicit_imports_nonrecursive(TestMod14, "test_mods.jl")) - @test isempty(improper_explicit_imports(TestMod14, "test_mods.jl")[1][2]) + @test isempty(improper_explicit_imports_nonrecursive(TestMod14, "test_mods.jl")) + @test isempty(improper_explicit_imports(TestMod14, "test_mods.jl")[1][2]) - @test isempty(improper_qualified_accesses_nonrecursive(TestMod14, "test_mods.jl")) + @test isempty(improper_qualified_accesses_nonrecursive(TestMod14, + "test_mods.jl")) - @test isempty(improper_qualified_accesses(TestMod14, "test_mods.jl")[1][2]) + @test isempty(improper_qualified_accesses(TestMod14, "test_mods.jl")[1][2]) + end end -end -@testset "imports" begin - cursor = TreeCursor(SyntaxNodeWrapper("imports.jl")) - leaves = collect(Leaves(cursor)) - import_type_pairs = get_val.(leaves) .=> analyze_import_type.(leaves) - filter!(import_type_pairs) do (k, v) - return v !== :not_import + @testset "imports" begin + cursor = TreeCursor(SyntaxNodeWrapper("imports.jl")) + leaves = collect(Leaves(cursor)) + import_type_pairs = get_val.(leaves) .=> analyze_import_type.(leaves) + filter!(import_type_pairs) do (k, v) + return v !== :not_import + end + @test import_type_pairs == + [:Exporter => :import_LHS, + :exported_a => :import_RHS, + :exported_c => :import_RHS, + :Exporter => :import_LHS, + :exported_c => :import_RHS, + :TestModA => :blanket_using_member, + :SubModB => :blanket_using, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h2 => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h3 => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h => :import_RHS, + :Exporter => :blanket_using, + :Exporter => :plain_import, + :LinearAlgebra => :import_LHS, + :map => :import_RHS, + :_svd! => :import_RHS, + :LinearAlgebra => :import_LHS, + :svd => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :exported_b => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :f => :import_RHS] + + inds = findall(==(:import_RHS), analyze_import_type.(leaves)) + lhs_rhs_pairs = get_import_lhs.(leaves[inds]) .=> get_val.(leaves[inds]) + @test lhs_rhs_pairs == [[:., :., :Exporter] => :exported_a, + [:., :., :Exporter] => :exported_c, + [:., :., :Exporter] => :exported_c, + [:., :., :TestModA, :SubModB] => :h2, + [:., :., :TestModA, :SubModB] => :h3, + [:., :., :TestModA, :SubModB] => :h, + [:LinearAlgebra] => :map, + [:LinearAlgebra] => :_svd!, + [:LinearAlgebra] => :svd, + [:., :., :TestModA, :SubModB] => :exported_b, + [:., :., :TestModA, :SubModB] => :f] + + imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; + allow_internal_imports=false)) + h_row = only(subset(imps, :name => ByRow(==(:h)))) + @test !h_row.public_import + # Note: if this fails locally, try `include("imports.jl")` to rebuild the module + @test h_row.whichmodule == TestModA.SubModB + @test h_row.importing_from == TestModA.SubModB + + h2_row = only(subset(imps, :name => ByRow(==(:h2)))) + @test h2_row.public_import + @test h2_row.whichmodule === TestModA.SubModB + @test h2_row.importing_from == TestModA.SubModB + _svd!_row = only(subset(imps, :name => ByRow(==(:_svd!)))) + @test !_svd!_row.public_import + + f_row = only(subset(imps, :name => ByRow(==(:f)))) + @test !f_row.public_import # not public in `TestModA.SubModB` + @test f_row.whichmodule == TestModA + @test f_row.importing_from == TestModA.SubModB + + imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; + allow_internal_imports=true)) + # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: + @test all(==(LinearAlgebra), imps.importing_from) end - @test import_type_pairs == - [:Exporter => :import_LHS, - :exported_a => :import_RHS, - :exported_c => :import_RHS, - :Exporter => :import_LHS, - :exported_c => :import_RHS, - :TestModA => :blanket_using_member, - :SubModB => :blanket_using, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h2 => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h3 => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h => :import_RHS, - :Exporter => :blanket_using, - :Exporter => :plain_import, - :LinearAlgebra => :import_LHS, - :map => :import_RHS, - :_svd! => :import_RHS, - :LinearAlgebra => :import_LHS, - :svd => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :exported_b => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :f => :import_RHS] - - inds = findall(==(:import_RHS), analyze_import_type.(leaves)) - lhs_rhs_pairs = get_import_lhs.(leaves[inds]) .=> get_val.(leaves[inds]) - @test lhs_rhs_pairs == [[:., :., :Exporter] => :exported_a, - [:., :., :Exporter] => :exported_c, - [:., :., :Exporter] => :exported_c, - [:., :., :TestModA, :SubModB] => :h2, - [:., :., :TestModA, :SubModB] => :h3, - [:., :., :TestModA, :SubModB] => :h, - [:LinearAlgebra] => :map, - [:LinearAlgebra] => :_svd!, - [:LinearAlgebra] => :svd, - [:., :., :TestModA, :SubModB] => :exported_b, - [:., :., :TestModA, :SubModB] => :f] - - imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; - allow_internal_imports=false)) - h_row = only(subset(imps, :name => ByRow(==(:h)))) - @test !h_row.public_import - # Note: if this fails locally, try `include("imports.jl")` to rebuild the module - @test h_row.whichmodule == TestModA.SubModB - @test h_row.importing_from == TestModA.SubModB - - h2_row = only(subset(imps, :name => ByRow(==(:h2)))) - @test h2_row.public_import - @test h2_row.whichmodule === TestModA.SubModB - @test h2_row.importing_from == TestModA.SubModB - _svd!_row = only(subset(imps, :name => ByRow(==(:_svd!)))) - @test !_svd!_row.public_import - - f_row = only(subset(imps, :name => ByRow(==(:f)))) - @test !f_row.public_import # not public in `TestModA.SubModB` - @test f_row.whichmodule == TestModA - @test f_row.importing_from == TestModA.SubModB - - imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; - allow_internal_imports=true)) - # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: - @test all(==(LinearAlgebra), imps.importing_from) -end -##### -##### To analyze a test case -##### -# using ExplicitImports: js_node, get_parent, kind, parents_match -# using JuliaSyntax: @K_str - -# cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")); -# leaves = collect(Leaves(cursor)) -# leaf = leaves[end - 2] # select a leaf -# js_node(leaf) # inspect it -# p = js_node(get_parent(leaf, 3)) # see the tree, etc -# kind(p) - -@testset "qualified access" begin - # analyze_qualified_names - qualified = analyze_qualified_names(TestQualifiedAccess, "test_qualified_access.jl") - @test length(qualified) == 6 - ABC, DEF, HIJ, X, map, x = qualified - @test ABC.name == :ABC - @test DEF.public_access - @test HIJ.public_access - @test DEF.name == :DEF - @test HIJ.name == :HIJ - @test X.name == :X - @test map.name == :map - @test x.name == :x - @test x.self_qualified - - # improper_qualified_accesses - ret = Dict(improper_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false)) - @test isempty(ret[TestQualifiedAccess.Bar]) - @test isempty(ret[TestQualifiedAccess.FooModule]) - @test !isempty(ret[TestQualifiedAccess]) - - @test length(ret[TestQualifiedAccess]) == 4 - ABC, X, map, x = ret[TestQualifiedAccess] - # Can add keys, but removing them is breaking - @test keys(ABC) ⊇ - [:name, :location, :value, :accessing_from, :whichmodule, :public_access, - :accessing_from_owns_name, :accessing_from_submodule_owns_name, :internal_access] - @test ABC.name == :ABC - @test ABC.location isa AbstractString - @test ABC.whichmodule == TestQualifiedAccess.Bar - @test ABC.accessing_from == TestQualifiedAccess.FooModule - @test ABC.public_access == false - @test ABC.accessing_from_submodule_owns_name == false - - @test X.name == :X - @test X.whichmodule == TestQualifiedAccess.FooModule.FooSub - @test X.accessing_from == TestQualifiedAccess.FooModule - @test X.public_access == false - @test X.accessing_from_submodule_owns_name == true - - @test map.name == :map - - @test x.name == :x - @test x.self_qualified - - imps = DataFrame(improper_qualified_accesses_nonrecursive(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=true)) - subset!(imps, :self_qualified => ByRow(!)) # drop self-qualified - # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: - @test all(==(LinearAlgebra), imps.accessing_from) - - # check_no_self_qualified_accesses - ex = SelfQualifiedAccessException - @test_throws ex check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl") - - str = exception_string() do - return check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl") - end - @test contains(str, "has self-qualified accesses:\n- `x` was accessed as") - - @test check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl"; ignore=(:x,)) === - nothing - - str = sprint(print_explicit_imports, TestQualifiedAccess, - "test_qualified_access.jl") - @test contains(str, "has 1 self-qualified access:\n\n • x was accessed as ") - - # check_all_qualified_accesses_via_owners - ex = QualifiedAccessesFromNonOwnerException - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - end - @test contains(str, - "has qualified accesses to names via modules other than their owner as determined") + ##### + ##### To analyze a test case + ##### + # using ExplicitImports: js_node, get_parent, kind, parents_match + # using JuliaSyntax: @K_str + + # cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")); + # leaves = collect(Leaves(cursor)) + # leaf = leaves[end - 2] # select a leaf + # js_node(leaf) # inspect it + # p = js_node(get_parent(leaf, 3)) # see the tree, etc + # kind(p) + + @testset "qualified access" begin + # analyze_qualified_names + qualified = analyze_qualified_names(TestQualifiedAccess, "test_qualified_access.jl") + @test length(qualified) == 6 + ABC, DEF, HIJ, X, map, x = qualified + @test ABC.name == :ABC + @test DEF.public_access + @test HIJ.public_access + @test DEF.name == :DEF + @test HIJ.name == :HIJ + @test X.name == :X + @test map.name == :map + @test x.name == :x + @test x.self_qualified + + # improper_qualified_accesses + ret = Dict(improper_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false)) + @test isempty(ret[TestQualifiedAccess.Bar]) + @test isempty(ret[TestQualifiedAccess.FooModule]) + @test !isempty(ret[TestQualifiedAccess]) + + @test length(ret[TestQualifiedAccess]) == 4 + ABC, X, map, x = ret[TestQualifiedAccess] + # Can add keys, but removing them is breaking + @test keys(ABC) ⊇ + [:name, :location, :value, :accessing_from, :whichmodule, :public_access, + :accessing_from_owns_name, :accessing_from_submodule_owns_name, + :internal_access] + @test ABC.name == :ABC + @test ABC.location isa AbstractString + @test ABC.whichmodule == TestQualifiedAccess.Bar + @test ABC.accessing_from == TestQualifiedAccess.FooModule + @test ABC.public_access == false + @test ABC.accessing_from_submodule_owns_name == false + + @test X.name == :X + @test X.whichmodule == TestQualifiedAccess.FooModule.FooSub + @test X.accessing_from == TestQualifiedAccess.FooModule + @test X.public_access == false + @test X.accessing_from_submodule_owns_name == true + + @test map.name == :map + + @test x.name == :x + @test x.self_qualified + + imps = DataFrame(improper_qualified_accesses_nonrecursive(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=true)) + subset!(imps, :self_qualified => ByRow(!)) # drop self-qualified + # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: + @test all(==(LinearAlgebra), imps.accessing_from) + + # check_no_self_qualified_accesses + ex = SelfQualifiedAccessException + @test_throws ex check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl") - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, ignore=(:map,), - allow_internal_accesses=false) === - nothing + str = exception_string() do + return check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl") + end + @test contains(str, "has self-qualified accesses:\n- `x` was accessed as") - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:ABC, :map), - allow_internal_accesses=false) === nothing + @test check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl"; ignore=(:x,)) === + nothing - # allow_internal_accesses=true - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl", - ignore=(:ABC,)) + str = sprint(print_explicit_imports, TestQualifiedAccess, + "test_qualified_access.jl") + @test contains(str, "has 1 self-qualified access:\n\n • x was accessed as ") - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:ABC, :map)) === nothing - - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, - require_submodule_access=true, - allow_internal_accesses=false) - - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar, - TestQualifiedAccess.FooModule => TestQualifiedAccess.FooModule.FooSub, - LinearAlgebra => Base) - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, - require_submodule_access=true, - allow_internal_accesses=false) === nothing - - # Printing via `print_explicit_imports` - str = sprint(io -> print_explicit_imports(io, TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "accesses 2 names from non-owner modules") - @test contains(str, "ABC has owner") - - ex = NonPublicQualifiedAccessException - @test_throws ex check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - str = exception_string() do - return check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - end - @test contains(str, "- `ABC` is not public in") + # check_all_qualified_accesses_via_owners + ex = QualifiedAccessesFromNonOwnerException + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + # Test the printing is hitting our formatted errors + str = exception_string() do + return check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + end + @test contains(str, + "has qualified accesses to names via modules other than their owner as determined") + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, ignore=(:map,), + allow_internal_accesses=false) === + nothing + + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:ABC, :map), + allow_internal_accesses=false) === + nothing + + # allow_internal_accesses=true + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl", + ignore=(:ABC,)) + + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:ABC, :map)) === nothing + + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, + require_submodule_access=true, + allow_internal_accesses=false) + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar, + TestQualifiedAccess.FooModule => TestQualifiedAccess.FooModule.FooSub, + LinearAlgebra => Base) + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, + require_submodule_access=true, + allow_internal_accesses=false) === + nothing + + # Printing via `print_explicit_imports` + str = sprint(io -> print_explicit_imports(io, TestQualifiedAccess, "test_qualified_access.jl"; - ignore=(:X, :ABC, :map), - allow_internal_accesses=false) === nothing - - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + allow_internal_accesses=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "accesses 2 names from non-owner modules") + @test contains(str, "ABC has owner") - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, ignore=(:X, :map), - allow_internal_accesses=false) === nothing + ex = NonPublicQualifiedAccessException + @test_throws ex check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + str = exception_string() do + return check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + end + @test contains(str, "- `ABC` is not public in") + + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:X, :ABC, :map), + allow_internal_accesses=false) === + nothing + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, ignore=(:X, :map), + allow_internal_accesses=false) === + nothing + + # allow_internal_accesses=true + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:map,)) === nothing + end - # allow_internal_accesses=true - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:map,)) === nothing -end + @testset "improper explicit imports" begin + imps = Dict(improper_explicit_imports(TestModA, "TestModA.jl"; + allow_internal_imports=false)) + row = only(imps[TestModA]) + @test row.name == :un_exported + @test row.whichmodule == Exporter + + row1, row2 = imps[TestModA.SubModB.TestModA.TestModC] + # Can add keys, but removing them is breaking + @test keys(row1) ⊇ + [:name, :location, :value, :importing_from, :whichmodule, :public_import, + :importing_from_owns_name, :importing_from_submodule_owns_name, :stale, + :internal_import] + @test row1.name == :exported_c + @test row1.stale == true + @test row2.name == :exported_d + @test row2.stale == true + + @test check_all_explicit_imports_via_owners(TestModA, "TestModA.jl"; + allow_internal_imports=false) === + nothing + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; + allow_internal_imports=false) + + # allow_internal_imports=true + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, + "imports.jl";) + @test check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; ignore=(:map,)) === + nothing + # Test the printing is hitting our formatted errors + str = exception_string() do + return check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; + allow_internal_imports=false) + end -@testset "improper explicit imports" begin - imps = Dict(improper_explicit_imports(TestModA, "TestModA.jl"; - allow_internal_imports=false)) - row = only(imps[TestModA]) - @test row.name == :un_exported - @test row.whichmodule == Exporter - - row1, row2 = imps[TestModA.SubModB.TestModA.TestModC] - # Can add keys, but removing them is breaking - @test keys(row1) ⊇ - [:name, :location, :value, :importing_from, :whichmodule, :public_import, - :importing_from_owns_name, :importing_from_submodule_owns_name, :stale, - :internal_import] - @test row1.name == :exported_c - @test row1.stale == true - @test row2.name == :exported_d - @test row2.stale == true - - @test check_all_explicit_imports_via_owners(TestModA, "TestModA.jl"; - allow_internal_imports=false) === nothing - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; - allow_internal_imports=false) - - # allow_internal_imports=true - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, - "imports.jl";) - @test check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; ignore=(:map,)) === nothing - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; - allow_internal_imports=false) + @test contains(str, + "explicit imports of names from modules other than their owner as determined ") + + @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; + ignore=(:exported_b, :f, :map), + allow_internal_imports=false) === + nothing + + # We can pass `skip` to ignore non-owning explicit imports from LinearAlgebra that are owned by Base + @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; + skip=(LinearAlgebra => Base,), + ignore=(:exported_b, :f), + allow_internal_imports=false) === + nothing + + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + allow_internal_imports=false) + + # test ignore + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC,), + allow_internal_imports=false) === + nothing + + # test skip + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + skip=(TestExplicitImports.FooModule => TestExplicitImports.Bar,), + allow_internal_imports=false) === + nothing + + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC,), + require_submodule_import=true, + allow_internal_imports=false) + + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC, :X), + require_submodule_import=true, + allow_internal_imports=false) === + nothing + + # allow_internal_imports = true + @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, + "imports.jl";) + @test check_all_explicit_imports_are_public(ModImports, + "imports.jl"; ignore=(:map, :_svd!)) === + nothing end - @test contains(str, - "explicit imports of names from modules other than their owner as determined ") - - @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; - ignore=(:exported_b, :f, :map), - allow_internal_imports=false) === nothing - - # We can pass `skip` to ignore non-owning explicit imports from LinearAlgebra that are owned by Base - @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; - skip=(LinearAlgebra => Base,), - ignore=(:exported_b, :f), - allow_internal_imports=false) === nothing - - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - allow_internal_imports=false) - - # test ignore - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC,), - allow_internal_imports=false) === nothing - - # test skip - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - skip=(TestExplicitImports.FooModule => TestExplicitImports.Bar,), - allow_internal_imports=false) === - nothing - - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC,), - require_submodule_import=true, - allow_internal_imports=false) - - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC, :X), - require_submodule_import=true, - allow_internal_imports=false) === nothing - - # allow_internal_imports = true - @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, - "imports.jl";) - @test check_all_explicit_imports_are_public(ModImports, - "imports.jl"; ignore=(:map, :_svd!)) === - nothing -end + @testset "structs" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) + @test map(get_val, filter(is_struct_type_param, leaves)) == [:X, :Y, :QR] -@testset "structs" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) - @test map(get_val, filter(is_struct_type_param, leaves)) == [:X, :Y, :QR] + @test map(get_val, filter(is_struct_field_name, leaves)) == [:x, :x, :x, :qr, :qr] - @test map(get_val, filter(is_struct_field_name, leaves)) == [:x, :x, :x, :qr, :qr] + # Tests #34 and #36 + @test using_statement.(explicit_imports_nonrecursive(TestMod5, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] + end - # Tests #34 and #36 - @test using_statement.(explicit_imports_nonrecursive(TestMod5, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] -end + if VERSION >= v"1.7-" + @testset "loops" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) + @test map(get_val, filter(is_for_arg, leaves)) == + [:i, :I, :j, :k, :k, :j, :xi, :yi] + + # Tests #35 + @test using_statement.(explicit_imports_nonrecursive(TestMod6, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] + end + end -if VERSION >= v"1.7-" - @testset "loops" begin + @testset "nested local scope" begin cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) leaves = collect(Leaves(cursor)) - @test map(get_val, filter(is_for_arg, leaves)) == [:i, :I, :j, :k, :k, :j, :xi, :yi] - - # Tests #35 - @test using_statement.(explicit_imports_nonrecursive(TestMod6, "test_mods.jl")) == + # Test nested local scope + @test using_statement.(explicit_imports_nonrecursive(TestMod7, "test_mods.jl")) == ["using LinearAlgebra: LinearAlgebra"] end -end -@testset "nested local scope" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) - # Test nested local scope - @test using_statement.(explicit_imports_nonrecursive(TestMod7, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] -end + @testset "types without values in function signatures" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/33 + @test using_statement.(explicit_imports_nonrecursive(TestMod8, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: QR"] + end -@testset "types without values in function signatures" begin - # https://github.com/ericphanson/ExplicitImports.jl/issues/33 - @test using_statement.(explicit_imports_nonrecursive(TestMod8, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: QR"] -end + @testset "generators" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) -@testset "generators" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) + v = [:i1, :I, :i2, :I, :i3, :I, :i4, :I] + w = [:i1, :I] + @test map(get_val, filter(is_generator_arg, leaves)) == + [v; v; w; w; w; w; w] - v = [:i1, :I, :i2, :I, :i3, :I, :i4, :I] - w = [:i1, :I] - @test map(get_val, filter(is_generator_arg, leaves)) == - [v; v; w; w; w; w; w] + if VERSION >= v"1.7-" + @test using_statement.(explicit_imports_nonrecursive(TestMod9, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] - if VERSION >= v"1.7-" - @test using_statement.(explicit_imports_nonrecursive(TestMod9, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] + per_usage_info, _ = analyze_all_names("test_mods.jl") + df = DataFrame(per_usage_info) + subset!(df, :module_path => ByRow(==([:TestMod9])), :name => ByRow(==(:i1))) + @test all(==(ExplicitImports.InternalGenerator), df.analysis_code) + end + end + + @testset "while loops" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod10, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: I"] per_usage_info, _ = analyze_all_names("test_mods.jl") df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod9])), :name => ByRow(==(:i1))) - @test all(==(ExplicitImports.InternalGenerator), df.analysis_code) + subset!(df, :module_path => ByRow(==([:TestMod10])), :name => ByRow(==(:I))) + # First one is internal, second one external + @test df.analysis_code == + [ExplicitImports.InternalAssignment, ExplicitImports.External] end -end -@testset "while loops" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod10, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: I"] - - per_usage_info, _ = analyze_all_names("test_mods.jl") - df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod10])), :name => ByRow(==(:I))) - # First one is internal, second one external - @test df.analysis_code == [ExplicitImports.InternalAssignment, ExplicitImports.External] -end + if VERSION >= v"1.7-" + @testset "do- syntax" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod11, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", + "using LinearAlgebra: Hermitian", + "using LinearAlgebra: svd"] + + per_usage_info, _ = analyze_all_names("test_mods.jl") + df = DataFrame(per_usage_info) + subset!(df, :module_path => ByRow(==([:TestMod11]))) + + I_codes = subset(df, :name => ByRow(==(:I))).analysis_code + @test I_codes == + [ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst] + svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code + @test svd_codes == + [ExplicitImports.InternalFunctionArg, ExplicitImports.External] + Hermitian_codes = subset(df, :name => ByRow(==(:Hermitian))).analysis_code + @test Hermitian_codes == + [ExplicitImports.External, ExplicitImports.IgnoredNonFirst] + end + end -if VERSION >= v"1.7-" - @testset "do- syntax" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod11, "test_mods.jl")) == + @testset "try-catch" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod12, "test_mods.jl")) == ["using LinearAlgebra: LinearAlgebra", - "using LinearAlgebra: Hermitian", + "using LinearAlgebra: I", "using LinearAlgebra: svd"] per_usage_info, _ = analyze_all_names("test_mods.jl") df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod11]))) + subset!(df, :module_path => ByRow(==([:TestMod12]))) I_codes = subset(df, :name => ByRow(==(:I))).analysis_code - @test I_codes == - [ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst] + @test I_codes == [ExplicitImports.InternalAssignment, + ExplicitImports.External, + ExplicitImports.External, + ExplicitImports.InternalAssignment, + ExplicitImports.InternalCatchArgument, + ExplicitImports.IgnoredNonFirst, + ExplicitImports.External] svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code - @test svd_codes == [ExplicitImports.InternalFunctionArg, ExplicitImports.External] - Hermitian_codes = subset(df, :name => ByRow(==(:Hermitian))).analysis_code - @test Hermitian_codes == [ExplicitImports.External, ExplicitImports.IgnoredNonFirst] + @test svd_codes == [ExplicitImports.InternalAssignment, + ExplicitImports.External, + ExplicitImports.InternalAssignment, + ExplicitImports.External] end -end - -@testset "try-catch" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod12, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", - "using LinearAlgebra: I", - "using LinearAlgebra: svd"] - - per_usage_info, _ = analyze_all_names("test_mods.jl") - df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod12]))) - - I_codes = subset(df, :name => ByRow(==(:I))).analysis_code - @test I_codes == [ExplicitImports.InternalAssignment, - ExplicitImports.External, - ExplicitImports.External, - ExplicitImports.InternalAssignment, - ExplicitImports.InternalCatchArgument, - ExplicitImports.IgnoredNonFirst, - ExplicitImports.External] - svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code - @test svd_codes == [ExplicitImports.InternalAssignment, - ExplicitImports.External, - ExplicitImports.InternalAssignment, - ExplicitImports.External] -end - -@testset "scripts" begin - str = sprint(print_explicit_imports_script, "script.jl") - str = replace(str, r"\s+" => " ") - @test contains(str, "Script script.jl") - @test contains(str, "relying on implicit imports for 1 name") - @test contains(str, "using LinearAlgebra: norm") - @test contains(str, "stale explicit imports for this 1 unused name") - @test contains(str, "• qr") -end - -@testset "Handle public symbols with same name as exported Base symbols (#88)" begin - statements = using_statement.(explicit_imports_nonrecursive(Mod88, "examples.jl")) - @test statements == ["using .ModWithTryparse: ModWithTryparse"] - -end -@testset "Don't skip source modules (#29)" begin - # In this case `UUID` is defined in Base but exported in UUIDs - ret = ExplicitImports.find_implicit_imports(Mod29)[:UUID] - @test ret.source == Base - @test ret.exporters == [UUIDs] - # We should NOT skip it, even though `skip` includes `Base`, since the exporters - # are not skipped. - statements = using_statement.(explicit_imports_nonrecursive(Mod29, "examples.jl")) - @test statements == ["using UUIDs: UUIDs", "using UUIDs: UUID"] -end - -@testset "Exported module (#24)" begin - statements = using_statement.(explicit_imports_nonrecursive(Mod24, "examples.jl")) - # The key thing here is we do not have `using .Exporter: exported_a`, - # since we haven't done `using .Exporter` in `Mod24`, only `using .Exporter2` - @test statements == ["using .Exporter2: Exporter2", "using .Exporter2: exported_a"] -end -@testset "string macros (#20)" begin - foo = only_name_source(explicit_imports_nonrecursive(Foo20, "examples.jl")) - @test foo == [(; name=:Markdown, source=Markdown), - (; name=Symbol("@doc_str"), source=Markdown)] - bar = explicit_imports_nonrecursive(Bar20, "examples.jl") - @test isempty(bar) -end - -@testset "TestModArgs" begin - # don't detect `a`! - statements = using_statement.(explicit_imports_nonrecursive(TestModArgs, - "TestModArgs.jl")) - @test statements == - ["using .Exporter4: Exporter4", "using .Exporter4: A", "using .Exporter4: Z"] + @testset "scripts" begin + str = sprint(print_explicit_imports_script, "script.jl") + str = replace(str, r"\s+" => " ") + @test contains(str, "Script script.jl") + @test contains(str, "relying on implicit imports for 1 name") + @test contains(str, "using LinearAlgebra: norm") + @test contains(str, "stale explicit imports for this 1 unused name") + @test contains(str, "• qr") + end - statements = using_statement.(explicit_imports_nonrecursive(ThreadPinning, - "examples.jl")) + @testset "Handle public symbols with same name as exported Base symbols (#88)" begin + statements = using_statement.(explicit_imports_nonrecursive(Mod88, "examples.jl")) + @test statements == ["using .ModWithTryparse: ModWithTryparse"] + end + @testset "Don't skip source modules (#29)" begin + # In this case `UUID` is defined in Base but exported in UUIDs + ret = ExplicitImports.find_implicit_imports(Mod29)[:UUID] + @test ret.source == Base + @test ret.exporters == [UUIDs] + # We should NOT skip it, even though `skip` includes `Base`, since the exporters + # are not skipped. + statements = using_statement.(explicit_imports_nonrecursive(Mod29, "examples.jl")) + @test statements == ["using UUIDs: UUIDs", "using UUIDs: UUID"] + end - @test statements == ["using LinearAlgebra: LinearAlgebra"] -end + @testset "Exported module (#24)" begin + statements = using_statement.(explicit_imports_nonrecursive(Mod24, "examples.jl")) + # The key thing here is we do not have `using .Exporter: exported_a`, + # since we haven't done `using .Exporter` in `Mod24`, only `using .Exporter2` + @test statements == ["using .Exporter2: Exporter2", "using .Exporter2: exported_a"] + end -@testset "is_function_definition_arg" begin - cursor = TreeCursor(SyntaxNodeWrapper("TestModArgs.jl")) - leaves = collect(Leaves(cursor)) - purported_function_args = filter(is_function_definition_arg, leaves) + @testset "string macros (#20)" begin + foo = only_name_source(explicit_imports_nonrecursive(Foo20, "examples.jl")) + @test foo == [(; name=:Markdown, source=Markdown), + (; name=Symbol("@doc_str"), source=Markdown)] + bar = explicit_imports_nonrecursive(Bar20, "examples.jl") + @test isempty(bar) + end - # written this way to get clearer test failure messages - vals = unique(get_val.(purported_function_args)) - @test vals == [:a] + @testset "TestModArgs" begin + # don't detect `a`! + statements = using_statement.(explicit_imports_nonrecursive(TestModArgs, + "TestModArgs.jl")) + @test statements == + ["using .Exporter4: Exporter4", "using .Exporter4: A", "using .Exporter4: Z"] - # we have 9*4 functions with one argument `a`, plus 2 macros - @test length(purported_function_args) == 9 * 4 + 2 - non_function_args = filter(!is_function_definition_arg, leaves) - missed = filter(x -> get_val(x) === :a, non_function_args) - @test isempty(missed) -end + statements = using_statement.(explicit_imports_nonrecursive(ThreadPinning, + "examples.jl")) -@testset "has_ancestor" begin - @test has_ancestor(TestModA.SubModB, TestModA) - @test !has_ancestor(TestModA, TestModA.SubModB) + @test statements == ["using LinearAlgebra: LinearAlgebra"] + end - @test should_skip(Base.Iterators; skip=(Base, Core)) -end + @testset "is_function_definition_arg" begin + cursor = TreeCursor(SyntaxNodeWrapper("TestModArgs.jl")) + leaves = collect(Leaves(cursor)) + purported_function_args = filter(is_function_definition_arg, leaves) -function get_per_scope(per_usage_info) - per_usage_df = DataFrame(per_usage_info) - dropmissing!(per_usage_df, :external_global_name) - return per_usage_df -end + # written this way to get clearer test failure messages + vals = unique(get_val.(purported_function_args)) + @test vals == [:a] -@testset "file not found" begin - for f in (check_no_implicit_imports, check_no_stale_explicit_imports, - check_all_explicit_imports_via_owners, check_all_qualified_accesses_via_owners, - explicit_imports, - explicit_imports_nonrecursive, print_explicit_imports, - improper_explicit_imports, improper_explicit_imports_nonrecursive, - improper_qualified_accesses, improper_qualified_accesses_nonrecursive) - @test_throws FileNotFoundException f(TestModA) + # we have 9*4 functions with one argument `a`, plus 2 macros + @test length(purported_function_args) == 9 * 4 + 2 + non_function_args = filter(!is_function_definition_arg, leaves) + missed = filter(x -> get_val(x) === :a, non_function_args) + @test isempty(missed) end - str = sprint(Base.showerror, FileNotFoundException()) - @test contains(str, "module which is not top-level in a package") -end -@testset "ExplicitImports.jl" begin - @test using_statement.(explicit_imports_nonrecursive(TestModA, "TestModA.jl")) == - ["using .Exporter: Exporter", "using .Exporter: @mac", - "using .Exporter2: Exporter2", - "using .Exporter2: exported_a", "using .Exporter3: Exporter3"] - - per_usage_info, _ = analyze_all_names("TestModA.jl") - df = get_per_scope(per_usage_info) - locals = contains.(string.(df.name), Ref("local")) - @test all(!, df.external_global_name[locals]) - - # we use `x` in two scopes - xs = subset(df, :name => ByRow(==(:x))) - @test !xs[1, :external_global_name] - @test !xs[2, :external_global_name] - @test xs[2, :analysis_code] == ExplicitImports.InternalAssignment - - # we use `exported_a` in two scopes; both times refer to the global name - exported_as = subset(df, :name => ByRow(==(:exported_a))) - @test exported_as[1, :external_global_name] - @test exported_as[2, :external_global_name] - @test !exported_as[2, :is_assignment] - - # Test submodules - @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB, "TestModA.jl")) == - ["using .Exporter3: Exporter3", "using .Exporter3: exported_b", - "using .TestModA: f"] - - mod_path = module_path(TestModA.SubModB) - @test mod_path == [:SubModB, :TestModA, :Main] - sub_df = restrict_to_module(df, TestModA.SubModB) - - h = only(subset(sub_df, :name => ByRow(==(:h)))) - @test h.external_global_name - @test !h.is_assignment - - # Nested submodule with same name as outer module... - @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA, - "TestModA.jl")) == - ["using .Exporter3: Exporter3", "using .Exporter3: exported_b"] - - # Check we are getting innermost names and not outer ones - subsub_df = restrict_to_module(df, TestModA.SubModB.TestModA) - @test :inner_h in subsub_df.name - @test :h ∉ subsub_df.name - # ...we do currently get the outer ones when the module path prefixes collide - @test_broken :f ∉ subsub_df.name - @test_broken :func ∉ subsub_df.name - - # starts from innermost - @test module_path(TestModA.SubModB.TestModA.TestModC) == - [:TestModC, :TestModA, :SubModB, :TestModA, :Main] - - from_outer_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModA.jl")) - from_inner_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl")) - @test from_inner_file == from_outer_file - @test "using .TestModA: f" in from_inner_file - # This one isn't needed bc all usages are fully qualified - @test "using .Exporter: exported_a" ∉ from_inner_file - - # This one isn't needed; it is already explicitly imported - @test "using .Exporter: exported_b" ∉ from_inner_file - - # This one shouldn't be there; we never use it, only explicitly import it. - # So actually it should be on a list of unnecessary imports. BUT it can show up - # because by importing it, we have the name in the file, so we used to detect it. - @test "using .Exporter: exported_c" ∉ from_inner_file - - @test from_inner_file == ["using .TestModA: TestModA", "using .TestModA: f"] - - ret = improper_explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - allow_internal_imports=false) - - @test [(; row.name) for row in ret if row.stale] == - [(; name=:exported_c), (; name=:exported_d)] - - # Recursive version - lookup = Dict(improper_explicit_imports(TestModA, - "TestModA.jl"; allow_internal_imports=false)) - ret = lookup[TestModA.SubModB.TestModA.TestModC] - - @test [(; row.name) for row in ret if row.stale] == - [(; name=:exported_c), (; name=:exported_d)] - @test isempty((row for row in lookup[TestModA] if row.stale)) - - per_usage_info, _ = analyze_all_names("TestModC.jl") - testmodc = DataFrame(per_usage_info) - qualified_row = only(subset(testmodc, :name => ByRow(==(:exported_a)))) - @test qualified_row.analysis_code == ExplicitImports.IgnoredQualified - @test qualified_row.qualified_by == [:Exporter] - - qualified_row2 = only(subset(testmodc, :name => ByRow(==(:h)))) - @test qualified_row2.qualified_by == [:TestModA, :SubModB] - - @test using_statement.(explicit_imports_nonrecursive(TestMod1, - "test_mods.jl")) == - ["using ExplicitImports: print_explicit_imports"] - - # Recursion - nested = explicit_imports(TestModA, "TestModA.jl") - @test nested isa Vector{Pair{Module, - Vector{@NamedTuple{name::Symbol,source::Module, - exporters::Vector{Module},location::String}}}} - @test TestModA in first.(nested) - @test TestModA.SubModB in first.(nested) - @test TestModA.SubModB.TestModA in first.(nested) - @test TestModA.SubModB.TestModA.TestModC in first.(nested) - - # Printing - # should be no logs - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "Module Main.TestModA is relying on implicit imports") - @test contains(str, "using .Exporter2: Exporter2, exported_a") - @test contains(str, - "However, module Main.TestModA.SubModB.TestModA.TestModC has stale explicit imports for these 2 unused names") - - # should be no logs - # try with linewidth tiny - should put one name per line - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - linewidth=0)) - @test contains(str, "using .Exporter2: Exporter2,\n exported_a") - - # test `show_locations=true` - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - show_locations=true, - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "using .Exporter3: Exporter3 # used at TestModA.jl:") - @test contains(str, "is unused but it was imported from Main.Exporter at TestModC.jl") - - # test `separate_lines=true`` - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - separate_lines=true, - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "using .Exporter3: Exporter3 using .Exporter3: exported_b") - - # `warn_improper_explicit_imports=false` does something (also still no logs) - str_no_warn = @test_logs sprint(io -> print_explicit_imports(io, TestModA, - "TestModA.jl"; - warn_improper_explicit_imports=false)) - str = replace(str, r"\s+" => " ") - @test length(str_no_warn) <= length(str) - - # in particular, this ensures we add `using Foo: Foo` as the first line - @test using_statement.(explicit_imports_nonrecursive(TestMod4, "test_mods.jl")) == - ["using .Exporter4: Exporter4" - "using .Exporter4: A" - "using .Exporter4: Z" - "using .Exporter4: a" - "using .Exporter4: z"] -end + @testset "has_ancestor" begin + @test has_ancestor(TestModA.SubModB, TestModA) + @test !has_ancestor(TestModA, TestModA.SubModB) -@testset "checks" begin - @test check_no_implicit_imports(TestModEmpty, "test_mods.jl") === nothing - @test check_no_stale_explicit_imports(TestModEmpty, "test_mods.jl") === nothing - @test check_no_stale_explicit_imports(TestMod1, "test_mods.jl") === nothing - - @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, - "test_mods.jl") - - # test name ignores - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports,)) === nothing - - # test name mod pair ignores - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports => ExplicitImports,)) === - nothing - - # if you pass the module in the pair, you must get the right one - @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, - "test_mods.jl"; - ignore=(:print_explicit_imports => TestModA,)) === - nothing - - # non-existent names are OK - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports => ExplicitImports, - :does_not_exist)) === nothing - - # you can use skip to skip whole modules - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - skip=(Base, Core, ExplicitImports)) === nothing - - @test_throws ImplicitImportsException check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") - - # test submodule ignores - @test check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, "TestModC.jl"; - ignore=(TestModA.SubModB.TestModA.TestModC,)) === - nothing - - @test_throws StaleImportsException check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") - - # make sure ignored names don't show up in error - e = try - check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - ignore=(:exported_d,)) - @test false # should error before this - catch e - e - end - str = sprint(Base.showerror, e) - @test contains(str, "exported_c") - @test !contains(str, "exported_d") - - # ignore works: - @test check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - ignore=(:exported_c, :exported_d)) === - nothing - - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_no_implicit_imports(TestMod1, "test_mods.jl") + @test should_skip(Base.Iterators; skip=(Base, Core)) end - @test contains(str, "is relying on the following implicit imports") - str = exception_string() do - return check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") + function get_per_scope(per_usage_info) + per_usage_df = DataFrame(per_usage_info) + dropmissing!(per_usage_df, :external_global_name) + return per_usage_df end - @test contains(str, "has stale (unused) explicit imports for:") - @test check_all_explicit_imports_are_public(TestMod1, "test_mods.jl") === nothing - @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, - "imports.jl") - str = exception_string() do - return check_all_explicit_imports_are_public(ModImports, "imports.jl") - end - @test contains(str, "`_svd!` is not public in `LinearAlgebra` but it was imported") - @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; - ignore=(:_svd!, :exported_b, :f, :h, :map)) === - nothing - - @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; - ignore=(:_svd!, :exported_b, :f, :h), - skip=(LinearAlgebra => Base,)) === - nothing - - @testset "Tainted modules" begin - # 3 dynamic include statements - l = (:warn, r"Dynamic") - log = (l, l, l) - - @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl")) == - [DynMod => nothing, DynMod.Hidden => nothing] - @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl"; - strict=false)) == - [DynMod => [(; name=:print_explicit_imports, - source=ExplicitImports)], - # Wrong! Missing explicit export - DynMod.Hidden => []] - - @test_logs log... @test explicit_imports_nonrecursive(DynMod, "DynMod.jl") === - nothing - - @test_logs log... @test only_name_source(explicit_imports_nonrecursive(DynMod, - "DynMod.jl"; - strict=false)) == - [(; name=:print_explicit_imports, source=ExplicitImports)] - - @test @test_logs log... improper_explicit_imports(DynMod, - "DynMod.jl") == - [DynMod => nothing, - DynMod.Hidden => nothing] - - @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, - "DynMod.jl") === - nothing - - @test_logs log... @test improper_explicit_imports(DynMod, - "DynMod.jl"; - strict=false) == - [DynMod => [], - # Wrong! Missing stale explicit export - DynMod.Hidden => []] - - @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, - "DynMod.jl"; - strict=false) == - [] - - str = @test_logs log... sprint(print_explicit_imports, DynMod, "DynMod.jl") - @test contains(str, "DynMod could not be accurately analyzed") - - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - # Ignore also works - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod,), - ignore=(DynMod.Hidden,)) === - nothing - - e = UnanalyzableModuleException - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, - "DynMod.jl") - - # Missed `Hidden` - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, - "DynMod.jl"; - allow_unanalyzable=(DynMod,),) - - @test_logs log... @test check_no_stale_explicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - @test_logs log... @test_throws e check_no_stale_explicit_imports(DynMod, - "DynMod.jl") - - str = sprint(Base.showerror, UnanalyzableModuleException(DynMod)) - @test contains(str, "was found to be unanalyzable") - - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod,)) + @testset "file not found" begin + for f in (check_no_implicit_imports, check_no_stale_explicit_imports, + check_all_explicit_imports_via_owners, check_all_qualified_accesses_via_owners, + explicit_imports, + explicit_imports_nonrecursive, print_explicit_imports, + improper_explicit_imports, improper_explicit_imports_nonrecursive, + improper_qualified_accesses, improper_qualified_accesses_nonrecursive) + @test_throws FileNotFoundException f(TestModA) + end + str = sprint(Base.showerror, FileNotFoundException()) + @test contains(str, "module which is not top-level in a package") end -end -@testset "Aqua" begin - Aqua.test_all(ExplicitImports; ambiguities=false) -end + @testset "ExplicitImports.jl" begin + @test using_statement.(explicit_imports_nonrecursive(TestModA, "TestModA.jl")) == + ["using .Exporter: Exporter", "using .Exporter: @mac", + "using .Exporter2: Exporter2", + "using .Exporter2: exported_a", "using .Exporter3: Exporter3"] + + per_usage_info, _ = analyze_all_names("TestModA.jl") + df = get_per_scope(per_usage_info) + locals = contains.(string.(df.name), Ref("local")) + @test all(!, df.external_global_name[locals]) + + # we use `x` in two scopes + xs = subset(df, :name => ByRow(==(:x))) + @test !xs[1, :external_global_name] + @test !xs[2, :external_global_name] + @test xs[2, :analysis_code] == ExplicitImports.InternalAssignment + + # we use `exported_a` in two scopes; both times refer to the global name + exported_as = subset(df, :name => ByRow(==(:exported_a))) + @test exported_as[1, :external_global_name] + @test exported_as[2, :external_global_name] + @test !exported_as[2, :is_assignment] + + # Test submodules + @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB, + "TestModA.jl")) == + ["using .Exporter3: Exporter3", "using .Exporter3: exported_b", + "using .TestModA: f"] + + mod_path = module_path(TestModA.SubModB) + @test mod_path == [:SubModB, :TestModA, :Main] + sub_df = restrict_to_module(df, TestModA.SubModB) + + h = only(subset(sub_df, :name => ByRow(==(:h)))) + @test h.external_global_name + @test !h.is_assignment + + # Nested submodule with same name as outer module... + @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA, + "TestModA.jl")) == + ["using .Exporter3: Exporter3", "using .Exporter3: exported_b"] + + # Check we are getting innermost names and not outer ones + subsub_df = restrict_to_module(df, TestModA.SubModB.TestModA) + @test :inner_h in subsub_df.name + @test :h ∉ subsub_df.name + # ...we do currently get the outer ones when the module path prefixes collide + @test_broken :f ∉ subsub_df.name + @test_broken :func ∉ subsub_df.name + + # starts from innermost + @test module_path(TestModA.SubModB.TestModA.TestModC) == + [:TestModC, :TestModA, :SubModB, :TestModA, :Main] + + from_outer_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModA.jl")) + from_inner_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl")) + @test from_inner_file == from_outer_file + @test "using .TestModA: f" in from_inner_file + # This one isn't needed bc all usages are fully qualified + @test "using .Exporter: exported_a" ∉ from_inner_file + + # This one isn't needed; it is already explicitly imported + @test "using .Exporter: exported_b" ∉ from_inner_file + + # This one shouldn't be there; we never use it, only explicitly import it. + # So actually it should be on a list of unnecessary imports. BUT it can show up + # because by importing it, we have the name in the file, so we used to detect it. + @test "using .Exporter: exported_c" ∉ from_inner_file + + @test from_inner_file == ["using .TestModA: TestModA", "using .TestModA: f"] + + ret = improper_explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + allow_internal_imports=false) -@testset "`inspect_session`" begin - # We just want to make sure we are robust enough that this doesn't error - big_str = with_logger(Logging.NullLogger()) do - return sprint(inspect_session) - end -end + @test [(; row.name) for row in ret if row.stale] == + [(; name=:exported_c), (; name=:exported_d)] + + # Recursive version + lookup = Dict(improper_explicit_imports(TestModA, + "TestModA.jl"; + allow_internal_imports=false)) + ret = lookup[TestModA.SubModB.TestModA.TestModC] + + @test [(; row.name) for row in ret if row.stale] == + [(; name=:exported_c), (; name=:exported_d)] + @test isempty((row for row in lookup[TestModA] if row.stale)) + + per_usage_info, _ = analyze_all_names("TestModC.jl") + testmodc = DataFrame(per_usage_info) + qualified_row = only(subset(testmodc, :name => ByRow(==(:exported_a)))) + @test qualified_row.analysis_code == ExplicitImports.IgnoredQualified + @test qualified_row.qualified_by == [:Exporter] + + qualified_row2 = only(subset(testmodc, :name => ByRow(==(:h)))) + @test qualified_row2.qualified_by == [:TestModA, :SubModB] + + @test using_statement.(explicit_imports_nonrecursive(TestMod1, + "test_mods.jl")) == + ["using ExplicitImports: print_explicit_imports"] + + # Recursion + nested = explicit_imports(TestModA, "TestModA.jl") + @test nested isa Vector{Pair{Module, + Vector{@NamedTuple{name::Symbol,source::Module, + exporters::Vector{Module},location::String}}}} + @test TestModA in first.(nested) + @test TestModA.SubModB in first.(nested) + @test TestModA.SubModB.TestModA in first.(nested) + @test TestModA.SubModB.TestModA.TestModC in first.(nested) + + # Printing + # should be no logs + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "Module Main.TestModA is relying on implicit imports") + @test contains(str, "using .Exporter2: Exporter2, exported_a") + @test contains(str, + "However, module Main.TestModA.SubModB.TestModA.TestModC has stale explicit imports for these 2 unused names") + + # should be no logs + # try with linewidth tiny - should put one name per line + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + linewidth=0)) + @test contains(str, "using .Exporter2: Exporter2,\n exported_a") + + # test `show_locations=true` + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + show_locations=true, + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "using .Exporter3: Exporter3 # used at TestModA.jl:") + @test contains(str, + "is unused but it was imported from Main.Exporter at TestModC.jl") + + # test `separate_lines=true`` + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + separate_lines=true, + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "using .Exporter3: Exporter3 using .Exporter3: exported_b") -@testset "backtick modules and locations" begin - @testset "print_explicit_imports" begin - # Test that module names and file:line locations are surrounded by backticks - # and that underscores in module and file names are printed and do not cause italics. - str = sprint() do io - print_explicit_imports(io, Test_Mod_Underscores, "Test_Mod_Underscores.jl"; report_non_public=true) - end + # `warn_improper_explicit_imports=false` does something (also still no logs) + str_no_warn = @test_logs sprint(io -> print_explicit_imports(io, TestModA, + "TestModA.jl"; + warn_improper_explicit_imports=false)) str = replace(str, r"\s+" => " ") - # stale import - @test contains(str, "Test_Mod_Underscores has stale explicit imports") - @test contains(str, "svd is unused but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # non-owner module - @test contains(str, "Test_Mod_Underscores explicitly imports 1 name from non-owner module") - @test contains(str, "map has owner Base but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # non-public name - @test contains(str, "Test_Mod_Underscores explicitly imports 1 non-public name") - @test contains(str, "_svd! is not public in LinearAlgebra but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # self-qualified access - @test contains(str, "Test_Mod_Underscores has 1 self-qualified access") - @test contains(str, "foo was accessed as Main.Test_Mod_Underscores.foo inside Main.TestModUnderscores at Test_Mod_Underscores.jl") - # access non-owner module - @test contains(str, "Test_Mod_Underscores accesses 1 name from non-owner modules") - @test contains(str, "Number has owner Base but it was accessed from Base.Sys at Test_Mod_Underscores.jl") - # access non-public name - @test contains(str, "Test_Mod_Underscores accesses 1 non-public name") - @test contains(str, "__unsafe_string! is not public in Base but it was accessed via Base at Test_Mod_Underscores.jl") + @test length(str_no_warn) <= length(str) + + # in particular, this ensures we add `using Foo: Foo` as the first line + @test using_statement.(explicit_imports_nonrecursive(TestMod4, "test_mods.jl")) == + ["using .Exporter4: Exporter4" + "using .Exporter4: A" + "using .Exporter4: Z" + "using .Exporter4: a" + "using .Exporter4: z"] end - @testset "check_*" begin - str = exception_string() do - check_no_implicit_imports(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + + @testset "checks" begin + @test check_no_implicit_imports(TestModEmpty, "test_mods.jl") === nothing + @test check_no_stale_explicit_imports(TestModEmpty, "test_mods.jl") === nothing + @test check_no_stale_explicit_imports(TestMod1, "test_mods.jl") === nothing + + @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, + "test_mods.jl") + + # test name ignores + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports,)) === nothing + + # test name mod pair ignores + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports => ExplicitImports,)) === + nothing + + # if you pass the module in the pair, you must get the right one + @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, + "test_mods.jl"; + ignore=(:print_explicit_imports => TestModA,)) === + nothing + + # non-existent names are OK + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports => ExplicitImports, + :does_not_exist)) === nothing + + # you can use skip to skip whole modules + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + skip=(Base, Core, ExplicitImports)) === nothing + + @test_throws ImplicitImportsException check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") + + # test submodule ignores + @test check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, "TestModC.jl"; + ignore=(TestModA.SubModB.TestModA.TestModC,)) === + nothing + + @test_throws StaleImportsException check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") + + # make sure ignored names don't show up in error + e = try + check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + ignore=(:exported_d,)) + @test false # should error before this + catch e + e end - @test contains(str, "`Main.Test_Mod_Underscores` is relying on") + str = sprint(Base.showerror, e) + @test contains(str, "exported_c") + @test !contains(str, "exported_d") + + # ignore works: + @test check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + ignore=(:exported_c, :exported_d)) === + nothing + + # Test the printing is hitting our formatted errors str = exception_string() do - check_all_explicit_imports_via_owners(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_no_implicit_imports(TestMod1, "test_mods.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + @test contains(str, "is relying on the following implicit imports") + str = exception_string() do - check_all_explicit_imports_are_public(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + @test contains(str, "has stale (unused) explicit imports for:") + + @test check_all_explicit_imports_are_public(TestMod1, "test_mods.jl") === nothing + @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, + "imports.jl") str = exception_string() do - check_no_stale_explicit_imports(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_all_explicit_imports_are_public(ModImports, "imports.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has stale") - str = exception_string() do - check_all_qualified_accesses_via_owners(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + @test contains(str, "`_svd!` is not public in `LinearAlgebra` but it was imported") + @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; + ignore=(:_svd!, :exported_b, :f, :h, + :map)) === + nothing + + @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; + ignore=(:_svd!, :exported_b, :f, :h), + skip=(LinearAlgebra => Base,)) === + nothing + + @testset "Tainted modules" begin + # 3 dynamic include statements + l = (:warn, r"Dynamic") + log = (l, l, l) + + @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl")) == + [DynMod => nothing, DynMod.Hidden => nothing] + @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl"; + strict=false)) == + [DynMod => [(; name=:print_explicit_imports, + source=ExplicitImports)], + # Wrong! Missing explicit export + DynMod.Hidden => []] + + @test_logs log... @test explicit_imports_nonrecursive(DynMod, "DynMod.jl") === + nothing + + @test_logs log... @test only_name_source(explicit_imports_nonrecursive(DynMod, + "DynMod.jl"; + strict=false)) == + [(; name=:print_explicit_imports, + source=ExplicitImports)] + + @test @test_logs log... improper_explicit_imports(DynMod, + "DynMod.jl") == + [DynMod => nothing, + DynMod.Hidden => nothing] + + @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, + "DynMod.jl") === + nothing + + @test_logs log... @test improper_explicit_imports(DynMod, + "DynMod.jl"; + strict=false) == + [DynMod => [], + # Wrong! Missing stale explicit export + DynMod.Hidden => []] + + @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, + "DynMod.jl"; + strict=false) == + [] + + str = @test_logs log... sprint(print_explicit_imports, DynMod, "DynMod.jl") + @test contains(str, "DynMod could not be accurately analyzed") + + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + # Ignore also works + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod,), + ignore=(DynMod.Hidden,)) === + nothing + + e = UnanalyzableModuleException + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, + "DynMod.jl") + + # Missed `Hidden` + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, + "DynMod.jl"; + allow_unanalyzable=(DynMod,),) + + @test_logs log... @test check_no_stale_explicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + @test_logs log... @test_throws e check_no_stale_explicit_imports(DynMod, + "DynMod.jl") + + str = sprint(Base.showerror, UnanalyzableModuleException(DynMod)) + @test contains(str, "was found to be unanalyzable") + + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod,)) end - @test contains(str, "Module `Main.Test_Mod_Underscores` has qualified accesses") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") - str = exception_string() do - check_all_qualified_accesses_are_public(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + end + + @testset "Aqua" begin + Aqua.test_all(ExplicitImports; ambiguities=false) + end + + @testset "`inspect_session`" begin + # We just want to make sure we are robust enough that this doesn't error + big_str = with_logger(Logging.NullLogger()) do + return sprint(inspect_session) end - @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports of") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") - str = exception_string() do - check_no_self_qualified_accesses(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + end + + @testset "backtick modules and locations" begin + @testset "print_explicit_imports" begin + # Test that module names and file:line locations are surrounded by backticks + # and that underscores in module and file names are printed and do not cause italics. + str = sprint() do io + return print_explicit_imports(io, Test_Mod_Underscores, + "Test_Mod_Underscores.jl"; + report_non_public=true) + end + str = replace(str, r"\s+" => " ") + # stale import + @test contains(str, "Test_Mod_Underscores has stale explicit imports") + @test contains(str, + "svd is unused but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # non-owner module + @test contains(str, + "Test_Mod_Underscores explicitly imports 1 name from non-owner module") + @test contains(str, + "map has owner Base but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # non-public name + @test contains(str, "Test_Mod_Underscores explicitly imports 1 non-public name") + @test contains(str, + "_svd! is not public in LinearAlgebra but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # self-qualified access + @test contains(str, "Test_Mod_Underscores has 1 self-qualified access") + @test contains(str, + "foo was accessed as Main.Test_Mod_Underscores.foo inside Main.TestModUnderscores at Test_Mod_Underscores.jl") + # access non-owner module + @test contains(str, + "Test_Mod_Underscores accesses 1 name from non-owner modules") + @test contains(str, + "Number has owner Base but it was accessed from Base.Sys at Test_Mod_Underscores.jl") + # access non-public name + @test contains(str, "Test_Mod_Underscores accesses 1 non-public name") + @test contains(str, + "__unsafe_string! is not public in Base but it was accessed via Base at Test_Mod_Underscores.jl") + end + @testset "check_*" begin + str = exception_string() do + return check_no_implicit_imports(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "`Main.Test_Mod_Underscores` is relying on") + str = exception_string() do + return check_all_explicit_imports_via_owners(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_all_explicit_imports_are_public(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_no_stale_explicit_imports(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has stale") + str = exception_string() do + return check_all_qualified_accesses_via_owners(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has qualified accesses") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_all_qualified_accesses_are_public(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, + "Module `Main.Test_Mod_Underscores` has explicit imports of") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_no_self_qualified_accesses(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, + "Module `Main.Test_Mod_Underscores` has self-qualified accesses") + @test contains(str, + r"accessed as `Main.Test_Mod_Underscores.foo` inside `Main.Test_Mod_Underscores` at `Test_Mod_Underscores.jl:10:40`") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has self-qualified accesses") - @test contains(str, r"accessed as `Main.Test_Mod_Underscores.foo` inside `Main.Test_Mod_Underscores` at `Test_Mod_Underscores.jl:10:40`") end end From c14d8b44018e6c9dcc86dae40ca69a63edf378b8 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:39:15 +0200 Subject: [PATCH 02/19] gitignore for ]dev --local --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1c02e5e..603f9ec 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ *.jl.mem Manifest.toml /docs/build/ +dev From 3ec279df467b286238a4d305759eef2407b66900 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:43:02 +0200 Subject: [PATCH 03/19] undo simplify hashing hack --- src/get_names_used.jl | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 2373a84..9748580 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -490,11 +490,6 @@ function analyze_all_names(file) return analyze_per_usage_info(per_usage_info), untainted_modules end -# this would ideally be identity, but hashing SyntaxNode's is slow on v1.0.2 -# https://github.com/JuliaLang/JuliaSyntax.jl/issues/558 -# so we will settle for some unlikely chance at collisions and just check the string rep of the values -_simplify_hashing(scope_path) = map(string ∘ get_val, scope_path) - function is_name_internal_in_higher_local_scope(name, scope_path, seen) # We will recurse up the `scope_path`. Note the order is "reversed", # so the first entry of `scope_path` is deepest. @@ -507,7 +502,7 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) end # Ok, now pop off the first scope and check. scope_path = scope_path[2:end] - ret = get(seen, (; name, scope_path=_simplify_hashing(scope_path)), nothing) + ret = get(seen, (; name, scope_path), nothing) if ret === nothing # Not introduced here yet, trying recursing further continue @@ -528,9 +523,9 @@ function analyze_per_usage_info(per_usage_info) # Otherwise, we are in local scope: # 1. Next, if the name is a function arg, then this is not a global name (essentially first usage is assignment) # 2. Otherwise, if first usage is assignment, then it is local, otherwise it is global - seen = Dict{@NamedTuple{name::Symbol,scope_path::Vector{String}},Bool}() + seen = IdDict{@NamedTuple{name::Symbol,scope_path::Vector{JuliaSyntax.SyntaxNode}},Bool}() return map(per_usage_info) do nt - @compat if (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) in keys(seen) + @compat if (; nt.name, nt.scope_path) in keys(seen) return PerUsageInfo(; nt..., first_usage_in_scope=false, external_global_name=missing, analysis_code=IgnoredNonFirst) @@ -563,7 +558,7 @@ function analyze_per_usage_info(per_usage_info) if is_local external_global_name = false push!(seen, - (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) + (; nt.name, nt.scope_path) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=reason) @@ -575,14 +570,14 @@ function analyze_per_usage_info(per_usage_info) seen) external_global_name = false push!(seen, - (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) + (; nt.name, nt.scope_path) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=InternalHigherScope) end external_global_name = true push!(seen, - (; nt.name, scope_path=_simplify_hashing(nt.scope_path)) => external_global_name) + (; nt.name, nt.scope_path) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=External) end From e0c4f8df175c5e61258370265f84aab55c071154 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:43:54 +0200 Subject: [PATCH 04/19] pkg roundtrip --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 9c440d0..a2bca17 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ExplicitImports" uuid = "7d51a73a-1435-4ff3-83d9-f097790105c7" -version = "1.11.3" authors = ["Eric P. Hanson"] +version = "1.11.3" [deps] AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c" From 27fcf8d15b9c0502bb54714b42e364f414e73674 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 23:27:52 +0200 Subject: [PATCH 05/19] improve debugging --- src/improper_qualified_accesses.jl | 5 ++++- src/parse_utilities.jl | 11 +++++++++++ test/runtests.jl | 11 +++++++---- test/start_tests.jl | 8 ++++++++ 4 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 test/start_tests.jl diff --git a/src/improper_qualified_accesses.jl b/src/improper_qualified_accesses.jl index b9754dd..8589ead 100644 --- a/src/improper_qualified_accesses.jl +++ b/src/improper_qualified_accesses.jl @@ -11,9 +11,12 @@ function analyze_qualified_names(mod::Module, file=pathof(mod); # something there would invalidate the qualified names with issues we did find. # For now let's ignore it. + @debug "[analyze_qualified_names] per_usage_info has $(length(per_usage_info)) rows" # Filter to qualified names qualified = [row for row in per_usage_info if row.qualified_by !== nothing] + @debug "[analyze_qualified_names] qualified has $(length(qualified)) rows" + # which are in our module mod_path = module_path(mod) match = module_path -> all(Base.splat(isequal), zip(module_path, mod_path)) @@ -54,7 +57,7 @@ end function process_qualified_row(row, mod) # for JET @assert !isnothing(row.qualified_by) - + isempty(row.qualified_by) && return nothing current_mod = mod for submod in row.qualified_by diff --git a/src/parse_utilities.jl b/src/parse_utilities.jl index cfab037..2c4e019 100644 --- a/src/parse_utilities.jl +++ b/src/parse_utilities.jl @@ -139,6 +139,17 @@ function parents_match(n::TreeCursor, kinds::Tuple) return parents_match(p, Base.tail(kinds)) end + +function parent_kinds(n::TreeCursor) + kinds = [] + while true + n = parent(n) + n === nothing && return kinds + push!(kinds, kind(n)) + end + return kinds +end + function get_parent(n, i=1) for _ in i:-1:1 n = parent(n) diff --git a/test/runtests.jl b/test/runtests.jl index 45707fa..5f902b4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -25,7 +25,9 @@ function exception_string(f) catch e sprint(showerror, e) end - @test str isa String + if str === false + error("Expected `f` to throw an exception, but it did not") + end return str end @@ -1046,9 +1048,10 @@ include("Test_Mod_Underscores.jl") end end - @testset "Aqua" begin - Aqua.test_all(ExplicitImports; ambiguities=false) - end + # TODO reenable this + # @testset "Aqua" begin + # Aqua.test_all(ExplicitImports; ambiguities=false) + # end @testset "`inspect_session`" begin # We just want to make sure we are robust enough that this doesn't error diff --git a/test/start_tests.jl b/test/start_tests.jl new file mode 100644 index 0000000..5eef013 --- /dev/null +++ b/test/start_tests.jl @@ -0,0 +1,8 @@ +using TestEnv # assumed globally installed +using Pkg +Pkg.activate(joinpath(@__DIR__, "..")) +TestEnv.activate() +using ExplicitImports + +cd(joinpath(pkgdir(ExplicitImports), "test")) +include("runtests.jl") From b6a5e4e033a64a867e3d00fd6233c139e5a8ed6e Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 23:28:09 +0200 Subject: [PATCH 06/19] change for quoting in qualified names (JS#324) --- src/get_names_used.jl | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 9748580..e203f1f 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -23,6 +23,8 @@ Base.@kwdef struct PerUsageInfo analysis_code::AnalysisCode end +Base.show(io::IO, r::PerUsageInfo) = print(io, "PerUsageInfo (`$(r.name)` @ $(r.location), `qualified_by`=$(r.qualified_by))") + function Base.NamedTuple(r::PerUsageInfo) names = fieldnames(typeof(r)) return NamedTuple{names}(map(x -> getfield(r, x), names)) @@ -53,12 +55,18 @@ end # returns `nothing` for no qualifying module, otherwise a symbol function qualifying_module(leaf) + @debug "[qualifying_module] leaf: $(js_node(leaf)) start" + # introspect leaf and its tree of parents + @debug "[qualifying_module] leaf: $(js_node(leaf)) parents: $(parent_kinds(leaf))" + # is this name being used in a qualified context, like `X.y`? - parents_match(leaf, (K"quote", K".")) || return nothing + parents_match(leaf, (K".",)) || return nothing + @debug "[qualifying_module] leaf: $(js_node(leaf)) passed dot" # Are we on the right-hand side? - child_index(parent(leaf)) == 2 || return nothing + child_index(leaf) == 2 || return nothing + @debug "[qualifying_module] leaf: $(js_node(leaf)) passed right-hand side" # Ok, now try to retrieve the child on the left-side - node = first(AbstractTrees.children(get_parent(leaf, 2))) + node = first(AbstractTrees.children(parent(leaf))) path = Symbol[] retrieve_module_path!(path, node) return path From b3871aa7b102893bf594a32e2ce86ad7a6580034 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Thu, 12 Jun 2025 23:56:19 +0200 Subject: [PATCH 07/19] wip: hashing fix --- src/get_names_used.jl | 44 +++++++++++++++++++++++++++++++++++++------ test/runtests.jl | 6 ++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index e203f1f..1748124 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -499,6 +499,10 @@ function analyze_all_names(file) end function is_name_internal_in_higher_local_scope(name, scope_path, seen) + if name == :QR + println("--------------------------------") + println("is_name_internal_in_higher_local_scope: $name, length(scope_path)=$(length(scope_path)), scope_path= $(map(nodevalue,scope_path)))") + end # We will recurse up the `scope_path`. Note the order is "reversed", # so the first entry of `scope_path` is deepest. @@ -510,7 +514,10 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) end # Ok, now pop off the first scope and check. scope_path = scope_path[2:end] - ret = get(seen, (; name, scope_path), nothing) + ret = get(seen, (; name, scope_path=SyntaxNodeList(scope_path)), nothing) + if name == :QR + println("[is_name_internal_in_higher_local_scope] ret: $ret, length(scope_path): $(length(scope_path))") + end if ret === nothing # Not introduced here yet, trying recursing further continue @@ -523,6 +530,31 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) return false end +struct SyntaxNodeList + nodes::Vector{JuliaSyntax.SyntaxNode} +end + +Base.:(==)(a::SyntaxNodeList, b::SyntaxNodeList) = a.nodes == b.nodes + +function syntax_node_weak_hash(node::JuliaSyntax.SyntaxNode, h::UInt) + # We want to hash the node cheaply instead of fully recursively + h = hash(kind(node), h) + for child in js_children(node) + h = hash(kind(child), h) + h = hash(length(js_children(child)), h) + end + return h +end +function Base.hash(a::SyntaxNodeList, h::UInt) + # We implement a workaround for https://github.com/JuliaLang/JuliaSyntax.jl/issues/558 + # There, hashing is extremely slow on SyntaxNode since it was changed to be fully recursive + # We can tolerate some collisions, so we want to implement our own hashing that is not fully recursive. + for node in a.nodes + h = syntax_node_weak_hash(node, h) + end + return h +end + function analyze_per_usage_info(per_usage_info) # For each scope, we want to understand if there are any global usages of the name in that scope # First, throw away all qualified usages, they are irrelevant @@ -531,9 +563,9 @@ function analyze_per_usage_info(per_usage_info) # Otherwise, we are in local scope: # 1. Next, if the name is a function arg, then this is not a global name (essentially first usage is assignment) # 2. Otherwise, if first usage is assignment, then it is local, otherwise it is global - seen = IdDict{@NamedTuple{name::Symbol,scope_path::Vector{JuliaSyntax.SyntaxNode}},Bool}() + seen = Dict{@NamedTuple{name::Symbol,scope_path::SyntaxNodeList},Bool}() return map(per_usage_info) do nt - @compat if (; nt.name, nt.scope_path) in keys(seen) + @compat if (; nt.name, scope_path=SyntaxNodeList(nt.scope_path)) in keys(seen) return PerUsageInfo(; nt..., first_usage_in_scope=false, external_global_name=missing, analysis_code=IgnoredNonFirst) @@ -566,7 +598,7 @@ function analyze_per_usage_info(per_usage_info) if is_local external_global_name = false push!(seen, - (; nt.name, nt.scope_path) => external_global_name) + (; nt.name, scope_path=SyntaxNodeList(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=reason) @@ -578,14 +610,14 @@ function analyze_per_usage_info(per_usage_info) seen) external_global_name = false push!(seen, - (; nt.name, nt.scope_path) => external_global_name) + (; nt.name, scope_path=SyntaxNodeList(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=InternalHigherScope) end external_global_name = true push!(seen, - (; nt.name, nt.scope_path) => external_global_name) + (; nt.name, scope_path=SyntaxNodeList(nt.scope_path)) => external_global_name) return PerUsageInfo(; nt..., first_usage_in_scope=true, external_global_name, analysis_code=External) end diff --git a/test/runtests.jl b/test/runtests.jl index 5f902b4..bf0fde2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -498,6 +498,12 @@ include("Test_Mod_Underscores.jl") @test map(get_val, filter(is_struct_field_name, leaves)) == [:x, :x, :x, :qr, :qr] + df = DataFrame(get_names_used("test_mods.jl").per_usage_info) + subset!(df, :name => ByRow(==(:QR)), :module_path => ByRow(==([:TestMod5]))) + # two uses of QR: one is as a type param, the other a usage of that type param in the same struct definition + @test df.analysis_code == [ExplicitImports.InternalStruct, ExplicitImports.IgnoredNonFirst] + @test df.struct_field_or_type_param == [true, false] + # Tests #34 and #36 @test using_statement.(explicit_imports_nonrecursive(TestMod5, "test_mods.jl")) == ["using LinearAlgebra: LinearAlgebra"] From 3303b8c22d25d6ea6540aa8862b983468a385dbc Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:07:35 +0200 Subject: [PATCH 08/19] switch to object identity --- src/get_names_used.jl | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 1748124..6e60606 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -499,10 +499,6 @@ function analyze_all_names(file) end function is_name_internal_in_higher_local_scope(name, scope_path, seen) - if name == :QR - println("--------------------------------") - println("is_name_internal_in_higher_local_scope: $name, length(scope_path)=$(length(scope_path)), scope_path= $(map(nodevalue,scope_path)))") - end # We will recurse up the `scope_path`. Note the order is "reversed", # so the first entry of `scope_path` is deepest. @@ -515,9 +511,6 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) # Ok, now pop off the first scope and check. scope_path = scope_path[2:end] ret = get(seen, (; name, scope_path=SyntaxNodeList(scope_path)), nothing) - if name == :QR - println("[is_name_internal_in_higher_local_scope] ret: $ret, length(scope_path): $(length(scope_path))") - end if ret === nothing # Not introduced here yet, trying recursing further continue @@ -530,29 +523,19 @@ function is_name_internal_in_higher_local_scope(name, scope_path, seen) return false end +# We implement a workaround for https://github.com/JuliaLang/JuliaSyntax.jl/issues/558 +# Hashing and equality for SyntaxNodes were changed from object identity to a recursive comparison +# in JuliaSyntax 1.0. This is very slow and also not quite the semantics we want anyway. +# Here, we wrap our nodes in a custom type that only compares object identity. struct SyntaxNodeList nodes::Vector{JuliaSyntax.SyntaxNode} end -Base.:(==)(a::SyntaxNodeList, b::SyntaxNodeList) = a.nodes == b.nodes +Base.:(==)(a::SyntaxNodeList, b::SyntaxNodeList) = map(objectid, a.nodes) == map(objectid, b.nodes) +Base.isequal(a::SyntaxNodeList, b::SyntaxNodeList) = isequal(map(objectid, a.nodes), map(objectid, b.nodes)) -function syntax_node_weak_hash(node::JuliaSyntax.SyntaxNode, h::UInt) - # We want to hash the node cheaply instead of fully recursively - h = hash(kind(node), h) - for child in js_children(node) - h = hash(kind(child), h) - h = hash(length(js_children(child)), h) - end - return h -end function Base.hash(a::SyntaxNodeList, h::UInt) - # We implement a workaround for https://github.com/JuliaLang/JuliaSyntax.jl/issues/558 - # There, hashing is extremely slow on SyntaxNode since it was changed to be fully recursive - # We can tolerate some collisions, so we want to implement our own hashing that is not fully recursive. - for node in a.nodes - h = syntax_node_weak_hash(node, h) - end - return h + return hash(map(objectid, a.nodes), h) end function analyze_per_usage_info(per_usage_info) From 010701974a41f50d99777452906a7f6c15b76ac3 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:17:00 +0200 Subject: [PATCH 09/19] fix for and generator iteration --- src/get_names_used.jl | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 6e60606..5b03c2c 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -23,7 +23,10 @@ Base.@kwdef struct PerUsageInfo analysis_code::AnalysisCode end -Base.show(io::IO, r::PerUsageInfo) = print(io, "PerUsageInfo (`$(r.name)` @ $(r.location), `qualified_by`=$(r.qualified_by))") +function Base.show(io::IO, r::PerUsageInfo) + return print(io, + "PerUsageInfo (`$(r.name)` @ $(r.location), `qualified_by`=$(r.qualified_by))") +end function Base.NamedTuple(r::PerUsageInfo) names = fieldnames(typeof(r)) @@ -278,7 +281,16 @@ function in_for_argument_position(node) # We must be on the LHS of a `for` `equal`. if !has_parent(node, 2) return false - elseif parents_match(node, (K"iteration", K"for")) + elseif parents_match(node, (K"in", K"iteration", K"for")) + @debug """ + [in_for_argument_position] node: $(js_node(node)) + parents: $(parent_kinds(node)) + child_index=$(child_index(node)) + parent_child_index=$(child_index(get_parent(node, 1))) + parent_child_index2=$(child_index(get_parent(node, 2))) + """ + + # child_index(node) == 1 means we are the first argument of the `in`, like `yi in y` return child_index(node) == 1 elseif kind(parent(node)) in (K"tuple", K"parameters") return in_for_argument_position(get_parent(node)) @@ -302,8 +314,8 @@ function in_generator_arg_position(node) # (possibly inside a filter, possibly inside a `iteration`) if !has_parent(node, 2) return false - elseif parents_match(node, (K"iteration", K"generator")) || - parents_match(node, (K"iteration", K"filter")) + elseif parents_match(node, (K"in", K"iteration", K"generator")) || + parents_match(node, (K"in", K"iteration", K"filter")) return child_index(node) == 1 elseif kind(parent(node)) in (K"tuple", K"parameters") return in_generator_arg_position(get_parent(node)) @@ -531,8 +543,12 @@ struct SyntaxNodeList nodes::Vector{JuliaSyntax.SyntaxNode} end -Base.:(==)(a::SyntaxNodeList, b::SyntaxNodeList) = map(objectid, a.nodes) == map(objectid, b.nodes) -Base.isequal(a::SyntaxNodeList, b::SyntaxNodeList) = isequal(map(objectid, a.nodes), map(objectid, b.nodes)) +function Base.:(==)(a::SyntaxNodeList, b::SyntaxNodeList) + return map(objectid, a.nodes) == map(objectid, b.nodes) +end +function Base.isequal(a::SyntaxNodeList, b::SyntaxNodeList) + return isequal(map(objectid, a.nodes), map(objectid, b.nodes)) +end function Base.hash(a::SyntaxNodeList, h::UInt) return hash(map(objectid, a.nodes), h) From a3afdec13db1b38b06dd30376072b82736ded65a Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:17:56 +0200 Subject: [PATCH 10/19] stricter --- src/get_names_used.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 5b03c2c..ed9947e 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -279,7 +279,7 @@ end # https://github.com/JuliaLang/JuliaSyntax.jl/issues/432 function in_for_argument_position(node) # We must be on the LHS of a `for` `equal`. - if !has_parent(node, 2) + if !has_parent(node, 3) return false elseif parents_match(node, (K"in", K"iteration", K"for")) @debug """ @@ -312,7 +312,7 @@ end function in_generator_arg_position(node) # We must be on the LHS of a `=` inside a generator # (possibly inside a filter, possibly inside a `iteration`) - if !has_parent(node, 2) + if !has_parent(node, 3) return false elseif parents_match(node, (K"in", K"iteration", K"generator")) || parents_match(node, (K"in", K"iteration", K"filter")) From 73c8b4b6c85dc25b59f3b4c4052e387ea060af18 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:21:14 +0200 Subject: [PATCH 11/19] inline function defs do not have K"=" anymore --- src/get_names_used.jl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index ed9947e..940ebe7 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -242,10 +242,6 @@ function call_is_func_def(node) # note: macros only support full-form function definitions # (not inline) kind(p) in (K"function", K"macro") && return true - if kind(p) == K"=" - # call should be the first arg in an inline function def - return child_index(node) == 1 - end return false end From 43f6b77e455f3529dcb24e2875bcc39f4267dfcd Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:28:05 +0200 Subject: [PATCH 12/19] fix do-blocks --- src/get_names_used.jl | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/get_names_used.jl b/src/get_names_used.jl index 940ebe7..1219364 100644 --- a/src/get_names_used.jl +++ b/src/get_names_used.jl @@ -150,8 +150,8 @@ function is_anonymous_do_function_definition_arg(leaf) if !has_parent(leaf, 2) return false elseif parents_match(leaf, (K"tuple", K"do")) - # second argument of `do`-block - return child_index(parent(leaf)) == 2 + # first argument of `do`-block (args then function body since JuliaSyntax 1.0) + return child_index(parent(leaf)) == 1 elseif kind(parent(leaf)) in (K"tuple", K"parameters") # Ok, let's just step up one level and see again return is_anonymous_do_function_definition_arg(parent(leaf)) @@ -374,10 +374,7 @@ function analyze_name(leaf; debug=false) # Constructs that start a new local scope. Note `let` & `macro` *arguments* are not explicitly supported/tested yet, # but we can at least keep track of scope properly. if k in - (K"let", K"for", K"function", K"struct", K"generator", K"while", K"macro") || - # Or do-block when we are considering a path that did not go through the first-arg - # (which is the function name, and NOT part of the local scope) - (k == K"do" && child_index(prev_node) > 1) || + (K"let", K"for", K"function", K"struct", K"generator", K"while", K"macro", K"do") || # any child of `try` gets it's own individual scope (I think) (parents_match(node, (K"try",))) push!(scope_path, nodevalue(node).node) From 1099a73a89594b5d2b7506d1ace910ac33d95408 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:31:26 +0200 Subject: [PATCH 13/19] bump version --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index a2bca17..d1db8fe 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ExplicitImports" uuid = "7d51a73a-1435-4ff3-83d9-f097790105c7" authors = ["Eric P. Hanson"] -version = "1.11.3" +version = "1.12.0" [deps] AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c" From 9bb164ad0cf0a26e557944d803e716daea730487 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 00:41:42 +0200 Subject: [PATCH 14/19] support new error on 1.12 --- src/find_implicit_imports.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/find_implicit_imports.jl b/src/find_implicit_imports.jl index 490f78e..9769669 100644 --- a/src/find_implicit_imports.jl +++ b/src/find_implicit_imports.jl @@ -45,6 +45,7 @@ function find_implicit_imports(mod::Module; skip=(mod, Base, Core)) # `WARNING: both Exporter3 and Exporter2 export "exported_a"; uses of it in module TestModA must be qualified` # and there is an ambiguity, and the name is in fact not resolved in `mod` clash = (err == ErrorException("\"$name\" is not defined in module $mod"))::Bool + clash |= (err == ErrorException("Constant binding was imported from multiple modules"))::Bool # if it is something else, rethrow clash || rethrow() missing From 7f21afd53bf142245539ddc4fba886cd1cee0239 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 22:35:13 +0200 Subject: [PATCH 15/19] support module aliases --- .gitignore | 2 ++ Project.toml | 2 +- src/improper_explicit_imports.jl | 12 ------------ test/module_alias.jl | 12 ++++++++++++ test/runtests.jl | 8 ++++++++ 5 files changed, 23 insertions(+), 13 deletions(-) create mode 100644 test/module_alias.jl diff --git a/.gitignore b/.gitignore index 1c02e5e..ad7d911 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,6 @@ *.jl.cov *.jl.mem Manifest.toml +Manifest-v*.toml /docs/build/ +dev diff --git a/Project.toml b/Project.toml index 23ac81b..319c010 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "ExplicitImports" uuid = "7d51a73a-1435-4ff3-83d9-f097790105c7" -version = "1.11.2" +version = "1.11.3" authors = ["Eric P. Hanson"] [deps] diff --git a/src/improper_explicit_imports.jl b/src/improper_explicit_imports.jl index 901509a..69acfb9 100644 --- a/src/improper_explicit_imports.jl +++ b/src/improper_explicit_imports.jl @@ -125,18 +125,6 @@ function process_explicitly_imported_row(row, mod) isempty(explicitly_imported_by) && return nothing - if explicitly_imported_by[end] !== nameof(current_mod) - error(""" - Encountered implementation bug in `process_explicitly_imported_row`. - Please file an issue on ExplicitImports.jl (https://github.com/ericphanson/ExplicitImports.jl/issues/new). - - Info: - - `explicitly_imported_by`=$(explicitly_imported_by) - `nameof(current_mod)`=$(nameof(current_mod)) - """) - end - # Ok, now `current_mod` should contain the actual module we imported the name from # This lets us query if the name is public in *that* module, get the value, etc value = trygetproperty(current_mod, row.name) diff --git a/test/module_alias.jl b/test/module_alias.jl new file mode 100644 index 0000000..07096bf --- /dev/null +++ b/test/module_alias.jl @@ -0,0 +1,12 @@ +# https://github.com/ericphanson/ExplicitImports.jl/issues/106 +module ModAlias + +module M1 + const g = 9.8 +end + +const M1′ = M1 + +using .M1′: g + +end # module ModAlias diff --git a/test/runtests.jl b/test/runtests.jl index 5ef9731..d753167 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -73,6 +73,7 @@ include("test_qualified_access.jl") include("test_explicit_imports.jl") include("main.jl") include("Test_Mod_Underscores.jl") +include("module_alias.jl") # For deprecations, we are using `maxlog`, which # the TestLogger only respects in Julia 1.8+. @@ -101,6 +102,13 @@ if VERSION > v"1.9-" end end +@testset "module aliases (#106)" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/106 + ret = Dict(improper_explicit_imports(ModAlias, "module_alias.jl")) + @test isempty(ret[ModAlias]) + @test isempty(ret[ModAlias.M1]) +end + @testset "function arg bug" begin # https://github.com/ericphanson/ExplicitImports.jl/issues/62 df = DataFrame(get_names_used("test_mods.jl").per_usage_info) From ed96729820c2e27a05f0bdda270433a19d0afd17 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 22:41:20 +0200 Subject: [PATCH 16/19] format tests & wrap in testset --- test/runtests.jl | 1936 ++++++++++++++++++++++++---------------------- 1 file changed, 995 insertions(+), 941 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index d753167..1b6315f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -25,7 +25,9 @@ function exception_string(f) catch e sprint(showerror, e) end - @test str isa String + if str === false + error("Expected `f` to throw an exception, but it did not") + end return str end @@ -75,1033 +77,1085 @@ include("main.jl") include("Test_Mod_Underscores.jl") include("module_alias.jl") -# For deprecations, we are using `maxlog`, which -# the TestLogger only respects in Julia 1.8+. -# (https://github.com/JuliaLang/julia/commit/02f7332027bd542b0701956a0f838bc75fa2eebd) -if VERSION >= v"1.8-" - @testset "deprecations" begin - include("deprecated.jl") +@testset "ExplicitImports" begin + # For deprecations, we are using `maxlog`, which + # the TestLogger only respects in Julia 1.8+. + # (https://github.com/JuliaLang/julia/commit/02f7332027bd542b0701956a0f838bc75fa2eebd) + if VERSION >= v"1.8-" + @testset "deprecations" begin + include("deprecated.jl") + end end -end -# package extension support needs Julia 1.9+ -if VERSION > v"1.9-" - @testset "Extensions" begin - submods = ExplicitImports.find_submodules(TestPkg) - @test length(submods) == 2 - DataFramesExt = Base.get_extension(TestPkg, :DataFramesExt) - @test haskey(Dict(submods), DataFramesExt) - - ext_imports = Dict(only_name_source(explicit_imports(TestPkg)))[DataFramesExt] - @test ext_imports == [(; name=:DataFrames, source=DataFrames), - (; name=:DataFrame, source=DataFrames), - (; name=:groupby, source=DataFrames)] || - ext_imports == [(; name=:DataFrames, source=DataFrames), - (; name=:DataFrame, source=DataFrames), - (; name=:groupby, source=DataFrames.DataAPI)] + # package extension support needs Julia 1.9+ + if VERSION > v"1.9-" + @testset "Extensions" begin + submods = ExplicitImports.find_submodules(TestPkg) + @test length(submods) == 2 + DataFramesExt = Base.get_extension(TestPkg, :DataFramesExt) + @test haskey(Dict(submods), DataFramesExt) + + ext_imports = Dict(only_name_source(explicit_imports(TestPkg)))[DataFramesExt] + @test ext_imports == [(; name=:DataFrames, source=DataFrames), + (; name=:DataFrame, source=DataFrames), + (; name=:groupby, source=DataFrames)] || + ext_imports == [(; name=:DataFrames, source=DataFrames), + (; name=:DataFrame, source=DataFrames), + (; name=:groupby, source=DataFrames.DataAPI)] + end end -end -@testset "module aliases (#106)" begin - # https://github.com/ericphanson/ExplicitImports.jl/issues/106 - ret = Dict(improper_explicit_imports(ModAlias, "module_alias.jl")) - @test isempty(ret[ModAlias]) - @test isempty(ret[ModAlias.M1]) -end + @testset "module aliases (#106)" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/106 + ret = Dict(improper_explicit_imports(ModAlias, "module_alias.jl")) + @test isempty(ret[ModAlias]) + @test isempty(ret[ModAlias.M1]) + end -@testset "function arg bug" begin - # https://github.com/ericphanson/ExplicitImports.jl/issues/62 - df = DataFrame(get_names_used("test_mods.jl").per_usage_info) - subset!(df, :name => ByRow(==(:norm)), :module_path => ByRow(==([:TestMod13]))) + @testset "function arg bug" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/62 + df = DataFrame(get_names_used("test_mods.jl").per_usage_info) + subset!(df, :name => ByRow(==(:norm)), :module_path => ByRow(==([:TestMod13]))) - @test_broken check_no_stale_explicit_imports(TestMod13, "test_mods.jl") === nothing -end + @test_broken check_no_stale_explicit_imports(TestMod13, "test_mods.jl") === nothing + end -@testset "owner_mod_for_printing" begin - @test owner_mod_for_printing(Core, :throw, Core.throw) == Base - @test owner_mod_for_printing(Core, :println, Core.println) == Core -end + @testset "owner_mod_for_printing" begin + @test owner_mod_for_printing(Core, :throw, Core.throw) == Base + @test owner_mod_for_printing(Core, :println, Core.println) == Core + end -# https://github.com/ericphanson/ExplicitImports.jl/issues/69 -@testset "Reexport support" begin - @test check_no_stale_explicit_imports(TestMod15, "test_mods.jl") === nothing - @test isempty(improper_explicit_imports_nonrecursive(TestMod15, "test_mods.jl")) - @test isempty(improper_explicit_imports(TestMod15, "test_mods.jl")[1][2]) -end + # https://github.com/ericphanson/ExplicitImports.jl/issues/69 + @testset "Reexport support" begin + @test check_no_stale_explicit_imports(TestMod15, "test_mods.jl") === nothing + @test isempty(improper_explicit_imports_nonrecursive(TestMod15, "test_mods.jl")) + @test isempty(improper_explicit_imports(TestMod15, "test_mods.jl")[1][2]) + end -if VERSION >= v"1.7-" - # https://github.com/ericphanson/ExplicitImports.jl/issues/70 - @testset "Compat skipping" begin - @test check_all_explicit_imports_via_owners(TestMod14, "test_mods.jl") === nothing - @test check_all_qualified_accesses_via_owners(TestMod14, "test_mods.jl") === nothing + if VERSION >= v"1.7-" + # https://github.com/ericphanson/ExplicitImports.jl/issues/70 + @testset "Compat skipping" begin + @test check_all_explicit_imports_via_owners(TestMod14, "test_mods.jl") === + nothing + @test check_all_qualified_accesses_via_owners(TestMod14, "test_mods.jl") === + nothing - @test isempty(improper_explicit_imports_nonrecursive(TestMod14, "test_mods.jl")) - @test isempty(improper_explicit_imports(TestMod14, "test_mods.jl")[1][2]) + @test isempty(improper_explicit_imports_nonrecursive(TestMod14, "test_mods.jl")) + @test isempty(improper_explicit_imports(TestMod14, "test_mods.jl")[1][2]) - @test isempty(improper_qualified_accesses_nonrecursive(TestMod14, "test_mods.jl")) + @test isempty(improper_qualified_accesses_nonrecursive(TestMod14, + "test_mods.jl")) - @test isempty(improper_qualified_accesses(TestMod14, "test_mods.jl")[1][2]) + @test isempty(improper_qualified_accesses(TestMod14, "test_mods.jl")[1][2]) + end end -end -@testset "imports" begin - cursor = TreeCursor(SyntaxNodeWrapper("imports.jl")) - leaves = collect(Leaves(cursor)) - import_type_pairs = get_val.(leaves) .=> analyze_import_type.(leaves) - filter!(import_type_pairs) do (k, v) - return v !== :not_import + @testset "imports" begin + cursor = TreeCursor(SyntaxNodeWrapper("imports.jl")) + leaves = collect(Leaves(cursor)) + import_type_pairs = get_val.(leaves) .=> analyze_import_type.(leaves) + filter!(import_type_pairs) do (k, v) + return v !== :not_import + end + @test import_type_pairs == + [:Exporter => :import_LHS, + :exported_a => :import_RHS, + :exported_c => :import_RHS, + :Exporter => :import_LHS, + :exported_c => :import_RHS, + :TestModA => :blanket_using_member, + :SubModB => :blanket_using, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h2 => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h3 => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :h => :import_RHS, + :Exporter => :blanket_using, + :Exporter => :plain_import, + :LinearAlgebra => :import_LHS, + :map => :import_RHS, + :_svd! => :import_RHS, + :LinearAlgebra => :import_LHS, + :svd => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :exported_b => :import_RHS, + :TestModA => :import_LHS, + :SubModB => :import_LHS, + :f => :import_RHS] + + inds = findall(==(:import_RHS), analyze_import_type.(leaves)) + lhs_rhs_pairs = get_import_lhs.(leaves[inds]) .=> get_val.(leaves[inds]) + @test lhs_rhs_pairs == [[:., :., :Exporter] => :exported_a, + [:., :., :Exporter] => :exported_c, + [:., :., :Exporter] => :exported_c, + [:., :., :TestModA, :SubModB] => :h2, + [:., :., :TestModA, :SubModB] => :h3, + [:., :., :TestModA, :SubModB] => :h, + [:LinearAlgebra] => :map, + [:LinearAlgebra] => :_svd!, + [:LinearAlgebra] => :svd, + [:., :., :TestModA, :SubModB] => :exported_b, + [:., :., :TestModA, :SubModB] => :f] + + imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; + allow_internal_imports=false)) + h_row = only(subset(imps, :name => ByRow(==(:h)))) + @test !h_row.public_import + # Note: if this fails locally, try `include("imports.jl")` to rebuild the module + @test h_row.whichmodule == TestModA.SubModB + @test h_row.importing_from == TestModA.SubModB + + h2_row = only(subset(imps, :name => ByRow(==(:h2)))) + @test h2_row.public_import + @test h2_row.whichmodule === TestModA.SubModB + @test h2_row.importing_from == TestModA.SubModB + _svd!_row = only(subset(imps, :name => ByRow(==(:_svd!)))) + @test !_svd!_row.public_import + + f_row = only(subset(imps, :name => ByRow(==(:f)))) + @test !f_row.public_import # not public in `TestModA.SubModB` + @test f_row.whichmodule == TestModA + @test f_row.importing_from == TestModA.SubModB + + imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; + allow_internal_imports=true)) + # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: + @test all(==(LinearAlgebra), imps.importing_from) end - @test import_type_pairs == - [:Exporter => :import_LHS, - :exported_a => :import_RHS, - :exported_c => :import_RHS, - :Exporter => :import_LHS, - :exported_c => :import_RHS, - :TestModA => :blanket_using_member, - :SubModB => :blanket_using, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h2 => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h3 => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :h => :import_RHS, - :Exporter => :blanket_using, - :Exporter => :plain_import, - :LinearAlgebra => :import_LHS, - :map => :import_RHS, - :_svd! => :import_RHS, - :LinearAlgebra => :import_LHS, - :svd => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :exported_b => :import_RHS, - :TestModA => :import_LHS, - :SubModB => :import_LHS, - :f => :import_RHS] - - inds = findall(==(:import_RHS), analyze_import_type.(leaves)) - lhs_rhs_pairs = get_import_lhs.(leaves[inds]) .=> get_val.(leaves[inds]) - @test lhs_rhs_pairs == [[:., :., :Exporter] => :exported_a, - [:., :., :Exporter] => :exported_c, - [:., :., :Exporter] => :exported_c, - [:., :., :TestModA, :SubModB] => :h2, - [:., :., :TestModA, :SubModB] => :h3, - [:., :., :TestModA, :SubModB] => :h, - [:LinearAlgebra] => :map, - [:LinearAlgebra] => :_svd!, - [:LinearAlgebra] => :svd, - [:., :., :TestModA, :SubModB] => :exported_b, - [:., :., :TestModA, :SubModB] => :f] - - imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; - allow_internal_imports=false)) - h_row = only(subset(imps, :name => ByRow(==(:h)))) - @test !h_row.public_import - # Note: if this fails locally, try `include("imports.jl")` to rebuild the module - @test h_row.whichmodule == TestModA.SubModB - @test h_row.importing_from == TestModA.SubModB - - h2_row = only(subset(imps, :name => ByRow(==(:h2)))) - @test h2_row.public_import - @test h2_row.whichmodule === TestModA.SubModB - @test h2_row.importing_from == TestModA.SubModB - _svd!_row = only(subset(imps, :name => ByRow(==(:_svd!)))) - @test !_svd!_row.public_import - - f_row = only(subset(imps, :name => ByRow(==(:f)))) - @test !f_row.public_import # not public in `TestModA.SubModB` - @test f_row.whichmodule == TestModA - @test f_row.importing_from == TestModA.SubModB - - imps = DataFrame(improper_explicit_imports_nonrecursive(ModImports, "imports.jl"; - allow_internal_imports=true)) - # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: - @test all(==(LinearAlgebra), imps.importing_from) -end -##### -##### To analyze a test case -##### -# using ExplicitImports: js_node, get_parent, kind, parents_match -# using JuliaSyntax: @K_str - -# cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")); -# leaves = collect(Leaves(cursor)) -# leaf = leaves[end - 2] # select a leaf -# js_node(leaf) # inspect it -# p = js_node(get_parent(leaf, 3)) # see the tree, etc -# kind(p) - -@testset "qualified access" begin - # analyze_qualified_names - qualified = analyze_qualified_names(TestQualifiedAccess, "test_qualified_access.jl") - @test length(qualified) == 6 - ABC, DEF, HIJ, X, map, x = qualified - @test ABC.name == :ABC - @test DEF.public_access - @test HIJ.public_access - @test DEF.name == :DEF - @test HIJ.name == :HIJ - @test X.name == :X - @test map.name == :map - @test x.name == :x - @test x.self_qualified - - # improper_qualified_accesses - ret = Dict(improper_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false)) - @test isempty(ret[TestQualifiedAccess.Bar]) - @test isempty(ret[TestQualifiedAccess.FooModule]) - @test !isempty(ret[TestQualifiedAccess]) - - @test length(ret[TestQualifiedAccess]) == 4 - ABC, X, map, x = ret[TestQualifiedAccess] - # Can add keys, but removing them is breaking - @test keys(ABC) ⊇ - [:name, :location, :value, :accessing_from, :whichmodule, :public_access, - :accessing_from_owns_name, :accessing_from_submodule_owns_name, :internal_access] - @test ABC.name == :ABC - @test ABC.location isa AbstractString - @test ABC.whichmodule == TestQualifiedAccess.Bar - @test ABC.accessing_from == TestQualifiedAccess.FooModule - @test ABC.public_access == false - @test ABC.accessing_from_submodule_owns_name == false - - @test X.name == :X - @test X.whichmodule == TestQualifiedAccess.FooModule.FooSub - @test X.accessing_from == TestQualifiedAccess.FooModule - @test X.public_access == false - @test X.accessing_from_submodule_owns_name == true - - @test map.name == :map - - @test x.name == :x - @test x.self_qualified - - imps = DataFrame(improper_qualified_accesses_nonrecursive(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=true)) - subset!(imps, :self_qualified => ByRow(!)) # drop self-qualified - # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: - @test all(==(LinearAlgebra), imps.accessing_from) - - # check_no_self_qualified_accesses - ex = SelfQualifiedAccessException - @test_throws ex check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl") - - str = exception_string() do - return check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl") - end - @test contains(str, "has self-qualified accesses:\n- `x` was accessed as") - - @test check_no_self_qualified_accesses(TestQualifiedAccess, - "test_qualified_access.jl"; ignore=(:x,)) === - nothing - - str = sprint(print_explicit_imports, TestQualifiedAccess, - "test_qualified_access.jl") - @test contains(str, "has 1 self-qualified access:\n\n • x was accessed as ") - - # check_all_qualified_accesses_via_owners - ex = QualifiedAccessesFromNonOwnerException - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - end - @test contains(str, - "has qualified accesses to names via modules other than their owner as determined") + ##### + ##### To analyze a test case + ##### + # using ExplicitImports: js_node, get_parent, kind, parents_match + # using JuliaSyntax: @K_str + + # cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")); + # leaves = collect(Leaves(cursor)) + # leaf = leaves[end - 2] # select a leaf + # js_node(leaf) # inspect it + # p = js_node(get_parent(leaf, 3)) # see the tree, etc + # kind(p) + + @testset "qualified access" begin + # analyze_qualified_names + qualified = analyze_qualified_names(TestQualifiedAccess, "test_qualified_access.jl") + @test length(qualified) == 6 + ABC, DEF, HIJ, X, map, x = qualified + @test ABC.name == :ABC + @test DEF.public_access + @test HIJ.public_access + @test DEF.name == :DEF + @test HIJ.name == :HIJ + @test X.name == :X + @test map.name == :map + @test x.name == :x + @test x.self_qualified + + # improper_qualified_accesses + ret = Dict(improper_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false)) + @test isempty(ret[TestQualifiedAccess.Bar]) + @test isempty(ret[TestQualifiedAccess.FooModule]) + @test !isempty(ret[TestQualifiedAccess]) + + @test length(ret[TestQualifiedAccess]) == 4 + ABC, X, map, x = ret[TestQualifiedAccess] + # Can add keys, but removing them is breaking + @test keys(ABC) ⊇ + [:name, :location, :value, :accessing_from, :whichmodule, :public_access, + :accessing_from_owns_name, :accessing_from_submodule_owns_name, + :internal_access] + @test ABC.name == :ABC + @test ABC.location isa AbstractString + @test ABC.whichmodule == TestQualifiedAccess.Bar + @test ABC.accessing_from == TestQualifiedAccess.FooModule + @test ABC.public_access == false + @test ABC.accessing_from_submodule_owns_name == false + + @test X.name == :X + @test X.whichmodule == TestQualifiedAccess.FooModule.FooSub + @test X.accessing_from == TestQualifiedAccess.FooModule + @test X.public_access == false + @test X.accessing_from_submodule_owns_name == true + + @test map.name == :map + + @test x.name == :x + @test x.self_qualified + + imps = DataFrame(improper_qualified_accesses_nonrecursive(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=true)) + subset!(imps, :self_qualified => ByRow(!)) # drop self-qualified + # in this case we rule out all the `Main` ones, so only LinearAlgebra is left: + @test all(==(LinearAlgebra), imps.accessing_from) + + # check_no_self_qualified_accesses + ex = SelfQualifiedAccessException + @test_throws ex check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl") - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, ignore=(:map,), - allow_internal_accesses=false) === - nothing + str = exception_string() do + return check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl") + end + @test contains(str, "has self-qualified accesses:\n- `x` was accessed as") - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:ABC, :map), - allow_internal_accesses=false) === nothing + @test check_no_self_qualified_accesses(TestQualifiedAccess, + "test_qualified_access.jl"; ignore=(:x,)) === + nothing - # allow_internal_accesses=true - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl", - ignore=(:ABC,)) + str = sprint(print_explicit_imports, TestQualifiedAccess, + "test_qualified_access.jl") + @test contains(str, "has 1 self-qualified access:\n\n • x was accessed as ") - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:ABC, :map)) === nothing - - @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, - require_submodule_access=true, - allow_internal_accesses=false) - - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar, - TestQualifiedAccess.FooModule => TestQualifiedAccess.FooModule.FooSub, - LinearAlgebra => Base) - @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, - require_submodule_access=true, - allow_internal_accesses=false) === nothing - - # Printing via `print_explicit_imports` - str = sprint(io -> print_explicit_imports(io, TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "accesses 2 names from non-owner modules") - @test contains(str, "ABC has owner") - - ex = NonPublicQualifiedAccessException - @test_throws ex check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - str = exception_string() do - return check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - allow_internal_accesses=false) - end - @test contains(str, "- `ABC` is not public in") + # check_all_qualified_accesses_via_owners + ex = QualifiedAccessesFromNonOwnerException + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + # Test the printing is hitting our formatted errors + str = exception_string() do + return check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + end + @test contains(str, + "has qualified accesses to names via modules other than their owner as determined") + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, ignore=(:map,), + allow_internal_accesses=false) === + nothing + + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:ABC, :map), + allow_internal_accesses=false) === + nothing + + # allow_internal_accesses=true + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl", + ignore=(:ABC,)) + + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:ABC, :map)) === nothing + + @test_throws ex check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, + require_submodule_access=true, + allow_internal_accesses=false) + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar, + TestQualifiedAccess.FooModule => TestQualifiedAccess.FooModule.FooSub, + LinearAlgebra => Base) + @test check_all_qualified_accesses_via_owners(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, + require_submodule_access=true, + allow_internal_accesses=false) === + nothing + + # Printing via `print_explicit_imports` + str = sprint(io -> print_explicit_imports(io, TestQualifiedAccess, "test_qualified_access.jl"; - ignore=(:X, :ABC, :map), - allow_internal_accesses=false) === nothing - - skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + allow_internal_accesses=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "accesses 2 names from non-owner modules") + @test contains(str, "ABC has owner") - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - skip, ignore=(:X, :map), - allow_internal_accesses=false) === nothing + ex = NonPublicQualifiedAccessException + @test_throws ex check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + str = exception_string() do + return check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + allow_internal_accesses=false) + end + @test contains(str, "- `ABC` is not public in") + + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:X, :ABC, :map), + allow_internal_accesses=false) === + nothing + + skip = (TestQualifiedAccess.FooModule => TestQualifiedAccess.Bar,) + + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + skip, ignore=(:X, :map), + allow_internal_accesses=false) === + nothing + + # allow_internal_accesses=true + @test check_all_qualified_accesses_are_public(TestQualifiedAccess, + "test_qualified_access.jl"; + ignore=(:map,)) === nothing + end - # allow_internal_accesses=true - @test check_all_qualified_accesses_are_public(TestQualifiedAccess, - "test_qualified_access.jl"; - ignore=(:map,)) === nothing -end + @testset "improper explicit imports" begin + imps = Dict(improper_explicit_imports(TestModA, "TestModA.jl"; + allow_internal_imports=false)) + row = only(imps[TestModA]) + @test row.name == :un_exported + @test row.whichmodule == Exporter + + row1, row2 = imps[TestModA.SubModB.TestModA.TestModC] + # Can add keys, but removing them is breaking + @test keys(row1) ⊇ + [:name, :location, :value, :importing_from, :whichmodule, :public_import, + :importing_from_owns_name, :importing_from_submodule_owns_name, :stale, + :internal_import] + @test row1.name == :exported_c + @test row1.stale == true + @test row2.name == :exported_d + @test row2.stale == true + + @test check_all_explicit_imports_via_owners(TestModA, "TestModA.jl"; + allow_internal_imports=false) === + nothing + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; + allow_internal_imports=false) + + # allow_internal_imports=true + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, + "imports.jl";) + @test check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; ignore=(:map,)) === + nothing + # Test the printing is hitting our formatted errors + str = exception_string() do + return check_all_explicit_imports_via_owners(ModImports, + "imports.jl"; + allow_internal_imports=false) + end -@testset "improper explicit imports" begin - imps = Dict(improper_explicit_imports(TestModA, "TestModA.jl"; - allow_internal_imports=false)) - row = only(imps[TestModA]) - @test row.name == :un_exported - @test row.whichmodule == Exporter - - row1, row2 = imps[TestModA.SubModB.TestModA.TestModC] - # Can add keys, but removing them is breaking - @test keys(row1) ⊇ - [:name, :location, :value, :importing_from, :whichmodule, :public_import, - :importing_from_owns_name, :importing_from_submodule_owns_name, :stale, - :internal_import] - @test row1.name == :exported_c - @test row1.stale == true - @test row2.name == :exported_d - @test row2.stale == true - - @test check_all_explicit_imports_via_owners(TestModA, "TestModA.jl"; - allow_internal_imports=false) === nothing - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; - allow_internal_imports=false) - - # allow_internal_imports=true - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(ModImports, - "imports.jl";) - @test check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; ignore=(:map,)) === nothing - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_all_explicit_imports_via_owners(ModImports, - "imports.jl"; - allow_internal_imports=false) + @test contains(str, + "explicit imports of names from modules other than their owner as determined ") + + @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; + ignore=(:exported_b, :f, :map), + allow_internal_imports=false) === + nothing + + # We can pass `skip` to ignore non-owning explicit imports from LinearAlgebra that are owned by Base + @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; + skip=(LinearAlgebra => Base,), + ignore=(:exported_b, :f), + allow_internal_imports=false) === + nothing + + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + allow_internal_imports=false) + + # test ignore + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC,), + allow_internal_imports=false) === + nothing + + # test skip + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + skip=(TestExplicitImports.FooModule => TestExplicitImports.Bar,), + allow_internal_imports=false) === + nothing + + @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC,), + require_submodule_import=true, + allow_internal_imports=false) + + @test check_all_explicit_imports_via_owners(TestExplicitImports, + "test_explicit_imports.jl"; + ignore=(:ABC, :X), + require_submodule_import=true, + allow_internal_imports=false) === + nothing + + # allow_internal_imports = true + @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, + "imports.jl";) + @test check_all_explicit_imports_are_public(ModImports, + "imports.jl"; ignore=(:map, :_svd!)) === + nothing end - @test contains(str, - "explicit imports of names from modules other than their owner as determined ") - - @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; - ignore=(:exported_b, :f, :map), - allow_internal_imports=false) === nothing - - # We can pass `skip` to ignore non-owning explicit imports from LinearAlgebra that are owned by Base - @test check_all_explicit_imports_via_owners(ModImports, "imports.jl"; - skip=(LinearAlgebra => Base,), - ignore=(:exported_b, :f), - allow_internal_imports=false) === nothing - - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - allow_internal_imports=false) - - # test ignore - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC,), - allow_internal_imports=false) === nothing - - # test skip - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - skip=(TestExplicitImports.FooModule => TestExplicitImports.Bar,), - allow_internal_imports=false) === - nothing - - @test_throws ExplicitImportsFromNonOwnerException check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC,), - require_submodule_import=true, - allow_internal_imports=false) - - @test check_all_explicit_imports_via_owners(TestExplicitImports, - "test_explicit_imports.jl"; - ignore=(:ABC, :X), - require_submodule_import=true, - allow_internal_imports=false) === nothing - - # allow_internal_imports = true - @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, - "imports.jl";) - @test check_all_explicit_imports_are_public(ModImports, - "imports.jl"; ignore=(:map, :_svd!)) === - nothing -end + @testset "structs" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) + @test map(get_val, filter(is_struct_type_param, leaves)) == [:X, :Y, :QR] -@testset "structs" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) - @test map(get_val, filter(is_struct_type_param, leaves)) == [:X, :Y, :QR] + @test map(get_val, filter(is_struct_field_name, leaves)) == [:x, :x, :x, :qr, :qr] - @test map(get_val, filter(is_struct_field_name, leaves)) == [:x, :x, :x, :qr, :qr] + df = DataFrame(get_names_used("test_mods.jl").per_usage_info) + subset!(df, :name => ByRow(==(:QR)), :module_path => ByRow(==([:TestMod5]))) + # two uses of QR: one is as a type param, the other a usage of that type param in the same struct definition + @test df.analysis_code == + [ExplicitImports.InternalStruct, ExplicitImports.IgnoredNonFirst] + @test df.struct_field_or_type_param == [true, false] - # Tests #34 and #36 - @test using_statement.(explicit_imports_nonrecursive(TestMod5, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] -end + # Tests #34 and #36 + @test using_statement.(explicit_imports_nonrecursive(TestMod5, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] + end -if VERSION >= v"1.7-" - @testset "loops" begin + if VERSION >= v"1.7-" + @testset "loops" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) + @test map(get_val, filter(is_for_arg, leaves)) == + [:i, :I, :j, :k, :k, :j, :xi, :yi] + + # Tests #35 + @test using_statement.(explicit_imports_nonrecursive(TestMod6, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] + end + end + + @testset "nested local scope" begin cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) leaves = collect(Leaves(cursor)) - @test map(get_val, filter(is_for_arg, leaves)) == [:i, :I, :j, :k, :k, :j, :xi, :yi] - - # Tests #35 - @test using_statement.(explicit_imports_nonrecursive(TestMod6, "test_mods.jl")) == + # Test nested local scope + @test using_statement.(explicit_imports_nonrecursive(TestMod7, "test_mods.jl")) == ["using LinearAlgebra: LinearAlgebra"] end -end -@testset "nested local scope" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) - # Test nested local scope - @test using_statement.(explicit_imports_nonrecursive(TestMod7, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] -end + @testset "types without values in function signatures" begin + # https://github.com/ericphanson/ExplicitImports.jl/issues/33 + @test using_statement.(explicit_imports_nonrecursive(TestMod8, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: QR"] + end -@testset "types without values in function signatures" begin - # https://github.com/ericphanson/ExplicitImports.jl/issues/33 - @test using_statement.(explicit_imports_nonrecursive(TestMod8, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: QR"] -end + @testset "generators" begin + cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) + leaves = collect(Leaves(cursor)) -@testset "generators" begin - cursor = TreeCursor(SyntaxNodeWrapper("test_mods.jl")) - leaves = collect(Leaves(cursor)) + v = [:i1, :I, :i2, :I, :i3, :I, :i4, :I] + w = [:i1, :I] + @test map(get_val, filter(is_generator_arg, leaves)) == + [v; v; w; w; w; w; w] - v = [:i1, :I, :i2, :I, :i3, :I, :i4, :I] - w = [:i1, :I] - @test map(get_val, filter(is_generator_arg, leaves)) == - [v; v; w; w; w; w; w] + if VERSION >= v"1.7-" + @test using_statement.(explicit_imports_nonrecursive(TestMod9, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra"] - if VERSION >= v"1.7-" - @test using_statement.(explicit_imports_nonrecursive(TestMod9, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra"] + per_usage_info, _ = analyze_all_names("test_mods.jl") + df = DataFrame(per_usage_info) + subset!(df, :module_path => ByRow(==([:TestMod9])), :name => ByRow(==(:i1))) + @test all(==(ExplicitImports.InternalGenerator), df.analysis_code) + end + end + + @testset "while loops" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod10, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: I"] per_usage_info, _ = analyze_all_names("test_mods.jl") df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod9])), :name => ByRow(==(:i1))) - @test all(==(ExplicitImports.InternalGenerator), df.analysis_code) + subset!(df, :module_path => ByRow(==([:TestMod10])), :name => ByRow(==(:I))) + # First one is internal, second one external + @test df.analysis_code == + [ExplicitImports.InternalAssignment, ExplicitImports.External] end -end -@testset "while loops" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod10, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", "using LinearAlgebra: I"] - - per_usage_info, _ = analyze_all_names("test_mods.jl") - df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod10])), :name => ByRow(==(:I))) - # First one is internal, second one external - @test df.analysis_code == [ExplicitImports.InternalAssignment, ExplicitImports.External] -end + if VERSION >= v"1.7-" + @testset "do- syntax" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod11, "test_mods.jl")) == + ["using LinearAlgebra: LinearAlgebra", + "using LinearAlgebra: Hermitian", + "using LinearAlgebra: svd"] + + per_usage_info, _ = analyze_all_names("test_mods.jl") + df = DataFrame(per_usage_info) + subset!(df, :module_path => ByRow(==([:TestMod11]))) + + I_codes = subset(df, :name => ByRow(==(:I))).analysis_code + @test I_codes == + [ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, + ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst] + svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code + @test svd_codes == + [ExplicitImports.InternalFunctionArg, ExplicitImports.External] + Hermitian_codes = subset(df, :name => ByRow(==(:Hermitian))).analysis_code + @test Hermitian_codes == + [ExplicitImports.External, ExplicitImports.IgnoredNonFirst] + end + end -if VERSION >= v"1.7-" - @testset "do- syntax" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod11, "test_mods.jl")) == + @testset "try-catch" begin + @test using_statement.(explicit_imports_nonrecursive(TestMod12, "test_mods.jl")) == ["using LinearAlgebra: LinearAlgebra", - "using LinearAlgebra: Hermitian", + "using LinearAlgebra: I", "using LinearAlgebra: svd"] per_usage_info, _ = analyze_all_names("test_mods.jl") df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod11]))) + subset!(df, :module_path => ByRow(==([:TestMod12]))) I_codes = subset(df, :name => ByRow(==(:I))).analysis_code - @test I_codes == - [ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst, - ExplicitImports.InternalFunctionArg, ExplicitImports.IgnoredNonFirst] + @test I_codes == [ExplicitImports.InternalAssignment, + ExplicitImports.External, + ExplicitImports.External, + ExplicitImports.InternalAssignment, + ExplicitImports.InternalCatchArgument, + ExplicitImports.IgnoredNonFirst, + ExplicitImports.External] svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code - @test svd_codes == [ExplicitImports.InternalFunctionArg, ExplicitImports.External] - Hermitian_codes = subset(df, :name => ByRow(==(:Hermitian))).analysis_code - @test Hermitian_codes == [ExplicitImports.External, ExplicitImports.IgnoredNonFirst] + @test svd_codes == [ExplicitImports.InternalAssignment, + ExplicitImports.External, + ExplicitImports.InternalAssignment, + ExplicitImports.External] end -end - -@testset "try-catch" begin - @test using_statement.(explicit_imports_nonrecursive(TestMod12, "test_mods.jl")) == - ["using LinearAlgebra: LinearAlgebra", - "using LinearAlgebra: I", - "using LinearAlgebra: svd"] - - per_usage_info, _ = analyze_all_names("test_mods.jl") - df = DataFrame(per_usage_info) - subset!(df, :module_path => ByRow(==([:TestMod12]))) - - I_codes = subset(df, :name => ByRow(==(:I))).analysis_code - @test I_codes == [ExplicitImports.InternalAssignment, - ExplicitImports.External, - ExplicitImports.External, - ExplicitImports.InternalAssignment, - ExplicitImports.InternalCatchArgument, - ExplicitImports.IgnoredNonFirst, - ExplicitImports.External] - svd_codes = subset(df, :name => ByRow(==(:svd))).analysis_code - @test svd_codes == [ExplicitImports.InternalAssignment, - ExplicitImports.External, - ExplicitImports.InternalAssignment, - ExplicitImports.External] -end - -@testset "scripts" begin - str = sprint(print_explicit_imports_script, "script.jl") - str = replace(str, r"\s+" => " ") - @test contains(str, "Script script.jl") - @test contains(str, "relying on implicit imports for 1 name") - @test contains(str, "using LinearAlgebra: norm") - @test contains(str, "stale explicit imports for this 1 unused name") - @test contains(str, "• qr") -end - -@testset "Handle public symbols with same name as exported Base symbols (#88)" begin - statements = using_statement.(explicit_imports_nonrecursive(Mod88, "examples.jl")) - @test statements == ["using .ModWithTryparse: ModWithTryparse"] -end -@testset "Don't skip source modules (#29)" begin - # In this case `UUID` is defined in Base but exported in UUIDs - ret = ExplicitImports.find_implicit_imports(Mod29)[:UUID] - @test ret.source == Base - @test ret.exporters == [UUIDs] - # We should NOT skip it, even though `skip` includes `Base`, since the exporters - # are not skipped. - statements = using_statement.(explicit_imports_nonrecursive(Mod29, "examples.jl")) - @test statements == ["using UUIDs: UUIDs", "using UUIDs: UUID"] -end - -@testset "Exported module (#24)" begin - statements = using_statement.(explicit_imports_nonrecursive(Mod24, "examples.jl")) - # The key thing here is we do not have `using .Exporter: exported_a`, - # since we haven't done `using .Exporter` in `Mod24`, only `using .Exporter2` - @test statements == ["using .Exporter2: Exporter2", "using .Exporter2: exported_a"] -end - -@testset "string macros (#20)" begin - foo = only_name_source(explicit_imports_nonrecursive(Foo20, "examples.jl")) - @test foo == [(; name=:Markdown, source=Markdown), - (; name=Symbol("@doc_str"), source=Markdown)] - bar = explicit_imports_nonrecursive(Bar20, "examples.jl") - @test isempty(bar) -end - -@testset "TestModArgs" begin - # don't detect `a`! - statements = using_statement.(explicit_imports_nonrecursive(TestModArgs, - "TestModArgs.jl")) - @test statements == - ["using .Exporter4: Exporter4", "using .Exporter4: A", "using .Exporter4: Z"] + @testset "scripts" begin + str = sprint(print_explicit_imports_script, "script.jl") + str = replace(str, r"\s+" => " ") + @test contains(str, "Script script.jl") + @test contains(str, "relying on implicit imports for 1 name") + @test contains(str, "using LinearAlgebra: norm") + @test contains(str, "stale explicit imports for this 1 unused name") + @test contains(str, "• qr") + end - statements = using_statement.(explicit_imports_nonrecursive(ThreadPinning, - "examples.jl")) + @testset "Handle public symbols with same name as exported Base symbols (#88)" begin + statements = using_statement.(explicit_imports_nonrecursive(Mod88, "examples.jl")) + @test statements == ["using .ModWithTryparse: ModWithTryparse"] + end + @testset "Don't skip source modules (#29)" begin + # In this case `UUID` is defined in Base but exported in UUIDs + ret = ExplicitImports.find_implicit_imports(Mod29)[:UUID] + @test ret.source == Base + @test ret.exporters == [UUIDs] + # We should NOT skip it, even though `skip` includes `Base`, since the exporters + # are not skipped. + statements = using_statement.(explicit_imports_nonrecursive(Mod29, "examples.jl")) + @test statements == ["using UUIDs: UUIDs", "using UUIDs: UUID"] + end - @test statements == ["using LinearAlgebra: LinearAlgebra"] -end + @testset "Exported module (#24)" begin + statements = using_statement.(explicit_imports_nonrecursive(Mod24, "examples.jl")) + # The key thing here is we do not have `using .Exporter: exported_a`, + # since we haven't done `using .Exporter` in `Mod24`, only `using .Exporter2` + @test statements == ["using .Exporter2: Exporter2", "using .Exporter2: exported_a"] + end -@testset "is_function_definition_arg" begin - cursor = TreeCursor(SyntaxNodeWrapper("TestModArgs.jl")) - leaves = collect(Leaves(cursor)) - purported_function_args = filter(is_function_definition_arg, leaves) + @testset "string macros (#20)" begin + foo = only_name_source(explicit_imports_nonrecursive(Foo20, "examples.jl")) + @test foo == [(; name=:Markdown, source=Markdown), + (; name=Symbol("@doc_str"), source=Markdown)] + bar = explicit_imports_nonrecursive(Bar20, "examples.jl") + @test isempty(bar) + end - # written this way to get clearer test failure messages - vals = unique(get_val.(purported_function_args)) - @test vals == [:a] + @testset "TestModArgs" begin + # don't detect `a`! + statements = using_statement.(explicit_imports_nonrecursive(TestModArgs, + "TestModArgs.jl")) + @test statements == + ["using .Exporter4: Exporter4", "using .Exporter4: A", "using .Exporter4: Z"] - # we have 9*4 functions with one argument `a`, plus 2 macros - @test length(purported_function_args) == 9 * 4 + 2 - non_function_args = filter(!is_function_definition_arg, leaves) - missed = filter(x -> get_val(x) === :a, non_function_args) - @test isempty(missed) -end + statements = using_statement.(explicit_imports_nonrecursive(ThreadPinning, + "examples.jl")) -@testset "has_ancestor" begin - @test has_ancestor(TestModA.SubModB, TestModA) - @test !has_ancestor(TestModA, TestModA.SubModB) + @test statements == ["using LinearAlgebra: LinearAlgebra"] + end - @test should_skip(Base.Iterators; skip=(Base, Core)) -end + @testset "is_function_definition_arg" begin + cursor = TreeCursor(SyntaxNodeWrapper("TestModArgs.jl")) + leaves = collect(Leaves(cursor)) + purported_function_args = filter(is_function_definition_arg, leaves) -function get_per_scope(per_usage_info) - per_usage_df = DataFrame(per_usage_info) - dropmissing!(per_usage_df, :external_global_name) - return per_usage_df -end + # written this way to get clearer test failure messages + vals = unique(get_val.(purported_function_args)) + @test vals == [:a] -@testset "file not found" begin - for f in (check_no_implicit_imports, check_no_stale_explicit_imports, - check_all_explicit_imports_via_owners, check_all_qualified_accesses_via_owners, - explicit_imports, - explicit_imports_nonrecursive, print_explicit_imports, - improper_explicit_imports, improper_explicit_imports_nonrecursive, - improper_qualified_accesses, improper_qualified_accesses_nonrecursive) - @test_throws FileNotFoundException f(TestModA) + # we have 9*4 functions with one argument `a`, plus 2 macros + @test length(purported_function_args) == 9 * 4 + 2 + non_function_args = filter(!is_function_definition_arg, leaves) + missed = filter(x -> get_val(x) === :a, non_function_args) + @test isempty(missed) end - str = sprint(Base.showerror, FileNotFoundException()) - @test contains(str, "module which is not top-level in a package") -end -@testset "ExplicitImports.jl" begin - @test using_statement.(explicit_imports_nonrecursive(TestModA, "TestModA.jl")) == - ["using .Exporter: Exporter", "using .Exporter: @mac", - "using .Exporter2: Exporter2", - "using .Exporter2: exported_a", "using .Exporter3: Exporter3"] - - per_usage_info, _ = analyze_all_names("TestModA.jl") - df = get_per_scope(per_usage_info) - locals = contains.(string.(df.name), Ref("local")) - @test all(!, df.external_global_name[locals]) - - # we use `x` in two scopes - xs = subset(df, :name => ByRow(==(:x))) - @test !xs[1, :external_global_name] - @test !xs[2, :external_global_name] - @test xs[2, :analysis_code] == ExplicitImports.InternalAssignment - - # we use `exported_a` in two scopes; both times refer to the global name - exported_as = subset(df, :name => ByRow(==(:exported_a))) - @test exported_as[1, :external_global_name] - @test exported_as[2, :external_global_name] - @test !exported_as[2, :is_assignment] - - # Test submodules - @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB, "TestModA.jl")) == - ["using .Exporter3: Exporter3", "using .Exporter3: exported_b", - "using .TestModA: f"] - - mod_path = module_path(TestModA.SubModB) - @test mod_path == [:SubModB, :TestModA, :Main] - sub_df = restrict_to_module(df, TestModA.SubModB) - - h = only(subset(sub_df, :name => ByRow(==(:h)))) - @test h.external_global_name - @test !h.is_assignment - - # Nested submodule with same name as outer module... - @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA, - "TestModA.jl")) == - ["using .Exporter3: Exporter3", "using .Exporter3: exported_b"] - - # Check we are getting innermost names and not outer ones - subsub_df = restrict_to_module(df, TestModA.SubModB.TestModA) - @test :inner_h in subsub_df.name - @test :h ∉ subsub_df.name - # ...we do currently get the outer ones when the module path prefixes collide - @test_broken :f ∉ subsub_df.name - @test_broken :func ∉ subsub_df.name - - # starts from innermost - @test module_path(TestModA.SubModB.TestModA.TestModC) == - [:TestModC, :TestModA, :SubModB, :TestModA, :Main] - - from_outer_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModA.jl")) - from_inner_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl")) - @test from_inner_file == from_outer_file - @test "using .TestModA: f" in from_inner_file - # This one isn't needed bc all usages are fully qualified - @test "using .Exporter: exported_a" ∉ from_inner_file - - # This one isn't needed; it is already explicitly imported - @test "using .Exporter: exported_b" ∉ from_inner_file - - # This one shouldn't be there; we never use it, only explicitly import it. - # So actually it should be on a list of unnecessary imports. BUT it can show up - # because by importing it, we have the name in the file, so we used to detect it. - @test "using .Exporter: exported_c" ∉ from_inner_file - - @test from_inner_file == ["using .TestModA: TestModA", "using .TestModA: f"] - - ret = improper_explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - allow_internal_imports=false) - - @test [(; row.name) for row in ret if row.stale] == - [(; name=:exported_c), (; name=:exported_d)] - - # Recursive version - lookup = Dict(improper_explicit_imports(TestModA, - "TestModA.jl"; allow_internal_imports=false)) - ret = lookup[TestModA.SubModB.TestModA.TestModC] - - @test [(; row.name) for row in ret if row.stale] == - [(; name=:exported_c), (; name=:exported_d)] - @test isempty((row for row in lookup[TestModA] if row.stale)) - - per_usage_info, _ = analyze_all_names("TestModC.jl") - testmodc = DataFrame(per_usage_info) - qualified_row = only(subset(testmodc, :name => ByRow(==(:exported_a)))) - @test qualified_row.analysis_code == ExplicitImports.IgnoredQualified - @test qualified_row.qualified_by == [:Exporter] - - qualified_row2 = only(subset(testmodc, :name => ByRow(==(:h)))) - @test qualified_row2.qualified_by == [:TestModA, :SubModB] - - @test using_statement.(explicit_imports_nonrecursive(TestMod1, - "test_mods.jl")) == - ["using ExplicitImports: print_explicit_imports"] - - # Recursion - nested = explicit_imports(TestModA, "TestModA.jl") - @test nested isa Vector{Pair{Module, - Vector{@NamedTuple{name::Symbol,source::Module, - exporters::Vector{Module},location::String}}}} - @test TestModA in first.(nested) - @test TestModA.SubModB in first.(nested) - @test TestModA.SubModB.TestModA in first.(nested) - @test TestModA.SubModB.TestModA.TestModC in first.(nested) - - # Printing - # should be no logs - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "Module Main.TestModA is relying on implicit imports") - @test contains(str, "using .Exporter2: Exporter2, exported_a") - @test contains(str, - "However, module Main.TestModA.SubModB.TestModA.TestModC has stale explicit imports for these 2 unused names") - - # should be no logs - # try with linewidth tiny - should put one name per line - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - linewidth=0)) - @test contains(str, "using .Exporter2: Exporter2,\n exported_a") - - # test `show_locations=true` - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - show_locations=true, - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "using .Exporter3: Exporter3 # used at TestModA.jl:") - @test contains(str, "is unused but it was imported from Main.Exporter at TestModC.jl") - - # test `separate_lines=true`` - str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; - separate_lines=true, - allow_internal_imports=false)) - str = replace(str, r"\s+" => " ") - @test contains(str, "using .Exporter3: Exporter3 using .Exporter3: exported_b") - - # `warn_improper_explicit_imports=false` does something (also still no logs) - str_no_warn = @test_logs sprint(io -> print_explicit_imports(io, TestModA, - "TestModA.jl"; - warn_improper_explicit_imports=false)) - str = replace(str, r"\s+" => " ") - @test length(str_no_warn) <= length(str) - - # in particular, this ensures we add `using Foo: Foo` as the first line - @test using_statement.(explicit_imports_nonrecursive(TestMod4, "test_mods.jl")) == - ["using .Exporter4: Exporter4" - "using .Exporter4: A" - "using .Exporter4: Z" - "using .Exporter4: a" - "using .Exporter4: z"] -end + @testset "has_ancestor" begin + @test has_ancestor(TestModA.SubModB, TestModA) + @test !has_ancestor(TestModA, TestModA.SubModB) -@testset "checks" begin - @test check_no_implicit_imports(TestModEmpty, "test_mods.jl") === nothing - @test check_no_stale_explicit_imports(TestModEmpty, "test_mods.jl") === nothing - @test check_no_stale_explicit_imports(TestMod1, "test_mods.jl") === nothing - - @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, - "test_mods.jl") - - # test name ignores - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports,)) === nothing - - # test name mod pair ignores - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports => ExplicitImports,)) === - nothing - - # if you pass the module in the pair, you must get the right one - @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, - "test_mods.jl"; - ignore=(:print_explicit_imports => TestModA,)) === - nothing - - # non-existent names are OK - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - ignore=(:print_explicit_imports => ExplicitImports, - :does_not_exist)) === nothing - - # you can use skip to skip whole modules - @test check_no_implicit_imports(TestMod1, "test_mods.jl"; - skip=(Base, Core, ExplicitImports)) === nothing - - @test_throws ImplicitImportsException check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") - - # test submodule ignores - @test check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, "TestModC.jl"; - ignore=(TestModA.SubModB.TestModA.TestModC,)) === - nothing - - @test_throws StaleImportsException check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") - - # make sure ignored names don't show up in error - e = try - check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - ignore=(:exported_d,)) - @test false # should error before this - catch e - e + @test should_skip(Base.Iterators; skip=(Base, Core)) end - str = sprint(Base.showerror, e) - @test contains(str, "exported_c") - @test !contains(str, "exported_d") - - # ignore works: - @test check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl"; - ignore=(:exported_c, :exported_d)) === - nothing - - # Test the printing is hitting our formatted errors - str = exception_string() do - return check_no_implicit_imports(TestMod1, "test_mods.jl") - end - @test contains(str, "is relying on the following implicit imports") - str = exception_string() do - return check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, - "TestModC.jl") + function get_per_scope(per_usage_info) + per_usage_df = DataFrame(per_usage_info) + dropmissing!(per_usage_df, :external_global_name) + return per_usage_df end - @test contains(str, "has stale (unused) explicit imports for:") - @test check_all_explicit_imports_are_public(TestMod1, "test_mods.jl") === nothing - @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, - "imports.jl") - str = exception_string() do - return check_all_explicit_imports_are_public(ModImports, "imports.jl") - end - @test contains(str, "`_svd!` is not public in `LinearAlgebra` but it was imported") - @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; - ignore=(:_svd!, :exported_b, :f, :h, :map)) === - nothing - - @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; - ignore=(:_svd!, :exported_b, :f, :h), - skip=(LinearAlgebra => Base,)) === - nothing - - @testset "Tainted modules" begin - # 3 dynamic include statements - l = (:warn, r"Dynamic") - log = (l, l, l) - - @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl")) == - [DynMod => nothing, DynMod.Hidden => nothing] - @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl"; - strict=false)) == - [DynMod => [(; name=:print_explicit_imports, - source=ExplicitImports)], - # Wrong! Missing explicit export - DynMod.Hidden => []] - - @test_logs log... @test explicit_imports_nonrecursive(DynMod, "DynMod.jl") === - nothing - - @test_logs log... @test only_name_source(explicit_imports_nonrecursive(DynMod, - "DynMod.jl"; - strict=false)) == - [(; name=:print_explicit_imports, source=ExplicitImports)] - - @test @test_logs log... improper_explicit_imports(DynMod, - "DynMod.jl") == - [DynMod => nothing, - DynMod.Hidden => nothing] - - @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, - "DynMod.jl") === - nothing - - @test_logs log... @test improper_explicit_imports(DynMod, - "DynMod.jl"; - strict=false) == - [DynMod => [], - # Wrong! Missing stale explicit export - DynMod.Hidden => []] - - @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, - "DynMod.jl"; - strict=false) == - [] - - str = @test_logs log... sprint(print_explicit_imports, DynMod, "DynMod.jl") - @test contains(str, "DynMod could not be accurately analyzed") - - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - # Ignore also works - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod,), - ignore=(DynMod.Hidden,)) === - nothing - - e = UnanalyzableModuleException - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, - "DynMod.jl") - - # Missed `Hidden` - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, - "DynMod.jl"; - allow_unanalyzable=(DynMod,),) - - @test_logs log... @test check_no_stale_explicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - @test_logs log... @test_throws e check_no_stale_explicit_imports(DynMod, - "DynMod.jl") - - str = sprint(Base.showerror, UnanalyzableModuleException(DynMod)) - @test contains(str, "was found to be unanalyzable") - - @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod, - DynMod.Hidden)) === - nothing - - @test_logs log... @test_throws e check_no_implicit_imports(DynMod, "DynMod.jl"; - allow_unanalyzable=(DynMod,)) + @testset "file not found" begin + for f in (check_no_implicit_imports, check_no_stale_explicit_imports, + check_all_explicit_imports_via_owners, check_all_qualified_accesses_via_owners, + explicit_imports, + explicit_imports_nonrecursive, print_explicit_imports, + improper_explicit_imports, improper_explicit_imports_nonrecursive, + improper_qualified_accesses, improper_qualified_accesses_nonrecursive) + @test_throws FileNotFoundException f(TestModA) + end + str = sprint(Base.showerror, FileNotFoundException()) + @test contains(str, "module which is not top-level in a package") end -end -@testset "Aqua" begin - Aqua.test_all(ExplicitImports; ambiguities=false) -end + @testset "ExplicitImports.jl" begin + @test using_statement.(explicit_imports_nonrecursive(TestModA, "TestModA.jl")) == + ["using .Exporter: Exporter", "using .Exporter: @mac", + "using .Exporter2: Exporter2", + "using .Exporter2: exported_a", "using .Exporter3: Exporter3"] + + per_usage_info, _ = analyze_all_names("TestModA.jl") + df = get_per_scope(per_usage_info) + locals = contains.(string.(df.name), Ref("local")) + @test all(!, df.external_global_name[locals]) + + # we use `x` in two scopes + xs = subset(df, :name => ByRow(==(:x))) + @test !xs[1, :external_global_name] + @test !xs[2, :external_global_name] + @test xs[2, :analysis_code] == ExplicitImports.InternalAssignment + + # we use `exported_a` in two scopes; both times refer to the global name + exported_as = subset(df, :name => ByRow(==(:exported_a))) + @test exported_as[1, :external_global_name] + @test exported_as[2, :external_global_name] + @test !exported_as[2, :is_assignment] + + # Test submodules + @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB, + "TestModA.jl")) == + ["using .Exporter3: Exporter3", "using .Exporter3: exported_b", + "using .TestModA: f"] + + mod_path = module_path(TestModA.SubModB) + @test mod_path == [:SubModB, :TestModA, :Main] + sub_df = restrict_to_module(df, TestModA.SubModB) + + h = only(subset(sub_df, :name => ByRow(==(:h)))) + @test h.external_global_name + @test !h.is_assignment + + # Nested submodule with same name as outer module... + @test using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA, + "TestModA.jl")) == + ["using .Exporter3: Exporter3", "using .Exporter3: exported_b"] + + # Check we are getting innermost names and not outer ones + subsub_df = restrict_to_module(df, TestModA.SubModB.TestModA) + @test :inner_h in subsub_df.name + @test :h ∉ subsub_df.name + # ...we do currently get the outer ones when the module path prefixes collide + @test_broken :f ∉ subsub_df.name + @test_broken :func ∉ subsub_df.name + + # starts from innermost + @test module_path(TestModA.SubModB.TestModA.TestModC) == + [:TestModC, :TestModA, :SubModB, :TestModA, :Main] + + from_outer_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModA.jl")) + from_inner_file = using_statement.(explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl")) + @test from_inner_file == from_outer_file + @test "using .TestModA: f" in from_inner_file + # This one isn't needed bc all usages are fully qualified + @test "using .Exporter: exported_a" ∉ from_inner_file + + # This one isn't needed; it is already explicitly imported + @test "using .Exporter: exported_b" ∉ from_inner_file + + # This one shouldn't be there; we never use it, only explicitly import it. + # So actually it should be on a list of unnecessary imports. BUT it can show up + # because by importing it, we have the name in the file, so we used to detect it. + @test "using .Exporter: exported_c" ∉ from_inner_file + + @test from_inner_file == ["using .TestModA: TestModA", "using .TestModA: f"] + + ret = improper_explicit_imports_nonrecursive(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + allow_internal_imports=false) -@testset "`inspect_session`" begin - # We just want to make sure we are robust enough that this doesn't error - big_str = with_logger(Logging.NullLogger()) do - return sprint(inspect_session) - end -end + @test [(; row.name) for row in ret if row.stale] == + [(; name=:exported_c), (; name=:exported_d)] + + # Recursive version + lookup = Dict(improper_explicit_imports(TestModA, + "TestModA.jl"; + allow_internal_imports=false)) + ret = lookup[TestModA.SubModB.TestModA.TestModC] + + @test [(; row.name) for row in ret if row.stale] == + [(; name=:exported_c), (; name=:exported_d)] + @test isempty((row for row in lookup[TestModA] if row.stale)) + + per_usage_info, _ = analyze_all_names("TestModC.jl") + testmodc = DataFrame(per_usage_info) + qualified_row = only(subset(testmodc, :name => ByRow(==(:exported_a)))) + @test qualified_row.analysis_code == ExplicitImports.IgnoredQualified + @test qualified_row.qualified_by == [:Exporter] + + qualified_row2 = only(subset(testmodc, :name => ByRow(==(:h)))) + @test qualified_row2.qualified_by == [:TestModA, :SubModB] + + @test using_statement.(explicit_imports_nonrecursive(TestMod1, + "test_mods.jl")) == + ["using ExplicitImports: print_explicit_imports"] + + # Recursion + nested = explicit_imports(TestModA, "TestModA.jl") + @test nested isa Vector{Pair{Module, + Vector{@NamedTuple{name::Symbol,source::Module, + exporters::Vector{Module},location::String}}}} + @test TestModA in first.(nested) + @test TestModA.SubModB in first.(nested) + @test TestModA.SubModB.TestModA in first.(nested) + @test TestModA.SubModB.TestModA.TestModC in first.(nested) + + # Printing + # should be no logs + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "Module Main.TestModA is relying on implicit imports") + @test contains(str, "using .Exporter2: Exporter2, exported_a") + @test contains(str, + "However, module Main.TestModA.SubModB.TestModA.TestModC has stale explicit imports for these 2 unused names") + + # should be no logs + # try with linewidth tiny - should put one name per line + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + linewidth=0)) + @test contains(str, "using .Exporter2: Exporter2,\n exported_a") + + # test `show_locations=true` + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + show_locations=true, + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "using .Exporter3: Exporter3 # used at TestModA.jl:") + @test contains(str, + "is unused but it was imported from Main.Exporter at TestModC.jl") + + # test `separate_lines=true`` + str = @test_logs sprint(io -> print_explicit_imports(io, TestModA, "TestModA.jl"; + separate_lines=true, + allow_internal_imports=false)) + str = replace(str, r"\s+" => " ") + @test contains(str, "using .Exporter3: Exporter3 using .Exporter3: exported_b") -@testset "backtick modules and locations" begin - @testset "print_explicit_imports" begin - # Test that module names and file:line locations are surrounded by backticks - # and that underscores in module and file names are printed and do not cause italics. - str = sprint() do io - print_explicit_imports(io, Test_Mod_Underscores, "Test_Mod_Underscores.jl"; report_non_public=true) - end + # `warn_improper_explicit_imports=false` does something (also still no logs) + str_no_warn = @test_logs sprint(io -> print_explicit_imports(io, TestModA, + "TestModA.jl"; + warn_improper_explicit_imports=false)) str = replace(str, r"\s+" => " ") - # stale import - @test contains(str, "Test_Mod_Underscores has stale explicit imports") - @test contains(str, "svd is unused but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # non-owner module - @test contains(str, "Test_Mod_Underscores explicitly imports 1 name from non-owner module") - @test contains(str, "map has owner Base but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # non-public name - @test contains(str, "Test_Mod_Underscores explicitly imports 1 non-public name") - @test contains(str, "_svd! is not public in LinearAlgebra but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") - # self-qualified access - @test contains(str, "Test_Mod_Underscores has 1 self-qualified access") - @test contains(str, "foo was accessed as Main.Test_Mod_Underscores.foo inside Main.TestModUnderscores at Test_Mod_Underscores.jl") - # access non-owner module - @test contains(str, "Test_Mod_Underscores accesses 1 name from non-owner modules") - @test contains(str, "Number has owner Base but it was accessed from Base.Sys at Test_Mod_Underscores.jl") - # access non-public name - @test contains(str, "Test_Mod_Underscores accesses 1 non-public name") - @test contains(str, "__unsafe_string! is not public in Base but it was accessed via Base at Test_Mod_Underscores.jl") + @test length(str_no_warn) <= length(str) + + # in particular, this ensures we add `using Foo: Foo` as the first line + @test using_statement.(explicit_imports_nonrecursive(TestMod4, "test_mods.jl")) == + ["using .Exporter4: Exporter4" + "using .Exporter4: A" + "using .Exporter4: Z" + "using .Exporter4: a" + "using .Exporter4: z"] end - @testset "check_*" begin - str = exception_string() do - check_no_implicit_imports(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + + @testset "checks" begin + @test check_no_implicit_imports(TestModEmpty, "test_mods.jl") === nothing + @test check_no_stale_explicit_imports(TestModEmpty, "test_mods.jl") === nothing + @test check_no_stale_explicit_imports(TestMod1, "test_mods.jl") === nothing + + @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, + "test_mods.jl") + + # test name ignores + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports,)) === nothing + + # test name mod pair ignores + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports => ExplicitImports,)) === + nothing + + # if you pass the module in the pair, you must get the right one + @test_throws ImplicitImportsException check_no_implicit_imports(TestMod1, + "test_mods.jl"; + ignore=(:print_explicit_imports => TestModA,)) === + nothing + + # non-existent names are OK + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + ignore=(:print_explicit_imports => ExplicitImports, + :does_not_exist)) === nothing + + # you can use skip to skip whole modules + @test check_no_implicit_imports(TestMod1, "test_mods.jl"; + skip=(Base, Core, ExplicitImports)) === nothing + + @test_throws ImplicitImportsException check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") + + # test submodule ignores + @test check_no_implicit_imports(TestModA.SubModB.TestModA.TestModC, "TestModC.jl"; + ignore=(TestModA.SubModB.TestModA.TestModC,)) === + nothing + + @test_throws StaleImportsException check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") + + # make sure ignored names don't show up in error + e = try + check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + ignore=(:exported_d,)) + @test false # should error before this + catch e + e end - @test contains(str, "`Main.Test_Mod_Underscores` is relying on") + str = sprint(Base.showerror, e) + @test contains(str, "exported_c") + @test !contains(str, "exported_d") + + # ignore works: + @test check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl"; + ignore=(:exported_c, :exported_d)) === + nothing + + # Test the printing is hitting our formatted errors str = exception_string() do - check_all_explicit_imports_via_owners(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_no_implicit_imports(TestMod1, "test_mods.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + @test contains(str, "is relying on the following implicit imports") + str = exception_string() do - check_all_explicit_imports_are_public(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_no_stale_explicit_imports(TestModA.SubModB.TestModA.TestModC, + "TestModC.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + @test contains(str, "has stale (unused) explicit imports for:") + + @test check_all_explicit_imports_are_public(TestMod1, "test_mods.jl") === nothing + @test_throws NonPublicExplicitImportsException check_all_explicit_imports_are_public(ModImports, + "imports.jl") str = exception_string() do - check_no_stale_explicit_imports(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + return check_all_explicit_imports_are_public(ModImports, "imports.jl") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has stale") - str = exception_string() do - check_all_qualified_accesses_via_owners(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + @test contains(str, "`_svd!` is not public in `LinearAlgebra` but it was imported") + @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; + ignore=(:_svd!, :exported_b, :f, :h, + :map)) === + nothing + + @test check_all_explicit_imports_are_public(ModImports, "imports.jl"; + ignore=(:_svd!, :exported_b, :f, :h), + skip=(LinearAlgebra => Base,)) === + nothing + + @testset "Tainted modules" begin + # 3 dynamic include statements + l = (:warn, r"Dynamic") + log = (l, l, l) + + @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl")) == + [DynMod => nothing, DynMod.Hidden => nothing] + @test_logs log... @test only_name_source(explicit_imports(DynMod, "DynMod.jl"; + strict=false)) == + [DynMod => [(; name=:print_explicit_imports, + source=ExplicitImports)], + # Wrong! Missing explicit export + DynMod.Hidden => []] + + @test_logs log... @test explicit_imports_nonrecursive(DynMod, "DynMod.jl") === + nothing + + @test_logs log... @test only_name_source(explicit_imports_nonrecursive(DynMod, + "DynMod.jl"; + strict=false)) == + [(; name=:print_explicit_imports, + source=ExplicitImports)] + + @test @test_logs log... improper_explicit_imports(DynMod, + "DynMod.jl") == + [DynMod => nothing, + DynMod.Hidden => nothing] + + @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, + "DynMod.jl") === + nothing + + @test_logs log... @test improper_explicit_imports(DynMod, + "DynMod.jl"; + strict=false) == + [DynMod => [], + # Wrong! Missing stale explicit export + DynMod.Hidden => []] + + @test_logs log... @test improper_explicit_imports_nonrecursive(DynMod, + "DynMod.jl"; + strict=false) == + [] + + str = @test_logs log... sprint(print_explicit_imports, DynMod, "DynMod.jl") + @test contains(str, "DynMod could not be accurately analyzed") + + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + # Ignore also works + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod,), + ignore=(DynMod.Hidden,)) === + nothing + + e = UnanalyzableModuleException + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, + "DynMod.jl") + + # Missed `Hidden` + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, + "DynMod.jl"; + allow_unanalyzable=(DynMod,),) + + @test_logs log... @test check_no_stale_explicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + @test_logs log... @test_throws e check_no_stale_explicit_imports(DynMod, + "DynMod.jl") + + str = sprint(Base.showerror, UnanalyzableModuleException(DynMod)) + @test contains(str, "was found to be unanalyzable") + + @test_logs log... @test check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod, + DynMod.Hidden)) === + nothing + + @test_logs log... @test_throws e check_no_implicit_imports(DynMod, "DynMod.jl"; + allow_unanalyzable=(DynMod,)) end - @test contains(str, "Module `Main.Test_Mod_Underscores` has qualified accesses") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") - str = exception_string() do - check_all_qualified_accesses_are_public(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + end + + # TODO reenable this + # @testset "Aqua" begin + # Aqua.test_all(ExplicitImports; ambiguities=false) + # end + + @testset "`inspect_session`" begin + # We just want to make sure we are robust enough that this doesn't error + big_str = with_logger(Logging.NullLogger()) do + return sprint(inspect_session) end - @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports of") - @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") - str = exception_string() do - check_no_self_qualified_accesses(Test_Mod_Underscores, "Test_Mod_Underscores.jl") + end + + @testset "backtick modules and locations" begin + @testset "print_explicit_imports" begin + # Test that module names and file:line locations are surrounded by backticks + # and that underscores in module and file names are printed and do not cause italics. + str = sprint() do io + return print_explicit_imports(io, Test_Mod_Underscores, + "Test_Mod_Underscores.jl"; + report_non_public=true) + end + str = replace(str, r"\s+" => " ") + # stale import + @test contains(str, "Test_Mod_Underscores has stale explicit imports") + @test contains(str, + "svd is unused but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # non-owner module + @test contains(str, + "Test_Mod_Underscores explicitly imports 1 name from non-owner module") + @test contains(str, + "map has owner Base but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # non-public name + @test contains(str, "Test_Mod_Underscores explicitly imports 1 non-public name") + @test contains(str, + "_svd! is not public in LinearAlgebra but it was imported from LinearAlgebra at Test_Mod_Underscores.jl") + # self-qualified access + @test contains(str, "Test_Mod_Underscores has 1 self-qualified access") + @test contains(str, + "foo was accessed as Main.Test_Mod_Underscores.foo inside Main.TestModUnderscores at Test_Mod_Underscores.jl") + # access non-owner module + @test contains(str, + "Test_Mod_Underscores accesses 1 name from non-owner modules") + @test contains(str, + "Number has owner Base but it was accessed from Base.Sys at Test_Mod_Underscores.jl") + # access non-public name + @test contains(str, "Test_Mod_Underscores accesses 1 non-public name") + @test contains(str, + "__unsafe_string! is not public in Base but it was accessed via Base at Test_Mod_Underscores.jl") + end + @testset "check_*" begin + str = exception_string() do + return check_no_implicit_imports(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "`Main.Test_Mod_Underscores` is relying on") + str = exception_string() do + return check_all_explicit_imports_via_owners(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has explicit imports") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_all_explicit_imports_are_public(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_no_stale_explicit_imports(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has stale") + str = exception_string() do + return check_all_qualified_accesses_via_owners(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, "Module `Main.Test_Mod_Underscores` has qualified accesses") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_all_qualified_accesses_are_public(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, + "Module `Main.Test_Mod_Underscores` has explicit imports of") + @test contains(str, r"at `Test_Mod_Underscores.jl:\d+:\d+`") + str = exception_string() do + return check_no_self_qualified_accesses(Test_Mod_Underscores, + "Test_Mod_Underscores.jl") + end + @test contains(str, + "Module `Main.Test_Mod_Underscores` has self-qualified accesses") + @test contains(str, + r"accessed as `Main.Test_Mod_Underscores.foo` inside `Main.Test_Mod_Underscores` at `Test_Mod_Underscores.jl:10:40`") end - @test contains(str, "Module `Main.Test_Mod_Underscores` has self-qualified accesses") - @test contains(str, r"accessed as `Main.Test_Mod_Underscores.foo` inside `Main.Test_Mod_Underscores` at `Test_Mod_Underscores.jl:10:40`") end end From ebdf549be740bacddf3535fd47ecf7ef7fef03a8 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 22:44:31 +0200 Subject: [PATCH 17/19] re-enable aqua --- test/runtests.jl | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 1b6315f..d1bbf65 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1062,12 +1062,6 @@ include("module_alias.jl") allow_unanalyzable=(DynMod,)) end end - - # TODO reenable this - # @testset "Aqua" begin - # Aqua.test_all(ExplicitImports; ambiguities=false) - # end - @testset "`inspect_session`" begin # We just want to make sure we are robust enough that this doesn't error big_str = with_logger(Logging.NullLogger()) do @@ -1158,4 +1152,9 @@ include("module_alias.jl") r"accessed as `Main.Test_Mod_Underscores.foo` inside `Main.Test_Mod_Underscores` at `Test_Mod_Underscores.jl:10:40`") end end + + + @testset "Aqua" begin + Aqua.test_all(ExplicitImports; ambiguities=false) + end end From 42eec55e6dcba56de4dca3ad542e095fcb33cfd4 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 22:46:24 +0200 Subject: [PATCH 18/19] support 1.12-beta --- src/find_implicit_imports.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/find_implicit_imports.jl b/src/find_implicit_imports.jl index 490f78e..9769669 100644 --- a/src/find_implicit_imports.jl +++ b/src/find_implicit_imports.jl @@ -45,6 +45,7 @@ function find_implicit_imports(mod::Module; skip=(mod, Base, Core)) # `WARNING: both Exporter3 and Exporter2 export "exported_a"; uses of it in module TestModA must be qualified` # and there is an ambiguity, and the name is in fact not resolved in `mod` clash = (err == ErrorException("\"$name\" is not defined in module $mod"))::Bool + clash |= (err == ErrorException("Constant binding was imported from multiple modules"))::Bool # if it is something else, rethrow clash || rethrow() missing From 326f419c843c74f43c0572c6c1de7ad145e16cea Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Fri, 13 Jun 2025 22:56:36 +0200 Subject: [PATCH 19/19] format --- test/runtests.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index d1bbf65..7b49a8e 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1153,7 +1153,6 @@ include("module_alias.jl") end end - @testset "Aqua" begin Aqua.test_all(ExplicitImports; ambiguities=false) end