Fixes updating columns when using alternative formats

lib/annotate/annotate_models.rb

@@ -364,6 +364,10 @@ def get_foreign_key_info(klass, options = {})
     #  :force<Symbol>:: whether to update the file even if it doesn't seem to need it.
     #  :position_in_*<Symbol>:: where to place the annotated section in fixture or model file,
     #                           :before, :top, :after or :bottom. Default is :before.
+    #  :format_bare<Boolean>:: whether to format annotations using default, bare syntax
+    #  :format_markdown<Boolean>:: whether to format annotations using Markdown syntax
+    #  :format_rdoc<Boolean>:: whether to format annotations using RDoc syntax
+    #  :format_yard<Boolean>:: whether to format annotations using Yard syntax
     #
     def annotate_one_file(file_name, info_block, position, options = {})
       return false unless File.exist?(file_name)
@@ -375,7 +379,9 @@ def annotate_one_file(file_name, info_block, position, options = {})
       old_header = old_content.match(header_pattern).to_s
       new_header = info_block.match(header_pattern).to_s
 
-      column_pattern = /^#[\t ]+[\w\*\.`]+[\t ]+.+$/
+      annotation_format = format_from(options)
+      column_pattern =
+        /#{column_pattern_for(annotation_format)}|#{foreign_key_column_pattern_for(annotation_format)}/
       old_columns = old_header && old_header.scan(column_pattern).sort
       new_columns = new_header && new_header.scan(column_pattern).sort
 
@@ -908,6 +914,79 @@ def get_attributes(column, column_type, klass, options)
 
       attrs
     end
+
+    # Returns the regular expression for finding a column definition based on
+    # the format of the annotations being made
+    #
+    # == Returns:
+    # Regular expression
+    #
+    # @param [Symbol] symbol representation of the format being used for
+    #   annotations
+    def column_pattern_for(format_type)
+      case format_type
+      when :markdown then /^#\s+\*{2}`\w+`\*{2}(?:\s+\|\s`.*`)*$/
+      when :rdoc then
+        field_name_regex = /^#\s+\*\w+\*\:{2}/
+        field_info_regex = /\s+\<tt\>.*\<\/tt\>$/
+
+        /#{field_name_regex}#{field_info_regex}/
+      when :yard
+        attribute_regex = /^#\s@!attribute\s\w+$/
+        return_regex = /^#\s+@return\s+\[\w+\]$/
+
+        /#{attribute_regex}|#{return_regex}/
+      else # :bare/default
+        /^#\s+\w+\s?:\w+.*$/
+      end
+    end
+
+    # == Returns:
+    # Regular expression
+    #
+    # Checks for annotation comments that could represent a foreign key
+    # constraint. Handles the different formats that are supported. Also
+    # handles the optiona to shorten foreign keys with "...".
+    #
+    # @note Foreign key formatting only varies for markdown formatting. All
+    #   other formats use the "bare" format.
+    #
+    # @param [Symbol] symbol representation of the format being used for
+    #   annotations
+    def foreign_key_column_pattern_for(format_type)
+      case format_type
+      when :markdown
+        /^#\s+\*\s`[\w_]+(?:\.{3})?`\s.*$/
+      else
+        /^#\s+[\w_]+(?:\.{3})?\s+.*/
+      end
+    end
+
+    # Determine the current format being used for making annotations based on the options.
+    #
+    # == Returns:
+    # Symbol representing the format being used. Options include:
+    #   :markdown
+    #   :rdoc
+    #   :yard
+    #   :bare - this is the default option
+    #
+    # === Options (opts)
+    #  :format_bare<Boolean>:: whether to format annotations using default, bare syntax
+    #  :format_markdown<Boolean>:: whether to format annotations using Markdown syntax
+    #  :format_rdoc<Boolean>:: whether to format annotations using RDoc syntax
+    #  :format_yard<Boolean>:: whether to format annotations using Yard syntax
+    def format_from(options)
+      if options[:format_markdown]
+        :markdown
+      elsif options[:format_rdoc]
+        :rdoc
+      elsif options[:format_yard]
+        :yard
+      else
+        :bare
+      end
+    end
   end
 
   class BadModelFileError < LoadError

spec/lib/annotate/annotate_models_spec.rb

@@ -2526,44 +2526,255 @@ def annotate_one_file(options = {})
 
     describe 'with existing annotation' do
       context 'of a foreign key' do
+        let(:class_name) { :users }
+        let(:primary_key) { :id }
+        let(:original_columns) do
+          [
+            mock_column(:id, :integer),
+            mock_column(:foreign_thing_id, :integer)
+          ]
+        end
+        let(:original_foreign_keys) do
+          [
+            mock_foreign_key('fk_rails_cf2568e89e',
+                             'foreign_thing_id',
+                             'foreign_things',
+                             'id',
+                             on_delete: :cascade)
+          ]
+        end
+
+        let(:options) do
+          { show_foreign_keys: true }.merge(format_option)
+        end
+
         before do
