chore: ensure Options are initialised with defaults

lib/annotate_rb/model_annotator/model_wrapper.rb

@@ -6,6 +6,8 @@ class ModelWrapper
       # Should be the wrapper for an ActiveRecord model that serves as the source of truth of the model
       # of the model that we're annotating
 
+      DEFAULT_TIMESTAMP_COLUMNS = %w[created_at updated_at]
+
       def initialize(klass, options)
         @klass = klass
         @options = options
@@ -143,18 +145,24 @@ def classified_sort(cols)
         associations = []
         id = nil
 
+        # specs don't load defaults, so ensure we have defaults here
+        timestamp_columns = @options[:timestamp_columns] || DEFAULT_TIMESTAMP_COLUMNS
+
         cols.each do |c|
           if c.name.eql?("id")
             id = c
-          elsif c.name.eql?("created_at") || c.name.eql?("updated_at")
+          elsif timestamp_columns.include?(c.name)
             timestamps << c
           elsif c.name[-3, 3].eql?("_id")
             associations << c
           else
             rest_cols << c
           end
         end
-        [rest_cols, timestamps, associations].each { |a| a.sort_by!(&:name) }
+
+        timestamp_order = timestamp_columns.each_with_index.to_h
+        timestamps.sort_by! { |col| timestamp_order[col.name] }
+        [rest_cols, associations].each { |a| a.sort_by!(&:name) }
 
         ([id] << rest_cols << timestamps << associations).flatten.compact
       end

lib/annotate_rb/options.rb

@@ -10,7 +10,7 @@ class Options
 
     class << self
       def from(options = {}, state = {})
-        new(options, state).load_defaults
+        new(options, state)
       end
     end
 
@@ -36,7 +36,6 @@ def from(options = {}, state = {})
       exclude_sti_subclasses: false, # ModelAnnotator
       exclude_tests: false, # ModelAnnotator
       force: false, # ModelAnnotator, but should be used by both
-      format_bare: true, # Unused
       format_markdown: false, # ModelAnnotator, RouteAnnotator
       format_rdoc: false, # ModelAnnotator
       format_yard: false, # ModelAnnotator
@@ -68,6 +67,9 @@ def from(options = {}, state = {})
       # ModelAnnotator
       hide_limit_column_types: "",
 
+      # ModelAnnotator
+      timestamp_columns: ModelAnnotator::ModelWrapper::DEFAULT_TIMESTAMP_COLUMNS,
+
       ignore_columns: nil, # ModelAnnotator
       ignore_routes: nil, # RouteAnnotator
       ignore_unknown_models: false, # ModelAnnotator
@@ -92,102 +94,61 @@ def from(options = {}, state = {})
 
     DEFAULT_OPTIONS = {}.merge(POSITION_OPTIONS, FLAG_OPTIONS, OTHER_OPTIONS, PATH_OPTIONS).freeze
 
-    FLAG_OPTION_KEYS = [
-      :classified_sort,
-      :exclude_controllers,
-      :exclude_factories,
-      :exclude_fixtures,
-      :exclude_helpers,
-      :exclude_scaffolds,
-      :exclude_serializers,
-      :exclude_sti_subclasses,
-      :exclude_tests,
-      :force,
-      :format_markdown,
-      :format_rdoc,
-      :format_yard,
-      :frozen,
-      :ignore_model_sub_dir,
-      :ignore_unknown_models,
-      :include_version,
-      :show_check_constraints,
-      :show_complete_foreign_keys,
-      :show_foreign_keys,
-      :show_indexes,
-      :simple_indexes,
-      :sort,
-      :timestamp,
-      :trace,
-      :with_comment,
-      :with_column_comments,
-      :with_table_comments
-    ].freeze
-
-    OTHER_OPTION_KEYS = [
-      :active_admin,
-      :command,
-      :debug,
-      :hide_default_column_types,
-      :hide_limit_column_types,
-      :ignore_columns,
-      :ignore_routes,
-      :ignore_unknown_models,
-      :models,
-      :routes,
-      :skip_on_db_migrate,
-      :target_action,
-      :wrapper,
-      :wrapper_close,
-      :wrapper_open,
-      :classes_default_to_s
-    ].freeze
-
-    PATH_OPTION_KEYS = [
-      :additional_file_patterns,
-      :model_dir,
-      :require,
-      :root_dir
-    ].freeze
-
-    ALL_OPTION_KEYS = [
-      POSITION_OPTIONS.keys, FLAG_OPTION_KEYS, OTHER_OPTION_KEYS, PATH_OPTION_KEYS
-    ].flatten.freeze
-
     POSITION_DEFAULT = "before"
 
     # Want this to be read only after initializing
