AbstractTrees.jl icon indicating copy to clipboard operation
AbstractTrees.jl copied to clipboard

guard against trait type piracy in a dependent package

Open nsajko opened this issue 6 months ago • 2 comments

Prevent dependent packages from commiting type piracy easily and unintentionally when defining an AbstractTrees trait for all subtypes of a type. Specifically: the bottom type, Union{}, is a subtype of each type, so add a method for the bottom type for each AbstractTrees trait. This method will take precedence over sane methods in dependent packages, thus preventing spurious type piracy.

nsajko avatar May 11 '25 17:05 nsajko

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 71.49%. Comparing base (4fe5615) to head (2c0f73b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   71.16%   71.49%   +0.32%     
==========================================
  Files           8        8              
  Lines         437      442       +5     
==========================================
+ Hits          311      316       +5     
  Misses        126      126              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar May 11 '25 17:05 codecov-commenter

This is how Base does the same thing, for eltype, IteratorSize and IteratorEltype:

  • https://github.com/JuliaLang/julia/blob/8567a3a10f4b746b91bf406bfe3171c3399aed8d/base/abstractarray.jl#L240-L240

  • https://github.com/JuliaLang/julia/blob/8567a3a10f4b746b91bf406bfe3171c3399aed8d/base/generator.jl#L96-L96

  • https://github.com/JuliaLang/julia/blob/8567a3a10f4b746b91bf406bfe3171c3399aed8d/base/generator.jl#L130-L130

The slurp stuff is presumably there to help inference somehow. I don't think it's important to have that.

nsajko avatar Jun 11 '25 11:06 nsajko