From dbf19d7c82ec7e26898bccbf092d245b83951d83 Mon Sep 17 00:00:00 2001 From: Andrey Krivko Date: Mon, 18 Jul 2016 20:38:10 +0300 Subject: [PATCH 1/2] Provide :hide_default_column_types option to make hiding of default configurable --- .rubocop_todo.yml | 2 +- bin/annotate | 4 + lib/annotate.rb | 2 +- lib/annotate/annotate_models.rb | 9 +- .../templates/auto_annotate_models.rake | 73 ++++---- spec/annotate/annotate_models_spec.rb | 171 ++++++++++-------- 6 files changed, 146 insertions(+), 115 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2a2429aca..26feaffbb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -105,7 +105,7 @@ Metrics/MethodLength: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 522 + Max: 527 # Offense count: 7 Metrics/PerceivedComplexity: diff --git a/bin/annotate b/bin/annotate index ca770c2e0..bec90b041 100755 --- a/bin/annotate +++ b/bin/annotate @@ -185,6 +185,10 @@ OptionParser.new do |opts| ENV['hide_limit_column_types'] = "#{values}" end + opts.on('--hide-default-column-types VALUES', "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values| + ENV['hide_default_column_types'] = "#{values}" + end + opts.on('--ignore-unknown-models', "don't display warnings for bad model files") do |values| ENV['ignore_unknown_models'] = 'true' end diff --git a/lib/annotate.rb b/lib/annotate.rb index 247afe6e0..c001bdec5 100755 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -34,7 +34,7 @@ module Annotate ].freeze OTHER_OPTIONS = [ :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close, :wrapper, :routes, - :hide_limit_column_types, :ignore_routes, :active_admin + :hide_limit_column_types, :hide_default_column_types, :ignore_routes, :active_admin ].freeze PATH_OPTIONS = [ :require, :model_dir, :root_dir diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 1c1e9d00a..08a70c662 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -220,7 +220,7 @@ def get_schema_info(klass, header, options = {}) col_type = (col.type || col.sql_type).to_s attrs = [] - attrs << "default(#{schema_default(klass, col)})" unless col.default.nil? || NO_DEFAULT_COL_TYPES.include?(col_type) + attrs << "default(#{schema_default(klass, col)})" unless col.default.nil? || hide_default?(col_type, options) attrs << 'not null' unless col.null attrs << 'primary key' if klass.primary_key && (klass.primary_key.is_a?(Array) ? klass.primary_key.collect(&:to_sym).include?(col.name.to_sym) : col.name.to_sym == klass.primary_key.to_sym) @@ -320,6 +320,13 @@ def hide_limit?(col_type, options) excludes.include?(col_type) end + def hide_default?(col_type, options) + return false if options[:hide_default_column_types].blank? + + excludes = options[:hide_default_column_types].split(',') + excludes.include?(col_type) + end + def get_foreign_key_info(klass, options={}) if options[:format_markdown] fk_info = "#\n# ### Foreign Keys\n#\n" diff --git a/lib/generators/annotate/templates/auto_annotate_models.rake b/lib/generators/annotate/templates/auto_annotate_models.rake index fc7dd9581..7f13f26c5 100644 --- a/lib/generators/annotate/templates/auto_annotate_models.rake +++ b/lib/generators/annotate/templates/auto_annotate_models.rake @@ -6,42 +6,43 @@ if Rails.env.development? # You can override any of these by setting an environment variable of the # same name. Annotate.set_defaults( - 'routes' => 'false', - 'position_in_routes' => 'before', - 'position_in_class' => 'before', - 'position_in_test' => 'before', - 'position_in_fixture' => 'before', - 'position_in_factory' => 'before', - 'position_in_serializer' => 'before', - 'show_foreign_keys' => 'true', - 'show_indexes' => 'true', - 'simple_indexes' => 'false', - 'model_dir' => 'app/models', - 'root_dir' => '', - 'include_version' => 'false', - 'require' => '', - 'exclude_tests' => 'false', - 'exclude_fixtures' => 'false', - 'exclude_factories' => 'false', - 'exclude_serializers' => 'false', - 'exclude_scaffolds' => 'true', - 'exclude_controllers' => 'true', - 'exclude_helpers' => 'true', - 'exclude_sti_subclasses' => 'false', - 'ignore_model_sub_dir' => 'false', - 'ignore_columns' => nil, - 'ignore_routes' => nil, - 'ignore_unknown_models' => 'false', - 'hide_limit_column_types' => '<%= AnnotateModels::NO_LIMIT_COL_TYPES.join(",") %>', - 'skip_on_db_migrate' => 'false', - 'format_bare' => 'true', - 'format_rdoc' => 'false', - 'format_markdown' => 'false', - 'sort' => 'false', - 'force' => 'false', - 'trace' => 'false', - 'wrapper_open' => nil, - 'wrapper_close' => nil + 'routes' => 'false', + 'position_in_routes' => 'before', + 'position_in_class' => 'before', + 'position_in_test' => 'before', + 'position_in_fixture' => 'before', + 'position_in_factory' => 'before', + 'position_in_serializer' => 'before', + 'show_foreign_keys' => 'true', + 'show_indexes' => 'true', + 'simple_indexes' => 'false', + 'model_dir' => 'app/models', + 'root_dir' => '', + 'include_version' => 'false', + 'require' => '', + 'exclude_tests' => 'false', + 'exclude_fixtures' => 'false', + 'exclude_factories' => 'false', + 'exclude_serializers' => 'false', + 'exclude_scaffolds' => 'true', + 'exclude_controllers' => 'true', + 'exclude_helpers' => 'true', + 'exclude_sti_subclasses' => 'false', + 'ignore_model_sub_dir' => 'false', + 'ignore_columns' => nil, + 'ignore_routes' => nil, + 'ignore_unknown_models' => 'false', + 'hide_limit_column_types' => '<%= AnnotateModels::NO_LIMIT_COL_TYPES.join(",") %>', + 'hide_default_column_types' => '<%= AnnotateModels::NO_DEFAULT_COL_TYPES.join(",") %>', + 'skip_on_db_migrate' => 'false', + 'format_bare' => 'true', + 'format_rdoc' => 'false', + 'format_markdown' => 'false', + 'sort' => 'false', + 'force' => 'false', + 'trace' => 'false', + 'wrapper_open' => nil, + 'wrapper_close' => nil ) end diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index fdda19eb9..0ec556fc2 100755 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -153,27 +153,6 @@ def mock_column(name, type, options={}) EOS end - it "should ignore default value of json and hstore columns " do - klass = mock_class(:users, nil, [ - mock_column(:id, :integer), - mock_column(:profile, :json, :default => '{}'), - mock_column(:settings, :jsonb, :default => '{}'), - mock_column(:parameters, :hstore, :default => '{}'), - ]) - - expect(AnnotateModels.get_schema_info(klass, "Schema Info")).to eql(<<-EOS) -# Schema Info -# -# Table name: users -# -# id :integer not null -# profile :json not null -# settings :jsonb not null -# parameters :hstore not null -# -EOS - end - it "should get foreign key info" do klass = mock_class(:users, :id, [ mock_column(:id, :integer), @@ -271,61 +250,101 @@ def self.when_called_with(options = {}) end end - when_called_with hide_limit_column_types: '', returns: <<-EOS.strip_heredoc - # Schema Info - # - # Table name: users - # - # id :integer not null, primary key - # active :boolean not null - # name :string(50) not null - # notes :text(55) not null - # - EOS - - when_called_with hide_limit_column_types: 'integer,boolean', returns: - <<-EOS.strip_heredoc - # Schema Info - # - # Table name: users - # - # id :integer not null, primary key - # active :boolean not null - # name :string(50) not null - # notes :text(55) not null - # - EOS - - when_called_with hide_limit_column_types: 'integer,boolean,string,text', returns: - <<-EOS.strip_heredoc - # Schema Info - # - # Table name: users - # - # id :integer not null, primary key - # active :boolean not null - # name :string not null - # notes :text not null - # - EOS - - mocked_columns_without_id = [ - [:active, :boolean, { :limit => 1 }], - [:name, :string, { :limit => 50 }], - [:notes, :text, { :limit => 55 }] - ] - - when_called_with classified_sort: 'yes', with_columns: mocked_columns_without_id, returns: - <<-EOS.strip_heredoc - # Schema Info - # - # Table name: users - # - # active :boolean not null - # name :string(50) not null - # notes :text(55) not null - # - EOS + describe 'hide_limit_column_types option' do + when_called_with hide_limit_column_types: '', returns: <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # id :integer not null, primary key + # active :boolean not null + # name :string(50) not null + # notes :text(55) not null + # + EOS + + when_called_with hide_limit_column_types: 'integer,boolean', returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # id :integer not null, primary key + # active :boolean not null + # name :string(50) not null + # notes :text(55) not null + # + EOS + + when_called_with hide_limit_column_types: 'integer,boolean,string,text', returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # id :integer not null, primary key + # active :boolean not null + # name :string not null + # notes :text not null + # + EOS + end + + describe 'hide_default_column_types option' do + mocked_columns_without_id = [ + [:profile, :json, default: {}], + [:settings, :jsonb, default: {}], + [:parameters, :hstore, default: {}] + ] + + when_called_with hide_default_column_types: '', + with_columns: mocked_columns_without_id, + returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # profile :json default({}), not null + # settings :jsonb default({}), not null + # parameters :hstore default({}), not null + # + EOS + + when_called_with hide_default_column_types: 'json', + with_columns: mocked_columns_without_id, + returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # profile :json not null + # settings :jsonb default({}), not null + # parameters :hstore default({}), not null + # + EOS + end + + describe 'classified_sort option' do + mocked_columns_without_id = [ + [:active, :boolean, { :limit => 1 }], + [:name, :string, { :limit => 50 }], + [:notes, :text, { :limit => 55 }] + ] + + when_called_with classified_sort: 'yes', with_columns: mocked_columns_without_id, returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # + # active :boolean not null + # name :string(50) not null + # notes :text(55) not null + # + EOS + end end describe "#get_model_class" do From e0077659836620c1ac194967e58ffcf702451308 Mon Sep 17 00:00:00 2001 From: Andrey Krivko Date: Tue, 19 Jul 2016 11:23:36 +0300 Subject: [PATCH 2/2] Use `NO_DEFAULT_COL_TYPES` as default when the `hide_default_column_types` option is not provided --- .rubocop_todo.yml | 2 +- lib/annotate/annotate_models.rb | 8 ++++++-- spec/annotate/annotate_models_spec.rb | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 26feaffbb..75ebe75bf 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -105,7 +105,7 @@ Metrics/MethodLength: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 527 + Max: 531 # Offense count: 7 Metrics/PerceivedComplexity: diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 08a70c662..c17d1d887 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -321,9 +321,13 @@ def hide_limit?(col_type, options) end def hide_default?(col_type, options) - return false if options[:hide_default_column_types].blank? + excludes = + if options[:hide_default_column_types].blank? + NO_DEFAULT_COL_TYPES + else + options[:hide_default_column_types].split(',') + end - excludes = options[:hide_default_column_types].split(',') excludes.include?(col_type) end diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 0ec556fc2..c2e04e93e 100755 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -305,6 +305,20 @@ def self.when_called_with(options = {}) # # Table name: users # + # profile :json not null + # settings :jsonb not null + # parameters :hstore not null + # + EOS + + when_called_with hide_default_column_types: 'skip', + with_columns: mocked_columns_without_id, + returns: + <<-EOS.strip_heredoc + # Schema Info + # + # Table name: users + # # profile :json default({}), not null # settings :jsonb default({}), not null # parameters :hstore default({}), not null