-    def_delegator :@options, :[]
+    def_delegators :@options, :[], :to_h
 
     def initialize(options = {}, state = {})
       @options = options
 
       # For now, state is a hash to store state that we need but is not a configuration option
       @state = state
 
-      symbolize_exclude_tests
+      load_defaults
+      @options.freeze
+    end
+
+    def set_state(key, value, overwrite = false)
+      if @state.key?(key) && !overwrite
+        val = @state[key]
+        raise ArgumentError, "Attempting to write '#{value}' to state with key '#{key}', but it already exists with '#{val}'."
+      end
+
+      @state[key] = value
     end
 
-    def to_h
-      @options.to_h
+    def get_state(key)
+      @state[key]
     end
 
+    def print
+      # TODO: prints options and state
+    end
+
+    private
+
     def load_defaults
-      ALL_OPTION_KEYS.each do |key|
-        @options[key] = DEFAULT_OPTIONS[key] unless @options.key?(key)
+      @options = DEFAULT_OPTIONS.merge(@options)
+
+      # `:exclude_tests` option is being expanded to function as a boolean OR an array of symbols
+      # https://github.com/drwl/annotaterb/issues/103
+      if @options[:exclude_tests].is_a?(Array)
+        @options[:exclude_tests].map! { |item| item.to_s.strip.to_sym }
       end
 
       # Set all of the position options in the following order:
       # 1) Use the value if it's defined
       # 2) Use value from :position if it's defined
       # 3) Use default
-      POSITION_OPTIONS.keys.each do |key|
+      POSITION_OPTIONS.each_key do |key|
         @options[key] = Helper.fallback(
           @options[key], @options[:position], POSITION_DEFAULT
         )
       end
 
       # Unpack path options if we're passed in a String
-      PATH_OPTION_KEYS.each do |key|
+      PATH_OPTIONS.each_key do |key|
         if @options[key].is_a?(String)
           @options[key] = @options[key].split(",").map(&:strip).reject(&:empty?)
         end
@@ -200,41 +161,6 @@ def load_defaults
       # Set column and table comments to default to :with_comment, if not set
       @options[:with_column_comments] = @options[:with_comment] if @options[:with_column_comments].nil?
       @options[:with_table_comments] = @options[:with_comment] if @options[:with_table_comments].nil?
-
-      self
-    end
-
-    def set_state(key, value, overwrite = false)
-      if @state.key?(key) && !overwrite
-        val = @state[key]
-        raise ArgumentError, "Attempting to write '#{value}' to state with key '#{key}', but it already exists with '#{val}'."
-      end
-
-      @state[key] = value
-    end
-
-    def get_state(key)
-      @state[key]
-    end
-
-    def print
-      # TODO: prints options and state
-    end
-
-    private
-
-    # Guard against user inputting strings instead of symbols
-    def symbolize_exclude_tests
-      # `:exclude_tests` option is being expanded to function as a boolean OR an array of symbols
-      # https://github.com/drwl/annotaterb/issues/103
-
-      if @options[:exclude_tests].is_a?(Array)
-        @options[:exclude_tests].map! do |item|
-          item = item.strip if item.respond_to?(:strip)
-
-          item.to_sym
-        end
-      end
     end
   end
 end

spec/lib/annotate_rb/annotate_models/annotate_models_annotating_a_file_spec.rb

@@ -22,7 +22,7 @@ class User < ActiveRecord::Base
     end
 
     it "works with namespaced models (i.e. models inside modules/subdirectories)" do
-      (model_file_name, file_content) = write_model "foo/user.rb", <<~EOS
+      (model_file_name, file_content) = write_model(File.join("foo", "user.rb"), <<~EOS)
         class Foo::User < ActiveRecord::Base
         end
       EOS
@@ -42,58 +42,61 @@ class Foo::User < ActiveRecord::Base
       expect(File.read(model_file_name)).to eq("#{schema_info}#{file_content}")
     end
 
