Enhance test_ambiguities' exclude options for functions with kwargs (…

…#204)
JuliaTesting · Oct 31, 2023 · 8348f74 · 8348f74
1 parent dd5f929
commit 8348f74
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - The minimum requirement for julia was raised from `1.0` to `1.4`. ([#221](https://github.com/JuliaTesting/Aqua.jl/pull/221))
+- `test_ambiguities` now excludes the keyword sorter of all `exclude`d functions with keyword arguments as well. ([#203](https://github.com/JuliaTesting/Aqua.jl/pull/204))
 - In `test_deps_compat`, the two subtests `check_extras` and `check_weakdeps` are now run by default. ([#202](https://github.com/JuliaTesting/Aqua.jl/pull/202)) [BREAKING]
 - `test_deps_compat` now reqiures compat entries for all dependencies. Stdlibs no longer get ignored. This change is motivated by similar changes in the General registry. ([#215](https://github.com/JuliaTesting/Aqua.jl/pull/215)) [BREAKING]
 

diff --git a/src/ambiguities.jl b/src/ambiguities.jl
@@ -176,16 +176,11 @@ end
 struct _NoValue end
 
 function getobj(m::Method)
-    ty = try
-        fieldtype(m.sig, 1)
-    catch err
-        @error(
-            "Failed to obtain a function from `Method`.",
-            exception = (err, catch_backtrace())
-        )
-        # If this doesn't work, `Base` internal was probably changed
-        # too much compared to what it is now.  So, bailing out.
-        return _NoValue()
+    signature = Base.unwrap_unionall(m.sig)
+    ty = if is_kwcall(signature)
+        signature.parameters[3]
+    else
+        signature.parameters[1]
     end
     ty = Base.unwrap_unionall(ty)
     if ty <: Function

diff --git a/test/pkgs/PkgWithAmbiguities.jl b/test/pkgs/PkgWithAmbiguities.jl
@@ -3,6 +3,15 @@ module PkgWithAmbiguities
 # 1 ambiguity
 f(::Any, ::Int) = 1
 f(::Int, ::Any) = 2
+const num_ambs_f = 1
+
+# 2 ambiguities:
+#   1 for g
+#   1 for Core.kwfunc(g) if VERSION >= 1.4
+#   2 for Core.kwfunc(g) if VERSION < 1.4
+g(::Any, ::Int; kw) = 1
+g(::Int, ::Any; kw) = 2
+const num_ambs_g = VERSION >= v"1.4-" ? 2 : 3
 
 abstract type AbstractType end
 struct SingletonType <: AbstractType end
@@ -20,6 +29,8 @@ SingletonType(::Int, ::Any, ::Any) = 3
 (::SingletonType)(::Any, ::Float64) = 1
 (::SingletonType)(::Float64, ::Any) = 2
 
+const num_ambs_SingletonType = 3
+
 # 3 ambiguities
 ConcreteType(::Any, ::Any, ::Int) = 1
 ConcreteType(::Any, ::Int, ::Any) = 2
@@ -29,10 +40,14 @@ ConcreteType(::Int, ::Any, ::Any) = 3
 (::ConcreteType)(::Any, ::Float64) = 1
 (::ConcreteType)(::Float64, ::Any) = 2
 
-# 1 ambiguitiy
+const num_ambs_ConcreteType = 4
+
+# 1 ambiguity 
 abstract type AbstractParameterizedType{T} end
 struct ConcreteParameterizedType{T} <: AbstractParameterizedType{T} end
 (::AbstractParameterizedType{T})(::Tuple{Tuple{Int}}) where {T} = 1
 (::ConcreteParameterizedType)(::Tuple) = 2
 
+const num_ambs_ParameterizedType = 1
+
 end  # module
diff --git a/test/test_ambiguities.jl b/test/test_ambiguities.jl
@@ -2,9 +2,28 @@ module TestAmbiguities
 
 include("preamble.jl")
 
-using PkgWithAmbiguities
-
 @testset begin
+    using PkgWithAmbiguities
+
+    @static if VERSION >= v"1.3-"
+        using PkgWithAmbiguities:
+            num_ambs_f,
+            num_ambs_g,
+            num_ambs_SingletonType,
+            num_ambs_ConcreteType,
+            num_ambs_ParameterizedType
+        total =
+            num_ambs_f +
+            num_ambs_g +
+            num_ambs_SingletonType +
+            num_ambs_ConcreteType +
+            num_ambs_ParameterizedType
+    else
+        using PkgWithAmbiguities:
+            num_ambs_f, num_ambs_g, num_ambs_SingletonType, num_ambs_ConcreteType
+        total = num_ambs_f + num_ambs_g + num_ambs_SingletonType + num_ambs_ConcreteType
+    end
+
     function check_testcase(exclude, num_ambiguities::Int; broken::Bool = false)
         pkgids = Aqua.aspkgids([PkgWithAmbiguities, Core]) # include Core to find constructor ambiguities
         num_ambiguities_, strout, strerr = Aqua._find_ambiguities(pkgids; exclude = exclude)
@@ -16,32 +35,39 @@ using PkgWithAmbiguities
         @test isempty(strerr)
     end
 
-    total = 9
-
-
     check_testcase([], total)
 
     # exclude just anything irrelevant, see #49
     check_testcase([convert], total)
 
     # exclude function
-    check_testcase([PkgWithAmbiguities.f], total - 1)
+    check_testcase([PkgWithAmbiguities.f], total - num_ambs_f)
+
+    # exclude function and kwsorter
+    check_testcase([PkgWithAmbiguities.g], total - num_ambs_g)
 
     # exclude callables and constructors
-    check_testcase([PkgWithAmbiguities.SingletonType], total - 2 - 1)
-    check_testcase([PkgWithAmbiguities.ConcreteType], total - 3 - 1)
+    check_testcase([PkgWithAmbiguities.SingletonType], total - num_ambs_SingletonType)
+    check_testcase([PkgWithAmbiguities.ConcreteType], total - num_ambs_ConcreteType)
 
     # exclude abstract supertype without callables and constructors
     check_testcase([PkgWithAmbiguities.AbstractType], total)
 
     # for ambiguities between abstract and concrete type callables, only one needs to be excluded
-    check_testcase([PkgWithAmbiguities.AbstractParameterizedType], total - 1)
-    check_testcase([PkgWithAmbiguities.ConcreteParameterizedType], total - 1)
+    check_testcase(
+        [PkgWithAmbiguities.AbstractParameterizedType],
+        total - num_ambs_ParameterizedType,
+    )
+    check_testcase(
+        [PkgWithAmbiguities.ConcreteParameterizedType],
+        total - num_ambs_ParameterizedType,
+    )
 
     # exclude everything
     check_testcase(
         [
             PkgWithAmbiguities.f,
+            PkgWithAmbiguities.g,
             PkgWithAmbiguities.SingletonType,
             PkgWithAmbiguities.ConcreteType,
             PkgWithAmbiguities.ConcreteParameterizedType,