Merge pull request #2241 from ksss/variable-duplication-error

Check variable duplication

Author: soutaro

lib/rbs/definition_builder.rb

@@ -151,6 +151,7 @@ def define_instance(definition, type_name, subst)
             end
 
           when AST::Members::InstanceVariable
+            InstanceVariableDuplicationError.check!(variables: definition.instance_variables, member: member, type_name: type_name)
             insert_variable(
               type_name,
               definition.instance_variables,
@@ -159,6 +160,7 @@ def define_instance(definition, type_name, subst)
             )
 
           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)
           end
         end
@@ -287,9 +289,11 @@ def build_singleton0(type_name)
                 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)
 
               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)
               end
             end

lib/rbs/errors.rb

@@ -322,6 +322,51 @@ def type_name
     end
   end
 
+  class VariableDuplicationError < DefinitionError
+    include DetailedMessageable
+
+    attr_reader :member
+
+    def initialize(member:)
+      @member = member
+
+      super "#{Location.to_string location}: Duplicated variable name #{member.name}"
+    end
+
+    def location
+      loc = @member.location or raise
+      loc[:name]
+    end
+  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
     include DetailedMessageable
 

sig/errors.rbs

@@ -182,6 +182,28 @@ module RBS
     def location: () -> AST::Members::Mixin::loc?
   end
 
+  class VariableDuplicationError < DefinitionError
+    include DetailedMessageable
+
+    attr_reader member: AST::Members::Var
+
+    def initialize: (member: AST::Members::Var) -> void
+
+    def location: () -> Location[bot, bot]
+  end
+
+  class InstanceVariableDuplicationError < VariableDuplicationError
+    def self.check!: (variables: Hash[Symbol, Definition::Variable], member: AST::Members::InstanceVariable, type_name: TypeName) -> void
+  end
+
+  class ClassInstanceVariableDuplicationError < VariableDuplicationError
+    def self.check!: (variables: Hash[Symbol, Definition::Variable], member: AST::Members::ClassInstanceVariable, type_name: TypeName) -> void
+  end
+
+  class ClassVariableDuplicationError < VariableDuplicationError
+    def self.check!: (variables: Hash[Symbol, Definition::Variable], member: AST::Members::ClassVariable, type_name: TypeName) -> void
+  end
+
   # The `alias` member declares an alias from unknown method
   #
   class UnknownMethodAliasError < DefinitionError

test/rbs/definition_builder_test.rb

@@ -2705,4 +2705,64 @@ module Foo
       end
     end
   end
+
+  def test_duplicated_variable
+    SignatureManager.new do |manager|
+      manager.add_file("instance.rbs", <<-EOF)
+class InstanceVariable
+  @instance: Integer
+  @instance: Integer
+end
+
+class ClassInstanceVariable
+  self.@class_instance: Integer
+  self.@class_instance: Integer
+end
+
+class ClassVariable
+  @@class: Integer
+  @@class: Integer
+end
+      EOF
+
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        assert_raises(RBS::InstanceVariableDuplicationError) do
+          builder.build_instance(type_name("::InstanceVariable"))
+        end
+        assert_raises(RBS::ClassInstanceVariableDuplicationError) do
+          builder.build_singleton(type_name("::ClassInstanceVariable"))
+        end
+        assert_raises(RBS::ClassVariableDuplicationError) do
+          builder.build_instance(type_name("::ClassVariable"))
+        end
+      end
+    end
+
+    SignatureManager.new do |manager|
+      manager.add_file("inherited.rbs", <<-EOF)
+class A
+  @instance: Integer
+  self.@class_instance: Integer
+  @@class: Integer
+end
+
+class B < A
+  @instance: Integer
+  self.@class_instance: Integer
+  @@class: Integer
+end
+      EOF
+
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::A"))
+        assert_raises(RBS::ClassVariableDuplicationError) do
+          builder.build_instance(type_name("::B"))
+        end
+      end
+    end
+  end
 end