Merge pull request ruby#2301 from ruby/annotations

Fix method annotations

Author: soutaro

lib/rbs/definition.rb

@@ -42,9 +42,9 @@ def initialize(type:, member:, defined_in:, implemented_in:, overload_annotation
           @member = member
           @defined_in = defined_in
           @implemented_in = implemented_in
-          @member_annotations = member.annotations
-          @overload_annotations = overload_annotations
-          @annotations = member.annotations + overload_annotations
+          @member_annotations = []
+          @overload_annotations = []
+          @annotations = []
         end
 
         def ==(other)
@@ -66,7 +66,10 @@ def comment
         end
 
         def update(type: self.type, member: self.member, defined_in: self.defined_in, implemented_in: self.implemented_in)
-          TypeDef.new(type: type, member: member, defined_in: defined_in, implemented_in: implemented_in, overload_annotations: overload_annotations)
+          TypeDef.new(type: type, member: member, defined_in: defined_in, implemented_in: implemented_in).tap do |type_def|
+            type_def.overload_annotations.replace(self.overload_annotations)
+            type_def.member_annotations.replace(self.member_annotations)
+          end
         end
 
         def overload?
@@ -77,20 +80,33 @@ def overload?
             false
           end
         end
+
+        def each_annotation(&block)
+          if block
+            member_annotations.each(&block)
+            overload_annotations.each(&block)
+          else
+            enum_for :each_annotation
+          end
+        end
       end
 
       attr_reader :super_method
       attr_reader :defs
       attr_reader :accessibility
       attr_reader :extra_annotations
+      attr_reader :annotations
       attr_reader :alias_of
+      attr_reader :alias_member
 
-      def initialize(super_method:, defs:, accessibility:, annotations: [], alias_of:)
+      def initialize(super_method:, defs:, accessibility:, annotations: [], alias_of:, alias_member: nil)
         @super_method = super_method
         @defs = defs
         @accessibility = accessibility
         @extra_annotations = []
+        @annotations = []
         @alias_of = alias_of
+        @alias_member = alias_member
       end
 
       def ==(other)
@@ -99,7 +115,8 @@ def ==(other)
           other.defs == defs &&
           other.accessibility == accessibility &&
           other.annotations == annotations &&
-          other.alias_of == alias_of
+          other.alias_of == alias_of &&
+          other.alias_member == alias_member
       end
 
       alias eql? ==
@@ -130,10 +147,6 @@ def comments
         @comments ||= defs.map(&:comment).compact.uniq
       end
 
-      def annotations
-        @annotations ||= defs.flat_map {|d| d.member_annotations }
-      end
-
       def members
         @members ||= defs.map(&:member).uniq
       end
@@ -149,49 +162,42 @@ def private?
       def sub(s)
         return self if s.empty?
 
-        self.class.new(
+        update(
           super_method: super_method&.sub(s),
-          defs: defs.map {|defn| defn.update(type: defn.type.sub(s)) },
-          accessibility: @accessibility,
-          alias_of: alias_of
+          defs: defs.map {|defn| defn.update(type: defn.type.sub(s)) }
         )
       end
 
       def map_type(&block)
-        self.class.new(
+        update(
           super_method: super_method&.map_type(&block),
-          defs: defs.map {|defn| defn.update(type: defn.type.map_type(&block)) },
-          accessibility: @accessibility,
-          alias_of: alias_of
+          defs: defs.map {|defn| defn.update(type: defn.type.map_type(&block)) }
         )
       end
 
       def map_type_bound(&block)
-        self.class.new(
+        update(
           super_method: super_method&.map_type_bound(&block),
-          defs: defs.map {|defn| defn.update(type: defn.type.map_type_bound(&block)) },
-          accessibility: @accessibility,
-          alias_of: alias_of
+          defs: defs.map {|defn| defn.update(type: defn.type.map_type_bound(&block)) }
         )
       end
 
       def map_method_type(&block)
-        self.class.new(
-          super_method: super_method,
+        update(
           defs: defs.map {|defn| defn.update(type: yield(defn.type)) },
-          accessibility: @accessibility,
-          alias_of: alias_of
         )
       end
 
-      def update(super_method: self.super_method, defs: self.defs, accessibility: self.accessibility, alias_of: self.alias_of, annotations: self.annotations)
+      def update(super_method: self.super_method, defs: self.defs, accessibility: self.accessibility, alias_of: self.alias_of, annotations: self.annotations, alias_member: self.alias_member)
         self.class.new(
           super_method: super_method,
           defs: defs,
           accessibility: accessibility,
           alias_of: alias_of,
-          annotations: annotations
-        )
+          alias_member: alias_member
+        ).tap do |method|
+          method.annotations.replace(annotations)
+        end
       end
     end
 

lib/rbs/definition_builder.rb

@@ -389,12 +389,14 @@ def build_singleton(type_name)
                       type: method_type,
                       member: initialize_def.member,
                       defined_in: initialize_def.defined_in,
-                      implemented_in: initialize_def.implemented_in,
-                      overload_annotations: initialize_def.overload_annotations
-                    )
+                      implemented_in: initialize_def.implemented_in
+                    ).tap do |type_def|
+                      type_def.overload_annotations.replace(initialize_def.overload_annotations)
+                    end
                   end,
                   accessibility: :public,
-                  alias_of: nil
+                  alias_of: nil,
+                  alias_member: nil
                 )
 
                 definition.methods[:new] = typed_new
@@ -663,8 +665,11 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
             defn.update(defined_in: defined_in, implemented_in: implemented_in)
           end,
           accessibility: original_method.accessibility,
-          alias_of: original_method
+          alias_of: original_method,
+          alias_member: original
         )
