Keep fixture annotations out of ERB blocks (#345)

lib/annotate_rb/model_annotator/file_parser/yml_parser.rb

@@ -43,10 +43,15 @@ def parse_yml
           begin
             parser.parse(@input)
           rescue Psych::SyntaxError => _e
-            # "Dynamic fixtures with ERB" exist in Rails, and will cause Psych.parser to error
-            # This is a hacky solution to get around this and still have it parse
-            erb_yml = ERB.new(@input).result
-            parser.parse(erb_yml)
+            # "Dynamic fixtures with ERB" exist in Rails and cause Psych.parser to error.
+            #
+            # We deliberately do not evaluate the ERB and read line numbers off the
+            # result: evaluating runs arbitrary code, and the line numbers from the
+            # evaluated output do not map back to the original file (ERB tags spanning
+            # multiple lines shift the offsets), which would place annotations inside
+            # an ERB tag. Instead we derive the content bounds straight from the
+            # original lines so annotations land around the ERB body.
+            return record_erb_positions
           end
 
           stream = parser.handler.root
@@ -68,6 +73,29 @@ def parse_yml
             @ends << [nil, stream.end_line]
           end
         end
+
+        # Locates the content bounds of an ERB fixture directly from the original
+        # lines, treating the ERB/YAML body as the doc. The start is the first
+        # non-blank, non-comment line so annotations are written above the ERB
+        # block (and after any leading comments), never inside a tag.
+        def record_erb_positions
+          lines = @input.split($/)
+          content_start = lines.index { |line| content_line?(line) }
+
+          if content_start.nil?
+            @starts << [nil, 0]
+            @ends << [nil, 0]
+          else
+            content_end = lines.rindex { |line| content_line?(line) }
+            @starts << [nil, content_start]
+            @ends << [nil, content_end + 1]
+          end
+        end
+
+        def content_line?(line)
+          stripped = line.strip
+          !stripped.empty? && !stripped.start_with?("#")
+        end
       end
     end
   end

spec/lib/annotate_rb/model_annotator/annotated_file/generator_spec.rb

@@ -1113,5 +1113,93 @@ class TestModel < ApplicationRecord
         end
       end
     end
+
+    context "with an erb fixture file" do
+      let(:annotation_position) { :position_in_fixture }
+      let(:parser_klass) { AnnotateRb::ModelAnnotator::FileToParserMapper.map("users.yml") }
+      let(:new_annotations) do
+        <<~ANNOTATIONS
+          # == Schema Information
+          #
+          # Table name: users
+          #
+          #  id   :integer          not null, primary key
+          #  name :string
+          #
+        ANNOTATIONS
+      end
+
+      context "when the file starts with a multi-line erb block" do
+        let(:options) { AnnotateRb::Options.new({position_in_fixture: "before_doc"}) }
+        let(:file_content) do
+          <<~FILE
+            <%
+              total = 2
+            %>
+            <% total.times do |i| %>
+            user_<%= i %>:
+              name: User <%= i %>
+            <% end %>
+          FILE
+        end
+        let(:expected_content) do
+          <<~CONTENT
+            # == Schema Information
+            #
+            # Table name: users
+            #
+            #  id   :integer          not null, primary key
+            #  name :string
+            #
+            <%
+              total = 2
+            %>
+            <% total.times do |i| %>
+            user_<%= i %>:
+              name: User <%= i %>
+            <% end %>
+          CONTENT
+        end
+
+        it "writes the annotation above the erb block, not inside it" do
+          is_expected.to eq(expected_content)
+        end
+      end
+
+      context "when the erb block is preceded by a comment" do
+        let(:options) { AnnotateRb::Options.new({position_in_fixture: "before_doc"}) }
+        let(:file_content) do
+          <<~FILE
+            # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
+
+            <% 1.upto(3) do |i| %>
+            user_<%= i %>:
+              name: User <%= i %>
+            <% end %>
+          FILE
+        end
+        let(:expected_content) do
+          <<~CONTENT
+            # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
+
+            # == Schema Information
+            #
+            # Table name: users
+            #
+            #  id   :integer          not null, primary key
+            #  name :string
+            #
+            <% 1.upto(3) do |i| %>
+            user_<%= i %>:
+              name: User <%= i %>
+            <% end %>
+          CONTENT
+        end
+
+        it "writes the annotation above the erb block, not inside the loop" do
+          is_expected.to eq(expected_content)
+        end
+      end
+    end
   end
 end

spec/lib/annotate_rb/model_annotator/file_parser/yml_parser_spec.rb

@@ -127,14 +127,39 @@ def check_it_parses_correctly
         ["# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html", 0]
       ]
     end
-    let(:expected_starts) { [[nil, 3]] }
-    let(:expected_ends) { [[nil, 404]] }
+    # Content bounds are taken from the original file (not the evaluated ERB),
+    # so the annotation is placed around the ERB body instead of inside a tag.
+    let(:expected_starts) { [[nil, 2]] }
+    let(:expected_ends) { [[nil, 7]] }
 
     it "parses without errors" do
       check_it_parses_correctly
     end
   end
 
+  context "with a yml file that starts with a multi-line erb block" do
+    let(:input) do
+      <<~FILE
+        <%
+          total = 2
+        %>
+        <% total.times do |i| %>
+        user_<%= i %>:
+          name: User <%= i %>
+        <% end %>
+      FILE
+    end
+    let(:expected_comments) { [] }
+    # Start must be line 0 (the opening `<%`) so the annotation is written
+    # above the ERB block, not inside it.
+    let(:expected_starts) { [[nil, 0]] }
+    let(:expected_ends) { [[nil, 7]] }
+
+    it "parses without inserting into the erb block" do
+      check_it_parses_correctly
+    end
+  end
+
   context "with an empty yml file" do
     let(:input) do
       <<~FILE

spec/lib/annotate_rb/model_annotator/single_file_annotator_spec.rb

@@ -345,7 +345,6 @@ class User < ApplicationRecord
         <<~FILE
           # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
 
-          <% 1.upto(100) do |i| %>
           # == Schema Information
           #
           # Table name: users
@@ -354,6 +353,7 @@ class User < ApplicationRecord
           #  name([sensitivity: medium]) :string(50)       not null
           #  email                       :string
           #
+          <% 1.upto(100) do |i| %>
           user_<%= i %>:
             name: User <%= i %>
             email: user_<%= i %>@example.com