Merge pull request #2294 from ksss/fix-variable-bug

Exclude `attr_*` methods from duplicate checks.

Author: soutaro

lib/rbs/definition.rb

@@ -6,11 +6,13 @@ class Variable
       attr_reader :parent_variable
       attr_reader :type
       attr_reader :declared_in
+      attr_reader :source
 
-      def initialize(parent_variable:, type:, declared_in:)
+      def initialize(parent_variable:, type:, declared_in:, source:)
         @parent_variable = parent_variable
         @type = type
         @declared_in = declared_in
+        @source = source
       end
 
       def sub(s)
@@ -19,7 +21,8 @@ def sub(s)
         self.class.new(
           parent_variable: parent_variable,
           type: type.sub(s),
-          declared_in: declared_in
+          declared_in: declared_in,
+          source: source
         )
       end
     end

lib/rbs/definition_builder.rb

@@ -145,23 +145,23 @@ def define_instance(definition, type_name, subst)
                   type_name,
                   definition.instance_variables,
                   name: ivar_name,
-                  type: member.type.sub(subst_)
+                  type: member.type.sub(subst_),
+                  source: member
                 )
               end
             end
 
           when AST::Members::InstanceVariable
-            InstanceVariableDuplicationError.check!(variables: definition.instance_variables, member: member, type_name: type_name)
             insert_variable(
               type_name,
               definition.instance_variables,
               name: member.name,
-              type: member.type.sub(subst_)
+              type: member.type.sub(subst_),
+              source: member
             )
 
           when AST::Members::ClassVariable
-            ClassVariableDuplicationError.check!(variables: definition.class_variables, member: member, type_name: type_name)
-            insert_variable(type_name, definition.class_variables, name: member.name, type: member.type)
+            insert_variable(type_name, definition.class_variables, name: member.name, type: member.type, source: member)
           end
         end
       end
@@ -284,17 +284,15 @@ def build_singleton0(type_name)
                               end
 
                   if ivar_name
-                    insert_variable(type_name, definition.instance_variables, name: ivar_name, type: member.type)
+                    insert_variable(type_name, definition.instance_variables, name: ivar_name, type: member.type, source: member)
                   end
                 end
 
               when AST::Members::ClassInstanceVariable
-                ClassInstanceVariableDuplicationError.check!(variables: definition.instance_variables, member: member, type_name: type_name)
-                insert_variable(type_name, definition.instance_variables, name: member.name, type: member.type)
+                insert_variable(type_name, definition.instance_variables, name: member.name, type: member.type, source: member)
 
               when AST::Members::ClassVariable
-                ClassVariableDuplicationError.check!(variables: definition.class_variables, member: member, type_name: type_name)
-                insert_variable(type_name, definition.class_variables, name: member.name, type: member.type)
+                insert_variable(type_name, definition.class_variables, name: member.name, type: member.type, source: member)
               end
             end
           end
@@ -539,12 +537,46 @@ def validate_type_params(definition, ancestors:, methods:)
       end
     end
 
-    def insert_variable(type_name, variables, name:, type:)
+    def insert_variable(type_name, variables, name:, type:, source:)
       variables[name] = Definition::Variable.new(
         parent_variable: variables[name],
         type: type,
-        declared_in: type_name
+        declared_in: type_name,
+        source: source
       )
+      validate_variable(variables[name])
+    end
+
+    def validate_variable(var)
+      return unless var.parent_variable
+
+      # Ignore attrs
+      variables = [] #: Array[Definition::Variable]
+      tmp_var = var
+      while tmp_var
+        variables << tmp_var if tmp_var.source.is_a?(AST::Members::Var)
+        tmp_var = tmp_var.parent_variable
+      end
+
+      # Duplicates should be eliminated, so there can't be more than 3.
+      return unless variables.length == 2
+
+      l, r = variables #: [Definition::Variable, Definition::Variable]
+
+      case l.source
+      when AST::Members::InstanceVariable
+        if r.source.instance_of?(AST::Members::InstanceVariable) && l.declared_in == r.declared_in
+          raise InstanceVariableDuplicationError.new(member: l.source)
+        end
+      when AST::Members::ClassInstanceVariable
+        if r.source.instance_of?(AST::Members::ClassInstanceVariable) && l.declared_in == r.declared_in
+          raise ClassInstanceVariableDuplicationError.new(member: l.source)
+        end
+      when AST::Members::ClassVariable
+        if r.source.instance_of?(AST::Members::ClassVariable)
+          raise ClassVariableDuplicationError.new(member: l.source)
+        end
+      end
     end
 
     def import_methods(definition, module_name, module_methods, interfaces_methods, subst, self_type_methods)