+
+        method_definition.annotations.replace(original.annotations)
       when AST::Members::MethodDefinition
         if duplicated_method = methods[method.name]
           raise DuplicatedMethodDefinitionError.new(
@@ -679,9 +684,11 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
             type: subst.empty? ? overload.method_type : overload.method_type.sub(subst),
             member: original,
             defined_in: defined_in,
-            implemented_in: implemented_in,
-            overload_annotations: overload.annotations
-          )
+            implemented_in: implemented_in
+          ).tap do |type_def|
+            # Keep the original annotations given to overloads.
+            type_def.overload_annotations.replace(overload.annotations)
+          end
         end
 
         # @type var accessibility: RBS::Definition::accessibility
@@ -701,8 +708,11 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
           super_method: super_method,
           defs: defs,
           accessibility: accessibility,
-          alias_of: nil
+          alias_of: nil,
+          alias_member: nil
         )
+
+        method_definition.annotations.replace(original.annotations)
       when AST::Members::AttrReader, AST::Members::AttrWriter, AST::Members::AttrAccessor
         if duplicated_method = methods[method.name]
           raise DuplicatedMethodDefinitionError.new(
@@ -751,8 +761,11 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
             )
           ],
           accessibility: method.accessibility,
-          alias_of: nil
+          alias_of: nil,
+          alias_member: nil
         )
+
+        method_definition.annotations.replace(original.annotations)
       when nil
         # Overloading method definition only
 
@@ -779,8 +792,11 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
             defn.update(implemented_in: implemented_in)
           end,
           accessibility: existing_method.accessibility,
-          alias_of: existing_method.alias_of
+          alias_of: existing_method.alias_of,
+          alias_member: nil
         )
+
+        method_definition.annotations.replace(existing_method.annotations)
       end
 
       method.overloads.each do |overloading_def|
@@ -789,12 +805,19 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
             type: subst.empty? ? overload.method_type : overload.method_type.sub(subst),
             member: overloading_def,
             defined_in: defined_in,
-            implemented_in: implemented_in,
-            overload_annotations: overload.annotations
+            implemented_in: implemented_in
           )
 
+          type_def.overload_annotations.replace(overload.annotations)
+
           method_definition.defs.unshift(type_def)
         end
+
+        method_definition.annotations.concat(overloading_def.annotations)
+      end
+
+      method_definition.defs.each do |type_def|
+        type_def.member_annotations.replace(method_definition.annotations)
       end
 
       methods[method.name] = method_definition

lib/rbs/unit_test/type_assertions.rb

@@ -183,7 +183,7 @@ def send_setup(method_type, receiver, method, args, proc)
           assert_empty errors.map {|x| RBS::Test::Errors.to_string(x) }, "Call trace does not match with given method type: #{trace.inspect}"
 
           method_defs = method_defs(method)
-          all_errors = method_defs.map {|t| typecheck.method_call(method, t.type, trace, errors: [], annotations: t.annotations) }
+          all_errors = method_defs.map {|t| typecheck.method_call(method, t.type, trace, errors: [], annotations: t.each_annotation.to_a) }
           assert all_errors.any? {|es| es.empty? }, "Call trace does not match one of method definitions:\n  #{trace.inspect}\n  #{method_defs.map(&:type).join(" | ")}"
 
           raise exception if exception
