Merge pull request #3067 from ruby/claude/module-self-type-param-alignment-2wpbja

Align type params across declarations in module-self types and superclass validation

Author: soutaro

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)

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

lib/rbs/definition_builder/ancestor_builder.rb

@@ -179,7 +179,13 @@ def validate_super_class!(type_name, entry)
 
         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
+
+          if align_params = entry.align_params(decl)
+            args = args.map {|type| type.sub(align_params) }
+          end
+
+          Types::ClassInstance.new(name: super_class.name, args: args, location: nil)
         end
 
         super_types.uniq!
@@ -473,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,

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,22 +115,22 @@ 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
                     when AST::Members::AttrReader, AST::Members::AttrWriter, AST::Members::AttrAccessor
                       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

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

lib/rbs/environment/module_entry.rb

@@ -42,10 +42,35 @@ def type_params
 
       def self_types
         each_decl.flat_map do |decl|
-          decl.self_types
+          self_types = decl.self_types
+          subst = align_params(decl)
+
+          if self_types.empty? || subst.nil?
+            self_types
+          else
+            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
 
+      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

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

sig/environment/module_entry.rbs

@@ -44,7 +44,22 @@ 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]
+
+      # 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

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")] = <<EOF
+interface _EachItem[out T]
+end
+
+interface _FooItem[out T]
+end
+
+module M[out A] : _EachItem[A]
+end
+EOF
+      manager.files[Pathname("b.rbs")] = <<EOF
+module M[out B] : _EachItem[B], _FooItem[Array[B]]
+end
+EOF
+      manager.build do |env|
+        builder = DefinitionBuilder::AncestorBuilder.new(env: env)
+
+        builder.one_instance_ancestors(type_name("::M")).tap do |a|
+          assert_equal type_name("::M"), a.type_name
+          assert_equal [:A], a.params
+
+          # Type parameters in self types are renamed to the primary declaration's type parameters,
+          # and `_EachItem[A]`/`_EachItem[B]` are deduplicated
+          assert_equal [
+                         Ancestor::Instance.new(name: type_name("::_EachItem"), args: [parse_type("A", variables: [:A])], source: nil),
+                         Ancestor::Instance.new(name: type_name("::_FooItem"), args: [parse_type("::Array[A]", variables: [:A])], source: nil)
+                       ],
+                       a.self_types
+
+          # The source of each self type keeps pointing to the original declaration
+          a.self_types or raise
+          a.self_types.each do |self_type|
+            source = self_type.source
+            assert_instance_of AST::Declarations::Module::Self, source
+            location = source.location or raise
+            expected_file = source.name == type_name("::_FooItem") ? "b.rbs" : "a.rbs"
+            assert_equal expected_file, Pathname(location.buffer.name).basename.to_s
+          end
+        end
+      end
+    end
+  end
+
   def test_one_ancestors_module_no_self_type
     SignatureManager.new(system_builtin: true) do |manager|
       manager.files[Pathname("foo.rbs")] = <<EOF
@@ -407,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")] = <<EOF

test/rbs/definition_builder_test.rb

@@ -375,6 +375,36 @@ module M : _StringConvertible
     end
   end
 
+  def test_build_instance_module_self_types_type_param_alignment
+    SignatureManager.new do |manager|
+      manager.files[Pathname("a.rbs")] = <<EOF
+interface _Reader[T]
+  def read: () -> T
+end
+
+module M[A] : _Reader[A]
+end
+EOF
+      manager.files[Pathname("b.rbs")] = <<EOF
+module M[B] : _Reader[B]
+end
+EOF
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::M")).tap do |definition|
+          assert_instance_of Definition, definition
+          assert_equal type_name("::M"), definition.type_name
+          assert_equal [:A], definition.type_params
+
+          # The self type from `b.rbs` is aligned to the primary declaration's type parameters
+          assert_equal Set[:read], Set.new(definition.methods.keys)
+          assert_method_definition definition.methods[:read], ["() -> A"], accessibility: :public
+        end
+      end
+    end
+  end
+
   def test_build_instance_class_basic_object
     SignatureManager.new do |manager|
       manager.build do |env|