lib/rbs/errors.rb

@@ -340,31 +340,12 @@ def location
   end
 
   class InstanceVariableDuplicationError < VariableDuplicationError
-    def self.check!(variables:, member:, type_name:)
-      if old = variables[member.name]
-        if old.declared_in == type_name
-          raise new(member: member)
-        end
-      end
-    end
   end
 
   class ClassInstanceVariableDuplicationError < VariableDuplicationError
-    def self.check!(variables:, member:, type_name:)
-      if old = variables[member.name]
-        if old.declared_in == type_name
-          raise new(member: member)
-        end
-      end
-    end
   end
 
   class ClassVariableDuplicationError < VariableDuplicationError
-    def self.check!(variables:, member:, type_name:)
-      if old = variables[member.name]
-        raise new(member: member)
-      end
-    end
   end
 
   class UnknownMethodAliasError < DefinitionError

sig/definition.rbs

@@ -6,8 +6,15 @@ module RBS
       attr_reader parent_variable: Variable?
       attr_reader type: Types::t
       attr_reader declared_in: TypeName
-
-      def initialize: (parent_variable: Variable?, type: Types::t, declared_in: TypeName) -> void
+      type source = AST::Members::AttrAccessor
+                  | AST::Members::AttrReader
+                  | AST::Members::AttrWriter
+                  | AST::Members::InstanceVariable
+                  | AST::Members::ClassVariable
+                  | AST::Members::ClassInstanceVariable
+      attr_reader source: source
+
+      def initialize: (parent_variable: Variable?, type: Types::t, declared_in: TypeName, source: source) -> void
 
       def sub: (Substitution) -> Variable
     end

sig/definition_builder.rbs

@@ -105,7 +105,9 @@ module RBS
 
     def source_location: (Definition::Ancestor::Instance::source, AST::Declarations::t) -> Location[untyped, untyped]?
 
-    def insert_variable: (TypeName, Hash[Symbol, Definition::Variable], name: Symbol, type: Types::t) -> void
+    def validate_variable: (Definition::Variable) -> 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
     #

test/rbs/definition_builder_test.rb

@@ -2714,6 +2714,28 @@ class InstanceVariable
   @instance: Integer
 end
 
+class AttrInstanceVariable
+  attr_accessor instance: Integer
+  @instance: Integer
+end
+
+class InstanceVariableAttr
+  @instance: Integer
+  attr_accessor instance: Integer
+end
+
+class InstanceVariableAttrInstanceVariable
+  @instance: Integer
+  attr_accessor instance: Integer
+  @instance: Integer
+end
+
+class ClassInstanceVariableSingletonAttrClassInstanceVariable
+  self.@class_instance: Integer
+  attr_accessor self.class_instance: Integer
+  self.@class_instance: Integer
+end
+
 class ClassInstanceVariable
   self.@class_instance: Integer
   self.@class_instance: Integer
@@ -2731,6 +2753,18 @@ class ClassVariable
         assert_raises(RBS::InstanceVariableDuplicationError) do
           builder.build_instance(type_name("::InstanceVariable"))
         end
+        assert_nothing_raised do
+          builder.build_instance(type_name("::AttrInstanceVariable"))
+        end
+        assert_nothing_raised do
+          builder.build_instance(type_name("::InstanceVariableAttr"))
+        end
+        assert_raises(RBS::InstanceVariableDuplicationError) do
+          builder.build_instance(type_name("::InstanceVariableAttrInstanceVariable"))
+        end
+        assert_raises(RBS::ClassInstanceVariableDuplicationError) do
+          builder.build_singleton(type_name("::ClassInstanceVariableSingletonAttrClassInstanceVariable"))
+        end
         assert_raises(RBS::ClassInstanceVariableDuplicationError) do
           builder.build_singleton(type_name("::ClassInstanceVariable"))
         end