From b954d29d25f85307cf95ba047969d32fd226f4f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 02:10:25 +0000 Subject: [PATCH 1/4] Align module-self type params across declarations in ModuleEntry#self_types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE --- lib/rbs/ast/declarations.rb | 2 +- lib/rbs/environment/module_entry.rb | 25 +++++++++++++++- sig/environment/module_entry.rbs | 9 ++++++ test/rbs/ancestor_builder_test.rb | 45 +++++++++++++++++++++++++++++ test/rbs/definition_builder_test.rb | 30 +++++++++++++++++++ test/rbs/environment_test.rb | 45 +++++++++++++++++++++++++++++ 6 files changed, 154 insertions(+), 2 deletions(-) diff --git a/lib/rbs/ast/declarations.rb b/lib/rbs/ast/declarations.rb index 31be847f49..a989805d95 100644 --- a/lib/rbs/ast/declarations.rb +++ b/lib/rbs/ast/declarations.rb @@ -163,7 +163,7 @@ def ==(other) alias eql? == def hash - self.class.hash ^ name.hash ^ args.hash ^ location.hash + self.class.hash ^ name.hash ^ args.hash end def to_json(state = nil) diff --git a/lib/rbs/environment/module_entry.rb b/lib/rbs/environment/module_entry.rb index be466169cf..a0e8d7eeea 100644 --- a/lib/rbs/environment/module_entry.rb +++ b/lib/rbs/environment/module_entry.rb @@ -41,8 +41,31 @@ def type_params end def self_types + params = type_params + param_names = params.map(&:name) + each_decl.flat_map do |decl| - decl.self_types + self_types = decl.self_types + decl_param_names = decl.type_params.map(&:name) + + if self_types.empty? || decl_param_names == param_names + self_types + else + # The declaration uses different type parameter names from the primary declaration. + # Rename the type variables in the self types, so that they are aligned to `#type_params`. + subst = Substitution.build( + decl_param_names, + params.map {|param| Types::Variable.new(name: param.name, location: param.location) } + ) + + self_types.map do |self_type| + AST::Declarations::Module::Self.new( + name: self_type.name, + args: self_type.args.map {|type| type.sub(subst) }, + location: self_type.location + ) + end + end end.uniq end diff --git a/sig/environment/module_entry.rbs b/sig/environment/module_entry.rbs index 1dee2fefed..dc30945594 100644 --- a/sig/environment/module_entry.rbs +++ b/sig/environment/module_entry.rbs @@ -44,6 +44,15 @@ module RBS # def validate_type_params: () -> void + # Returns the self types of the declarations + # + # The type variables in the self types are aligned to `#type_params`, + # so that the self types from declarations with different type parameter + # names can be compared and used with `#type_params`. + # + # Note that the returned objects may be different from the ones in the + # declarations, but `#location` points to the original declaration. + # def self_types: () -> Array[AST::Declarations::Module::Self] end end diff --git a/test/rbs/ancestor_builder_test.rb b/test/rbs/ancestor_builder_test.rb index 4a47432e91..3824de5edc 100644 --- a/test/rbs/ancestor_builder_test.rb +++ b/test/rbs/ancestor_builder_test.rb @@ -141,6 +141,51 @@ module Hello[X] : _I1[Array[X]] end end + def test_one_ancestors_module_self_types_type_param_alignment + SignatureManager.new(system_builtin: true) do |manager| + manager.files[Pathname("a.rbs")] = < T +end + +module M[A] : _Reader[A] +end +EOF + manager.files[Pathname("b.rbs")] = < A"], accessibility: :public + end + end + end + end + def test_build_instance_class_basic_object SignatureManager.new do |manager| manager.build do |env| diff --git a/test/rbs/environment_test.rb b/test/rbs/environment_test.rb index 2943f8ab0e..d9d6c6ddb4 100644 --- a/test/rbs/environment_test.rb +++ b/test/rbs/environment_test.rb @@ -316,6 +316,51 @@ module Bar : _Animal end end + def test_module_self_type_type_param_alignment + _, _, decls = RBS::Parser.parse_signature(< T +end + +module Foo[A] : _Animal[A] +end + +module Foo[B] : _Animal[B] +end + +module Foo[C] : _Animal[Integer] +end +EOF + + Environment.new.tap do |env| + decls.each do |decl| + env.insert_rbs_decl(decl, context: nil, namespace: RBS::Namespace.root) + end + + foo = env.class_decls[type_name("::Foo")] + + assert_equal [:A], foo.type_params.map(&:name) + + # Self types are aligned to the primary declaration's type parameters, and + # `_Animal[A]` and `_Animal[B]` are deduplicated + assert_equal [ + RBS::AST::Declarations::Module::Self.new( + name: type_name("_Animal"), + args: [RBS::Types::Variable.new(name: :A, location: nil)], + location: nil + ), + RBS::AST::Declarations::Module::Self.new( + name: type_name("_Animal"), + args: [RBS::Types::ClassInstance.new(name: type_name("Integer"), args: [], location: nil)], + location: nil + ), + ], foo.self_types + + # The locations of the self types point to the original declarations + assert_equal ["_Animal[A]", "_Animal[Integer]"], foo.self_types.map {|self_type| self_type.location&.source } + end + end + def test_absolute_type env = Environment.new From 1e2caae1ac9e9226ec0b4287150225c5f7481e34 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 03:37:28 +0000 Subject: [PATCH 2/4] Align type params in AncestorBuilder#validate_super_class! 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 Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE --- .../definition_builder/ancestor_builder.rb | 15 ++++++- test/rbs/ancestor_builder_test.rb | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/lib/rbs/definition_builder/ancestor_builder.rb b/lib/rbs/definition_builder/ancestor_builder.rb index 122f5f5230..63d44cbc1b 100644 --- a/lib/rbs/definition_builder/ancestor_builder.rb +++ b/lib/rbs/definition_builder/ancestor_builder.rb @@ -177,9 +177,22 @@ def validate_super_class!(type_name, entry) return if with_super_classes.size <= 1 + entry_param_names = entry.type_params.map(&:name) + super_types = with_super_classes.map do |decl| super_class = decl.super_class or raise - Types::ClassInstance.new(name: super_class.name, args: super_class.args, location: nil) + args = super_class.args + + decl_param_names = decl.type_params.map(&:name) + unless decl_param_names == entry_param_names || args.empty? + align_params = Substitution.build( + decl_param_names, + entry.type_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } + ) + args = args.map {|type| type.sub(align_params) } + end + + Types::ClassInstance.new(name: super_class.name, args: args, location: nil) end super_types.uniq! diff --git a/test/rbs/ancestor_builder_test.rb b/test/rbs/ancestor_builder_test.rb index 3824de5edc..bbd4d2d613 100644 --- a/test/rbs/ancestor_builder_test.rb +++ b/test/rbs/ancestor_builder_test.rb @@ -452,6 +452,45 @@ class B < ::String end end + def test_instance_ancestors_super_class_validation_renamed_params + SignatureManager.new do |manager| + manager.files.merge!(Pathname("foo.rbs") => <<-EOF) +class Base[T] +end + +class A[X] < Base[X] +end + +class B[X] < Base[X] +end + +class B[Y] < Base[Integer] +end + EOF + + manager.files.merge!(Pathname("foo2.rbs") => <<-EOF) +class A[Y] < Base[Y] +end + EOF + + manager.build do |env| + builder = DefinitionBuilder::AncestorBuilder.new(env: env) + + # ::A is valid: the declarations declare the same superclass modulo type parameter renaming. + builder.one_instance_ancestors(type_name("::A")).tap do |a| + assert_equal Ancestor::Instance.new(name: type_name("::Base"), args: [parse_type("X", variables: [:X])], source: :super), + a.super_class + end + + # ::B is invalid: the superclass args are different. + error = assert_raises SuperclassMismatchError do + builder.one_instance_ancestors(type_name("::B")) + end + assert_equal error.name, type_name("::B") + end + end + end + def test_singleton_ancestors SignatureManager.new do |manager| manager.files[Pathname("foo.rbs")] = < Date: Fri, 7 Aug 2026 00:41:39 +0000 Subject: [PATCH 3/4] Extract per-declaration type param alignment into entry-level align_params 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 Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE --- lib/rbs/definition_builder.rb | 7 +++-- .../definition_builder/ancestor_builder.rb | 14 ++-------- lib/rbs/definition_builder/method_builder.rb | 8 +++--- lib/rbs/environment/class_entry.rb | 12 +++++++++ lib/rbs/environment/module_entry.rb | 26 ++++++++++--------- sig/environment/class_entry.rbs | 6 +++++ sig/environment/module_entry.rbs | 6 +++++ test/rbs/environment_test.rb | 24 +++++++++++++++++ 8 files changed, 73 insertions(+), 30 deletions(-) diff --git a/lib/rbs/definition_builder.rb b/lib/rbs/definition_builder.rb index b4d36b721e..715b417a96 100644 --- a/lib/rbs/definition_builder.rb +++ b/lib/rbs/definition_builder.rb @@ -124,10 +124,13 @@ def define_instance(definition, type_name, subst, define_class_vars:) end entry = env.class_decls[type_name] or raise "Unknown name for build_instance: #{type_name}" - args = entry.type_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } entry.each_decl do |decl| - subst_ = subst + Substitution.build(decl.type_params.each.map(&:name), args) + if align_params = entry.align_params(decl) + subst_ = subst + align_params + else + subst_ = subst + end decl.members.each do |member| case member diff --git a/lib/rbs/definition_builder/ancestor_builder.rb b/lib/rbs/definition_builder/ancestor_builder.rb index 63d44cbc1b..7cc2c75430 100644 --- a/lib/rbs/definition_builder/ancestor_builder.rb +++ b/lib/rbs/definition_builder/ancestor_builder.rb @@ -177,18 +177,11 @@ def validate_super_class!(type_name, entry) return if with_super_classes.size <= 1 - entry_param_names = entry.type_params.map(&:name) - super_types = with_super_classes.map do |decl| super_class = decl.super_class or raise args = super_class.args - decl_param_names = decl.type_params.map(&:name) - unless decl_param_names == entry_param_names || args.empty? - align_params = Substitution.build( - decl_param_names, - entry.type_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } - ) + if align_params = entry.align_params(decl) args = args.map {|type| type.sub(align_params) } end @@ -486,10 +479,7 @@ def mixin_ancestors0(decl, type_name, align_params:, included_modules:, included def mixin_ancestors(entry, type_name, included_modules:, included_interfaces:, extended_modules:, prepended_modules:, extended_interfaces:) entry.each_decl do |decl| - align_params = Substitution.build( - decl.type_params.each.map(&:name), - entry.type_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } - ) + align_params = entry.align_params(decl) mixin_ancestors0(decl, type_name, diff --git a/lib/rbs/definition_builder/method_builder.rb b/lib/rbs/definition_builder/method_builder.rb index a4f47e4051..530a9d2089 100644 --- a/lib/rbs/definition_builder/method_builder.rb +++ b/lib/rbs/definition_builder/method_builder.rb @@ -104,7 +104,7 @@ def build_instance(type_name) type = Types::ClassInstance.new(name: type_name, args: args, location: nil) Methods.new(type: type).tap do |methods| entry.each_decl do |decl| - subst = Substitution.build(decl.type_params.each.map(&:name), args) + subst = entry.align_params(decl) case decl when AST::Declarations::Base each_rbs_member_with_accessibility(decl.members) do |member, accessibility| @@ -115,14 +115,14 @@ def build_instance(type_name) build_method( methods, type, - member: member.update(overloads: member.overloads.map {|overload| overload.sub(subst) }), + member: subst ? member.update(overloads: member.overloads.map {|overload| overload.sub(subst) }) : member, accessibility: member.visibility || accessibility ) when :singleton_instance build_method( methods, type, - member: member.update(overloads: member.overloads.map {|overload| overload.sub(subst) }), + member: subst ? member.update(overloads: member.overloads.map {|overload| overload.sub(subst) }) : member, accessibility: :private ) end @@ -130,7 +130,7 @@ def build_instance(type_name) if member.kind == :instance build_attribute(methods, type, - member: member.update(type: member.type.sub(subst)), + member: subst ? member.update(type: member.type.sub(subst)) : member, accessibility: member.visibility || accessibility) end when AST::Members::Alias diff --git a/lib/rbs/environment/class_entry.rb b/lib/rbs/environment/class_entry.rb index 121310cf67..73762330a5 100644 --- a/lib/rbs/environment/class_entry.rb +++ b/lib/rbs/environment/class_entry.rb @@ -64,6 +64,18 @@ def validate_type_params end end end + + def align_params(decl) + entry_params = type_params + decl_param_names = decl.type_params.map(&:name) + + return nil if decl_param_names == entry_params.map(&:name) + + Substitution.build( + decl_param_names, + entry_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } + ) + end end end end diff --git a/lib/rbs/environment/module_entry.rb b/lib/rbs/environment/module_entry.rb index a0e8d7eeea..1f65cf4905 100644 --- a/lib/rbs/environment/module_entry.rb +++ b/lib/rbs/environment/module_entry.rb @@ -41,23 +41,13 @@ def type_params end def self_types - params = type_params - param_names = params.map(&:name) - each_decl.flat_map do |decl| self_types = decl.self_types - decl_param_names = decl.type_params.map(&:name) + subst = align_params(decl) - if self_types.empty? || decl_param_names == param_names + if self_types.empty? || subst.nil? self_types else - # The declaration uses different type parameter names from the primary declaration. - # Rename the type variables in the self types, so that they are aligned to `#type_params`. - subst = Substitution.build( - decl_param_names, - params.map {|param| Types::Variable.new(name: param.name, location: param.location) } - ) - self_types.map do |self_type| AST::Declarations::Module::Self.new( name: self_type.name, @@ -69,6 +59,18 @@ def self_types end.uniq end + def align_params(decl) + entry_params = type_params + decl_param_names = decl.type_params.map(&:name) + + return nil if decl_param_names == entry_params.map(&:name) + + Substitution.build( + decl_param_names, + entry_params.map {|param| Types::Variable.new(name: param.name, location: param.location) } + ) + end + def validate_type_params unless context_decls.empty? first_decl, *rest_decls = each_decl.to_a diff --git a/sig/environment/class_entry.rbs b/sig/environment/class_entry.rbs index 854e191eb3..ff97cc5c10 100644 --- a/sig/environment/class_entry.rbs +++ b/sig/environment/class_entry.rbs @@ -45,6 +45,12 @@ module RBS # * Raises `GenericParameterMismatchError` if incompatible declaration is detected. # def validate_type_params: () -> void + + # Returns a substitution that renames the type parameters of the declaration to the entry's type parameters (`#type_params`) + # + # Returns `nil` if the declaration uses the same type parameter names as `#type_params`. + # + def align_params: (declaration | ModuleEntry::declaration) -> Substitution? end end end diff --git a/sig/environment/module_entry.rbs b/sig/environment/module_entry.rbs index dc30945594..c556b09791 100644 --- a/sig/environment/module_entry.rbs +++ b/sig/environment/module_entry.rbs @@ -54,6 +54,12 @@ module RBS # declarations, but `#location` points to the original declaration. # def self_types: () -> Array[AST::Declarations::Module::Self] + + # Returns a substitution that renames the type parameters of the declaration to the entry's type parameters (`#type_params`) + # + # Returns `nil` if the declaration uses the same type parameter names as `#type_params`. + # + def align_params: (declaration | ClassEntry::declaration) -> Substitution? end end end diff --git a/test/rbs/environment_test.rb b/test/rbs/environment_test.rb index d9d6c6ddb4..a124f309be 100644 --- a/test/rbs/environment_test.rb +++ b/test/rbs/environment_test.rb @@ -361,6 +361,30 @@ module Foo[C] : _Animal[Integer] end end + def test_module_entry_align_params + _, _, decls = RBS::Parser.parse_signature(< Date: Fri, 7 Aug 2026 01:49:55 +0000 Subject: [PATCH 4/4] Add test for align_params with arity mismatch 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 Claude-Session: https://claude.ai/code/session_01TBY8ct4HpkVkZNPdsNHEDE --- test/rbs/environment_test.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/rbs/environment_test.rb b/test/rbs/environment_test.rb index a124f309be..2198da9261 100644 --- a/test/rbs/environment_test.rb +++ b/test/rbs/environment_test.rb @@ -368,6 +368,9 @@ module Foo[A, B] module Foo[X, Y] end + +module Foo[X] +end EOF Environment::ModuleEntry.new(type_name("::Foo")).tap do |entry| @@ -383,6 +386,16 @@ module Foo[X, Y] assert_equal RBS::Types::Variable.new(name: :B, location: nil), subst[RBS::Types::Variable.new(name: :Y, location: nil)] end end + + Environment::ModuleEntry.new(type_name("::Foo")).tap do |entry| + entry << [nil, decls[0]] + entry << [nil, decls[2]] + + # The type params validation runs before the alignment, so the arity mismatch is detected first + assert_raises RBS::GenericParameterMismatchError do + entry.align_params(decls[2]) + end + end end def test_absolute_type