Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor lib/annotate.rb #707

Merged
merged 13 commits into from
Dec 30, 2019
26 changes: 12 additions & 14 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-09-02 16:23:56 -0700 using RuboCop version 0.68.1.
# on 2019-12-24 20:33:53 -0800 using RuboCop version 0.68.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -51,7 +51,7 @@ Layout/AlignArray:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 107
# Offense count: 111
# Cop supports --auto-correct.
# Configuration parameters: EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle.
# SupportedHashRocketStyles: key, separator, table
Expand All @@ -74,7 +74,7 @@ Layout/BlockAlignment:
Exclude:
- 'lib/annotate/annotate_models.rb'

# Offense count: 45
# Offense count: 47
# Cop supports --auto-correct.
Layout/ClosingHeredocIndentation:
Exclude:
Expand All @@ -87,7 +87,7 @@ Layout/ClosingHeredocIndentation:
- 'spec/integration/standalone.rb'
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 13
# Offense count: 14
# Cop supports --auto-correct.
Layout/EmptyLineAfterGuardClause:
Exclude:
Expand Down Expand Up @@ -173,7 +173,7 @@ Layout/ExtraSpacing:
Layout/IndentFirstHashElement:
EnforcedStyle: consistent

# Offense count: 54
# Offense count: 55
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent
Expand Down Expand Up @@ -410,11 +410,11 @@ Lint/ShadowingOuterLocalVariable:
Metrics/AbcSize:
Max: 141

# Offense count: 28
# Offense count: 31
# Configuration parameters: CountComments, ExcludedMethods.
# ExcludedMethods: refine
Metrics/BlockLength:
Max: 259
Max: 268

# Offense count: 1
# Configuration parameters: CountBlocks.
Expand All @@ -439,7 +439,7 @@ Naming/AccessorMethodName:
Exclude:
- 'lib/annotate.rb'

# Offense count: 76
# Offense count: 79
# Configuration parameters: Blacklist.
# Blacklist: (?-mix:(^|\s)(EO[A-Z]{1}|END)(\s|$))
Naming/HeredocDelimiterNaming:
Expand Down Expand Up @@ -572,7 +572,7 @@ Style/FormatStringToken:
Exclude:
- 'lib/annotate/annotate_models.rb'

# Offense count: 186
# Offense count: 189
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: when_needed, always, never
Expand Down Expand Up @@ -633,14 +633,12 @@ Style/MultilineBlockChain:
- 'lib/annotate/annotate_models.rb'
- 'spec/spec_helper.rb'

# Offense count: 5
# Offense count: 3
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: literals, strict
Style/MutableConstant:
Exclude:
- 'lib/annotate.rb'
- 'lib/annotate/annotate_models.rb'
- 'lib/annotate/annotate_routes.rb'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'
- 'spec/integration/rails_2.3_with_bundler/config/environment.rb'
Expand Down Expand Up @@ -778,7 +776,7 @@ Style/StderrPuts:
- 'lib/annotate/annotate_models.rb'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'

# Offense count: 247
# Offense count: 249
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
# SupportedStyles: single_quotes, double_quotes
Expand Down Expand Up @@ -830,7 +828,7 @@ Style/UnneededPercentQ:
- 'annotate.gemspec'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'

# Offense count: 465
# Offense count: 477
# Cop supports --auto-correct.
# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Expand Down
77 changes: 11 additions & 66 deletions lib/annotate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'annotate/annotate_models'
require 'annotate/annotate_routes'
require 'annotate/constants'
require 'annotate/helpers'

begin
# ActiveSupport 3.x...
Expand All @@ -17,36 +18,6 @@
end

module Annotate
##
# The set of available options to customize the behavior of Annotate.
#
POSITION_OPTIONS = [
:position_in_routes, :position_in_class, :position_in_test,
:position_in_fixture, :position_in_factory, :position,
:position_in_serializer
].freeze
FLAG_OPTIONS = [
:show_indexes, :simple_indexes, :include_version, :exclude_tests,
:exclude_fixtures, :exclude_factories, :ignore_model_sub_dir,
:format_bare, :format_rdoc, :format_markdown, :sort, :force, :frozen,
:trace, :timestamp, :exclude_serializers, :classified_sort,
:show_foreign_keys, :show_complete_foreign_keys,
:exclude_scaffolds, :exclude_controllers, :exclude_helpers,
:exclude_sti_subclasses, :ignore_unknown_models, :with_comment
].freeze
OTHER_OPTIONS = [
:additional_file_patterns, :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close,
:wrapper, :routes, :models, :hide_limit_column_types, :hide_default_column_types,
:ignore_routes, :active_admin
].freeze
PATH_OPTIONS = [
:require, :model_dir, :root_dir
].freeze

