diff --git a/lib/annotate_rb/model_annotator/file_parser/yml_parser.rb b/lib/annotate_rb/model_annotator/file_parser/yml_parser.rb index 83c2692b..8b2441e2 100644 --- a/lib/annotate_rb/model_annotator/file_parser/yml_parser.rb +++ b/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 diff --git a/spec/lib/annotate_rb/model_annotator/annotated_file/generator_spec.rb b/spec/lib/annotate_rb/model_annotator/annotated_file/generator_spec.rb index 787ac171..192b3fa9 100644 --- a/spec/lib/annotate_rb/model_annotator/annotated_file/generator_spec.rb +++ b/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 diff --git a/spec/lib/annotate_rb/model_annotator/file_parser/yml_parser_spec.rb b/spec/lib/annotate_rb/model_annotator/file_parser/yml_parser_spec.rb index 53704843..2d48a81b 100644 --- a/spec/lib/annotate_rb/model_annotator/file_parser/yml_parser_spec.rb +++ b/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 diff --git a/spec/lib/annotate_rb/model_annotator/single_file_annotator_spec.rb b/spec/lib/annotate_rb/model_annotator/single_file_annotator_spec.rb index e85388cc..f77396be 100644 --- a/spec/lib/annotate_rb/model_annotator/single_file_annotator_spec.rb +++ b/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