Merge pull request #2300 from ruby/mixin-class-var

Fix class variable

Author: soutaro

lib/rbs/definition_builder.rb

@@ -82,7 +82,7 @@ def tapp_subst(name, args)
       AST::TypeParam.application(params, args) || Substitution.new()
     end
 
-    def define_instance(definition, type_name, subst)
+    def define_instance(definition, type_name, subst, define_class_vars:)
       one_ancestors = ancestor_builder.one_instance_ancestors(type_name)
       methods = method_builder.build_instance(type_name)
 
@@ -104,7 +104,7 @@ def define_instance(definition, type_name, subst)
           validate_type_presence(arg)
         end
 
-        define_instance(definition, mod.name, subst + tapp_subst(mod.name, mod.args))
+        define_instance(definition, mod.name, subst + tapp_subst(mod.name, mod.args), define_class_vars: define_class_vars)
       end
 
       all_interfaces = one_ancestors.each_included_interface.flat_map do |interface|
@@ -120,7 +120,7 @@ def define_instance(definition, type_name, subst)
           validate_type_presence(arg)
         end
 
-        define_instance(definition, mod.name, subst + tapp_subst(mod.name, mod.args))
+        define_instance(definition, mod.name, subst + tapp_subst(mod.name, mod.args), define_class_vars: define_class_vars)
       end
 
       entry = env.class_decls[type_name] or raise "Unknown name for build_instance: #{type_name}"
@@ -161,7 +161,9 @@ def define_instance(definition, type_name, subst)
             )
 
           when AST::Members::ClassVariable
-            insert_variable(type_name, definition.class_variables, name: member.name, type: member.type, source: member)
+            if define_class_vars
+              insert_variable(type_name, definition.class_variables, name: member.name, type: member.type, source: member)
+            end
           end
         end
       end
@@ -216,13 +218,13 @@ def build_instance(type_name)
                 if ans.name.interface?
                   define_interface(definition, ans.name, subst)
                 else
-                  define_instance(definition, ans.name, subst)
+                  define_instance(definition, ans.name, subst, define_class_vars: true)
                 end
               end
             end
           end
 
-          define_instance(definition, type_name, Substitution.new)
+          define_instance(definition, type_name, Substitution.new, define_class_vars: true)
         end
       end
     end
@@ -251,7 +253,6 @@ def build_singleton0(type_name)
 
             definition.methods.merge!(defn.methods)
             definition.instance_variables.merge!(defn.instance_variables)
-            definition.class_variables.merge!(defn.class_variables)
           end
 
           one_ancestors.each_extended_module do |mod|
@@ -260,7 +261,7 @@ def build_singleton0(type_name)
             end
 
             subst = tapp_subst(mod.name, mod.args)
-            define_instance(definition, mod.name, subst)
+            define_instance(definition, mod.name, subst, define_class_vars: false)
           end
 
           all_interfaces = one_ancestors.each_extended_interface.flat_map do |interface|
@@ -290,12 +291,12 @@ def build_singleton0(type_name)
 
               when AST::Members::ClassInstanceVariable
                 insert_variable(type_name, definition.instance_variables, name: member.name, type: member.type, source: member)
-
-              when AST::Members::ClassVariable
-                insert_variable(type_name, definition.class_variables, name: member.name, type: member.type, source: member)
               end
             end
           end
+
+          instance_definition = build_instance(type_name)
+          definition.class_variables.replace(instance_definition.class_variables)
         end
       end
     end

sig/definition_builder.rbs

@@ -107,7 +107,13 @@ module RBS
 
     def validate_variable: (Definition::Variable) -> void
 
-    def insert_variable: (TypeName, Hash[Symbol, Definition::Variable], name: Symbol, type: Types::t, source: Definition::Variable::source) -> void
+    def insert_variable: (
+      TypeName,
+      Hash[Symbol, Definition::Variable],
+      name: Symbol,
+      type: Types::t,
+      source: Definition::Variable::source
+    ) -> void
 
     # Add method definition to `methods`, which will be merged to `class_definition` after defining all methods at this *level* -- class, module, or interface
     #
@@ -144,11 +150,17 @@ module RBS
       Hash[Symbol, Definition::Method]? self_type_methods,
     ) -> void
 
-    # Updates `definition` with methods and variables of `type_name` that can be a module or a class
+    # Updates `definition` with methods and instance variables of `type_name` that can be a module or a class
     #
     # It processes includes and prepends recursively.
+    # If `define_class_vars:` is falsy, it skips inserting class variables.
     #
-    def define_instance: (Definition definition, TypeName type_name, Substitution subst) -> void
+    def define_instance: (
+      Definition definition,
+      TypeName type_name,
+      Substitution subst,
+      define_class_vars: bool
+    ) -> void
 
     # Updates `definition` with methods defined in an interface `type_name`
     #

test/rbs/definition_builder_test.rb

@@ -2706,6 +2706,104 @@ module Foo
     end
   end
 
+  def test_class_var__mixin__include_defines_class_var
+    SignatureManager.new() do |manager|
+      manager.add_file("foo.rbs", <<-EOF)
+module M1
+  @@m1: Integer
+end
+
+class Foo
+  include M1
+end
+      EOF
+
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::Foo")).tap do |definition|
+          definition.class_variables[:@@m1].tap do |var|
+            assert_instance_of Definition::Variable, var
+            assert_equal type_name("::M1"), var.declared_in
+            assert_nil var.parent_variable
+            assert_equal "@@m1: Integer", var.source.location.source
+          end
+        end
+
+        builder.build_singleton(type_name("::Foo")).tap do |definition|
+          definition.class_variables[:@@m1].tap do |var|
+            assert_instance_of Definition::Variable, var
+            assert_equal type_name("::M1"), var.declared_in
+            assert_nil var.parent_variable
+            assert_equal "@@m1: Integer", var.source.location.source
+          end
+        end
+      end
+    end
+  end
+
+  def test_class_var__mixin__extend_no_class_var
+    SignatureManager.new() do |manager|
+      manager.add_file("foo.rbs", <<-EOF)
+module M1
+  @@m1: Integer
+end
+
+class Foo
+  extend M1
+end
+      EOF
+
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::Foo")).tap do |definition|
+          assert_nil definition.class_variables[:@@m1]
+        end
+
+        builder.build_singleton(type_name("::Foo")).tap do |definition|
+          assert_nil definition.class_variables[:@@m1]
+        end
+      end
+    end
+  end
+
+  def test_class_var__mixin__prepend_class_var
+    SignatureManager.new() do |manager|
+      manager.add_file("foo.rbs", <<-EOF)
+module M1
+  @@m1: Integer
+end
+
+class Foo
+  prepend M1
+end
+      EOF
+
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::Foo")).tap do |definition|
+          definition.class_variables[:@@m1].tap do |var|
+            assert_instance_of Definition::Variable, var
+            assert_equal type_name("::M1"), var.declared_in
+            assert_nil var.parent_variable
+            assert_equal "@@m1: Integer", var.source.location.source
+          end
+        end
+
+        builder.build_singleton(type_name("::Foo")).tap do |definition|
+          definition.class_variables[:@@m1].tap do |var|
+            assert_instance_of Definition::Variable, var
+            assert_equal type_name("::M1"), var.declared_in
+            assert_nil var.parent_variable
+            assert_equal "@@m1: Integer", var.source.location.source
+          end
+        end
+      end
+    end
+  end
+
   def test_duplicated_variable
     SignatureManager.new do |manager|
       manager.add_file("instance.rbs", <<-EOF)