go back to doing some structural evaluation, but more carefully using types

Even though that means there are cases that will be impossible to
exclude, the author of Aqua confirmed that is intentional.

Author: vtjnash

src/ambiguities.jl

@@ -29,30 +29,31 @@ test_ambiguities(packages; kwargs...) = _test_ambiguities(aspkgids(packages); kw
 
 const ExcludeSpec = Pair{Base.PkgId,String}
 
-strnameof(x) = string(x)
-strnameof(x::Type) = string(nameof(x))
-
-rootmodule(x) = rootmodule(parentmodule(x))
+rootmodule(x::Type) = 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.")
+# get the Type associated with x
+normalize_exclude_obj(@nospecialize x) = x isa Type ? x : typeof(x)
+
+function normalize_exclude(@nospecialize x)
+    x = normalize_exclude_obj(x)
+    return Base.PkgId(rootmodule(x)) => join((fullname(parentmodule(x))..., string(nameof(x))), ".")
+end
 
-function getobj((pkgid, name)::ExcludeSpec)
+function getexclude((pkgid, name)::ExcludeSpec)
     nameparts = Symbol.(split(name, "."))
     m = Base.require(pkgid)
     for name in nameparts
         m = getproperty(m, name)
     end
-    return m
+    return normalize_exclude_obj(m)
 end
 
 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 `$(spec[2])` is resolved to a different object.")
+    exspecs = ExcludeSpec[normalize_exclude(exspec) for exspec in exclude]
+    for (i, exspec) in enumerate(exspecs)
+        if getexclude(exspec) !== normalize_exclude_obj(exclude[i])
+            error("Name `$(exspec[2])` is resolved to a different object.")
         end
     end
     return exspecs::Vector{ExcludeSpec}
@@ -154,6 +155,20 @@ function reprpkgid(pkg::PkgId)
     return "Base.PkgId(Base.UUID($(repr(uuid.value))), $(repr(name)))"
 end
 
+# try to extract the called function, or nothing if it is hard to analyze
+function trygetft(m::Method)
+    sig = Base.unwrap_unionall(m.sig)::DataType
+    ft = sig.parameters[is_kwcall(sig) ? 3 : 1]
+    ft = Base.unwrap_unionall(ft)
+    if ft isa DataType && ft.name === Type.body.name
+        ft = Base.unwrap_unionall(ft.parameters[1])
+    end
+    if ft isa DataType
+        return ft.name.wrapper
+    end
+    return nothing # cannot exclude signatures with Union
+end
+
 function test_ambiguities_impl(
     packages::Vector{PkgId},
     options::NamedTuple,
@@ -165,26 +180,9 @@ function test_ambiguities_impl(
     ambiguities = detect_ambiguities(modules...; options...)
 
     if !isempty(exspecs)
-        exclude_ft = Any[getobj(spec) for spec in exspecs]
-        exclude_sig = Any[]
-        for ft in exclude_ft
-            if ft isa Type
-                push!(exclude_sig, Tuple{ft, Vararg})
-                push!(exclude_sig, Tuple{Core.kwftype(ft), Any, ft, Vararg})
-                ft = Type{<:ft} # alternatively, Type{ft}
-            else
-                ft = typeof(ft)
-            end
-            push!(exclude_sig, Tuple{ft, Vararg})
-            push!(exclude_sig, Tuple{Core.kwftype(ft), Any, ft, Vararg})
-        end
+        exclude_ft = Any[getexclude(spec) for spec in exspecs] # vector of Type objects
         ambiguities = filter(ambiguities) do (m1, m2)
-            for excl in exclude_sig
-                if m1.sig <: excl || m2.sig <: excl
-                    return false
-                end
-            end
-            return true
+            trygetft(m1) ∉ exclude_ft && trygetft(m2) ∉ exclude_ft
         end
     end
 

test/test_ambiguities.jl

@@ -49,7 +49,7 @@ include("preamble.jl")
     check_testcase([PkgWithAmbiguities.ConcreteType], total - num_ambs_ConcreteType)
 
     # exclude abstract supertype without callables and constructors
-    check_testcase([PkgWithAmbiguities.AbstractType], total - num_ambs_SingletonType - num_ambs_ConcreteType)
+    check_testcase([PkgWithAmbiguities.AbstractType], total)
 
     # for ambiguities between abstract and concrete type callables, only one needs to be excluded
     check_testcase(

test/test_exclude.jl

@@ -2,7 +2,7 @@ module TestExclude
 
 include("preamble.jl")
 using Base: PkgId
-using Aqua: getobj, normalize_exclude, normalize_and_check_exclude, rootmodule, reprexclude
+using Aqua: getexclude, normalize_exclude, normalize_exclude_obj, normalize_and_check_exclude, rootmodule, reprexclude
 
 @assert parentmodule(Tuple) === Core
 @assert parentmodule(foldl) === Base
@@ -11,27 +11,32 @@ using Aqua: getobj, normalize_exclude, normalize_and_check_exclude, rootmodule,
 @assert rootmodule(Broadcast.Broadcasted) === Base
 
 @testset "roundtrip" begin
-    @testset for x in [
+    @testset for x in Any[
         foldl
         Some
         Tuple
         Broadcast.Broadcasted
+        nothing
+        Any
     ]
-        @test getobj(normalize_exclude(x)) == x
+        @test getexclude(normalize_exclude(x)) === normalize_exclude_obj(x)
     end
+    @test_throws ErrorException normalize_and_check_exclude(Any[Pair{Int}])
 
     @testset "$(repr(last(spec)))" for spec in [
-        (PkgId(Base) => "Base.foldl")
+        (PkgId(Base) => "Base.#foldl")
         (PkgId(Base) => "Base.Some")
         (PkgId(Core) => "Core.Tuple")
         (PkgId(Base) => "Base.Broadcast.Broadcasted")
+        (PkgId(Core) => "Core.Nothing")
+        (PkgId(Core) => "Core.Any")
     ]
-        @test normalize_exclude(getobj(spec)) === spec
+        @test normalize_exclude(getexclude(spec)) === spec
     end
 end
 
 @testset "normalize_and_check_exclude" begin
-    @testset "$i" for (i, exclude) in enumerate([[foldl], [foldl, Some], [foldl, Tuple]])
+    @testset "$i" for (i, exclude) in enumerate([Any[foldl], Any[foldl, Some], Any[foldl, Tuple]])
         local specs
         @test (specs = normalize_and_check_exclude(exclude)) isa Vector
         @test Base.include_string(@__MODULE__, reprexclude(specs)) == specs