Align type params across declarations in module-self types and superclass validation - #3067
Merged
Merged
Conversation
…_types
When a module has multiple declarations with different (but compatible)
type parameter names, `Environment::ModuleEntry#self_types` collected the
self type constraints of each declaration as-is. The type variables from
non-primary declarations were left as free variables that are not bound
to any type parameter of the module, so downstream tools (e.g. Steep's
module self type check) could never satisfy the constraint:
# a.rbs
module M[out A] : _Foo[A]
end
# b.rbs
module M[out B] : _Foo[B]
end
entry.self_types # => [_Foo[A], _Foo[B]] (B is unbound)
Mixin members already get this alignment via `align_params` in
`DefinitionBuilder::AncestorBuilder#mixin_ancestors`, but module self
types did not. Fix it in `ModuleEntry#self_types` — the aggregation
point every consumer goes through — by renaming the type variables of
each declaration's self types to the primary declaration's type
parameters, using the same substitution as `mixin_ancestors`. The
`location` of substituted self types keeps pointing to the original
declaration, so error locations (NoSelfTypeFoundError,
InvalidTypeApplicationError) are unchanged.
Also drop `location` from `AST::Declarations::Module::Self#hash` to make
it consistent with `#==`/`#eql?`, which only compare `name` and `args`.
The inconsistency made the `.uniq` in `ModuleEntry#self_types`
ineffective across files, so identical self types from different
declarations were duplicated.
With both fixes, the example above now yields `[_Foo[A]]`.
This is what broke Steep's self check with rbs 4.1.2, where
core/enumerable.rbs renamed `Elem` to `E` while other environments still
declare `module Enumerable[unchecked out Elem] : _Each[Elem]`:
`one_instance_ancestors(::Enumerable).self_types` became
`[_Each[E, void], _Each[Elem, void]]`, failing every class that includes
Enumerable. (soutaro/steep#2256)
Note: `sig/shims/enumerable.rbs` intentionally keeps the `Elem` name —
this repository's own `steep check` runs on rbs 3.9 whose core still
uses `Elem`, and renaming the shim to `E` makes the self check fail
there. With the alignment fix the name difference is harmless.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE
The superclass comparison across multiple declarations compared the superclass args as written, so declarations that declare the same superclass with different type parameter names (`class C[A] < Base[A]` and `class C[B] < Base[B]`) raised a false SuperclassMismatchError. Align the args to the entry's type parameter names before comparing, like ModuleEntry#self_types and mixin_ancestors do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE
…arams The substitution that renames a declaration's type parameters to the entry's type parameters was built inline in five places: MethodBuilder, DefinitionBuilder#define_instance, AncestorBuilder#mixin_ancestors, ModuleEntry#self_types, and AncestorBuilder#validate_super_class!. Define it once as ModuleEntry#align_params / ClassEntry#align_params and use it from all of them. The method returns nil when the declaration already uses the entry's type parameter names, so the callers can skip the substitution in the common case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE
Type params validation runs before the alignment, so the arity mismatch raises GenericParameterMismatchError instead of building a broken substitution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE
soutaro
changed the title
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
2 participants
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.
When a module/class has multiple declarations with different (but compatible) type parameter names, the variables written in a non-primary declaration leaked through as free variables:
module M[A] : _Foo[A]+module M[B] : _Foo[B]madeModuleEntry#self_typesreturn[_Foo[A], _Foo[B]], andclass C[A] < Base[A]+class C[B] < Base[B]raised a falseSuperclassMismatchError. This broke Steep's module self type check for every class includingEnumerable, because rbs 4.1.2 renamed the coreEnumerable's type paramElemtoEwhilesig/shims/enumerable.rbsstill usesElem(soutaro/steep#2256).This PR renames the variables to the primary declaration's type parameter names in both places — the same alignment that methods, instance variables, and mixin arguments already receive — keeping the original
locations. The substitution is extracted intoModuleEntry#align_params/ClassEntry#align_params, shared by all five call sites, and returnsnilwhen the declaration already uses the entry's names. Also,Module::Self#hashno longer includeslocation.hash, matching==, so.uniqdeduplicates equal self types across files.