def self.all_options
[POSITION_OPTIONS, FLAG_OPTIONS, PATH_OPTIONS, OTHER_OPTIONS]
end

##
# Set default values that can be overridden via environment variables.
#
Expand All @@ -56,7 +27,7 @@ def self.set_defaults(options = {})

options = ActiveSupport::HashWithIndifferentAccess.new(options)

all_options.flatten.each do |key|
Constants::ALL_ANNOTATE_OPTIONS.flatten.each do |key|
if options.key?(key)
default_value = if options[key].is_a?(Array)
options[key].join(',')
Expand All @@ -74,16 +45,16 @@ def self.set_defaults(options = {})
# TODO: what is the difference between this and set_defaults?
#
def self.setup_options(options = {})
POSITION_OPTIONS.each do |key|
options[key] = fallback(ENV[key.to_s], ENV['position'], 'before')
Constants::POSITION_OPTIONS.each do |key|
options[key] = Annotate::Helpers.fallback(ENV[key.to_s], ENV['position'], 'before')
end
FLAG_OPTIONS.each do |key|
options[key] = true?(ENV[key.to_s])
Constants::FLAG_OPTIONS.each do |key|
options[key] = Annotate::Helpers.true?(ENV[key.to_s])
end
OTHER_OPTIONS.each do |key|
Constants::OTHER_OPTIONS.each do |key|
options[key] = !ENV[key.to_s].blank? ? ENV[key.to_s] : nil
end
PATH_OPTIONS.each do |key|
Constants::PATH_OPTIONS.each do |key|
options[key] = !ENV[key.to_s].blank? ? ENV[key.to_s].split(',') : []
end

Expand All @@ -94,29 +65,13 @@ def self.setup_options(options = {})
options[:wrapper_close] ||= options[:wrapper]

# These were added in 2.7.0 but so this is to revert to old behavior by default
options[:exclude_scaffolds] = Annotate.true?(ENV.fetch('exclude_scaffolds', 'true'))
options[:exclude_controllers] = Annotate.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV.fetch('exclude_scaffolds', 'true'))
options[:exclude_controllers] = Annotate::Helpers.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate::Helpers.true?(ENV.fetch('exclude_helpers', 'true'))

options
end

def self.reset_options
all_options.flatten.each { |key| ENV[key.to_s] = nil }
end

def self.skip_on_migration?
ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ Constants::TRUE_RE || ENV['skip_on_db_migrate'] =~ Constants::TRUE_RE
end

def self.include_routes?
ENV['routes'] =~ Constants::TRUE_RE
end

def self.include_models?
ENV['models'] =~ Constants::TRUE_RE
end

def self.loaded_tasks=(val)
@loaded_tasks = val
end
Expand Down Expand Up @@ -191,14 +146,4 @@ def self.bootstrap_rake
load_tasks
Rake::Task[:set_annotation_options].invoke
end

def self.fallback(*args)
args.detect { |arg| !arg.blank? }
end

def self.true?(val)
return false if val.blank?
return false unless val =~ Constants::TRUE_RE
true
end
end
33 changes: 33 additions & 0 deletions lib/annotate/constants.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
module Annotate
module Constants
TRUE_RE = /^(true|t|yes|y|1)$/i.freeze

##
# The set of available options to customize the behavior of Annotate.
#
POSITION_OPTIONS = [
:position_in_routes, :position_in_class, :position_in_test,
:position_in_fixture, :position_in_factory, :position,
:position_in_serializer
].freeze

FLAG_OPTIONS = [
:show_indexes, :simple_indexes, :include_version, :exclude_tests,
:exclude_fixtures, :exclude_factories, :ignore_model_sub_dir,
:format_bare, :format_rdoc, :format_markdown, :sort, :force, :frozen,
:trace, :timestamp, :exclude_serializers, :classified_sort,
:show_foreign_keys, :show_complete_foreign_keys,
:exclude_scaffolds, :exclude_controllers, :exclude_helpers,
:exclude_sti_subclasses, :ignore_unknown_models, :with_comment
].freeze