@@ -220,7 +220,7 @@ def send_setup(method_type, receiver, method, args, proc)
           assert_empty errors.map {|x| RBS::Test::Errors.to_string(x) }
 
           method_defs = method_defs(method)
-          all_errors = method_defs.map {|t| typecheck.method_call(method, t.type, trace, errors: [], annotations: t.annotations) }
+          all_errors = method_defs.map {|t| typecheck.method_call(method, t.type, trace, errors: [], annotations: t.each_annotation.to_a) }
           assert all_errors.all? {|es| es.size > 0 }, "Call trace unexpectedly matches one of method definitions:\n  #{trace.inspect}\n  #{method_defs.map(&:type).join(" | ")}"
 
           result

sig/definition.rbs

@@ -28,22 +28,50 @@ module RBS
         attr_reader defined_in: TypeName
         attr_reader implemented_in: TypeName?
 
-        # Annotations given to `#member`
+        # Annotations given to the method definition syntax
+        #
+        # If the method have multiple syntaxes, union of the annotations to the member will be included, without dedup.
+        #
+        # The value should be updated during setup.
+        #
         attr_reader member_annotations: Array[AST::Annotation]
 
         # Annotations given to the overload associated to the method type
+        #
+        # The value should be updated during setup.
+        #
         attr_reader overload_annotations: Array[AST::Annotation]
 
-        # Concatenation of `member_annotations` and `overload_annotations`
-        attr_reader annotations: Array[AST::Annotation]
-
-        def initialize: (type: MethodType, member: method_member, defined_in: TypeName, implemented_in: TypeName?, ?overload_annotations: Array[AST::Annotation]) -> void
+        # Always returns an empty array
+        %a{deprecated} attr_reader annotations: Array[AST::Annotation]
+
+        # `overload_annotations` is ignored.
+        def initialize: (
+            type: MethodType,
+            member: method_member,
+            defined_in: TypeName,
+            implemented_in: TypeName?
+          ) -> void
+        | %a{deprecated} (
+            type: MethodType,
+            member: method_member,
+            defined_in: TypeName,
+            implemented_in: TypeName?,
+            overload_annotations: nil
+          ) -> void
 
         def comment: () -> AST::Comment?
 
         def update: (?type: MethodType, ?member: method_member, ?defined_in: TypeName, ?implemented_in: TypeName?) -> TypeDef
 
         def overload?: () -> bool
+
+        # Yields member and overload annotations, without dedup
+        #
+        # Member annotation yields first.
+        #
+        def each_annotation: () { (AST::Annotation) -> void } -> void
+                           | () -> Enumerator[AST::Annotation]
       end
 
       attr_reader super_method: Method?
@@ -56,21 +84,38 @@ module RBS
       attr_reader comments: Array[AST::Comment]
 
       attr_reader members: Array[method_member]
+
+      # The original method when the method is defined with `alias` syntax
       attr_reader alias_of: Method?
 
+      # Present if the method is defined with `alias` syntax
+      attr_reader alias_member: AST::Members::Alias?
+
       # Unused, always returns empty array
-      attr_reader extra_annotations: Array[AST::Annotation]
+      %a{deprecated} attr_reader extra_annotations: Array[AST::Annotation]
 
-      # Union of annotations given to `defs`, not contains annotations given to each overload
+      # Union of annotations given to `def`s and `alias`, not contains annotations given to each overload
+      #
+      # The elements will be updated during `Method` object setup.
+      #
       attr_reader annotations: Array[AST::Annotation]
 
       # Note that the annotations given through `annotations:` keyword is ignored.
       #
-      def initialize: (super_method: Method?,
-                       defs: Array[TypeDef],
-                       accessibility: accessibility,
-                       alias_of: Method?,
-                       ?annotations: Array[AST::Annotation]) -> void
+      def initialize: (
+          super_method: Method?,
+          defs: Array[TypeDef],
+          accessibility: accessibility,
+          alias_of: Method?,
+          alias_member: AST::Members::Alias?
+        ) -> void
+      | %a{deprecated} (
+          super_method: Method?,
+          defs: Array[TypeDef],
+          accessibility: accessibility,
+          alias_of: Method?,
+          annotations: Array[AST::Annotation]
+        ) -> void
 
       def public?: () -> bool
 
@@ -97,7 +142,8 @@ module RBS
         ?defs: Array[TypeDef],
         ?accessibility: accessibility,
         ?alias_of: Method?,
-        ?annotations: Array[AST::Annotation]
+        ?annotations: Array[AST::Annotation],
+        ?alias_member: AST::Members::Alias?
       ) -> Method
     end