Refactor ModelAnnotator (#21)

* Refactors `ModelAnnotator` internals to (hopefully) improve
maintainability
* Fixes bugs in old Annotate project that looked at wrong option keys
for `excludes_*`

Author: drwl

lib/annotate_rb/model_annotator.rb

@@ -23,5 +23,8 @@ module ModelAnnotator
     autoload :ColumnAnnotationBuilder, 'annotate_rb/model_annotator/column_annotation_builder'
     autoload :IndexAnnotationBuilder, 'annotate_rb/model_annotator/index_annotation_builder'
     autoload :ForeignKeyAnnotationBuilder, 'annotate_rb/model_annotator/foreign_key_annotation_builder'
+    autoload :RelatedFilesListBuilder, 'annotate_rb/model_annotator/related_files_list_builder'
+    autoload :AnnotationDecider, 'annotate_rb/model_annotator/annotation_decider'
+    autoload :FileAnnotatorInstruction, 'annotate_rb/model_annotator/file_annotator_instruction'
   end
 end

lib/annotate_rb/model_annotator/annotation_decider.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module AnnotateRb
+  module ModelAnnotator
+    # Class that encapsulates the logic to decide whether to annotate a model file and its related files or not.
+    class AnnotationDecider
+      def initialize(file, options)
+        @file = file
+        @options = options
+      end
+
+      def annotate?
+        return false if file_contains_skip_annotation
+
+        begin
+          klass = ModelClassGetter.call(@file, @options)
+
+          klass_is_a_class = klass.is_a?(Class)
+          klass_inherits_active_record_base = klass < ActiveRecord::Base
+          klass_is_not_abstract = klass.respond_to?(:abstract_class) && !klass.abstract_class?
+          klass_table_exists = klass.respond_to?(:abstract_class) && klass.table_exists?
+
+          not_sure_this_conditional = (!@options[:exclude_sti_subclasses] || !(klass.superclass < ActiveRecord::Base && klass.table_name == klass.superclass.table_name))
+
+          annotate_conditions = [
+            klass_is_a_class,
+            klass_inherits_active_record_base,
+            not_sure_this_conditional,
+            klass_is_not_abstract,
+            klass_table_exists
+          ]
+
+          to_annotate = annotate_conditions.all?
+
+          return to_annotate
+        rescue BadModelFileError => e
+          unless @options[:ignore_unknown_models]
+            $stderr.puts "Unable to annotate #{@file}: #{e.message}"
+            $stderr.puts "\t" + e.backtrace.join("\n\t") if @options[:trace]
+          end
+        rescue StandardError => e
+          $stderr.puts "Unable to annotate #{@file}: #{e.message}"
+          $stderr.puts "\t" + e.backtrace.join("\n\t") if @options[:trace]
+        end
+
+        false
+      end
+
+      private
+
+      def file_contains_skip_annotation
+        file_string = File.exist?(@file) ? File.read(@file) : ''
+
+        if /#{Constants::SKIP_ANNOTATION_PREFIX}.*/ =~ file_string
+          true
+        else
+          false
+        end
+      end
+    end
+  end
+end

lib/annotate_rb/model_annotator/annotation_generator.rb

@@ -12,10 +12,9 @@ class AnnotationGenerator
       MD_NAMES_OVERHEAD = 6
       MD_TYPE_ALLOWANCE = 18
 
-      def initialize(klass, header, options = {})
-        @header = header
-        @options = options
+      def initialize(klass, options = {})
         @model = ModelWrapper.new(klass, options)
+        @options = options
         @info = "" # TODO: Make array and build string that way
       end
 
@@ -50,9 +49,15 @@ def generate
         @info
       end
 
-      # TODO: Move header logic into here from AnnotateRb::ModelAnnotator::Annotator.do_annotations
       def header
-        @header
+        header = @options[:format_markdown] ? PREFIX_MD.dup : PREFIX.dup
+        version = ActiveRecord::Migrator.current_version rescue 0
+
+        if @options[:include_version] && version > 0
+          header += "\n# Schema version: #{version}"
+        end
+
+        header
       end
 
       def schema_header_text

lib/annotate_rb/model_annotator/annotator.rb

@@ -1,31 +1,20 @@
-# require 'bigdecimal'
+# frozen_string_literal: true
 
 module AnnotateRb
   module ModelAnnotator
     class Annotator
-      # Annotate Models plugin use this header
-      PREFIX = '== Schema Information'.freeze
-      PREFIX_MD = '## Schema Information'.freeze
-
-      MAGIC_COMMENT_MATCHER = Regexp.new(/(^#\s*encoding:.*(?:\n|r\n))|(^# coding:.*(?:\n|\r\n))|(^# -\*- coding:.*(?:\n|\r\n))|(^# -\*- encoding\s?:.*(?:\n|\r\n))|(^#\s*frozen_string_literal:.+(?:\n|\r\n))|(^# -\*- frozen_string_literal\s*:.+-\*-(?:\n|\r\n))/).freeze
-
       class << self
-        # We're passed a name of things that might be
-        # ActiveRecord models. If we can find the class, and
-        # if its a subclass of ActiveRecord::Base,
-        # then pass it to the associated block
         def do_annotations(options = {})
-          header = options[:format_markdown] ? PREFIX_MD.dup : PREFIX.dup
-          version = ActiveRecord::Migrator.current_version rescue 0
-          if options[:include_version] && version > 0
-            header << "\n# Schema version: #{version}"
-          end
-
           annotated = []
-          model_files_to_annotate = ModelFilesGetter.call(options)
 
-          model_files_to_annotate.each do |path, filename|
-            ModelFileAnnotator.call(annotated, File.join(path, filename), header, options)
+          model_files_to_consider = ModelFilesGetter.call(options)
+
+          model_files_to_consider.each do |path, filename|
+            file = File.join(path, filename)
+
+            if AnnotationDecider.new(file, options).annotate?
+              ModelFileAnnotator.call(annotated, file, options)
+            end
           end
 
           if annotated.empty?
@@ -37,34 +26,41 @@ def do_annotations(options = {})
 
         def remove_annotations(options = {})
           deannotated = []
-          deannotated_klass = false
-          ModelFilesGetter.call(options).each do |file|
-            file = File.join(file)
+
+          model_files_to_consider = ModelFilesGetter.call(options)
+
+          model_files_to_consider.each do |path, filename|
+            deannotated_klass = false
+            file = File.join(path, filename)
+
             begin
               klass = ModelClassGetter.call(file, options)
               if klass < ActiveRecord::Base && !klass.abstract_class?
                 model_name = klass.name.underscore
                 table_name = klass.table_name
-                model_file_name = file
-                deannotated_klass = true if FileAnnotationRemover.call(model_file_name, options)
 
-                patterns = PatternGetter.call(options)
+                if FileAnnotationRemover.call(file, options)
+                  deannotated_klass = true
+                end
+
+                related_files = RelatedFilesListBuilder.new(file, model_name, table_name, options).build
 
-                patterns
-                  .map { |f| FileNameResolver.call(f, model_name, table_name) }
-                  .each do |f|
+                related_files.each do |f, _position_key|
                   if File.exist?(f)
                     FileAnnotationRemover.call(f, options)
-                    deannotated_klass = true
                   end
                 end
               end
-              deannotated << klass if deannotated_klass
+
+              if deannotated_klass
+                deannotated << klass
+              end
             rescue StandardError => e
               $stderr.puts "Unable to deannotate #{File.join(file)}: #{e.message}"
               $stderr.puts "\t" + e.backtrace.join("\n\t") if options[:trace]
             end
           end
+
           puts "Removed annotations from: #{deannotated.join(', ')}"
         end
       end

lib/annotate_rb/model_annotator/constants.rb

@@ -1,8 +1,6 @@
 module AnnotateRb
   module ModelAnnotator
     module Constants
-      TRUE_RE = /^(true|t|yes|y|1)$/i.freeze
-
       ##
       # The set of available options to customize the behavior of Annotate.
       #
@@ -17,6 +15,8 @@ module Constants
       ALL_ANNOTATE_OPTIONS = ::AnnotateRb::Options::ALL_OPTION_KEYS
 
       SKIP_ANNOTATION_PREFIX = '# -\*- SkipSchemaAnnotations'.freeze
+
+      MAGIC_COMMENT_MATCHER = Regexp.new(/(^#\s*encoding:.*(?:\n|r\n))|(^# coding:.*(?:\n|\r\n))|(^# -\*- coding:.*(?:\n|\r\n))|(^# -\*- encoding\s?:.*(?:\n|\r\n))|(^#\s*frozen_string_literal:.+(?:\n|\r\n))|(^# -\*- frozen_string_literal\s*:.+-\*-(?:\n|\r\n))/).freeze
     end
   end
 end

lib/annotate_rb/model_annotator/file_annotator.rb

@@ -4,6 +4,10 @@ module AnnotateRb
   module ModelAnnotator
     class FileAnnotator
       class << self
+        def call_with_instructions(instruction)
+          call(instruction.file, instruction.annotation, instruction.position, instruction.options)
+        end
+
         # Add a schema block to a file. If the file already contains
         # a schema info block (a comment starting with "== Schema Information"),
         # check if it matches the block that is already there. If so, leave it be.
@@ -33,7 +37,7 @@ def call(file_name, info_block, position, options = {})
 
           return false if old_columns == new_columns && !options[:force]
 
-          abort "annotate error. #{file_name} needs to be updated, but annotate was run with `--frozen`." if options[:frozen]
+          abort "AnnotateRb error. #{file_name} needs to be updated, but annotaterb was run with `--frozen`." if options[:frozen]
 
           # Replace inline the old schema info with the new schema info
           wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : ""
@@ -47,7 +51,7 @@ def call(file_name, info_block, position, options = {})
           # need to insert it in correct position
           if old_annotation.empty? || options[:force]
             magic_comments_block = Helper.magic_comments_as_string(old_content)
-            old_content.gsub!(Annotator::MAGIC_COMMENT_MATCHER, '')
+            old_content.gsub!(Constants::MAGIC_COMMENT_MATCHER, '')
 
             annotation_pattern = AnnotationPatternGenerator.call(options)
             old_content.sub!(annotation_pattern, '')

lib/annotate_rb/model_annotator/file_annotator_instruction.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module AnnotateRb
+  module ModelAnnotator
+    # A plain old Ruby object (PORO) that contains all necessary information for FileAnnotator
+    class FileAnnotatorInstruction
+      def initialize(file, annotation, position, options = {})
+        @file = file # Path to file
+        @annotation = annotation # Annotation string
+        @position = position # Position in the file where to write the annotation to
+        @options = options
+      end
+
+      attr_reader :file, :annotation, :position, :options
+    end
+  end
+end

lib/annotate_rb/model_annotator/helper.rb

@@ -3,8 +3,6 @@
 module AnnotateRb
   module ModelAnnotator
     module Helper
-      MATCHED_TYPES = %w(test fixture factory serializer scaffold controller helper).freeze
-
       INDEX_CLAUSES = {
         unique: {
           default: 'UNIQUE',
@@ -89,16 +87,8 @@ def width(string)
           string.chars.inject(0) { |acc, elem| acc + (elem.bytesize == 3 ? 2 : 1) }
         end
 
-        def matched_types(options)
-          types = MATCHED_TYPES.dup
-          types << 'admin' if options[:active_admin] =~ Constants::TRUE_RE && !types.include?('admin')
-          types << 'additional_file_patterns' if options[:additional_file_patterns].present?
-
-          types
-        end
-
         def magic_comments_as_string(content)
-          magic_comments = content.scan(Annotator::MAGIC_COMMENT_MATCHER).flatten.compact
+          magic_comments = content.scan(Constants::MAGIC_COMMENT_MATCHER).flatten.compact
 
           if magic_comments.any?
             magic_comments.join
@@ -107,10 +97,6 @@ def magic_comments_as_string(content)
           end
         end
 
-        def true?(val)
-          val.present? && Constants::TRUE_RE.match?(val)
-        end
-
         # TODO: Find another implementation that doesn't depend on ActiveSupport
         def fallback(*args)
           args.compact.detect(&:present?)