Narrow Resolv::DNS resource methods by typeclass#2979
Draft
zonuexe wants to merge 1 commit into
Draft
Conversation
`Resolv::DNS#getresource` / `#getresources` / `#each_resource` / `#extract_resources` accept a typeclass argument (e.g. `Resolv::DNS::Resource::IN::MX`) that fully determines the returned resource subclass. The signatures returned the upper bound `Resolv::DNS::Resource`, dropping that information; callers using subclass-specific methods like `MX#exchange` then need a downcast. A bounded generic (`[T < Resolv::DNS::Resource] (..., singleton(T)) -> Array[T]`) would be the cleanest shape, but RBS does not currently accept a type variable inside `singleton(...)` (`Could not find ::T`). Fall back to per-subclass overloads for the `Resolv::DNS::Resource::IN::*` leaf types and `Generic`, with the previous wide overload kept as the trailing fallback (which also covers `Resource::ANY` and other classes under `Resolv::DNS::Query`).
44b6bff to
6c96746
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolv::DNS#getresource/#getresources/#each_resource/#extract_resourcesaccept a typeclass argument (Resolv::DNS::Resource::IN::MX,IN::A,IN::AAAA, …) that fully determines the returned resource subclass. The current signatures return the upper boundResolv::DNS::Resource, dropping that information — so callers using subclass-specific methods likeMX#exchangehave to downcast.This adds per-subclass overloads for the leaf
Resolv::DNS::Resource::IN::*types andResolv::DNS::Resource::Generic, with the previous wide signature kept as the trailing fallback (which also coversResource::ANYand other classes underResolv::DNS::Query).Motivation
A typical caller pattern, from Mastodon (
app/validators/email_mx_validator.rb:49):#exchangeis defined onResolv::DNS::Resource::MX(inherited byIN::MX), not on the baseResolv::DNS::Resource. Under the existing signature,eis typedResolv::DNS::Resourceand.exchangedoes not resolve.Approach considered
A bounded generic would be the cleanest shape:
but RBS does not currently accept a type variable inside
singleton(...):Falling back to per-subclass overloads — verbose but the precision is concentrated in the four methods.
Change
For each of the four methods, 15 overloads:
Resolv::DNS::Resource::IN::{A, AAAA, CNAME, HINFO, LOC, MINFO, MX, NS, PTR, SOA, SRV, TXT, WKS}overloads — narrowed return / block-parameter type.Resolv::DNS::Resource::Genericoverload — narrowed return / block-parameter type.singleton(Resolv::DNS::Query)fallback — keeps the previous behaviour forResolv::DNS::Resource::ANY, the non-INresource bases (MX,NS,CNAME,DomainName,SOA,MINFO,HINFO,LOC,TXT), and any otherQuerysubclass.Resolv::DNS#getresourceResolv::DNS#getresourceResolv::DNS#getresourcesResolv::DNS#getresourcesResolv::DNS#each_resourceResolv::DNS#each_resourceResolv::DNS#extract_resourcesResolv::DNS#extract_resourcesOut of scope
Resolv::DNS#fetch_resourcealso accepts a typeclass argument, but its block yields(Message, Name)rather than aResource, so it does not benefit from the same narrowing.Test plan
bundle exec rbs -r resolv validatebundle exec ruby -Ilib bin/test_runner.rb test/stdlib/resolv/DNS_test.rb(67 tests pass)