Symbol type parameters can be piracy

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Fix some world age issues in `test_ambiguities`. ([#366])
+- Symbols as type-parameters do not disable piracy checks in `detect_piracies`. ([#375])
 - The minimum supported julia version is increased to 1.6. ([#328])
 
 ## Version [v0.8.14] - 2025-08-04

src/piracies.jl

@@ -56,36 +56,40 @@ end
 is_foreign(@nospecialize(x), pkg::Base.PkgId; treat_as_own) =
     is_foreign(typeof(x), pkg; treat_as_own = treat_as_own)
 
-# Symbols can be used as type params - we assume these are unique and not
-# piracy.  This implies that we have
-#
-#     julia> Aqua.Piracy.is_foreign(1, Base.PkgId(Aqua))
-#     true
-# 
-#     julia> Aqua.Piracy.is_foreign(:hello, Base.PkgId(Aqua))
-#     false
-#
-# and thus
-#
-#     julia> Aqua.Piracy.is_foreign(Val{1}, Base.PkgId(Aqua))
-#     true
-# 
-#     julia> Aqua.Piracy.is_foreign(Val{:hello}, Base.PkgId(Aqua))
-#     false
-#
-# Admittedly, this asymmetry is rather worrisome.  We do need to treat 1 foreign
-# to consider `Vector{Char}` (i.e., `Array{Char,1}`) foreign.  This may suggest
-# to treat the `Symbol` type foreign as well.  However, it means that we treat
-# definition such as
-#
-#     ForeignModule.api_function(::Val{:MyPackageName}) = ...
-# 
-# as a type piracy even if this is actually the intended use-case (which is not
-# a crazy API).  The symbol name may also come from `gensym`.  Since the aim of
-# `Aqua.test_piracies` is to detect only "obvious" piracies, let us play on the
-# safe side.
+# With Symbols, the challenge is to distinguish between uses that were expected
+# by the foreign module and which are novel to the package. A pattern like
+# `ForeignModule.api_function(::Val{:MyPkg}) = ...` is an accepted dispatch
+# idiom and we do not want to flag it as piracy. On the other hand, IntervalSets
+# defines the type alias
+#     const ClosedInterval{T} = Interval{:closed, :closed, T}
+# and that should be treated as foreign.
+
+# Historically all symbols were treated as non-foreign. Preserve that behavior
+# for the fallback case:
 is_foreign(x::Symbol, pkg::Base.PkgId; treat_as_own) = false
 
+# But when symbols appear as type-parameters, check whether the specific symbol
+# and enclosing type match a user-defined type alias in the owning module. This
+# check is somewhat loose in not insisting on a specific positional index within
+# the parameter list:
+function is_symbol_param_structural(sym::Symbol, @nospecialize(T::DataType))
+    parent_mod = parentmodule(T)
+    Tname = nameof(T)
+    for name in names(parent_mod; all = true)
+        isdefined(parent_mod, name) || continue
+        Base.isdeprecated(parent_mod, name) && continue
+        obj = getfield(parent_mod, name)
+        body = Base.unwrap_unionall(obj)
+        if isa(body, DataType) &&
+           parentmodule(body) === parent_mod &&
+           nameof(body) === Tname &&
+           sym in body.parameters
+            return true
+        end
+    end
+    return false
+end
+
 is_foreign_module(mod::Module, pkg::Base.PkgId) = Base.PkgId(mod) != pkg
 
 function is_foreign(@nospecialize(T::DataType), pkg::Base.PkgId; treat_as_own)
@@ -96,10 +100,19 @@ function is_foreign(@nospecialize(T::DataType), pkg::Base.PkgId; treat_as_own)
         @assert length(params) == 1
         return is_foreign(first(params), pkg; treat_as_own = treat_as_own)
     else
-        # Both the type itself and all of its parameters must be foreign
+        # Both the type itself and all of its parameters must be foreign.
+        # Symbol parameters are handled via is_symbol_param_structural: a
+        # Symbol is foreign only if the owning module explicitly uses it as
+        # a type parameter of T (e.g. ClosedInterval = Interval{:closed,…}).
         return !((C in treat_as_own)::Bool) &&
                is_foreign_module(parentmodule(T), pkg) &&
-               all(param -> is_foreign(param, pkg; treat_as_own = treat_as_own), params)
+               all(params) do param
+                   if param isa Symbol
+                       is_symbol_param_structural(param, T)
+                   else
+                       is_foreign(param, pkg; treat_as_own = treat_as_own)
+                   end
+               end
     end
 end
 

test/pkgs/PiracyForeignProject/src/PiracyForeignProject.jl

@@ -7,4 +7,12 @@ struct ForeignNonSingletonType
     x::Int
 end
 
+# Type with a Symbol type parameter plus a type alias that fixes the Symbol.
+# ForeignTaggedType tests that dispatching on the alias is detected as piracy
+# (the :tag symbol is structural — it lives in this package's type alias),
+# while dispatching on ForeignSymbolParamType{:user_symbol, T} directly is not
+# (the :user_symbol could be a caller-defined dispatch tag).
+struct ForeignSymbolParamType{S, T} end
+const ForeignTaggedType{T} = ForeignSymbolParamType{:tag, T}
+
 end

test/test_piracy.jl

@@ -2,7 +2,12 @@ push!(LOAD_PATH, joinpath(@__DIR__, "pkgs", "PiracyForeignProject"))
 
 baremodule PiracyModule
 
-using PiracyForeignProject: ForeignType, ForeignParameterizedType, ForeignNonSingletonType
+using PiracyForeignProject:
+    ForeignType,
+    ForeignParameterizedType,
+    ForeignNonSingletonType,
+    ForeignSymbolParamType,
+    ForeignTaggedType
 
 using Base:
     Base,
@@ -33,6 +38,9 @@ Base.findlast(::Type{Val{Foo}}, x::Int) = x + 1
 Base.findlast(::Tuple{Vararg{Bar{Set{Int}}}}, x::Int) = x + 1
 Base.findlast(::Val{:foo}, x::Int) = x + 1
 Base.findlast(::ForeignParameterizedType{Foo}, x::Int) = x + 1
+# Not piracy: :caller_tag is not defined in PiracyForeignProject's type aliases,
+# so it is treated as a user-defined dispatch tag (like Val{:foo} above)
+Base.findlast(::ForeignSymbolParamType{:caller_tag, T}, x::Int) where T = x + 1
 
 # Not piracy
 const MyUnion = Union{Int,Foo}
@@ -58,10 +66,21 @@ Base.findmin(::ForeignParameterizedType{Int}, x::Int) = x + 1
 Base.findmin(::Set{Vector{ForeignParameterizedType{Int}}}, x::Int) = x + 1
 Base.findmin(::Union{Foo,ForeignParameterizedType{Int}}, x::Int) = x + 1
 
+# Piracy: ForeignTaggedType = ForeignSymbolParamType{:tag, T} is a type alias
+# defined in PiracyForeignProject, so :tag is structural — not a user-defined
+# dispatch tag — and must not suppress piracy detection.
+# Compare with Val{:foo} and ForeignSymbolParamType{:caller_tag,T} above.
+Base.findlast(::ForeignTaggedType{T}, x::Int) where T = x + 1
+
 end # PiracyModule
 
 using Aqua: Piracy
-using PiracyForeignProject: ForeignType, ForeignParameterizedType, ForeignNonSingletonType
+using PiracyForeignProject:
+    ForeignType,
+    ForeignParameterizedType,
+    ForeignNonSingletonType,
+    ForeignSymbolParamType,
+    ForeignTaggedType
 
 # Get all methods - test length
 meths = filter(Piracy.all_methods(PiracyModule)) do m
@@ -73,7 +92,7 @@ end
       1 + # Bar constructor
       2 + # f
       4 + # MyUnion (incl. kwcall)
-      6 + # findlast
+      8 + # findlast (7 non-piracy + 1 ForeignTaggedType piracy)
       3 + # findfirst
       1 + # ForeignType callable
       1 + # ForeignNonSingletonType callable
@@ -97,19 +116,33 @@ pirates = Piracy.hunt(PiracyModule)
       3 + # findmax
       3 + # findmin
       1 + # ForeignType callable
-      1   # ForeignNonSingletonType callable
+      1 + # ForeignNonSingletonType callable
+      1   # findlast on ForeignTaggedType — :tag is structural in PiracyForeignProject
 @test all(pirates) do m
-    m.name in [:findfirst, :findmax, :findmin, :ForeignType, :ForeignNonSingletonType]
+    m.name in [:findfirst, :findmax, :findmin, :ForeignType, :ForeignNonSingletonType, :findlast]
+end
+
+# Specifically verify which findlast is the pirate: the one whose arg2 contains
+# :tag (structural — defined in PiracyForeignProject's ForeignTaggedType alias)
+# must be piracy, while the one with :caller_tag (user-defined) must not be.
+let arg2_params = m -> let sig = Base.unwrap_unionall(m.sig), p2 = sig.parameters[2]
+        p2 isa DataType ? p2.parameters : ()
+    end
+    tagged_findlast  = filter(m -> m.name === :findlast && :tag        in arg2_params(m), meths)
+    caller_findlast  = filter(m -> m.name === :findlast && :caller_tag in arg2_params(m), meths)
+    @test length(tagged_findlast)  == 1 &&  Piracy.is_pirate(only(tagged_findlast))
+    @test length(caller_findlast)  == 1 && !Piracy.is_pirate(only(caller_findlast))
 end
 
 # Test what is pirate (with treat_as_own=[ForeignType])
 pirates = Piracy.hunt(PiracyModule, treat_as_own = [ForeignType])
 @test length(pirates) ==
       3 + # findfirst
       3 + # findmin
-      1   # ForeignNonSingletonType callable
+      1 + # ForeignNonSingletonType callable
+      1   # findlast on ForeignSymbolParamType{:tag,T}
 @test all(pirates) do m
-    m.name in [:findfirst, :findmin, :ForeignNonSingletonType]
+    m.name in [:findfirst, :findmin, :ForeignNonSingletonType, :findlast]
 end
 
 # Test what is pirate (with treat_as_own=[ForeignParameterizedType])
@@ -118,9 +151,10 @@ pirates = Piracy.hunt(PiracyModule, treat_as_own = [ForeignParameterizedType])
       3 + # findfirst
       3 + # findmax
       1 + # ForeignType callable
-      1   # ForeignNonSingletonType callable
+      1 + # ForeignNonSingletonType callable
+      1   # findlast on ForeignSymbolParamType{:tag,T}
 @test all(pirates) do m
-    m.name in [:findfirst, :findmax, :ForeignType, :ForeignNonSingletonType]
+    m.name in [:findfirst, :findmax, :ForeignType, :ForeignNonSingletonType, :findlast]
 end
 
 # Test what is pirate (with treat_as_own=[ForeignType, ForeignParameterizedType])
@@ -130,19 +164,21 @@ pirates = filter(
 )
 @test length(pirates) ==
       3 + # findfirst
-      1   # ForeignNonSingletonType callable
+      1 + # ForeignNonSingletonType callable
+      1   # findlast on ForeignSymbolParamType{:tag,T}
 @test all(pirates) do m
-    m.name in [:findfirst, :ForeignNonSingletonType]
+    m.name in [:findfirst, :ForeignNonSingletonType, :findlast]
 end
 
 # Test what is pirate (with treat_as_own=[Base.findfirst, Base.findmax])
 pirates = Piracy.hunt(PiracyModule, treat_as_own = [Base.findfirst, Base.findmax])
 @test length(pirates) ==
       3 + # findmin
       1 + # ForeignType callable
-      1   # ForeignNonSingletonType callable
+      1 + # ForeignNonSingletonType callable
+      1   # findlast on ForeignSymbolParamType{:tag,T}
 @test all(pirates) do m
-    m.name in [:findmin, :ForeignType, :ForeignNonSingletonType]
+    m.name in [:findmin, :ForeignType, :ForeignNonSingletonType, :findlast]
 end
 
 # Test what is pirate (excluding a cover of everything)
@@ -153,6 +189,7 @@ pirates = filter(
             ForeignType,
             ForeignParameterizedType,
             ForeignNonSingletonType,
+            ForeignSymbolParamType,
             Base.findfirst,
         ],
     ),