-          klass = mock_class(:users,
-                             :id,
-                             [
-                               mock_column(:id, :integer),
-                               mock_column(:foreign_thing_id, :integer)
-                             ],
+          klass = mock_class(class_name,
+                             primary_key,
+                             original_columns,
                              [],
-                             [
-                               mock_foreign_key('fk_rails_cf2568e89e',
-                                                'foreign_thing_id',
-                                                'foreign_things',
-                                                'id',
-                                                on_delete: :cascade)
-                             ])
-          @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', show_foreign_keys: true)
+                             original_foreign_keys)
+          @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
           annotate_one_file
         end
 
-        it 'should update foreign key constraint' do
-          klass = mock_class(:users,
-                             :id,
-                             [
-                               mock_column(:id, :integer),
-                               mock_column(:foreign_thing_id, :integer)
-                             ],
-                             [],
-                             [
-                               mock_foreign_key('fk_rails_cf2568e89e',
-                                                'foreign_thing_id',
-                                                'foreign_things',
-                                                'id',
-                                                on_delete: :restrict)
-                             ])
-          @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', show_foreign_keys: true)
-          annotate_one_file
+        context 'updating a constraint' do
+          let(:updated_foreign_keys) do
+            [
+              mock_foreign_key('fk_rails_cf2568e89e',
+                               'foreign_thing_id',
+                               'foreign_things',
+                               'id',
+                               on_delete: :restrict)
+            ]
+          end
+
+          context 'when option "format_bare" is true' do
+            let(:format_option) { { format_bare: true } }
+
+            it 'should update foreign key constraint' do
+              klass = mock_class(class_name,
+                                 primary_key,
+                                 original_columns,
+                                 [],
+                                 updated_foreign_keys)
+
+              @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+              annotate_one_file(options)
+
+              expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+            end
+          end
+
+          context 'when option "format_markdown" is true' do
+            let(:format_option) { { format_markdown: true } }
+
+            it 'should update foreign key constraint' do
+              klass = mock_class(class_name,
+                                 primary_key,
+                                 original_columns,
+                                 [],
+                                 updated_foreign_keys)
+
+              @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+              annotate_one_file(options)
+
+              expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+            end
+          end
+        end
+      end
+
+      context 'adding a new field' do
+        let(:class_name) { :users }
+        let(:primary_key) { :id }
+        let(:original_columns) do
+          [
+            mock_column(primary_key, :integer),
+            mock_column(:name, :string)
+          ]
+        end
+
+        before do
+          klass = mock_class(class_name, primary_key, original_columns)
+          @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+          annotate_one_file(options)
+
+          # confirm we initialized annotaions in file before checking for changes
+          expect(@schema_info).not_to be_empty
           expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
         end
+
+        context 'when option "format_bare" is true' do
+          let :options do
+            { format_bare: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            new_column_list = original_columns + [mock_column(:new_column, :string)]
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_yard" is true' do
+          let :options do
+            { format_yard: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            new_column_list = original_columns + [mock_column(:new_column, :string)]
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_rdoc" is true' do
+          let :options do
+            { format_rdoc: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            new_column_list = original_columns + [mock_column(:new_column, :string)]
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_markdown" is true' do
+          let :options do
+            { format_markdown: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            # The new column name must be shorter than the existing columns
+            # becuase markdown formatting adds additional spacing. If the
+            # column name is long, the header row has space added triggering a
+            # difference even if we don't properly check the columns. By having
+            # a shorter column name we are testing our column list comparison
+            # and not an unintentional resizing.
+            new_column_list = original_columns + [mock_column(:a, :string)]
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+      end
+
+      context 'changing a field type' do
+        let(:class_name) { :users }
+        let(:primary_key) { :id }
+        let(:original_columns) do
+          [
+            mock_column(primary_key, :integer),
+            mock_column(:some_field, :string)
+          ]
+        end
+
+        # update `name` from `:string` to `:text`
+        let(:new_column_list) do
+          [
+            mock_column(primary_key, :integer),
+            mock_column(:some_field, :integer)
+          ]
+        end
+
+        before do
+          klass = mock_class(class_name, primary_key, original_columns)
+          @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+          annotate_one_file(options)
+
+          # confirm we initialized annotaions in file before checking for changes
+          expect(@schema_info).not_to be_empty
+          expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+        end
+
+        context 'when option "format_bare" is true' do
+          let :options do
+            { format_bare: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_yard" is true' do
+          let :options do
+            { format_yard: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_rdoc" is true' do
+          let :options do
+            { format_rdoc: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
+
+        context 'when option "format_markdown" is true' do
+          let :options do
+            { format_markdown: true }
+          end
+
+          it 'updates the fields list to include the new column' do
+            klass = mock_class(class_name, primary_key, new_column_list)
+            @schema_info = AnnotateModels.get_schema_info(klass, '== Schema Info', options)
+            annotate_one_file(options)
+
+            expect(File.read(@model_file_name)).to eq("#{@schema_info}#{@file_content}")
+          end
+        end
       end
     end