OTHER_OPTIONS = [
:additional_file_patterns, :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close,
:wrapper, :routes, :models, :hide_limit_column_types, :hide_default_column_types,
:ignore_routes, :active_admin
].freeze

PATH_OPTIONS = [
:require, :model_dir, :root_dir
].freeze

ALL_ANNOTATE_OPTIONS = [
POSITION_OPTIONS, FLAG_OPTIONS, OTHER_OPTIONS, PATH_OPTIONS
].freeze
end
end
33 changes: 33 additions & 0 deletions lib/annotate/helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
module Annotate
# Class for holding helper methods. Done to make lib/annotate.rb less bloated.
class Helpers
class << self
def skip_on_migration?
ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ Constants::TRUE_RE || ENV['skip_on_db_migrate'] =~ Constants::TRUE_RE
end

def include_routes?
ENV['routes'] =~ Constants::TRUE_RE
end

def include_models?
ENV['models'] =~ Constants::TRUE_RE
end

def true?(val)
return false if val.blank?
return false unless val =~ Constants::TRUE_RE

true
end

def fallback(*args)
args.detect { |arg| !arg.blank? }
end

def reset_options(options)
options.flatten.each { |key| ENV[key.to_s] = nil }
end
end
end
end
70 changes: 35 additions & 35 deletions lib/tasks/annotate_models.rake
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,46 @@ task annotate_models: :environment do
require "#{annotate_lib}/annotate/active_record_patch"

options = {is_rake: true}
ENV['position'] = options[:position] = Annotate.fallback(ENV['position'], 'before')
ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV['position'], 'before')
options[:additional_file_patterns] = ENV['additional_file_patterns'] ? ENV['additional_file_patterns'].split(',') : []
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be mixed in instead of specifying full path? It would also cut back the number of changes needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctran this is done intentionally for now to make it easier to see where everything is coming from. Do you think it would be okay to keep for now?

I'm thinking we could untangle lib/annotate.rb. I'm looking at the Pry gem as a source of inspiration.

Copy link
Owner

@ctran ctran Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine either way, was just wondering if you had specific reasons to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it more explicit where it's coming from right now. I'm expecting Helpers to be a temporary solution until it's clearer what the different pieces are.

