From 0da6453758bf1d10075e276a73b1b5541811c318 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 26 Jun 2019 21:10:42 -0700 Subject: [PATCH] Make it possible to exclude methods from non-direct dependencies --- src/ambiguities.jl | 62 +++++++++++++++++++++++--------------------- test/test_exclude.jl | 38 ++++++++++++--------------- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/ambiguities.jl b/src/ambiguities.jl index e4d39a4e..9a2e86e2 100644 --- a/src/ambiguities.jl +++ b/src/ambiguities.jl @@ -10,15 +10,13 @@ false-positive. - `color::Union{Bool, Nothing} = nothing`: Enable/disable colorful output if a `Bool`. `nothing` (default) means to inherit the setting in the current process. -- `exclude::AbstractArray = []`: A vector of functions, types, or - strings for excluding them from ambiguity testing. A function means - to exclude _all_ its methods. A type means to exclude _all_ its - methods of the callable (sometimes also called "functor"). That is - to say, `MyModule.MyType` means to ignore ambiguities between - `(::MyType)(x, y::Int)` and `(::MyType)(x::Int, y)`. Note that - there is no way to exclude the constructor of a specific type at the - moment. A fully qualified name of function or type can also be - specified as a string (e.g., `"Base.foldl"`). +- `exclude::AbstractArray = []`: A vector of functions or types to be + excluded from ambiguity testing. A function means to exclude _all_ + its methods. A type means to exclude _all_ its methods of the + callable (sometimes also called "functor"). That is to say, + `MyModule.MyType` means to ignore ambiguities between `(::MyType)(x, + y::Int)` and `(::MyType)(x::Int, y)`. Note that there is no way to + exclude the constructor of a specific type at the moment. - `recursive::Bool = true`: Passed to `Test.detect_ambiguities`. Note that the default here (`true`) is different from `detect_ambiguities`. This is for testing ambiguities in methods @@ -29,6 +27,8 @@ false-positive. test_ambiguities(packages; kwargs...) = _test_ambiguities(aspkgids(packages); kwargs...) +const ExcludeSpec = Pair{Base.PkgId,String} + aspkgids(pkg::Union{Module, PkgId}) = aspkgids([pkg]) aspkgids(packages) = mapfoldl(aspkgid, push!, packages, init=PkgId[]) @@ -55,32 +55,36 @@ ispackage(m::Module) = strnameof(x) = string(x) strnameof(x::Type) = string(nameof(x)) -normalize_exclude(x::String) = x +rootmodule(x) = rootmodule(parentmodule(x)) +rootmodule(m::Module) = Base.require(PkgId(m)) # this handles Base/Core well + normalize_exclude(x::Union{Type, Function}) = + Base.PkgId(rootmodule(x)) => join((fullname(parentmodule(x))..., strnameof(x)), ".") normalize_exclude(::Any) = error("Only a function and type can be excluded.") -function getobj(spec::String, modules) - nameparts = Symbol.(split(spec, ".")) - for m in modules - if nameparts[1] === nameof(m) - return foldl(getproperty, nameparts[2:end], init=m) - end - end - error("Object $spec not found in following modules:\n$modules") +function getobj((pkgid, name)::ExcludeSpec) + nameparts = Symbol.(split(name, ".")) + m = Base.require(pkgid) + return foldl(getproperty, nameparts, init=m) end -function normalize_and_check_exclude(exclude::AbstractVector, packages) - strexclude = mapfoldl(normalize_exclude, push!, exclude, init=String[]) - modules = map(Base.require, packages) - for (str, obj) in zip(strexclude, exclude) - obj isa String && continue - if getobj(str, modules) !== obj +function normalize_and_check_exclude(exclude::AbstractVector) + exspecs = mapfoldl(normalize_exclude, push!, exclude, init=ExcludeSpec[]) + for (spec, obj) in zip(exspecs, exclude) + if getobj(spec) !== obj error("Name `$str` is resolved to a different object.") end end - return strexclude :: Vector{String} + return exspecs :: Vector{ExcludeSpec} +end + +function reprexclude(exspecs::Vector{ExcludeSpec}) + itemreprs = map(exspecs) do (pkgid, name) + string("(", reprpkgid(pkgid), " => ", repr(name), ")") + end + return string("Aqua.ExcludeSpec[", join(itemreprs, ", "), "]") end function _test_ambiguities( @@ -98,7 +102,7 @@ function _test_ambiguities( imported = imported, ambiguous_bottom = ambiguous_bottom, )) - exclude_repr = repr(normalize_and_check_exclude(exclude, packages)) + exclude_repr = reprexclude(normalize_and_check_exclude(exclude)) # Ambiguity test is run inside a clean process. # https://github.com/JuliaLang/julia/issues/28804 @@ -145,14 +149,14 @@ getobj(m::Method) = getproperty(m.module, m.name) function test_ambiguities_impl( packages::Vector{PkgId}, options::NamedTuple, - exclude::Vector{String}, + exspecs::Vector{ExcludeSpec}, ) modules = map(Base.require, packages) @debug "Testing method ambiguities" modules ambiguities = detect_ambiguities(modules...; options...) - if !isempty(exclude) - exclude_objs = getobj.(exclude, Ref(modules)) + if !isempty(exspecs) + exclude_objs = getobj.(exspecs) ambiguities = filter(ambiguities) do (m1, m2) # `getobj(m1) == getobj(m2)` so no need to check `m2` getobj(m1) ∉ exclude_objs diff --git a/test/test_exclude.jl b/test/test_exclude.jl index a242bada..f1818907 100644 --- a/test/test_exclude.jl +++ b/test/test_exclude.jl @@ -1,10 +1,9 @@ module TestExclude include("preamble.jl") -using Aqua: getobj, normalize_exclude, normalize_and_check_exclude - -rootmodule(m::Module) = Base.require(Base.PkgId(m)) -rootmodule(x) = rootmodule(parentmodule(x)) +using Base: PkgId +using Aqua: getobj, normalize_exclude, normalize_and_check_exclude, rootmodule, + reprexclude @assert parentmodule(Tuple) === Core @assert parentmodule(foldl) === Base @@ -19,31 +18,28 @@ rootmodule(x) = rootmodule(parentmodule(x)) Tuple Broadcast.Broadcasted ] - modules = [rootmodule(x)] - @test getobj(normalize_exclude(x), modules) == x + @test getobj(normalize_exclude(x)) == x end - @testset "$(repr(y))" for (y, modules) in [ - ("Base.foldl", [Base]) - ("Base.Some", [Base]) - ("Core.Tuple", [Core]) - ("Base.Broadcast.Broadcasted", [Base]) + @testset "$(repr(last(spec)))" for spec in [ + (PkgId(Base) => "Base.foldl") + (PkgId(Base) => "Base.Some") + (PkgId(Core) => "Core.Tuple") + (PkgId(Base) => "Base.Broadcast.Broadcasted") ] - @test normalize_exclude(getobj(y, modules)) == y + @test normalize_exclude(getobj(spec)) === spec end end @testset "normalize_and_check_exclude" begin - @testset "$i" for (i, (exclude, modules)) in enumerate([ - ([foldl], [Base]) - (["Base.foldl"], [Base]) - ([foldl, Some], [Base]) - (["Base.foldl", "Base.Some"], [Base]) - ([foldl, Tuple], [Base, Core]) - (["Base.foldl", "Core.Tuple"], [Base, Core]) + @testset "$i" for (i, exclude) in enumerate([ + [foldl], + [foldl, Some], + [foldl, Tuple], ]) - packages = Base.PkgId.(modules) - @test normalize_and_check_exclude(exclude, packages) isa Vector{String} + local specs + @test (specs = normalize_and_check_exclude(exclude)) isa Vector + @test Base.include_string(@__MODULE__, reprexclude(specs)) == specs end end