Preserve order of tags when reference tags are mixed with local tags

lib/yard/docstring.rb

@@ -40,9 +40,6 @@ def parser(*args) default_parser.new(*args) end
 
     self.default_parser = DocstringParser
 
-    # @return [Array<Tags::RefTag>] the list of reference tags
-    attr_reader :ref_tags
-
     # @return [CodeObjects::Base] the object that owns the docstring.
     attr_accessor :object
 
@@ -131,8 +128,7 @@ def to_s
     # @param [String] content the raw comments to be parsed
     def replace(content, parse = true)
       content = content.join("\n") if content.is_a?(Array)
-      @tags = []
-      @ref_tags = []
+      @all_tags = []
       if parse
         super(parse_comments(content))
       else
@@ -153,7 +149,7 @@ def replace(content, parse = true)
     def dup
       resolve_reference
       obj = super
-      %w(all summary tags ref_tags).each do |name|
+      %w(all summary all_tags).each do |name|
         val = instance_variable_defined?("@#{name}") && instance_variable_get("@#{name}")
         obj.instance_variable_set("@#{name}", val ? val.dup : nil)
       end
@@ -244,9 +240,9 @@ def add_tag(*tags)
         case tag
         when Tags::Tag
           tag.object = object
-          @tags << tag
-        when Tags::RefTag, Tags::RefTagList
-          @ref_tags << tag
+          @all_tags << tag
+        when Tags::RefTagList
+          @all_tags << tag
         else
           raise ArgumentError, "expected Tag or RefTag, got #{tag.class} (at index #{i})"
         end
@@ -271,11 +267,17 @@ def tag(name)
     # @param [#to_s] name the tag name to return data for, or nil for all tags
     # @return [Array<Tags::Tag>] the list of tags by the specified tag name
     def tags(name = nil)
-      list = stable_sort_by(@tags + convert_ref_tags, &:tag_name)
+      list = @all_tags.map { |tag| convert_ref_tag(tag) }.flatten
+      list = stable_sort_by(list, &:tag_name)
       return list unless name
       list.select {|tag| tag.tag_name.to_s == name.to_s }
     end
 
+    # @return [Array<Tags::RefTag, Tags::RefTagList>] the list of reference tags
+    def ref_tags
+      @all_tags.select { |tag| tag.is_a?(Tags::RefTag) || tag.is_a?(Tags::RefTagList) }
+    end
+
     # Returns true if at least one tag by the name +name+ was declared
     #
     # @param [String] name the tag name to search for
@@ -298,8 +300,7 @@ def delete_tags(name)
     # @return [void]
     # @since 0.7.0
     def delete_tag_if(&block)
-      @tags.delete_if(&block)
-      @ref_tags.delete_if(&block)
+      @all_tags.delete_if(&block)
     end
 
     # Returns true if the docstring has no content that is visible to a template.
@@ -311,7 +312,7 @@ def blank?(only_visible_tags = true)
       if only_visible_tags
         empty? && !tags.any? {|tag| Tags::Library.visible_tags.include?(tag.tag_name.to_sym) }
       else
-        empty? && @tags.empty? && @ref_tags.empty?
+        empty? && @all_tags.empty?
       end
     end
 
@@ -340,20 +341,26 @@ def resolve_reference
 
     # Maps valid reference tags
     #
-    # @return [Array<Tags::RefTag>] the list of valid reference tags
-    def convert_ref_tags
-      list = @ref_tags.reject {|t| CodeObjects::Proxy === t.owner }
-
+    # @param tag [Tags::Tag, Tags::RefTagList]
+    # @return [Array<Tags::Tag>] dereferenced tags
+    def convert_ref_tag(tag)
       @ref_tag_recurse_count ||= 0
       @ref_tag_recurse_count += 1
       if @ref_tag_recurse_count > 2
+        tag_and_name = ["@#{tag.tag_name}", tag.name].compact.join(" ")
         log.error "#{@object.file}:#{@object.line}: Detected circular reference tag in " \
-                  "`#{@object}', ignoring all reference tags for this object " \
-                  "(#{@ref_tags.map {|t| "@#{t.tag_name}" }.join(", ")})."
-        @ref_tags = []
-        return @ref_tags
+                  "`#{@object}'. Ignoring reference tag from #{tag_and_name} to `#{tag.owner}'."
+        return []
+      end
+      if tag.is_a?(Tags::RefTagList)
+        if CodeObjects::Proxy === tag.owner
+          list = []
+        else
+          list = tag.tags
+        end
+      else
+        list = [tag]
       end
-      list = list.map(&:tags).flatten
       @ref_tag_recurse_count -= 1
       list
     end

spec/docstring_spec.rb

@@ -168,6 +168,47 @@
       expect(tags.size).to eq 0
     end
 
+    it "preserves the order of ref tags mixed with local tags" do
+      YARD.parse_string <<-eof
+        class A
+          # @param x X
+          # @param z Z
+          def a(x, z); end
+          # @param x (see #a)
+          # @param y Y
+          # @param z (see #a)
+          def b(x,y,z);end
+          # @param x cX
+          # @param y (see #b)
+          # @param z cZ
+          def c(x,y,z);end
+          # (see #c)
+          def d(x,y,z); end
+        end
+      eof
+
+      # local tag between refs
+      expect(YARD::Registry.at('A#b').tags.map { |t| [t.tag_name, t.name, t.text]}).to eq [
+        ['param', 'x', 'X'],
+        ['param', 'y', 'Y'],
+        ['param', 'z', 'Z'],
+      ]
+
+      # ref tag between locals
+      expect(YARD::Registry.at('A#c').tags.map { |t| [t.tag_name, t.name, t.text]}).to eq [
+        ['param', 'x', 'cX'],
+        ['param', 'y', 'Y'],
+        ['param', 'z', 'cZ'],
+      ]
+
+      # through a ref doctring
+      expect(YARD::Registry.at('A#d').tags.map { |t| [t.tag_name, t.name, t.text]}).to eq [
+        ['param', 'x', 'cX'],
+        ['param', 'y', 'Y'],
+        ['param', 'z', 'cZ'],
+      ]
+    end
+
     it "resolves references to methods in the same class with #methname" do
       klass = CodeObjects::ClassObject.new(:root, "Foo")
       o = CodeObjects::MethodObject.new(klass, "bar")
@@ -192,7 +233,8 @@ def b; end
         end
       eof
 
-      expect(log.io.string).to match(/error.*circular reference tag in `Foo#b'/)
+      err = "Detected circular reference tag in `Foo#b'. Ignoring reference tag from @param to `Foo#a'."
+      expect(log.io.string).to include(err)
       expect(Registry.at('Foo#a').tags).to be_empty
       expect(Registry.at('Foo#b').tags).to be_empty
     end
@@ -205,7 +247,8 @@ def bar; end
         end
       eof
 
-      expect(log.io.string).to match(/error.*circular reference tag in `Foo#bar'/)
+      err = "Detected circular reference tag in `Foo#bar'. Ignoring reference tag from @param to `Foo#bar'."
+      expect(log.io.string).to include(err)
       expect(Registry.at('Foo#bar').tags).to be_empty
     end
   end