use more precise structural inspection of types for ambiguity exclusions

src/ambiguities.jl

@@ -29,30 +29,31 @@
 
 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,35 +155,18 @@
     return "Base.PkgId(Base.UUID($(repr(uuid.value))), $(repr(name)))"
 end
 
-struct _NoValue end
-
-function getobj(m::Method)
-    signature = Base.unwrap_unionall(m.sig)
-    ty = if is_kwcall(signature)
-        signature.parameters[3]
-    else
-        signature.parameters[1]
+# 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
-    ty = Base.unwrap_unionall(ty)
-    if ty <: Function
-        try
-            return ty.instance  # this should work for functions
-        catch
-        end
-    end
-    try
-        if ty.name.wrapper === Type
-            return ty.parameters[1]
-        else
-            return ty.name.wrapper
-        end
-    catch err
-        @error(
-            "Failed to obtain a function from `Method`.",
-            exception = (err, catch_backtrace())
-        )
+    if ft isa DataType
+        return ft.name.wrapper
     end
-    return _NoValue()
+    return nothing # cannot exclude signatures with Union
 end
 
 function test_ambiguities_impl(
@@ -196,9 +180,9 @@
     ambiguities = detect_ambiguities(modules...; options...)
 
     if !isempty(exspecs)
-        exclude_objs = getobj.(exspecs)
+        exclude_ft = Any[getexclude(spec) for spec in exspecs] # vector of Type objects
         ambiguities = filter(ambiguities) do (m1, m2)
-            getobj(m1) ∉ exclude_objs && getobj(m2) ∉ exclude_objs
+            trygetft(m1) ∉ exclude_ft && trygetft(m2) ∉ exclude_ft
         end
     end
 

src/piracies.jl

@@ -156,7 +156,7 @@ function is_pirate(meth::Method; treat_as_own = Union{Function,Type}[])
     signature = Base.unwrap_unionall(meth.sig)
 
     function_type_index = 1
-    if is_kwcall(signature)
+    if is_kwcall(meth.sig)
         # kwcall is a special case, since it is not a real function
         # but a wrapper around a function, the third parameter is the original
         # function, its positional arguments follow.

src/utils.jl

@@ -55,8 +55,9 @@ function checked_repr(obj)
     return code
 end
 
-function is_kwcall(signature::DataType)
+function is_kwcall(signature::Type)
     @static if VERSION < v"1.9"
+        signature = Base.unwrap_unionall(signature)::DataType
         try
             length(signature.parameters) >= 3 || return false
             signature <: Tuple{Function,Any,Any,Vararg} || return false
@@ -69,6 +70,6 @@ function is_kwcall(signature::DataType)
             return false
         end
     else
-        return signature.parameters[1] === typeof(Core.kwcall)
+        return signature <: Tuple{typeof(Core.kwcall), Any, Any, Vararg}
     end
 end

test/test_ambiguities.jl

@@ -24,6 +24,11 @@ include("preamble.jl")
         if broken
             @test_broken num_ambiguities_ == num_ambiguities
         else
+            if num_ambiguities_ != num_ambiguities
+                @show exclude
+                println(strout)
+                println(strerr)
+            end
             @test num_ambiguities_ == num_ambiguities
         end
     end

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