options[:position_in_class] = Annotate.fallback(ENV['position_in_class'], ENV['position'])
options[:position_in_fixture] = Annotate.fallback(ENV['position_in_fixture'], ENV['position'])
options[:position_in_factory] = Annotate.fallback(ENV['position_in_factory'], ENV['position'])
options[:position_in_test] = Annotate.fallback(ENV['position_in_test'], ENV['position'])
options[:position_in_serializer] = Annotate.fallback(ENV['position_in_serializer'], ENV['position'])
options[:show_foreign_keys] = Annotate.true?(ENV['show_foreign_keys'])
options[:show_complete_foreign_keys] = Annotate.true?(ENV['show_complete_foreign_keys'])
options[:show_indexes] = Annotate.true?(ENV['show_indexes'])
options[:simple_indexes] = Annotate.true?(ENV['simple_indexes'])
options[:position_in_class] = Annotate::Helpers.fallback(ENV['position_in_class'], ENV['position'])
options[:position_in_fixture] = Annotate::Helpers.fallback(ENV['position_in_fixture'], ENV['position'])
options[:position_in_factory] = Annotate::Helpers.fallback(ENV['position_in_factory'], ENV['position'])
options[:position_in_test] = Annotate::Helpers.fallback(ENV['position_in_test'], ENV['position'])
options[:position_in_serializer] = Annotate::Helpers.fallback(ENV['position_in_serializer'], ENV['position'])
options[:show_foreign_keys] = Annotate::Helpers.true?(ENV['show_foreign_keys'])
options[:show_complete_foreign_keys] = Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])
options[:show_indexes] = Annotate::Helpers.true?(ENV['show_indexes'])
options[:simple_indexes] = Annotate::Helpers.true?(ENV['simple_indexes'])
options[:model_dir] = ENV['model_dir'] ? ENV['model_dir'].split(',') : ['app/models']
options[:root_dir] = ENV['root_dir']
options[:include_version] = Annotate.true?(ENV['include_version'])
options[:include_version] = Annotate::Helpers.true?(ENV['include_version'])
options[:require] = ENV['require'] ? ENV['require'].split(',') : []
options[:exclude_tests] = Annotate.true?(ENV['exclude_tests'])
options[:exclude_factories] = Annotate.true?(ENV['exclude_factories'])
options[:exclude_fixtures] = Annotate.true?(ENV['exclude_fixtures'])
options[:exclude_serializers] = Annotate.true?(ENV['exclude_serializers'])
options[:exclude_scaffolds] = Annotate.true?(ENV['exclude_scaffolds'])
options[:exclude_controllers] = Annotate.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_sti_subclasses] = Annotate.true?(ENV['exclude_sti_subclasses'])
options[:ignore_model_sub_dir] = Annotate.true?(ENV['ignore_model_sub_dir'])
options[:format_bare] = Annotate.true?(ENV['format_bare'])
options[:format_rdoc] = Annotate.true?(ENV['format_rdoc'])
options[:format_markdown] = Annotate.true?(ENV['format_markdown'])
options[:sort] = Annotate.true?(ENV['sort'])
options[:force] = Annotate.true?(ENV['force'])
options[:frozen] = Annotate.true?(ENV['frozen'])
options[:classified_sort] = Annotate.true?(ENV['classified_sort'])
options[:trace] = Annotate.true?(ENV['trace'])
options[:wrapper_open] = Annotate.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate.fallback(ENV['wrapper_close'], ENV['wrapper'])
options[:exclude_tests] = Annotate::Helpers.true?(ENV['exclude_tests'])
options[:exclude_factories] = Annotate::Helpers.true?(ENV['exclude_factories'])
options[:exclude_fixtures] = Annotate::Helpers.true?(ENV['exclude_fixtures'])
options[:exclude_serializers] = Annotate::Helpers.true?(ENV['exclude_serializers'])
options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV['exclude_scaffolds'])
options[:exclude_controllers] = Annotate::Helpers.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate::Helpers.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_sti_subclasses] = Annotate::Helpers.true?(ENV['exclude_sti_subclasses'])
options[:ignore_model_sub_dir] = Annotate::Helpers.true?(ENV['ignore_model_sub_dir'])
options[:format_bare] = Annotate::Helpers.true?(ENV['format_bare'])
options[:format_rdoc] = Annotate::Helpers.true?(ENV['format_rdoc'])
options[:format_markdown] = Annotate::Helpers.true?(ENV['format_markdown'])
options[:sort] = Annotate::Helpers.true?(ENV['sort'])
options[:force] = Annotate::Helpers.true?(ENV['force'])
options[:frozen] = Annotate::Helpers.true?(ENV['frozen'])
options[:classified_sort] = Annotate::Helpers.true?(ENV['classified_sort'])
options[:trace] = Annotate::Helpers.true?(ENV['trace'])
options[:wrapper_open] = Annotate::Helpers.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate::Helpers.fallback(ENV['wrapper_close'], ENV['wrapper'])
options[:ignore_columns] = ENV.fetch('ignore_columns', nil)
options[:ignore_routes] = ENV.fetch('ignore_routes', nil)
options[:hide_limit_column_types] = Annotate.fallback(ENV['hide_limit_column_types'], '')
options[:hide_default_column_types] = Annotate.fallback(ENV['hide_default_column_types'], '')
options[:with_comment] = Annotate.true?(ENV['with_comment'])
options[:ignore_unknown_models] = Annotate.true?(ENV.fetch('ignore_unknown_models', 'false'))
options[:hide_limit_column_types] = Annotate::Helpers.fallback(ENV['hide_limit_column_types'], '')
options[:hide_default_column_types] = Annotate::Helpers.fallback(ENV['hide_default_column_types'], '')
options[:with_comment] = Annotate::Helpers.true?(ENV['with_comment'])
options[:ignore_unknown_models] = Annotate::Helpers.true?(ENV.fetch('ignore_unknown_models', 'false'))

AnnotateModels.do_annotations(options)
end
Expand All @@ -64,6 +64,6 @@ task remove_annotation: :environment do
options[:model_dir] = ENV['model_dir']
options[:root_dir] = ENV['root_dir']
options[:require] = ENV['require'] ? ENV['require'].split(',') : []
options[:trace] = Annotate.true?(ENV['trace'])
options[:trace] = Annotate::Helpers.true?(ENV['trace'])
AnnotateModels.remove_annotations(options)
end
Loading