-    describe "if a file can't be annotated" do
-      before do
-        allow(AnnotateRb::ModelAnnotator::ModelClassGetter).to receive(:get_loaded_model_by_path).with("user").and_return(nil)
+    context "when an error occurs" do
+      let(:basename) { "user" }
+      let(:error_message) { a_string_including("Unable to process #{File.join(@model_dir, "#{basename}.rb")}: oops") }
+      let(:stacktrace) { a_string_including(%r{(^.*:\d+:in [`'].*'$\n?)+}m) }
 
-        write_model("user.rb", <<~EOS)
-          class User < ActiveRecord::Base
-            raise "oops"
-          end
-        EOS
-      end
+      describe "while annotating" do
+        before do
+          allow(AnnotateRb::ModelAnnotator::ModelClassGetter).to receive(:get_loaded_model_by_path).with(basename).and_return(nil)
 
-      it "displays just the error message with trace disabled (default)" do
-        options = AnnotateRb::Options.from({model_dir: @model_dir}, {working_args: []})
+          write_model("#{basename}.rb", <<~EOS)
+            class #{basename.classify} < ActiveRecord::Base
+              raise "oops"
+            end
+          EOS
+        end
 
-        expect { described_class.do_annotations(options) }.to output(a_string_including("Unable to process #{@model_dir}/user.rb: oops")).to_stderr
+        it "displays just the error message with trace disabled (default)" do
+          options = AnnotateRb::Options.from({model_dir: @model_dir}, {working_args: []})
 
-        # TODO: Find another way of testing trace without hardcoding the file name as part of the spec
-        # expect { described_class.do_annotations(options) }.not_to output(a_string_including('/spec/annotate/annotate_models_spec.rb:')).to_stderr
-      end
+          expect { described_class.do_annotations(options) }.to output(error_message).to_stderr
+          expect { described_class.do_annotations(options) }.not_to output(stacktrace).to_stderr
+        end
 
-      it "displays the error message and stacktrace with trace enabled" do
-        options = AnnotateRb::Options.from({model_dir: @model_dir, trace: true}, {working_args: []})
-        expect { described_class.do_annotations(options) }.to output(a_string_including("Unable to process #{@model_dir}/user.rb: oops")).to_stderr
+        it "displays the error message and stacktrace with trace enabled" do
+          options = AnnotateRb::Options.from({model_dir: @model_dir, trace: true}, {working_args: []})
 
-        # TODO: Find another way of testing trace without hardcoding the file name as part of the spec
-        # expect { described_class.do_annotations(options) }.to output(a_string_including('/spec/lib/annotate/annotate_models_spec.rb:')).to_stderr
+          expect { described_class.do_annotations(options) }.to output(error_message).to_stderr
+          expect { described_class.do_annotations(options) }.to output(stacktrace).to_stderr
+        end
       end
-    end
 
-    describe "if a file can't be deannotated" do
-      before do
-        allow(AnnotateRb::ModelAnnotator::ModelClassGetter).to receive(:get_loaded_model_by_path).with("user").and_return(nil)
+      describe "while removing annotations" do
+        before do
+          allow(AnnotateRb::ModelAnnotator::ModelClassGetter).to receive(:get_loaded_model_by_path).with(basename).and_return(nil)
 
-        write_model("user.rb", <<~EOS)
-          class User < ActiveRecord::Base
-            raise "oops"
-          end
-        EOS
-      end
+          write_model("#{basename}.rb", <<~EOS)
+            class #{basename.classify} < ActiveRecord::Base
+              raise "oops"
+            end
+          EOS
+        end
 
-      it "displays just the error message with trace disabled (default)" do
-        options = AnnotateRb::Options.from({model_dir: @model_dir}, {working_args: []})
+        it "displays just the error message with trace disabled (default)" do
+          options = AnnotateRb::Options.from({model_dir: @model_dir}, {working_args: []})
 
-        expect { described_class.remove_annotations(options) }.to output(a_string_including("Unable to process #{@model_dir}/user.rb: oops")).to_stderr
-        expect { described_class.remove_annotations(options) }.not_to output(a_string_including("/user.rb:2:in `<class:User>'")).to_stderr
-      end
+          expect { described_class.remove_annotations(options) }.to output(error_message).to_stderr
+          expect { described_class.remove_annotations(options) }.not_to output(stacktrace).to_stderr
+        end
 
-      it "displays the error message and stacktrace with trace enabled" do
-        options = AnnotateRb::Options.from({model_dir: @model_dir, trace: true}, {working_args: []})
+        it "displays the error message and stacktrace with trace enabled" do
+          options = AnnotateRb::Options.from({model_dir: @model_dir, trace: true}, {working_args: []})
 
-        expect { described_class.remove_annotations(options) }.to output(a_string_including("Unable to process #{@model_dir}/user.rb: oops")).to_stderr
-        expect { described_class.remove_annotations(options) }.to output(a_string_including("/user.rb:2:in `<class:User>'")).to_stderr
+          expect { described_class.remove_annotations(options) }.to output(error_message).to_stderr
+          expect { described_class.remove_annotations(options) }.to output(stacktrace).to_stderr
+        end
       end
     end
   end