From 9f3e2b73d4683900e7ec8b69b682fff748f614e8 Mon Sep 17 00:00:00 2001 From: Cuong Tran Date: Thu, 1 Jun 2017 18:56:19 -0700 Subject: [PATCH 01/37] Remove "private" since it's not valid in context (#471) --- lib/annotate/annotate_routes.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index c1b0a9628..7f8435ee2 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -76,8 +76,6 @@ def remove_annotations(options={}) end end - private - def self.app_routes_map(options) routes_map = `rake routes`.split(/\n/, -1) From c24f64c3f52f24088b20e4c11b4f082f6297587f Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 2 Jun 2017 14:52:13 -0700 Subject: [PATCH 02/37] Allow use of the fallback loading solution (#473) LoadError does not descend from `StandardError` so the fallback loading is never invoked. ``` >> LoadError.ancestors => [LoadError, ScriptError, Exception, Object, Kernel, BasicObject] ``` --- lib/annotate/annotate_models.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 1c9d2f950..f54e91f4b 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -613,7 +613,7 @@ def get_model_class(file) # Retrieve loaded model class by path to the file where it's supposed to be defined. def get_loaded_model(model_path) ActiveSupport::Inflector.constantize(ActiveSupport::Inflector.camelize(model_path)) - rescue + rescue StandardError, LoadError # Revert to the old way but it is not really robust ObjectSpace.each_object(::Class) .select do |c| From 22d796c45504ac6530f18803994fed1a8275cd88 Mon Sep 17 00:00:00 2001 From: Steve Klebanoff Date: Mon, 5 Jun 2017 17:48:39 -0700 Subject: [PATCH 03/37] Fix show_complete_foreign_keys setting (#475) * add show_complete_foreign_keys as setting * test for show_complete_foreign_keys setting --- lib/annotate.rb | 3 ++- spec/annotate/annotate_models_spec.rb | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/annotate.rb b/lib/annotate.rb index a17b14215..9f4daf4c2 100644 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -30,7 +30,8 @@ module Annotate :show_indexes, :simple_indexes, :include_version, :exclude_tests, :exclude_fixtures, :exclude_factories, :ignore_model_sub_dir, :format_bare, :format_rdoc, :format_markdown, :sort, :force, :trace, - :timestamp, :exclude_serializers, :classified_sort, :show_foreign_keys, + :timestamp, :exclude_serializers, :classified_sort, + :show_foreign_keys, :show_complete_foreign_keys, :exclude_scaffolds, :exclude_controllers, :exclude_helpers, :exclude_sti_subclasses, :ignore_unknown_models ].freeze diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index ccbc53a46..0ef008fef 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -484,6 +484,21 @@ def mock_column(name, type, options = {}) EOS end + describe '#set_defaults' do + it 'should default show_complete_foreign_keys to false' do + expect(Annotate.true?(ENV['show_complete_foreign_keys'])).to be(false) + end + + it 'should be able to set show_complete_foreign_keys to true' do + Annotate.set_defaults('show_complete_foreign_keys' => 'true') + expect(Annotate.true?(ENV['show_complete_foreign_keys'])).to be(true) + end + + after :each do + ENV.delete('show_complete_foreign_keys') + end + end + describe '#get_schema_info with custom options' do def self.when_called_with(options = {}) expected = options.delete(:returns) From 134b833ec90830b95628defcc162536d3670b9fa Mon Sep 17 00:00:00 2001 From: Daniel Hanke Date: Fri, 9 Jun 2017 05:07:01 +0200 Subject: [PATCH 04/37] Add missing complete-foreign-keys option (introduced in #476) to bin (#477) --- .rubocop_todo.yml | 2 +- README.rdoc | 2 ++ bin/annotate | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 03801c2e1..00104bc6f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -108,7 +108,7 @@ Metrics/AbcSize: # Offense count: 3 # Configuration parameters: CountComments. Metrics/BlockLength: - Max: 134 + Max: 140 # Offense count: 2 Metrics/BlockNesting: diff --git a/README.rdoc b/README.rdoc index efd634586..be0a043b9 100644 --- a/README.rdoc +++ b/README.rdoc @@ -184,6 +184,8 @@ you can do so with a simple environment variable, instead of editing the -m, --show-migration Include the migration version number in the annotation -i, --show-indexes List the table's database indexes in the annotation -k, --show-foreign-keys List the table's foreign key constraints in the annotation + --ck, --complete-foreign-keys + Complete foreign key names in the annotation -s, --simple-indexes Concat the column's related indexes in the annotation --model-dir dir Annotate model files stored in dir rather than app/models, separate multiple dirs with commas --ignore-model-subdirects Ignore subdirectories of the models directory diff --git a/bin/annotate b/bin/annotate index a5acf0818..6b45dd9d9 100755 --- a/bin/annotate +++ b/bin/annotate @@ -107,6 +107,12 @@ OptionParser.new do |opts| ENV['show_foreign_keys'] = 'yes' end + opts.on('--ck', + '--complete-foreign-keys', 'Complete foreign key names in the annotation') do + ENV['show_foreign_keys'] = 'yes' + ENV['show_complete_foreign_keys'] = 'yes' + end + opts.on('-i', '--show-indexes', "List the table's database indexes in the annotation") do ENV['show_indexes'] = 'yes' From 8e75c911217449ce95a95007eb0aaf7d8ba0eed9 Mon Sep 17 00:00:00 2001 From: yskkin Date: Sun, 2 Jul 2017 09:03:26 +0900 Subject: [PATCH 05/37] Drop support of Ruby < 2.2 (#481) * Drop support of Ruby < 2.2 --- .travis.yml | 4 ---- Gemfile | 2 ++ annotate.gemspec | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index c72f5d690..0aad6403a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,6 @@ sudo: false language: ruby rvm: - - 1.9.3 - - 2.0 - - 2.1 - 2.2.5 - 2.3.0 - 2.4.0 @@ -11,7 +8,6 @@ rvm: matrix: allow_failures: - rvm: ruby-head - - rvm: 1.9.3 before_install: - gem update --system - gem update bundler diff --git a/Gemfile b/Gemfile index f30abba0d..f6e85c21e 100644 --- a/Gemfile +++ b/Gemfile @@ -1,5 +1,7 @@ source 'https://rubygems.org' +ruby '>= 2.2.0' + gem 'activerecord', '>= 4.2.5', require: false gem 'rake', require: false diff --git a/annotate.gemspec b/annotate.gemspec index e46a4ff00..97e325eb2 100644 --- a/annotate.gemspec +++ b/annotate.gemspec @@ -7,7 +7,7 @@ Gem::Specification.new do |s| s.name = 'annotate' s.version = Annotate.version - s.required_ruby_version = '>= 1.9.3' + s.required_ruby_version = '>= 2.2.0' s.required_rubygems_version = Gem::Requirement.new('>= 0') if s.respond_to? :required_rubygems_version= s.authors = ['Alex Chaffee', 'Cuong Tran', 'Marcos Piccinini', 'Turadg Aleahmad', 'Jon Frisby'] s.description = 'Annotates Rails/ActiveRecord Models, routes, fixtures, and others based on the database schema.' From 5803fbaa46de130b6439b1389c0c30cd46d55224 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Sun, 2 Jul 2017 06:27:19 +0600 Subject: [PATCH 06/37] Annotate ordered indexes (#479) --- lib/annotate/annotate_models.rb | 22 +++++++++- spec/annotate/annotate_models_spec.rb | 58 +++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index f54e91f4b..e892e5650 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -337,15 +337,33 @@ def get_index_info(klass, options = {}) max_size = indexes.collect{|index| index.name.size}.max + 1 indexes.sort_by(&:name).each do |index| index_info << if options[:format_markdown] - sprintf("# * `%s`%s:\n# * **`%s`**\n", index.name, index.unique ? " (_unique_)" : "", Array(index.columns).join("`**\n# * **`")) + final_index_string_in_markdown(index) else - sprintf("# %-#{max_size}.#{max_size}s %s %s", index.name, "(#{Array(index.columns).join(",")})", index.unique ? "UNIQUE" : "").rstrip + "\n" + final_index_string(index, max_size) end end index_info end + def index_columns_info(index) + Array(index.columns).map do |col| + if index.try(:orders) && index.orders[col.to_s] + "#{col} #{index.orders[col.to_s].upcase}" + else + col.to_s + end + end + end + + def final_index_string_in_markdown(index) + sprintf("# * `%s`%s:\n# * **`%s`**\n", index.name, index.unique ? " (_unique_)" : "", index_columns_info(index).join("`**\n# * **`")) + end + + def final_index_string(index, max_size) + sprintf("# %-#{max_size}.#{max_size}s %s %s", index.name, "(#{index_columns_info(index).join(',')})", index.unique ? "UNIQUE" : "").rstrip + "\n" + end + def hide_limit?(col_type, options) excludes = if options[:hide_limit_column_types].blank? diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 0ef008fef..25ed081fc 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -5,11 +5,12 @@ require 'active_support/core_ext/string' describe AnnotateModels do - def mock_index(name, columns = [], unique = false) + def mock_index(name, columns = [], orders = {}, unique = false) double('IndexKeyDefinition', name: name, columns: columns, - unique: unique) + unique: unique, + orders: orders) end def mock_foreign_key(name, from_column, to_table, to_column = 'id', constraints = {}) @@ -320,14 +321,52 @@ def mock_column(name, type, options = {}) EOS end + it 'should get ordered indexes keys' do + klass = mock_class(:users, + :id, + [ + mock_column("id", :integer), + mock_column("firstname", :string), + mock_column("surname", :string), + mock_column("value", :string) + ], + [ + mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b8', + %w(firstname surname value), + 'surname' => :asc, 'value' => :desc) + ]) + expect(AnnotateModels.get_schema_info(klass, 'Schema Info', show_indexes: true)).to eql(<<-EOS) +# Schema Info +# +# Table name: users +# +# id :integer not null, primary key +# firstname :string not null +# surname :string not null +# value :string not null +# +# Indexes +# +# index_rails_02e851e3b7 (id) +# index_rails_02e851e3b8 (firstname,surname ASC,value DESC) +# +EOS + end + it 'should get simple indexes keys' do klass = mock_class(:users, :id, [ mock_column(:id, :integer), mock_column(:foreign_thing_id, :integer) - ], [mock_index('index_rails_02e851e3b7', ['id']), - mock_index('index_rails_02e851e3b8', ['foreign_thing_id'])]) + ], + [ + mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b8', + ['foreign_thing_id'], + 'foreign_thing_id' => :desc) + ]) expect(AnnotateModels.get_schema_info(klass, 'Schema Info', simple_indexes: true)).to eql(<<-EOS) # Schema Info # @@ -460,8 +499,13 @@ def mock_column(name, type, options = {}) [ mock_column(:id, :integer), mock_column(:name, :string, limit: 50) - ], [mock_index('index_rails_02e851e3b7', ['id']), - mock_index('index_rails_02e851e3b8', ['foreign_thing_id'])]) + ], + [ + mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b8', + ['foreign_thing_id'], + 'foreign_thing_id' => :desc) + ]) expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) # #{AnnotateModels::PREFIX} # @@ -479,7 +523,7 @@ def mock_column(name, type, options = {}) # * `index_rails_02e851e3b7`: # * **`id`** # * `index_rails_02e851e3b8`: -# * **`foreign_thing_id`** +# * **`foreign_thing_id DESC`** # EOS end From 6775e4a16d853c3484f7753198220cbeb99bba8d Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Tue, 4 Jul 2017 01:15:53 +0600 Subject: [PATCH 07/37] Indexes: add WHERE and USING (#482) --- lib/annotate/annotate_models.rb | 61 ++++++- spec/annotate/annotate_models_spec.rb | 242 ++++++++++++++++++++++++-- 2 files changed, 284 insertions(+), 19 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index e892e5650..f922b87d7 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -65,6 +65,21 @@ module AnnotateModels # Don't show default value for these column types NO_DEFAULT_COL_TYPES = %w(json jsonb hstore).freeze + INDEX_CLAUSES = { + unique: { + default: 'UNIQUE', + markdown: '_unique_' + }, + where: { + default: 'WHERE', + markdown: '_where_' + }, + using: { + default: 'USING', + markdown: '_using_' + } + }.freeze + class << self def annotate_pattern(options = {}) if options[:wrapper_open] @@ -356,12 +371,54 @@ def index_columns_info(index) end end + def index_unique_info(index, format = :default) + index.unique ? " #{INDEX_CLAUSES[:unique][format]}" : '' + end + + def index_where_info(index, format = :default) + value = index.try(:where).try(:to_s) + if value.blank? + '' + else + " #{INDEX_CLAUSES[:where][format]} #{value}" + end + end + + def index_using_info(index, format = :default) + value = index.try(:using) && index.using.try(:to_sym) + if !value.blank? && value != :btree + " #{INDEX_CLAUSES[:using][format]} #{value}" + else + '' + end + end + def final_index_string_in_markdown(index) - sprintf("# * `%s`%s:\n# * **`%s`**\n", index.name, index.unique ? " (_unique_)" : "", index_columns_info(index).join("`**\n# * **`")) + details = sprintf( + "%s%s%s", + index_unique_info(index, :markdown), + index_where_info(index, :markdown), + index_using_info(index, :markdown) + ).strip + details = " (#{details})" unless details.blank? + + sprintf( + "# * `%s`%s:\n# * **`%s`**\n", + index.name, + details, + index_columns_info(index).join("`**\n# * **`") + ) end def final_index_string(index, max_size) - sprintf("# %-#{max_size}.#{max_size}s %s %s", index.name, "(#{index_columns_info(index).join(',')})", index.unique ? "UNIQUE" : "").rstrip + "\n" + sprintf( + "# %-#{max_size}.#{max_size}s %s%s%s%s", + index.name, + "(#{index_columns_info(index).join(',')})", + index_unique_info(index), + index_where_info(index), + index_using_info(index) + ).rstrip + "\n" end def hide_limit?(col_type, options) diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 25ed081fc..2f74b1b2a 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -5,12 +5,14 @@ require 'active_support/core_ext/string' describe AnnotateModels do - def mock_index(name, columns = [], orders = {}, unique = false) + def mock_index(name, params = {}) double('IndexKeyDefinition', name: name, - columns: columns, - unique: unique, - orders: orders) + columns: params[:columns] || [], + unique: params[:unique] || false, + orders: params[:orders] || {}, + where: params[:where], + using: params[:using]) end def mock_foreign_key(name, from_column, to_table, to_column = 'id', constraints = {}) @@ -303,8 +305,8 @@ def mock_column(name, type, options = {}) [ mock_column(:id, :integer), mock_column(:foreign_thing_id, :integer) - ], [mock_index('index_rails_02e851e3b7', ['id']), - mock_index('index_rails_02e851e3b8', ['foreign_thing_id'])]) + ], [mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', columns: ['foreign_thing_id'])]) expect(AnnotateModels.get_schema_info(klass, 'Schema Info', show_indexes: true)).to eql(<<-EOS) # Schema Info # @@ -331,10 +333,10 @@ def mock_column(name, type, options = {}) mock_column("value", :string) ], [ - mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b7', columns: ['id']), mock_index('index_rails_02e851e3b8', - %w(firstname surname value), - 'surname' => :asc, 'value' => :desc) + columns: %w(firstname surname value), + orders: { 'surname' => :asc, 'value' => :desc }) ]) expect(AnnotateModels.get_schema_info(klass, 'Schema Info', show_indexes: true)).to eql(<<-EOS) # Schema Info @@ -354,6 +356,72 @@ def mock_column(name, type, options = {}) EOS end + it 'should get indexes keys with where clause' do + klass = mock_class(:users, + :id, + [ + mock_column("id", :integer), + mock_column("firstname", :string), + mock_column("surname", :string), + mock_column("value", :string) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: %w(firstname surname), + where: 'value IS NOT NULL') + ]) + expect(AnnotateModels.get_schema_info(klass, 'Schema Info', show_indexes: true)).to eql(<<-EOS) +# Schema Info +# +# Table name: users +# +# id :integer not null, primary key +# firstname :string not null +# surname :string not null +# value :string not null +# +# Indexes +# +# index_rails_02e851e3b7 (id) +# index_rails_02e851e3b8 (firstname,surname) WHERE value IS NOT NULL +# +EOS + end + + it 'should get indexes keys with using clause other than btree' do + klass = mock_class(:users, + :id, + [ + mock_column("id", :integer), + mock_column("firstname", :string), + mock_column("surname", :string), + mock_column("value", :string) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: %w(firstname surname), + using: 'hash') + ]) + expect(AnnotateModels.get_schema_info(klass, 'Schema Info', show_indexes: true)).to eql(<<-EOS) +# Schema Info +# +# Table name: users +# +# id :integer not null, primary key +# firstname :string not null +# surname :string not null +# value :string not null +# +# Indexes +# +# index_rails_02e851e3b7 (id) +# index_rails_02e851e3b8 (firstname,surname) USING hash +# +EOS + end + it 'should get simple indexes keys' do klass = mock_class(:users, :id, @@ -362,10 +430,10 @@ def mock_column(name, type, options = {}) mock_column(:foreign_thing_id, :integer) ], [ - mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b7', columns: ['id']), mock_index('index_rails_02e851e3b8', - ['foreign_thing_id'], - 'foreign_thing_id' => :desc) + columns: ['foreign_thing_id'], + orders: { 'foreign_thing_id' => :desc }) ]) expect(AnnotateModels.get_schema_info(klass, 'Schema Info', simple_indexes: true)).to eql(<<-EOS) # Schema Info @@ -384,8 +452,8 @@ def mock_column(name, type, options = {}) [ mock_column("id", :integer), mock_column("name", :string) - ], [mock_index('index_rails_02e851e3b7', ['id']), - mock_index('index_rails_02e851e3b8', 'LOWER(name)')]) + ], [mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', columns: 'LOWER(name)')]) expect(AnnotateModels.get_schema_info(klass, 'Schema Info', simple_indexes: true)).to eql(<<-EOS) # Schema Info # @@ -501,10 +569,79 @@ def mock_column(name, type, options = {}) mock_column(:name, :string, limit: 50) ], [ - mock_index('index_rails_02e851e3b7', ['id']), + mock_index('index_rails_02e851e3b7', columns: ['id']), mock_index('index_rails_02e851e3b8', - ['foreign_thing_id'], - 'foreign_thing_id' => :desc) + columns: ['foreign_thing_id']) + ]) + expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) +# #{AnnotateModels::PREFIX} +# +# Table name: `users` +# +# ### Columns +# +# Name | Type | Attributes +# ----------- | ------------------ | --------------------------- +# **`id`** | `integer` | `not null, primary key` +# **`name`** | `string(50)` | `not null` +# +# ### Indexes +# +# * `index_rails_02e851e3b7`: +# * **`id`** +# * `index_rails_02e851e3b8`: +# * **`foreign_thing_id`** +# +EOS + end + + it 'should get schema info as Markdown with unique indexes' do + klass = mock_class(:users, + :id, + [ + mock_column(:id, :integer), + mock_column(:name, :string, limit: 50) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: ['foreign_thing_id'], + unique: true) + ]) + expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) +# #{AnnotateModels::PREFIX} +# +# Table name: `users` +# +# ### Columns +# +# Name | Type | Attributes +# ----------- | ------------------ | --------------------------- +# **`id`** | `integer` | `not null, primary key` +# **`name`** | `string(50)` | `not null` +# +# ### Indexes +# +# * `index_rails_02e851e3b7`: +# * **`id`** +# * `index_rails_02e851e3b8` (_unique_): +# * **`foreign_thing_id`** +# +EOS + end + + it 'should get schema info as Markdown with ordered indexes' do + klass = mock_class(:users, + :id, + [ + mock_column(:id, :integer), + mock_column(:name, :string, limit: 50) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: ['foreign_thing_id'], + orders: { 'foreign_thing_id' => :desc }) ]) expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) # #{AnnotateModels::PREFIX} @@ -528,6 +665,77 @@ def mock_column(name, type, options = {}) EOS end + it 'should get schema info as Markdown with indexes with WHERE clause' do + klass = mock_class(:users, + :id, + [ + mock_column(:id, :integer), + mock_column(:name, :string, limit: 50) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: ['foreign_thing_id'], + unique: true, + where: 'name IS NOT NULL') + ]) + expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) +# #{AnnotateModels::PREFIX} +# +# Table name: `users` +# +# ### Columns +# +# Name | Type | Attributes +# ----------- | ------------------ | --------------------------- +# **`id`** | `integer` | `not null, primary key` +# **`name`** | `string(50)` | `not null` +# +# ### Indexes +# +# * `index_rails_02e851e3b7`: +# * **`id`** +# * `index_rails_02e851e3b8` (_unique_ _where_ name IS NOT NULL): +# * **`foreign_thing_id`** +# +EOS + end + + it 'should get schema info as Markdown with indexes with using clause other than btree' do + klass = mock_class(:users, + :id, + [ + mock_column(:id, :integer), + mock_column(:name, :string, limit: 50) + ], + [ + mock_index('index_rails_02e851e3b7', columns: ['id']), + mock_index('index_rails_02e851e3b8', + columns: ['foreign_thing_id'], + using: 'hash') + ]) + expect(AnnotateModels.get_schema_info(klass, AnnotateModels::PREFIX, format_markdown: true, show_indexes: true)).to eql(<<-EOS) +# #{AnnotateModels::PREFIX} +# +# Table name: `users` +# +# ### Columns +# +# Name | Type | Attributes +# ----------- | ------------------ | --------------------------- +# **`id`** | `integer` | `not null, primary key` +# **`name`** | `string(50)` | `not null` +# +# ### Indexes +# +# * `index_rails_02e851e3b7`: +# * **`id`** +# * `index_rails_02e851e3b8` (_using_ hash): +# * **`foreign_thing_id`** +# +EOS + end + describe '#set_defaults' do it 'should default show_complete_foreign_keys to false' do expect(Annotate.true?(ENV['show_complete_foreign_keys'])).to be(false) From 330f8bdb8dbcf6ea62b8f428d3cdca540dc4deb5 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Wed, 12 Jul 2017 11:42:52 +0600 Subject: [PATCH 08/37] Remove empty line with trailing space at the end of routes map (#483) --- lib/annotate/annotate_routes.rb | 2 +- spec/annotate/annotate_routes_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index 7f8435ee2..3931fc6dc 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -77,7 +77,7 @@ def remove_annotations(options={}) end def self.app_routes_map(options) - routes_map = `rake routes`.split(/\n/, -1) + routes_map = `rake routes`.chomp("\n").split(/\n/, -1) # In old versions of Rake, the first line of output was the cwd. Not so # much in newer ones. We ditch that line if it exists, and if not, we diff --git a/spec/annotate/annotate_routes_spec.rb b/spec/annotate/annotate_routes_spec.rb index 037a4a2bf..f53dc108c 100644 --- a/spec/annotate/annotate_routes_spec.rb +++ b/spec/annotate/annotate_routes_spec.rb @@ -22,10 +22,10 @@ def mock_file(stubs = {}) expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true) expect(File).to receive(:read).with(ROUTE_FILE).and_return("") - expect(AnnotateRoutes).to receive(:`).with('rake routes').and_return(' Prefix Verb URI Pattern Controller#Action + expect(AnnotateRoutes).to receive(:`).with('rake routes').and_return(" Prefix Verb URI Pattern Controller#Action myaction1 GET /url1(.:format) mycontroller1#action myaction2 POST /url2(.:format) mycontroller2#action - myaction3 DELETE|GET /url3(.:format) mycontroller3#action') + myaction3 DELETE|GET /url3(.:format) mycontroller3#action\n") expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED) end From 5e3820b8f154950b6f74ec8f9ab5afc61836897e Mon Sep 17 00:00:00 2001 From: Hitabis GmbH Date: Wed, 12 Jul 2017 21:27:43 +0200 Subject: [PATCH 09/37] annotate --routes modifies only routes.rb (#485) * annotate --routes modifies only routes.rb This change is a proposed solution to #357 and an alternative to the already proposed solution #361. In #361 it is needed to call `annotate --routes --ignore-models` to achieve the same as in this change with only `annotate --routes`. * Call eager_load only when models are included This to prevent activerecord model errors when using mongoid for example and calling `annotate --routes` --- bin/annotate | 2 +- lib/annotate.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/annotate b/bin/annotate index 6b45dd9d9..dc4749f39 100755 --- a/bin/annotate +++ b/bin/annotate @@ -202,7 +202,7 @@ end.parse! options = Annotate.setup_options( is_rake: ENV['is_rake'] && !ENV['is_rake'].empty? ) -Annotate.eager_load(options) +Annotate.eager_load(options) if Annotate.include_models? AnnotateModels.send(target_action, options) if Annotate.include_models? AnnotateRoutes.send(target_action, options) if Annotate.include_routes? diff --git a/lib/annotate.rb b/lib/annotate.rb index 9f4daf4c2..e98bdb4f3 100644 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -114,7 +114,7 @@ def self.include_routes? end def self.include_models? - true + ENV['routes'] !~ TRUE_RE end def self.loaded_tasks=(val) From d02ff2e12cb2633aabe18179496dd7fae8739766 Mon Sep 17 00:00:00 2001 From: Takahiro Uchiyama Date: Fri, 14 Jul 2017 04:26:59 +0900 Subject: [PATCH 10/37] Test against latest Rubies (2.2.7 / 2.3.4 / 2.4.1) on TravisCI (#486) --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0aad6403a..c1b3e3826 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,9 @@ sudo: false language: ruby rvm: - - 2.2.5 - - 2.3.0 - - 2.4.0 + - 2.2.7 + - 2.3.4 + - 2.4.1 - ruby-head matrix: allow_failures: From 9f3a2174ee3995bf12b4a4c5d4177bebd60afac5 Mon Sep 17 00:00:00 2001 From: yhirano55 Date: Wed, 26 Jul 2017 04:23:45 +0900 Subject: [PATCH 11/37] Fix README and require annotate in InstallGenerator (#494) * Fix InstallGenerator to require annotate * Fix README --- README.rdoc | 4 ++-- lib/generators/annotate/install_generator.rb | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/README.rdoc b/README.rdoc index be0a043b9..91e5c0e37 100644 --- a/README.rdoc +++ b/README.rdoc @@ -55,11 +55,11 @@ Also, if you pass the -r option, it'll annotate routes.rb with the output of Into Gemfile from rubygems.org: - gem 'annotate' + gem 'annotate', require: false Into Gemfile from Github: - gem 'annotate', git: 'https://github.com/ctran/annotate_models.git' + gem 'annotate', github: 'ctran/annotate_models', require: false Into environment gems from rubygems.org: diff --git a/lib/generators/annotate/install_generator.rb b/lib/generators/annotate/install_generator.rb index 2d2d6ec97..383c5cdd5 100644 --- a/lib/generators/annotate/install_generator.rb +++ b/lib/generators/annotate/install_generator.rb @@ -1,8 +1,10 @@ +require 'annotate' + module Annotate module Generators class InstallGenerator < Rails::Generators::Base desc 'Copy annotate_models rakefiles for automatic annotation' - source_root File.expand_path('../templates', __FILE__) + source_root File.expand_path('templates', __dir__) # copy rake tasks def copy_tasks From 1e2047d2606c0ca0587d3dc5e48e1891490291e5 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Wed, 26 Jul 2017 01:25:58 +0600 Subject: [PATCH 12/37] Fix #436 where extra line break is added when using windows CRLF (#490) Fix #436 where extra line break is added when using windows CRLF (#490) --- lib/annotate/annotate_models.rb | 4 +-- spec/annotate/annotate_models_spec.rb | 45 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index f922b87d7..860fcbe51 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -83,9 +83,9 @@ module AnnotateModels class << self def annotate_pattern(options = {}) if options[:wrapper_open] - return /(?:^\n?# (?:#{options[:wrapper_open]}).*\n?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?\n(#.*\n)*\n*)|^\n?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?\n(#.*\n)*\n*/ + return /(?:^(\n|\r\n)?# (?:#{options[:wrapper_open]}).*(\n|\r\n)?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?(\n|\r\n)(#.*(\n|\r\n))*(\n|\r\n)*)|^(\n|\r\n)?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?(\n|\r\n)(#.*(\n|\r\n))*(\n|\r\n)*/ end - /^\n?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?\n(#.*\n)*\n*/ + /^(\n|\r\n)?# (?:#{COMPAT_PREFIX}|#{COMPAT_PREFIX_MD}).*?(\n|\r\n)(#.*(\n|\r\n))*(\n|\r\n)*/ end def model_dir diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 2f74b1b2a..aee16ff71 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -1153,6 +1153,28 @@ def content(path) # updated_at :datetime # +class Foo < ActiveRecord::Base +end + EOS + + AnnotateModels.remove_annotation_of_file(path) + + expect(content(path)).to eq <<-EOS +class Foo < ActiveRecord::Base +end + EOS + end + + it 'should remove annotate if CRLF is used for line breaks' do + path = create 'before.rb', <<-EOS +# == Schema Information +# +# Table name: foo\r\n# +# id :integer not null, primary key +# created_at :datetime +# updated_at :datetime +# +\r\n class Foo < ActiveRecord::Base end EOS @@ -1201,6 +1223,29 @@ class Foo < ActiveRecord::Base # updated_at :datetime # +class Foo < ActiveRecord::Base +end + EOS + + AnnotateModels.remove_annotation_of_file(path, wrapper_open: 'wrapper') + + expect(content(path)).to eq <<-EOS +class Foo < ActiveRecord::Base +end + EOS + end + + it 'should remove wrapper if CRLF is used for line breaks' do + path = create 'opening_wrapper.rb', <<-EOS +# wrapper\r\n# == Schema Information +# +# Table name: foo +# +# id :integer not null, primary key +# created_at :datetime +# updated_at :datetime +# + class Foo < ActiveRecord::Base end EOS From 992ad11a924cf7fed7e871de6f9956b52c876925 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Wed, 26 Jul 2017 01:26:43 +0600 Subject: [PATCH 13/37] [Fix #464] Add an empty line between magic comment and schema (#491) --- lib/annotate/annotate_models.rb | 23 ++++++++++++++++++----- spec/annotate/annotate_models_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 860fcbe51..c877167ca 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -503,8 +503,7 @@ def annotate_one_file(file_name, info_block, position, options = {}) old_columns = old_header && old_header.scan(column_pattern).sort new_columns = new_header && new_header.scan(column_pattern).sort - magic_comment_matcher = Regexp.new(/(^#\s*encoding:.*\n)|(^# coding:.*\n)|(^# -\*- coding:.*\n)|(^# -\*- encoding\s?:.*\n)|(^#\s*frozen_string_literal:.+\n)|(^# -\*- frozen_string_literal\s*:.+-\*-\n)/) - magic_comments = old_content.scan(magic_comment_matcher).flatten.compact + magic_comments_block = magic_comments_as_string(old_content) if old_columns == new_columns && !options[:force] return false @@ -522,13 +521,13 @@ def annotate_one_file(file_name, info_block, position, options = {}) # if there *was* no old schema info (no substitution happened) or :force was passed, # we simply need to insert it in correct position if new_content == old_content || options[:force] - old_content.sub!(magic_comment_matcher, '') + old_content.gsub!(magic_comment_matcher, '') old_content.sub!(annotate_pattern(options), '') new_content = if %w(after bottom).include?(options[position].to_s) - magic_comments.join + (old_content.rstrip + "\n\n" + wrapped_info_block) + magic_comments_block + (old_content.rstrip + "\n\n" + wrapped_info_block) else - magic_comments.join + wrapped_info_block + "\n" + old_content + magic_comments_block + wrapped_info_block + "\n" + old_content end end @@ -540,6 +539,20 @@ def annotate_one_file(file_name, info_block, position, options = {}) end end + def 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))/) + end + + def magic_comments_as_string(content) + magic_comments = content.scan(magic_comment_matcher).flatten.compact + + if magic_comments.any? + magic_comments.join + "\n" + else + '' + end + end + def remove_annotation_of_file(file_name, options = {}) if File.exist?(file_name) content = File.read(file_name) diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index aee16ff71..abfebdbf2 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -1476,6 +1476,30 @@ class User < ActiveRecord::Base end end + it 'adds an empty line between magic comments and annotation (position :before)' do + content = "class User < ActiveRecord::Base\nend\n" + magic_comments_list_each do |magic_comment| + model_file_name, = write_model 'user.rb', "#{magic_comment}\n#{content}" + + annotate_one_file position: :before + schema_info = AnnotateModels.get_schema_info(@klass, '== Schema Info') + + expect(File.read(model_file_name)).to eq("#{magic_comment}\n\n#{schema_info}\n#{content}") + end + end + + it 'adds an empty line between magic comments and model file content (position :after)' do + content = "class User < ActiveRecord::Base\nend\n" + magic_comments_list_each do |magic_comment| + model_file_name, = write_model 'user.rb', "#{magic_comment}\n#{content}" + + annotate_one_file position: :after + schema_info = AnnotateModels.get_schema_info(@klass, '== Schema Info') + + expect(File.read(model_file_name)).to eq("#{magic_comment}\n\n#{content}\n#{schema_info}") + end + end + describe "if a file can't be annotated" do before do allow(AnnotateModels).to receive(:get_loaded_model).with('user').and_return(nil) From 240c6b265f378586f1031a9485b43eb7d99b29fe Mon Sep 17 00:00:00 2001 From: yuemori Date: Wed, 26 Jul 2017 04:28:11 +0900 Subject: [PATCH 14/37] Add pluralized file patterns for factory_girl (#489) --- lib/annotate/annotate_models.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index c877167ca..ea4684cc5 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -142,6 +142,8 @@ def factory_files(root_directory) File.join(root_directory, FACTORY_GIRL_SPEC_DIR, "%MODEL_NAME%_factory.rb"), # (old style) File.join(root_directory, FACTORY_GIRL_TEST_DIR, "%TABLE_NAME%.rb"), # (new style) File.join(root_directory, FACTORY_GIRL_SPEC_DIR, "%TABLE_NAME%.rb"), # (new style) + File.join(root_directory, FACTORY_GIRL_TEST_DIR, "%PLURALIZED_MODEL_NAME%.rb"), # (new style) + File.join(root_directory, FACTORY_GIRL_SPEC_DIR, "%PLURALIZED_MODEL_NAME%.rb"), # (new style) File.join(root_directory, FABRICATORS_TEST_DIR, "%MODEL_NAME%_fabricator.rb"), File.join(root_directory, FABRICATORS_SPEC_DIR, "%MODEL_NAME%_fabricator.rb") ] From e763429ebe1d02c2d4a4a3a0dc3d4eeb58c12100 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Wed, 26 Jul 2017 01:29:53 +0600 Subject: [PATCH 15/37] [Fix #401] Allow wrappers in routes annotation (#492) --- lib/annotate/annotate_routes.rb | 12 ++++++++++-- lib/tasks/annotate_routes.rake | 2 ++ spec/annotate/annotate_routes_spec.rb | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index 3931fc6dc..4c8e1c816 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -1,3 +1,5 @@ +# rubocop:disable Metrics/ModuleLength + # == Annotate Routes # # Based on: @@ -36,7 +38,10 @@ def content(line, maxs, options = {}) def header(options = {}) routes_map = app_routes_map(options) - out = ["# #{options[:format_markdown] ? PREFIX_MD : PREFIX}" + (options[:timestamp] ? " (Updated #{Time.now.strftime('%Y-%m-%d %H:%M')})" : '')] + out = [] + out += ["# #{options[:wrapper_open]}"] if options[:wrapper_open] + + out += ["# #{options[:format_markdown] ? PREFIX_MD : PREFIX}" + (options[:timestamp] ? " (Updated #{Time.now.strftime('%Y-%m-%d %H:%M')})" : '')] out += ['#'] return out if routes_map.size.zero? @@ -51,7 +56,10 @@ def header(options = {}) out += ["# #{content(routes_map[0], maxs, options)}"] end - out + routes_map[1..-1].map { |line| "# #{content(options[:format_markdown] ? line.split(' ') : line, maxs, options)}" } + out += routes_map[1..-1].map { |line| "# #{content(options[:format_markdown] ? line.split(' ') : line, maxs, options)}" } + out += ["# #{options[:wrapper_close]}"] if options[:wrapper_close] + + out end def do_annotations(options = {}) diff --git a/lib/tasks/annotate_routes.rake b/lib/tasks/annotate_routes.rake index 26a99fecc..208505d24 100644 --- a/lib/tasks/annotate_routes.rake +++ b/lib/tasks/annotate_routes.rake @@ -8,6 +8,8 @@ task :annotate_routes => :environment do options[:position_in_routes] = Annotate.fallback(ENV['position_in_routes'], ENV['position']) options[:ignore_routes] = Annotate.fallback(ENV['ignore_routes'], nil) options[:require] = ENV['require'] ? ENV['require'].split(',') : [] + options[:wrapper_open] = Annotate.fallback(ENV['wrapper_open'], ENV['wrapper']) + options[:wrapper_close] = Annotate.fallback(ENV['wrapper_close'], ENV['wrapper']) AnnotateRoutes.do_annotations(options) end diff --git a/spec/annotate/annotate_routes_spec.rb b/spec/annotate/annotate_routes_spec.rb index f53dc108c..9fef19770 100644 --- a/spec/annotate/annotate_routes_spec.rb +++ b/spec/annotate/annotate_routes_spec.rb @@ -56,6 +56,21 @@ def mock_file(stubs = {}) AnnotateRoutes.do_annotations(format_markdown: true) end + + it 'wraps annotation if wrapper is specified' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" +# START +# == Route Map +# +# Prefix Verb URI Pattern Controller#Action +# myaction1 GET /url1(.:format) mycontroller1#action +# myaction2 POST /url2(.:format) mycontroller2#action +# myaction3 DELETE|GET /url3(.:format) mycontroller3#action +# END\n") + + AnnotateRoutes.do_annotations(wrapper_open: 'START', wrapper_close: 'END') + end end describe 'When adding' do From d44f705184f92dc6853f6f80ab397e857382e904 Mon Sep 17 00:00:00 2001 From: Cuong Tran Date: Wed, 26 Jul 2017 17:08:25 -0700 Subject: [PATCH 16/37] Use long form instead github due to non-https url (#496) --- README.rdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rdoc b/README.rdoc index 91e5c0e37..1a0444944 100644 --- a/README.rdoc +++ b/README.rdoc @@ -59,7 +59,7 @@ Into Gemfile from rubygems.org: Into Gemfile from Github: - gem 'annotate', github: 'ctran/annotate_models', require: false + gem 'annotate', git: 'https://github.com/ctran/annotate_models.git', require: false Into environment gems from rubygems.org: From 7c8e916db46a1c8678736a33e8df560d536795f0 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Thu, 27 Jul 2017 06:10:03 +0600 Subject: [PATCH 17/37] [Fix #414] Fix: AnnotateRoutes.remove_annotations duplicates routes (#493) --- lib/annotate/annotate_routes.rb | 29 +++++++--- spec/annotate/annotate_routes_spec.rb | 76 +++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index 4c8e1c816..fbae767d5 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -66,19 +66,17 @@ def do_annotations(options = {}) return unless routes_exists? existing_text = File.read(routes_file) - if write_contents(existing_text, header(options), options) + if rewrite_contents_with_header(existing_text, header(options), options) puts "#{routes_file} annotated." end end - def remove_annotations(options={}) + def remove_annotations(_options={}) return unless routes_exists? existing_text = File.read(routes_file) content, where_header_found = strip_annotations(existing_text) - - content = strip_on_removal(content, where_header_found) - - if write_contents(existing_text, content, options) + new_content = strip_on_removal(content, where_header_found) + if rewrite_contents(existing_text, new_content) puts "Removed annotations from #{routes_file}." end end @@ -112,7 +110,22 @@ def self.routes_exists? routes_exists end - def self.write_contents(existing_text, header, options = {}) + # @param [String, Array] + def self.rewrite_contents(existing_text, new_content) + # Make sure we end on a trailing newline. + new_content << '' unless new_content.last == '' + new_text = new_content.join("\n") + + if existing_text == new_text + puts "#{routes_file} unchanged." + false + else + File.open(routes_file, 'wb') { |f| f.puts(new_text) } + true + end + end + + def self.rewrite_contents_with_header(existing_text, header, options = {}) content, where_header_found = strip_annotations(existing_text) new_content = annotate_routes(header, content, where_header_found, options) @@ -162,7 +175,7 @@ def self.strip_annotations(content) content.split(/\n/, -1).each_with_index do |line, line_number| if mode == :header && line !~ /\s*#/ mode = :content - next unless line == '' + real_content << line unless line.blank? elsif mode == :content if line =~ /^\s*#\s*== Route.*$/ header_found_at = line_number + 1 # index start's at 0 diff --git a/spec/annotate/annotate_routes_spec.rb b/spec/annotate/annotate_routes_spec.rb index 9fef19770..719d2fe4b 100644 --- a/spec/annotate/annotate_routes_spec.rb +++ b/spec/annotate/annotate_routes_spec.rb @@ -170,14 +170,82 @@ def mock_file(stubs = {}) end it 'should remove trailing annotation and trim trailing newlines, but leave leading newlines alone' do - expect(File).to receive(:read).with(ROUTE_FILE).and_return("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n# == Route Map\n#\n# another good line\n# good line\n") - expect(@mock_file).to receive(:puts).with(/\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n/) + expect(File).to receive(:read).with(ROUTE_FILE).and_return(<<-EOS + + + + ActionController::Routing... + foo + + + # == Route Map + # + # another good line + # good line + EOS + ) + expect(@mock_file).to receive(:puts).with(<<-EOS + + + + ActionController::Routing... + foo + EOS + ) AnnotateRoutes.remove_annotations end it 'should remove prepended annotation and trim leading newlines, but leave trailing newlines alone' do - expect(File).to receive(:read).with(ROUTE_FILE).and_return("# == Route Map\n#\n# another good line\n# good line\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n") - expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n/) + expect(File).to receive(:read).with(ROUTE_FILE).and_return(<<-EOS + # == Route Map + # + # another good line + # good line + + + + + Rails.application.routes.draw do + root 'root#index' + end + + + + EOS + ) + expect(@mock_file).to receive(:puts).with(<<-EOS + Rails.application.routes.draw do + root 'root#index' + end + + + + EOS + ) + AnnotateRoutes.remove_annotations + end + + it 'should not remove custom comments above route map' do + expect(File).to receive(:read).with(ROUTE_FILE).and_return(<<-EOS + # My comment + # == Route Map + # + # another good line + # good line + Rails.application.routes.draw do + root 'root#index' + end + EOS + ) + + expect(@mock_file).to receive(:puts).with(<<-EOS + # My comment + Rails.application.routes.draw do + root 'root#index' + end + EOS + ) + AnnotateRoutes.remove_annotations end end From 5275757559bba5796c1edc033ec784337fd6ea0d Mon Sep 17 00:00:00 2001 From: Hideki IGARASHI Date: Tue, 3 Oct 2017 13:47:01 +0900 Subject: [PATCH 18/37] Fix problem with specifying model files as argument (#514) * Refactor AnnotateModels#get_model_files * Add the tests for `AnnotateModels#get_model_files` * Remove unnecessary `reject` call --- lib/annotate/annotate_models.rb | 64 +++++++++----- spec/annotate/annotate_models_spec.rb | 116 ++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 23 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index ea4684cc5..67769c340 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -649,34 +649,52 @@ def options_with_position(options, position_in) # of model files from root dir. Otherwise we take all the model files # in the model_dir directory. def get_model_files(options) - models = [] - unless options[:is_rake] - models = ARGV.dup.reject { |m| m.match(/^(.*)=/) } - end + model_files = [] - if models.empty? - begin - model_dir.each do |dir| - Dir.chdir(dir) do - lst = - if options[:ignore_model_sub_dir] - Dir["*.rb"].map{ |f| [dir, f] } - else - Dir["**/*.rb"].reject{ |f| f["concerns/"] }.map{ |f| [dir, f] } - end - models.concat(lst) - end - end - rescue SystemCallError - puts "No models found in directory '#{model_dir.join("', '")}'." - puts "Either specify models on the command line, or use the --model-dir option." - puts "Call 'annotate --help' for more info." - exit 1 + model_files = list_model_files_from_argument unless options[:is_rake] + + return model_files unless model_files.empty? + + model_dir.each do |dir| + Dir.chdir(dir) do + list = if options[:ignore_model_sub_dir] + Dir["*.rb"].map { |f| [dir, f] } + else + Dir["**/*.rb"].reject { |f| f["concerns/"] }.map { |f| [dir, f] } + end + model_files.concat(list) end end - models + model_files + rescue SystemCallError + puts "No models found in directory '#{model_dir.join("', '")}'." + puts "Either specify models on the command line, or use the --model-dir option." + puts "Call 'annotate --help' for more info." + exit 1 + end + + def list_model_files_from_argument + return [] if ARGV.empty? + + specified_files = ARGV.map { |file| File.expand_path(file) } + + model_files = model_dir.flat_map do |dir| + absolute_dir_path = File.expand_path(dir) + specified_files + .find_all { |file| file.start_with?(absolute_dir_path) } + .map { |file| [dir, file.sub("#{absolute_dir_path}/", '')] } + end + + if model_files.size != specified_files.size + puts "The specified file could not be found in directory '#{model_dir.join("', '")}'." + puts "Call 'annotate --help' for more info." + exit 1 + end + + model_files end + private :list_model_files_from_argument # Retrieve the classes belonging to the model names we're asked to process # Check for namespaced models in subdirectories as well as models diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index abfebdbf2..ff02a8d87 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -3,6 +3,7 @@ require 'annotate/annotate_models' require 'annotate/active_record_patch' require 'active_support/core_ext/string' +require 'files' describe AnnotateModels do def mock_index(name, params = {}) @@ -951,6 +952,121 @@ def self.when_called_with(options = {}) end end + describe '#get_model_files' do + subject { described_class.get_model_files(options) } + + before do + ARGV.clear + + described_class.model_dir = [model_dir] + end + + context 'when `model_dir` is valid' do + let(:model_dir) do + Files do + file 'foo.rb' + dir 'bar' do + file 'baz.rb' + dir 'qux' do + file 'quux.rb' + end + end + dir 'concerns' do + file 'corge.rb' + end + end + end + + context 'when the model files are not specified' do + context 'when no option is specified' do + let(:options) { {} } + + it 'returns all model files under `model_dir` directory' do + is_expected.to contain_exactly( + [model_dir, 'foo.rb'], + [model_dir, File.join('bar', 'baz.rb')], + [model_dir, File.join('bar', 'qux', 'quux.rb')] + ) + end + end + + context 'when `ignore_model_sub_dir` option is enabled' do + let(:options) { { ignore_model_sub_dir: true } } + + it 'returns model files just below `model_dir` directory' do + is_expected.to contain_exactly([model_dir, 'foo.rb']) + end + end + end + + context 'when the model files are specified' do + let(:additional_model_dir) { 'additional_model' } + let(:model_files) do + [ + File.join(model_dir, 'foo.rb'), + "./#{File.join(additional_model_dir, 'corge/grault.rb')}" # Specification by relative path + ] + end + + before { ARGV.concat(model_files) } + + context 'when no option is specified' do + let(:options) { {} } + + context 'when all the specified files are in `model_dir` directory' do + before do + described_class.model_dir << additional_model_dir + end + + it 'returns specified files' do + is_expected.to contain_exactly( + [model_dir, 'foo.rb'], + [additional_model_dir, 'corge/grault.rb'] + ) + end + end + + context 'when a model file outside `model_dir` directory is specified' do + it 'exits with the status code' do + begin + subject + raise + rescue SystemExit => e + expect(e.status).to eq(1) + end + end + end + end + + context 'when `is_rake` option is enabled' do + let(:options) { { is_rake: true } } + + it 'returns all model files under `model_dir` directory' do + is_expected.to contain_exactly( + [model_dir, 'foo.rb'], + [model_dir, File.join('bar', 'baz.rb')], + [model_dir, File.join('bar', 'qux', 'quux.rb')] + ) + end + end + end + end + + context 'when `model_dir` is invalid' do + let(:model_dir) { '/not_exist_path' } + let(:options) { {} } + + it 'exits with the status code' do + begin + subject + raise + rescue SystemExit => e + expect(e.status).to eq(1) + end + end + end + end + describe '#get_model_class' do require 'tmpdir' From 63cc88c1ab2e81b237cb8584931197bd348fd848 Mon Sep 17 00:00:00 2001 From: Shinichi Maeshima Date: Wed, 11 Oct 2017 12:21:34 +0900 Subject: [PATCH 19/37] Annotate bigint columns as 'bigint' instead of 'integer' (#515) --- lib/annotate/annotate_models.rb | 10 +++++++++- spec/annotate/annotate_models_spec.rb | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 67769c340..88fd9e446 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -249,7 +249,7 @@ def get_schema_info(klass, header, options = {}) cols = cols.sort_by(&:name) if options[:sort] cols = classified_sort(cols) if options[:classified_sort] cols.each do |col| - col_type = (col.type || col.sql_type).to_s + col_type = get_col_type(col) attrs = [] attrs << "default(#{schema_default(klass, col)})" unless col.default.nil? || hide_default?(col_type, options) attrs << 'unsigned' if col.respond_to?(:unsigned?) && col.unsigned? @@ -363,6 +363,14 @@ def get_index_info(klass, options = {}) index_info end + def get_col_type(col) + if col.respond_to?(:bigint?) && col.bigint? + 'bigint' + else + (col.type || col.sql_type).to_s + end + end + def index_columns_info(index) Array(index.columns).map do |col| if index.try(:orders) && index.orders[col.to_s] diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index ff02a8d87..73121730e 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -158,7 +158,7 @@ def mock_column(name, type, options = {}) [ mock_column(:id, :integer), mock_column(:integer, :integer, unsigned?: true), - mock_column(:bigint, :bigint, unsigned?: true), + mock_column(:bigint, :integer, unsigned?: true, bigint?: true), mock_column(:float, :float, unsigned?: true), mock_column(:decimal, :decimal, unsigned?: true, precision: 10, scale: 2), ]) From 2a977bfc116b8c544139b4860a1d6621bd4902d4 Mon Sep 17 00:00:00 2001 From: ryu39 Date: Thu, 12 Oct 2017 05:27:22 +0900 Subject: [PATCH 20/37] Fix error when table_name is frozen (#517) --- lib/annotate/annotate_models.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 88fd9e446..449f903e0 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -212,8 +212,8 @@ def retrieve_indexes_from_table(klass) return indexes if indexes.any? || !klass.table_name_prefix # Try to search the table without prefix - table_name.to_s.slice!(klass.table_name_prefix) - klass.connection.indexes(table_name) + table_name_without_prefix = table_name.to_s.sub(klass.table_name_prefix, '') + klass.connection.indexes(table_name_without_prefix) end # Use the column information in an ActiveRecord class From 7a96e901aad71ee632daad25f13ab45c319b0993 Mon Sep 17 00:00:00 2001 From: Doug Hammond Date: Fri, 13 Oct 2017 21:52:05 +0200 Subject: [PATCH 21/37] Make comment annotations work (#518) * Hook up with-comment option to rake tasks and CL. * with_comment option handles commentless columns. * Style fixes. --- .rubocop_todo.yml | 72 +++++++-------------------- bin/annotate | 4 ++ lib/annotate.rb | 2 +- lib/annotate/annotate_models.rb | 30 ++++++++--- lib/tasks/annotate_models.rake | 1 + spec/annotate/annotate_models_spec.rb | 10 ++-- 6 files changed, 55 insertions(+), 64 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 00104bc6f..5828f8aee 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2016-12-17 10:16:05 +0100 using RuboCop version 0.46.0. +# on 2017-10-13 10:01:18 +0200 using RuboCop version 0.46.0. # 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 @@ -65,11 +65,6 @@ Lint/HandleExceptions: Exclude: - 'bin/annotate' -# Offense count: 8 -Lint/IneffectiveAccessModifier: - Exclude: - - 'lib/annotate/annotate_routes.rb' - # Offense count: 1 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. @@ -88,62 +83,51 @@ Lint/ShadowingOuterLocalVariable: Exclude: - 'Rakefile' -# Offense count: 6 +# Offense count: 7 # Cop supports --auto-correct. # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: Exclude: - 'bin/annotate' -# Offense count: 1 -# Configuration parameters: ContextCreatingMethods. -Lint/UselessAccessModifier: - Exclude: - - 'lib/annotate/annotate_routes.rb' - -# Offense count: 17 +# Offense count: 18 Metrics/AbcSize: - Max: 159 + Max: 142 # Offense count: 3 # Configuration parameters: CountComments. Metrics/BlockLength: - Max: 140 + Max: 142 # Offense count: 2 Metrics/BlockNesting: Max: 4 -# Offense count: 8 +# Offense count: 9 Metrics/CyclomaticComplexity: - Max: 42 + Max: 36 -# Offense count: 350 +# Offense count: 380 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: - Max: 543 + Max: 276 -# Offense count: 24 +# Offense count: 26 # Configuration parameters: CountComments. Metrics/MethodLength: - Max: 79 - -# Offense count: 1 -# Configuration parameters: CountComments. -Metrics/ModuleLength: - Max: 116 + Max: 75 # Offense count: 7 Metrics/PerceivedComplexity: - Max: 48 + Max: 42 # Offense count: 1 Style/AccessorMethodName: Exclude: - 'lib/annotate.rb' -# Offense count: 3 +# Offense count: 2 # Cop supports --auto-correct. Style/AlignArray: Exclude: @@ -259,7 +243,7 @@ Style/ExtraSpacing: - 'spec/integration/rails_4.2.0/Gemfile' - 'spec/integration/rails_4.2.0/config.ru' -# Offense count: 9 +# Offense count: 10 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: format, sprintf, percent Style/FormatString: @@ -267,13 +251,6 @@ Style/FormatString: - 'lib/annotate/annotate_models.rb' - 'lib/annotate/annotate_routes.rb' -# Offense count: 181 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: when_needed, always -Style/FrozenStringLiteralComment: - Enabled: false - # Offense count: 7 # Configuration parameters: MinBodyLength. Style/GuardClause: @@ -356,16 +333,6 @@ Style/NumericLiterals: # Offense count: 2 # Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle, SupportedStyles. -# SupportedStyles: predicate, comparison -Style/NumericPredicate: - Exclude: - - 'spec/**/*' - - 'lib/annotate.rb' - - 'lib/annotate/annotate_models.rb' - -# Offense count: 6 -# Cop supports --auto-correct. # Configuration parameters: PreferredDelimiters. Style/PercentLiteralDelimiters: Exclude: @@ -458,16 +425,15 @@ Style/SpaceAroundKeyword: - 'spec/integration/rails_4.2.0/Gemfile' - 'spec/integration/standalone/Gemfile' -# Offense count: 6 +# Offense count: 4 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment. Style/SpaceAroundOperators: Exclude: - 'lib/annotate/annotate_models.rb' - - 'lib/tasks/annotate_models.rake' - 'lib/tasks/annotate_routes.rake' -# Offense count: 4 +# Offense count: 1 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: space, no_space @@ -517,14 +483,14 @@ Style/SpaceInsideStringInterpolation: Exclude: - 'lib/annotate/annotate_models.rb' -# Offense count: 223 +# Offense count: 237 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes Style/StringLiterals: Enabled: false -# Offense count: 2 +# Offense count: 1 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: single_quotes, double_quotes @@ -581,7 +547,7 @@ Style/UnneededInterpolation: Exclude: - 'bin/annotate' -# Offense count: 8 +# Offense count: 4 # Cop supports --auto-correct. Style/UnneededPercentQ: Exclude: diff --git a/bin/annotate b/bin/annotate index dc4749f39..f03c29c85 100755 --- a/bin/annotate +++ b/bin/annotate @@ -197,6 +197,10 @@ OptionParser.new do |opts| opts.on('--ignore-unknown-models', "don't display warnings for bad model files") do |values| ENV['ignore_unknown_models'] = 'true' end + + opts.on('--with-comment', "include database comments") do |values| + ENV['with_comment'] = 'true' + end end.parse! options = Annotate.setup_options( diff --git a/lib/annotate.rb b/lib/annotate.rb index e98bdb4f3..675b6a3b2 100644 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -33,7 +33,7 @@ module Annotate :timestamp, :exclude_serializers, :classified_sort, :show_foreign_keys, :show_complete_foreign_keys, :exclude_scaffolds, :exclude_controllers, :exclude_helpers, - :exclude_sti_subclasses, :ignore_unknown_models + :exclude_sti_subclasses, :ignore_unknown_models, :with_comment ].freeze OTHER_OPTIONS = [ :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close, diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 449f903e0..aaeee20d1 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -224,11 +224,7 @@ def get_schema_info(klass, header, options = {}) info = "# #{header}\n" info << get_schema_header_text(klass, options) - max_size = klass.column_names.map(&:size).max || 0 - with_comment = options[:with_comment] && klass.columns.first.respond_to?(:comment) - max_size = klass.columns.map{|col| col.name.size + col.comment.size }.max || 0 if with_comment - max_size += 2 if with_comment - max_size += options[:format_rdoc] ? 5 : 1 + max_size = max_schema_info_width(klass, options) md_names_overhead = 6 md_type_allowance = 18 bare_type_allowance = 16 @@ -291,7 +287,7 @@ def get_schema_info(klass, header, options = {}) end end end - col_name = if with_comment + col_name = if with_comments?(klass, options) && col.comment "#{col.name}(#{col.comment})" else col.name @@ -861,6 +857,28 @@ def silence_warnings ensure $VERBOSE = old_verbose end + + private + + def with_comments?(klass, options) + options[:with_comment] && + klass.columns.first.respond_to?(:comment) && + klass.columns.any? { |col| !col.comment.nil? } + end + + def max_schema_info_width(klass, options) + if with_comments?(klass, options) + max_size = klass.columns.map do |column| + column.name.size + (column.comment ? column.comment.size : 0) + end.max || 0 + max_size += 2 + else + max_size = klass.column_names.map(&:size).max + end + max_size += options[:format_rdoc] ? 5 : 1 + + max_size + end end class BadModelFileError < LoadError diff --git a/lib/tasks/annotate_models.rake b/lib/tasks/annotate_models.rake index 95cee4768..495501233 100644 --- a/lib/tasks/annotate_models.rake +++ b/lib/tasks/annotate_models.rake @@ -47,6 +47,7 @@ task annotate_models: :environment do 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.fallback(ENV['with_comment'], '') AnnotateModels.do_annotations(options) end diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 73121730e..42963f871 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -888,10 +888,11 @@ def self.when_called_with(options = {}) describe 'with_comment option' do mocked_columns_with_comment = [ - [:id, :integer, { limit: 8, comment: 'ID' }], - [:active, :boolean, { limit: 1, comment: 'Active' }], - [:name, :string, { limit: 50, comment: 'Name' }], - [:notes, :text, { limit: 55, comment: 'Notes' }] + [:id, :integer, { limit: 8, comment: 'ID' }], + [:active, :boolean, { limit: 1, comment: 'Active' }], + [:name, :string, { limit: 50, comment: 'Name' }], + [:notes, :text, { limit: 55, comment: 'Notes' }], + [:no_comment, :text, { limit: 20, comment: nil }] ] when_called_with with_comment: 'yes', @@ -905,6 +906,7 @@ def self.when_called_with(options = {}) # active(Active) :boolean not null # name(Name) :string(50) not null # notes(Notes) :text(55) not null + # no_comment :text(20) not null # EOS From fd67ed46431d35d8e471f1143ac90bb1b0431f3f Mon Sep 17 00:00:00 2001 From: Doug Hammond Date: Sun, 15 Oct 2017 20:10:32 +0200 Subject: [PATCH 22/37] Update CLI options in README. (#519) --- README.rdoc | 16 +++++++++++++--- bin/annotate | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/README.rdoc b/README.rdoc index 1a0444944..57035de1c 100644 --- a/README.rdoc +++ b/README.rdoc @@ -161,7 +161,8 @@ you can do so with a simple environment variable, instead of editing the Usage: annotate [options] [model_file]* -d, --delete Remove annotations from all model files or the routes.rb file - -p, --position [before|top|after|bottom] Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/routes file(s) + -p [before|top|after|bottom], Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s) + --position --pc, --position-in-class [before|top|after|bottom] Place the annotations at the top (before) or the bottom (after) of the model file --pf, --position-in-factory [before|top|after|bottom] @@ -179,17 +180,19 @@ you can do so with a simple environment variable, instead of editing the --wo, --wrapper-open STR Annotation wrapper opening. --wc, --wrapper-close STR Annotation wrapper closing -r, --routes Annotate routes.rb with the output of 'rake routes' - -aa, --active-admin Annotate all activeadmin models + -a, --active-admin Annotate active_admin models -v, --version Show the current version of this gem -m, --show-migration Include the migration version number in the annotation - -i, --show-indexes List the table's database indexes in the annotation -k, --show-foreign-keys List the table's foreign key constraints in the annotation --ck, --complete-foreign-keys Complete foreign key names in the annotation + -i, --show-indexes List the table's database indexes in the annotation -s, --simple-indexes Concat the column's related indexes in the annotation --model-dir dir Annotate model files stored in dir rather than app/models, separate multiple dirs with commas + --root-dir dir Annotate files stored within root dir projects, separate multiple dirs with commas --ignore-model-subdirects Ignore subdirectories of the models directory --sort Sort columns alphabetically, rather than in creation order + --classified-sort Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns -R, --require path Additional file to require before loading models, may be used multiple times -e [tests,fixtures,factories,serializers], --exclude Do not annotate fixtures, test files, factories, and/or serializers @@ -199,6 +202,13 @@ you can do so with a simple environment variable, instead of editing the --timestamp Include timestamp in (routes) annotation --trace If unable to annotate a file, print the full stack trace, not just the exception message. -I, --ignore-columns REGEX don't annotate columns that match a given REGEX (i.e., `annotate -I '^(id|updated_at|created_at)'` + --ignore-routes REGEX don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'` + --hide-limit-column-types VALUES + don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`) + --hide-default-column-types VALUES + don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`) + --ignore-unknown-models don't display warnings for bad model files + --with-comment include database comments in model annotations diff --git a/bin/annotate b/bin/annotate index f03c29c85..0bf4db646 100755 --- a/bin/annotate +++ b/bin/annotate @@ -90,7 +90,7 @@ OptionParser.new do |opts| ENV['routes'] = 'true' end - opts.on('-aa', '--active-admin', 'Annotate active_admin models') do + opts.on('-a', '--active-admin', 'Annotate active_admin models') do ENV['active_admin'] = 'true' end @@ -198,7 +198,7 @@ OptionParser.new do |opts| ENV['ignore_unknown_models'] = 'true' end - opts.on('--with-comment', "include database comments") do |values| + opts.on('--with-comment', "include database comments in model annotations") do |values| ENV['with_comment'] = 'true' end end.parse! From e315405cc6a8aeb699d839344da837262e6a748d Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Thu, 19 Oct 2017 04:00:04 +0600 Subject: [PATCH 23/37] Do not remove magic comments (AnnotateRoutes) (#520) --- lib/annotate/annotate_routes.rb | 33 ++++- spec/annotate/annotate_routes_spec.rb | 168 ++++++++++++++++++++++---- 2 files changed, 177 insertions(+), 24 deletions(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index fbae767d5..e04960113 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -38,7 +38,13 @@ def content(line, maxs, options = {}) def header(options = {}) routes_map = app_routes_map(options) + magic_comments_map, routes_map = extract_magic_comments_from_array(routes_map) out = [] + + magic_comments_map.each do |magic_comment| + out << magic_comment + end + out += ["# #{options[:wrapper_open]}"] if options[:wrapper_open] out += ["# #{options[:format_markdown] ? PREFIX_MD : PREFIX}" + (options[:timestamp] ? " (Updated #{Time.now.strftime('%Y-%m-%d %H:%M')})" : '')] @@ -82,6 +88,28 @@ def remove_annotations(_options={}) end end + def self.magic_comment_matcher + Regexp.new(/(^#\s*encoding:.*)|(^# coding:.*)|(^# -\*- coding:.*)|(^# -\*- encoding\s?:.*)|(^#\s*frozen_string_literal:.+)|(^# -\*- frozen_string_literal\s*:.+-\*-)/) + end + + # @param [Array] content + # @return [Array] all found magic comments + # @return [Array] content without magic comments + def self.extract_magic_comments_from_array(content_array) + magic_comments = [] + new_content = [] + + content_array.map do |row| + if row =~ magic_comment_matcher + magic_comments << row.strip + else + new_content << row + end + end + + [magic_comments, new_content] + end + def self.app_routes_map(options) routes_map = `rake routes`.chomp("\n").split(/\n/, -1) @@ -143,9 +171,10 @@ def self.rewrite_contents_with_header(existing_text, header, options = {}) end def self.annotate_routes(header, content, where_header_found, options = {}) + magic_comments_map, content = extract_magic_comments_from_array(content) if %w(before top).include?(options[:position_in_routes]) header = header << '' if content.first != '' - new_content = header + content + new_content = magic_comments_map + header + content else # Ensure we have adequate trailing newlines at the end of the file to # ensure a blank line separating the content from the annotation. @@ -155,7 +184,7 @@ def self.annotate_routes(header, content, where_header_found, options = {}) # the spacer we put in the first time around. content.shift if where_header_found == :before && content.first == '' - new_content = content + header + new_content = magic_comments_map + content + header end new_content diff --git a/spec/annotate/annotate_routes_spec.rb b/spec/annotate/annotate_routes_spec.rb index 719d2fe4b..005f1ff70 100644 --- a/spec/annotate/annotate_routes_spec.rb +++ b/spec/annotate/annotate_routes_spec.rb @@ -11,6 +11,22 @@ def mock_file(stubs = {}) @mock_file ||= double(File, stubs) end + def magic_comments_list_each + [ + '# encoding: UTF-8', + '# coding: UTF-8', + '# -*- coding: UTF-8 -*-', + '#encoding: utf-8', + '# encoding: utf-8', + '# -*- encoding : utf-8 -*-', + "# encoding: utf-8\n# frozen_string_literal: true", + "# frozen_string_literal: true\n# encoding: utf-8", + '# frozen_string_literal: true', + '#frozen_string_literal: false', + '# -*- frozen_string_literal : true -*-' + ].each { |magic_comment| yield magic_comment } + end + it 'should check if routes.rb exists' do expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(false) expect(AnnotateRoutes).to receive(:puts).with("Can't find routes.rb") @@ -18,21 +34,29 @@ def mock_file(stubs = {}) end describe 'Annotate#example' do - before(:each) do - expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true) - - expect(File).to receive(:read).with(ROUTE_FILE).and_return("") - expect(AnnotateRoutes).to receive(:`).with('rake routes').and_return(" Prefix Verb URI Pattern Controller#Action + let(:rake_routes_content) do + " Prefix Verb URI Pattern Controller#Action myaction1 GET /url1(.:format) mycontroller1#action myaction2 POST /url2(.:format) mycontroller2#action - myaction3 DELETE|GET /url3(.:format) mycontroller3#action\n") + myaction3 DELETE|GET /url3(.:format) mycontroller3#action\n" + end - expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED) + before(:each) do + expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true).at_least(:once) + + expect(File).to receive(:read).with(ROUTE_FILE).and_return("").at_least(:once) + + expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED).at_least(:once) end - it 'annotate normal' do - expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) - expect(@mock_file).to receive(:puts).with(" + context 'without magic comments' do + before(:each) do + expect(AnnotateRoutes).to receive(:`).with('rake routes').and_return(rake_routes_content) + end + + it 'annotate normal' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" # == Route Map # # Prefix Verb URI Pattern Controller#Action @@ -40,12 +64,12 @@ def mock_file(stubs = {}) # myaction2 POST /url2(.:format) mycontroller2#action # myaction3 DELETE|GET /url3(.:format) mycontroller3#action\n") - AnnotateRoutes.do_annotations - end + AnnotateRoutes.do_annotations + end - it 'annotate markdown' do - expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) - expect(@mock_file).to receive(:puts).with(" + it 'annotate markdown' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" # ## Route Map # # Prefix | Verb | URI Pattern | Controller#Action @@ -54,12 +78,72 @@ def mock_file(stubs = {}) # myaction2 | POST | /url2(.:format) | mycontroller2#action # myaction3 | DELETE-GET | /url3(.:format) | mycontroller3#action\n") - AnnotateRoutes.do_annotations(format_markdown: true) + AnnotateRoutes.do_annotations(format_markdown: true) + end + + it 'wraps annotation if wrapper is specified' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" +# START +# == Route Map +# +# Prefix Verb URI Pattern Controller#Action +# myaction1 GET /url1(.:format) mycontroller1#action +# myaction2 POST /url2(.:format) mycontroller2#action +# myaction3 DELETE|GET /url3(.:format) mycontroller3#action +# END\n") + + AnnotateRoutes.do_annotations(wrapper_open: 'START', wrapper_close: 'END') + end end - it 'wraps annotation if wrapper is specified' do - expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) - expect(@mock_file).to receive(:puts).with(" + context 'file with magic comments' do + it 'should not remove magic comments' do + magic_comments_list_each do |magic_comment| + expect(AnnotateRoutes).to receive(:`).with('rake routes') + .and_return("#{magic_comment}\n#{rake_routes_content}") + + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" +#{magic_comment} +# == Route Map +# +# Prefix Verb URI Pattern Controller#Action +# myaction1 GET /url1(.:format) mycontroller1#action +# myaction2 POST /url2(.:format) mycontroller2#action +# myaction3 DELETE|GET /url3(.:format) mycontroller3#action\n") + + AnnotateRoutes.do_annotations + end + end + + it 'annotate markdown' do + magic_comments_list_each do |magic_comment| + expect(AnnotateRoutes).to receive(:`).with('rake routes') + .and_return("#{magic_comment}\n#{rake_routes_content}") + + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" +#{magic_comment} +# ## Route Map +# +# Prefix | Verb | URI Pattern | Controller#Action +# --------- | ---------- | --------------- | -------------------- +# myaction1 | GET | /url1(.:format) | mycontroller1#action +# myaction2 | POST | /url2(.:format) | mycontroller2#action +# myaction3 | DELETE-GET | /url3(.:format) | mycontroller3#action\n") + + AnnotateRoutes.do_annotations(format_markdown: true) + end + end + + it 'wraps annotation if wrapper is specified' do + magic_comments_list_each do |magic_comment| + expect(AnnotateRoutes).to receive(:`).with('rake routes') + .and_return("#{magic_comment}\n#{rake_routes_content}") + expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) + expect(@mock_file).to receive(:puts).with(" +#{magic_comment} # START # == Route Map # @@ -69,14 +153,18 @@ def mock_file(stubs = {}) # myaction3 DELETE|GET /url3(.:format) mycontroller3#action # END\n") - AnnotateRoutes.do_annotations(wrapper_open: 'START', wrapper_close: 'END') + AnnotateRoutes.do_annotations(wrapper_open: 'START', wrapper_close: 'END') + end + end end end describe 'When adding' do before(:each) do - expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true) - expect(AnnotateRoutes).to receive(:`).with('rake routes').and_return('') + expect(File).to receive(:exists?).with(ROUTE_FILE) + .and_return(true).at_least(:once) + expect(AnnotateRoutes).to receive(:`).with('rake routes') + .and_return('').at_least(:once) end it 'should insert annotations if file does not contain annotations' do @@ -112,6 +200,42 @@ def mock_file(stubs = {}) AnnotateRoutes.do_annotations end + + context 'file with magic comments' do + it 'leaves magic comment on top, adds an empty line between magic comment and annotation (position_in_routes :top)' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb') + .and_yield(mock_file).at_least(:once) + + magic_comments_list_each do |magic_comment| + expect(File).to receive(:read).with(ROUTE_FILE).and_return("#{magic_comment}\nSomething") + expect(@mock_file).to receive(:puts).with("#{magic_comment}\n# == Route Map\n#\n\nSomething\n") + expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED) + AnnotateRoutes.do_annotations(position_in_routes: 'top') + end + end + + it 'leaves magic comment on top, adds an empty line between magic comment and annotation (position_in_routes :bottom)' do + expect(File).to receive(:open).with(ROUTE_FILE, 'wb') + .and_yield(mock_file).at_least(:once) + + magic_comments_list_each do |magic_comment| + expect(File).to receive(:read).with(ROUTE_FILE).and_return("#{magic_comment}\nSomething") + expect(@mock_file).to receive(:puts).with("#{magic_comment}\nSomething\n\n# == Route Map\n#\n") + expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED) + AnnotateRoutes.do_annotations(position_in_routes: 'bottom') + end + end + + it 'skips annotations if file does already contain annotation' do + magic_comments_list_each do |magic_comment| + expect(File).to receive(:read).with(ROUTE_FILE) + .and_return("#{magic_comment}\n\n# == Route Map\n#\n") + expect(AnnotateRoutes).to receive(:puts).with(FILE_UNCHANGED) + + AnnotateRoutes.do_annotations + end + end + end end describe 'When adding with older Rake versions' do From f1fe52e251f8f57a2cbd8129ae9a25ae7acc5519 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Thu, 19 Oct 2017 04:02:36 +0600 Subject: [PATCH 24/37] [Fix #345] annotate --delete should not ignore SkipSchemaAnnotations flag --- lib/annotate/annotate_models.rb | 8 ++++++-- spec/annotate/annotate_models_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index aaeee20d1..7b4d82eb7 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -12,6 +12,8 @@ module AnnotateModels PREFIX_MD = '## Schema Information'.freeze END_MARK = '== Schema Information End'.freeze + SKIP_ANNOTATION_PREFIX = '# -\*- SkipSchemaAnnotations'.freeze + MATCHED_TYPES = %w(test fixture factory serializer scaffold controller helper).freeze # File.join for windows reverse bar compat? @@ -498,7 +500,7 @@ def get_foreign_key_info(klass, options = {}) def annotate_one_file(file_name, info_block, position, options = {}) if File.exist?(file_name) old_content = File.read(file_name) - return false if old_content =~ /# -\*- SkipSchemaAnnotations.*\n/ + return false if old_content =~ /#{SKIP_ANNOTATION_PREFIX}.*\n/ # Ignore the Schema version line because it changes with each migration header_pattern = /(^# Table name:.*?\n(#.*[\r]?\n)*[\r]?)/ @@ -562,6 +564,8 @@ def magic_comments_as_string(content) def remove_annotation_of_file(file_name, options = {}) if File.exist?(file_name) content = File.read(file_name) + return false if content =~ /#{SKIP_ANNOTATION_PREFIX}.*\n/ + wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : '' content.sub!(/(#{wrapper_open})?#{annotate_pattern(options)}/, '') @@ -767,7 +771,7 @@ def do_annotations(options = {}) def annotate_model_file(annotated, file, header, options) begin - return false if /# -\*- SkipSchemaAnnotations.*/ =~ (File.exist?(file) ? File.read(file) : '') + return false if /#{SKIP_ANNOTATION_PREFIX}.*/ =~ (File.exist?(file) ? File.read(file) : '') klass = get_model_class(file) do_annotate = klass && klass < ActiveRecord::Base && diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 42963f871..ca136885d 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -1400,6 +1400,28 @@ class Foo < ActiveRecord::Base end EOS end + + it 'does not change file with #SkipSchemaAnnotations' do + content = <<-EOS +# -*- SkipSchemaAnnotations +# == Schema Information +# +# Table name: foo +# +# id :integer not null, primary key +# created_at :datetime +# updated_at :datetime +# + +class Foo < ActiveRecord::Base +end + EOS + + path = create 'skip.rb', content + + AnnotateModels.remove_annotation_of_file(path) + expect(content(path)).to eq(content) + end end describe '#resolve_filename' do From 4db90efe4b90f0d361eebb58b9b99c76c7b6ee18 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Thu, 19 Oct 2017 04:04:48 +0600 Subject: [PATCH 25/37] [Fix #438] model_dir option should allow multiple dirs with comma as described in README --- lib/annotate/annotate_models.rb | 6 +++++- spec/annotate/annotate_models_spec.rb | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 7b4d82eb7..c89afdf56 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -740,10 +740,14 @@ def get_loaded_model(model_path) end def parse_options(options = {}) - self.model_dir = options[:model_dir] if options[:model_dir] + self.model_dir = split_model_dir(options[:model_dir]) if options[:model_dir] self.root_dir = options[:root_dir] if options[:root_dir] end + def split_model_dir(option_value) + option_value.split(',').map(&:strip).reject(&:empty?) + end + # 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, diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index ca136885d..9eff90294 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -72,6 +72,31 @@ def mock_column(name, type, options = {}) it { expect(AnnotateModels.quote(BigDecimal.new('1.2'))).to eql('1.2') } it { expect(AnnotateModels.quote([BigDecimal.new('1.2')])).to eql(['1.2']) } + describe '#parse_options' do + let(:options) do + { + root_dir: '/root', + model_dir: 'app/models,app/one, app/two ,,app/three' + } + end + + it 'sets @root_dir' do + AnnotateModels.send(:parse_options, options) + expect(AnnotateModels.instance_variable_get(:@root_dir)).to eq('/root') + end + + it 'sets @model_dir separated with a comma' do + AnnotateModels.send(:parse_options, options) + expected = [ + 'app/models', + 'app/one', + 'app/two', + 'app/three' + ] + expect(AnnotateModels.instance_variable_get(:@model_dir)).to eq(expected) + end + end + it 'should get schema info with default options' do klass = mock_class(:users, :id, From c4ecc11721c686e56af23dc9dc574cea6848bb68 Mon Sep 17 00:00:00 2001 From: Alexander Belozerov Date: Fri, 20 Oct 2017 21:45:09 +0600 Subject: [PATCH 26/37] Additional new line between magic comment and annotation (AnnotateRoutes) (#524) --- lib/annotate/annotate_routes.rb | 3 +++ spec/annotate/annotate_routes_spec.rb | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/annotate/annotate_routes.rb b/lib/annotate/annotate_routes.rb index e04960113..276c2cbc1 100644 --- a/lib/annotate/annotate_routes.rb +++ b/lib/annotate/annotate_routes.rb @@ -39,11 +39,13 @@ def header(options = {}) routes_map = app_routes_map(options) magic_comments_map, routes_map = extract_magic_comments_from_array(routes_map) + out = [] magic_comments_map.each do |magic_comment| out << magic_comment end + out << '' if magic_comments_map.any? out += ["# #{options[:wrapper_open]}"] if options[:wrapper_open] @@ -174,6 +176,7 @@ def self.annotate_routes(header, content, where_header_found, options = {}) magic_comments_map, content = extract_magic_comments_from_array(content) if %w(before top).include?(options[:position_in_routes]) header = header << '' if content.first != '' + magic_comments_map << '' if magic_comments_map.any? new_content = magic_comments_map + header + content else # Ensure we have adequate trailing newlines at the end of the file to diff --git a/spec/annotate/annotate_routes_spec.rb b/spec/annotate/annotate_routes_spec.rb index 005f1ff70..1a34bbee0 100644 --- a/spec/annotate/annotate_routes_spec.rb +++ b/spec/annotate/annotate_routes_spec.rb @@ -106,6 +106,7 @@ def magic_comments_list_each expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) expect(@mock_file).to receive(:puts).with(" #{magic_comment} + # == Route Map # # Prefix Verb URI Pattern Controller#Action @@ -125,6 +126,7 @@ def magic_comments_list_each expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) expect(@mock_file).to receive(:puts).with(" #{magic_comment} + # ## Route Map # # Prefix | Verb | URI Pattern | Controller#Action @@ -144,6 +146,7 @@ def magic_comments_list_each expect(File).to receive(:open).with(ROUTE_FILE, 'wb').and_yield(mock_file) expect(@mock_file).to receive(:puts).with(" #{magic_comment} + # START # == Route Map # @@ -208,7 +211,7 @@ def magic_comments_list_each magic_comments_list_each do |magic_comment| expect(File).to receive(:read).with(ROUTE_FILE).and_return("#{magic_comment}\nSomething") - expect(@mock_file).to receive(:puts).with("#{magic_comment}\n# == Route Map\n#\n\nSomething\n") + expect(@mock_file).to receive(:puts).with("#{magic_comment}\n\n# == Route Map\n#\n\nSomething\n") expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED) AnnotateRoutes.do_annotations(position_in_routes: 'top') end From 7c7fef304d60a8cf5d9623650afe256050034709 Mon Sep 17 00:00:00 2001 From: AleksandrKudashkin Date: Wed, 25 Oct 2017 11:33:57 -0400 Subject: [PATCH 27/37] Update Readme Gemfile declaration (#526) --- README.rdoc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.rdoc b/README.rdoc index 57035de1c..d52d5acd1 100644 --- a/README.rdoc +++ b/README.rdoc @@ -55,11 +55,15 @@ Also, if you pass the -r option, it'll annotate routes.rb with the output of Into Gemfile from rubygems.org: - gem 'annotate', require: false + group :development do + gem 'annotate' + end Into Gemfile from Github: - gem 'annotate', git: 'https://github.com/ctran/annotate_models.git', require: false + group :development do + gem 'annotate', git: 'https://github.com/ctran/annotate_models.git' + end Into environment gems from rubygems.org: From ba95021ada15c0fe400488e4f023a0c9daf5d6ea Mon Sep 17 00:00:00 2001 From: Tony Ta Date: Tue, 31 Oct 2017 22:32:44 -0700 Subject: [PATCH 28/37] Redirect error messages to current standard error stream (#528) --- Rakefile | 2 +- lib/annotate.rb | 2 +- lib/annotate/annotate_models.rb | 22 ++++----- spec/annotate/annotate_models_spec.rb | 48 +++++++++---------- .../rails_2.3_with_bundler/config/boot.rb | 2 +- 5 files changed, 38 insertions(+), 38 deletions(-) diff --git a/Rakefile b/Rakefile index 65aae5e59..70228652b 100644 --- a/Rakefile +++ b/Rakefile @@ -28,7 +28,7 @@ require 'mg' begin MG.new('annotate.gemspec') rescue Exception - STDERR.puts("WARNING: Couldn't read gemspec. As such, a number of tasks may be unavailable to you until you run 'rake gem:gemspec' to correct the issue.") + $stderr.puts("WARNING: Couldn't read gemspec. As such, a number of tasks may be unavailable to you until you run 'rake gem:gemspec' to correct the issue.") # Gemspec is probably in a broken state, so let's give ourselves a chance to # build a new one... end diff --git a/lib/annotate.rb b/lib/annotate.rb index 675b6a3b2..856e4714f 100644 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -169,7 +169,7 @@ def self.bootstrap_rake require 'rake/dsl_definition' rescue StandardError => e # We might just be on an old version of Rake... - puts e.message + $stderr.puts e.message exit e.status_code end require 'rake' diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index c89afdf56..9c3e2887a 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -640,8 +640,8 @@ def annotate(klass, file, header, options = {}) end end rescue StandardError => e - puts "Unable to annotate #{file}: #{e.message}" - puts "\t" + e.backtrace.join("\n\t") if options[:trace] + $stderr.puts "Unable to annotate #{file}: #{e.message}" + $stderr.puts "\t" + e.backtrace.join("\n\t") if options[:trace] end annotated @@ -676,9 +676,9 @@ def get_model_files(options) model_files rescue SystemCallError - puts "No models found in directory '#{model_dir.join("', '")}'." - puts "Either specify models on the command line, or use the --model-dir option." - puts "Call 'annotate --help' for more info." + $stderr.puts "No models found in directory '#{model_dir.join("', '")}'." + $stderr.puts "Either specify models on the command line, or use the --model-dir option." + $stderr.puts "Call 'annotate --help' for more info." exit 1 end @@ -786,12 +786,12 @@ def annotate_model_file(annotated, file, header, options) annotated.concat(annotate(klass, file, header, options)) if do_annotate rescue BadModelFileError => e unless options[:ignore_unknown_models] - puts "Unable to annotate #{file}: #{e.message}" - puts "\t" + e.backtrace.join("\n\t") if options[:trace] + $stderr.puts "Unable to annotate #{file}: #{e.message}" + $stderr.puts "\t" + e.backtrace.join("\n\t") if options[:trace] end rescue StandardError => e - puts "Unable to annotate #{file}: #{e.message}" - puts "\t" + e.backtrace.join("\n\t") if options[:trace] + $stderr.puts "Unable to annotate #{file}: #{e.message}" + $stderr.puts "\t" + e.backtrace.join("\n\t") if options[:trace] end end @@ -821,8 +821,8 @@ def remove_annotations(options = {}) end deannotated << klass if deannotated_klass rescue StandardError => e - puts "Unable to deannotate #{File.join(file)}: #{e.message}" - puts "\t" + e.backtrace.join("\n\t") if options[:trace] + $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(', ')}" diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 9eff90294..4951b3be2 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -1676,22 +1676,22 @@ class User < ActiveRecord::Base EOS end - it 'displays an error message' do - expect(capturing(:stdout) do + it 'displays just the error message with trace disabled (default)' do + error_output = capturing(:stderr) do AnnotateModels.do_annotations model_dir: @model_dir, is_rake: true - end).to include("Unable to annotate #{@model_dir}/user.rb: oops") - end + end - it 'displays the full stack trace with --trace' do - expect(capturing(:stdout) do - AnnotateModels.do_annotations model_dir: @model_dir, trace: true, is_rake: true - end).to include('/spec/annotate/annotate_models_spec.rb:') + expect(error_output).to include("Unable to annotate #{@model_dir}/user.rb: oops") + expect(error_output).not_to include('/spec/annotate/annotate_models_spec.rb:') end - it 'omits the full stack trace without --trace' do - expect(capturing(:stdout) do - AnnotateModels.do_annotations model_dir: @model_dir, trace: false, is_rake: true - end).not_to include('/spec/annotate/annotate_models_spec.rb:') + it 'displays the error message and stacktrace with trace enabled' do + error_output = capturing(:stderr) do + AnnotateModels.do_annotations model_dir: @model_dir, is_rake: true, trace: true + end + + expect(error_output).to include("Unable to annotate #{@model_dir}/user.rb: oops") + expect(error_output).to include('/spec/annotate/annotate_models_spec.rb:') end end @@ -1706,22 +1706,22 @@ class User < ActiveRecord::Base EOS end - it 'displays an error message' do - expect(capturing(:stdout) do + it 'displays just the error message with trace disabled (default)' do + error_output = capturing(:stderr) do AnnotateModels.remove_annotations model_dir: @model_dir, is_rake: true - end).to include("Unable to deannotate #{@model_dir}/user.rb: oops") - end + end - it 'displays the full stack trace' do - expect(capturing(:stdout) do - AnnotateModels.remove_annotations model_dir: @model_dir, trace: true, is_rake: true - end).to include("/user.rb:2:in `'") + expect(error_output).to include("Unable to deannotate #{@model_dir}/user.rb: oops") + expect(error_output).not_to include("/user.rb:2:in `'") end - it 'omits the full stack trace without --trace' do - expect(capturing(:stdout) do - AnnotateModels.remove_annotations model_dir: @model_dir, trace: false, is_rake: true - end).not_to include("/user.rb:2:in `'") + it 'displays the error message and stacktrace with trace enabled' do + error_output = capturing(:stderr) do + AnnotateModels.remove_annotations model_dir: @model_dir, is_rake: true, trace: true + end + + expect(error_output).to include("Unable to deannotate #{@model_dir}/user.rb: oops") + expect(error_output).to include("/user.rb:2:in `'") end end end diff --git a/spec/integration/rails_2.3_with_bundler/config/boot.rb b/spec/integration/rails_2.3_with_bundler/config/boot.rb index 10592d802..190b37add 100644 --- a/spec/integration/rails_2.3_with_bundler/config/boot.rb +++ b/spec/integration/rails_2.3_with_bundler/config/boot.rb @@ -63,7 +63,7 @@ def load_rails_gem end rescue Gem::LoadError => load_error if load_error.message =~ /Could not find RubyGem rails/ - STDERR.puts %(Missing the Rails #{version} gem. Please `gem install -v=#{version} rails`, update your RAILS_GEM_VERSION setting in config/environment.rb for the Rails version you do have installed, or comment out RAILS_GEM_VERSION to use the latest version installed.) + $stderr.puts "Missing the Rails #{version} gem. Please `gem install -v=#{version} rails`, update your RAILS_GEM_VERSION setting in config/environment.rb for the Rails version you do have installed, or comment out RAILS_GEM_VERSION to use the latest version installed." exit 1 else raise From 9cdf2d34e9f394b533a14b7c890eada643f3d6c2 Mon Sep 17 00:00:00 2001 From: 0x01f7 <0x01f7@users.noreply.github.com> Date: Sat, 2 Dec 2017 01:05:38 +0800 Subject: [PATCH 29/37] Fix parsing bug in bin/annotate (#531) (#534) --- lib/annotate/annotate_models.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index 9c3e2887a..c70b5d710 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -745,7 +745,8 @@ def parse_options(options = {}) end def split_model_dir(option_value) - option_value.split(',').map(&:strip).reject(&:empty?) + option_value = option_value.is_a?(Array) ? option_value : option_value.split(',') + option_value.map(&:strip).reject(&:empty?) end # We're passed a name of things that might be From e3e0bbe9fdcc23db49862cb1f9ce239e765e63b1 Mon Sep 17 00:00:00 2001 From: 0x01f7 <0x01f7@users.noreply.github.com> Date: Mon, 4 Dec 2017 08:29:01 +0800 Subject: [PATCH 30/37] Try to get proper loaded model when using Rails eager_load_paths (#535) --- lib/annotate/annotate_models.rb | 31 ++++++++++++++++----------- spec/annotate/annotate_models_spec.rb | 26 ++++++++++++++++++---- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index c70b5d710..c9d11659c 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -711,11 +711,11 @@ def get_model_class(file) model_path = file.gsub(/\.rb$/, '') model_dir.each { |dir| model_path = model_path.gsub(/^#{dir}/, '').gsub(/^\//, '') } begin - get_loaded_model(model_path) || raise(BadModelFileError.new) + get_loaded_model(model_path, file) || raise(BadModelFileError.new) rescue LoadError # this is for non-rails projects, which don't get Rails auto-require magic file_path = File.expand_path(file) - if File.file?(file_path) && silence_warnings { Kernel.require(file_path) } + if File.file?(file_path) && Kernel.require(file_path) retry elsif model_path =~ /\// model_path = model_path.split('/')[1..-1].join('/').to_s @@ -726,8 +726,24 @@ def get_model_class(file) end end + # Retrieve loaded model class + def get_loaded_model(model_path, file) + loaded_model_class = get_loaded_model_by_path(model_path) + return loaded_model_class if loaded_model_class + + # We cannot get loaded model when `model_path` is loaded by Rails + # auto_load/eager_load paths. Try all possible model paths one by one. + absolute_file = File.expand_path(file) + model_paths = + $LOAD_PATH.select { |path| absolute_file.include?(path) } + .map { |path| absolute_file.sub(path, '').sub(/\.rb$/, '').sub(/^\//, '') } + model_paths + .map { |path| get_loaded_model_by_path(path) } + .find { |loaded_model| !loaded_model.nil? } + end + # Retrieve loaded model class by path to the file where it's supposed to be defined. - def get_loaded_model(model_path) + def get_loaded_model_by_path(model_path) ActiveSupport::Inflector.constantize(ActiveSupport::Inflector.camelize(model_path)) rescue StandardError, LoadError # Revert to the old way but it is not really robust @@ -858,15 +874,6 @@ def classified_sort(cols) ([id] << rest_cols << timestamps << associations).flatten.compact end - # Ignore warnings for the duration of the block () - def silence_warnings - old_verbose = $VERBOSE - $VERBOSE = nil - yield - ensure - $VERBOSE = old_verbose - end - private def with_comments?(klass, options) diff --git a/spec/annotate/annotate_models_spec.rb b/spec/annotate/annotate_models_spec.rb index 4951b3be2..c1baace01 100644 --- a/spec/annotate/annotate_models_spec.rb +++ b/spec/annotate/annotate_models_spec.rb @@ -1257,11 +1257,29 @@ class LoadedClass < ActiveRecord::Base EOS path = File.expand_path('loaded_class', AnnotateModels.model_dir[0]) Kernel.load "#{path}.rb" - expect(Kernel).not_to receive(:require).with(path) + expect(Kernel).not_to receive(:require) expect(capturing(:stderr) do check_class_name 'loaded_class.rb', 'LoadedClass' - end).not_to include('warning: already initialized constant LoadedClass::CONSTANT') + end).to be_blank + end + + it 'should not require model files twice which is inside a subdirectory' do + dir = Array.new(8) { (0..9).to_a.sample(random: Random.new) }.join + $LOAD_PATH.unshift(File.join(AnnotateModels.model_dir[0], dir)) + + create "#{dir}/subdir_loaded_class.rb", <<-EOS + class SubdirLoadedClass < ActiveRecord::Base + CONSTANT = 1 + end + EOS + path = File.expand_path("#{dir}/subdir_loaded_class", AnnotateModels.model_dir[0]) + Kernel.load "#{path}.rb" + expect(Kernel).not_to receive(:require) + + expect(capturing(:stderr) do + check_class_name "#{dir}/subdir_loaded_class.rb", 'SubdirLoadedClass' + end).to be_blank end end @@ -1667,7 +1685,7 @@ class User < ActiveRecord::Base describe "if a file can't be annotated" do before do - allow(AnnotateModels).to receive(:get_loaded_model).with('user').and_return(nil) + allow(AnnotateModels).to receive(:get_loaded_model_by_path).with('user').and_return(nil) write_model('user.rb', <<-EOS) class User < ActiveRecord::Base @@ -1697,7 +1715,7 @@ class User < ActiveRecord::Base describe "if a file can't be deannotated" do before do - allow(AnnotateModels).to receive(:get_loaded_model).with('user').and_return(nil) + allow(AnnotateModels).to receive(:get_loaded_model_by_path).with('user').and_return(nil) write_model('user.rb', <<-EOS) class User < ActiveRecord::Base From c8ee0910a324b87e6ab3e044c3e6a7facf85bc70 Mon Sep 17 00:00:00 2001 From: ramaboo-deliv <32721566+ramaboo-deliv@users.noreply.github.com> Date: Thu, 22 Feb 2018 23:49:31 -0800 Subject: [PATCH 31/37] Update auto_annotate_models.rake (#542) --- lib/generators/annotate/templates/auto_annotate_models.rake | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/generators/annotate/templates/auto_annotate_models.rake b/lib/generators/annotate/templates/auto_annotate_models.rake index 6f7c00641..72b5844c7 100644 --- a/lib/generators/annotate/templates/auto_annotate_models.rake +++ b/lib/generators/annotate/templates/auto_annotate_models.rake @@ -42,6 +42,7 @@ if Rails.env.development? 'format_markdown' => 'false', 'sort' => 'false', 'force' => 'false', + 'classified_sort' => 'true', 'trace' => 'false', 'wrapper_open' => nil, 'wrapper_close' => nil, From 3c90753a44598b0b1d63ecad2084a56a479b3d4f Mon Sep 17 00:00:00 2001 From: Manuel Meurer Date: Fri, 23 Feb 2018 08:53:04 +0100 Subject: [PATCH 32/37] add spec for #413 (#533) --- .../rails_4.2.0/app/models/sub1/no_namespace.rb | 3 +++ .../db/migrate/20140705000020_create_no_namespaces.rb | 8 ++++++++ 2 files changed, 11 insertions(+) create mode 100644 spec/integration/rails_4.2.0/app/models/sub1/no_namespace.rb create mode 100644 spec/integration/rails_4.2.0/db/migrate/20140705000020_create_no_namespaces.rb diff --git a/spec/integration/rails_4.2.0/app/models/sub1/no_namespace.rb b/spec/integration/rails_4.2.0/app/models/sub1/no_namespace.rb new file mode 100644 index 000000000..0a72e0830 --- /dev/null +++ b/spec/integration/rails_4.2.0/app/models/sub1/no_namespace.rb @@ -0,0 +1,3 @@ +class NoNamespace < ActiveRecord::Base + enum foo: [:bar, :baz] +end diff --git a/spec/integration/rails_4.2.0/db/migrate/20140705000020_create_no_namespaces.rb b/spec/integration/rails_4.2.0/db/migrate/20140705000020_create_no_namespaces.rb new file mode 100644 index 000000000..64a5b4cec --- /dev/null +++ b/spec/integration/rails_4.2.0/db/migrate/20140705000020_create_no_namespaces.rb @@ -0,0 +1,8 @@ +class CreateUsers < ActiveRecord::Migration + def change + create_table :no_namespaces do |t| + t.integer :foo + t.timestamps + end + end +end From 3cfde3727795efe04c1a62bd5de199e0b01cfff5 Mon Sep 17 00:00:00 2001 From: Shou Ya Date: Fri, 23 Mar 2018 10:16:21 +0800 Subject: [PATCH 33/37] Escape newline character in index expression (#545) --- lib/annotate/annotate_models.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/annotate/annotate_models.rb b/lib/annotate/annotate_models.rb index c9d11659c..eb78def63 100644 --- a/lib/annotate/annotate_models.rb +++ b/lib/annotate/annotate_models.rb @@ -374,7 +374,7 @@ def index_columns_info(index) if index.try(:orders) && index.orders[col.to_s] "#{col} #{index.orders[col.to_s].upcase}" else - col.to_s + col.to_s.gsub("\r", '\r').gsub("\n", '\n') end end end From 4d77e24deebca64cba81473042c1a46db434a9a8 Mon Sep 17 00:00:00 2001 From: Trevor <673838+trevorrjohn@users.noreply.github.com> Date: Fri, 20 Apr 2018 14:56:53 -0400 Subject: [PATCH 34/37] Use ANNOTATE_SKIP_ON_DB_MIGRATE ENV variable (#553) --- README.rdoc | 2 +- lib/annotate.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rdoc b/README.rdoc index d52d5acd1..c282c82b3 100644 --- a/README.rdoc +++ b/README.rdoc @@ -158,7 +158,7 @@ If you want to run rake db:migrate as a one-off without running ann you can do so with a simple environment variable, instead of editing the +.rake+ file: - skip_on_db_migrate=1 rake db:migrate + ANNOTATE_SKIP_ON_DB_MIGRATE=1 rake db:migrate == Options diff --git a/lib/annotate.rb b/lib/annotate.rb index 856e4714f..4a0ebb5c0 100644 --- a/lib/annotate.rb +++ b/lib/annotate.rb @@ -106,7 +106,7 @@ def self.reset_options end def self.skip_on_migration? - ENV['skip_on_db_migrate'] =~ TRUE_RE + ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ TRUE_RE || ENV['skip_on_db_migrate'] =~ TRUE_RE end def self.include_routes? From 85a9473e03d5a208b037c8f8ef0a15de61dddd32 Mon Sep 17 00:00:00 2001 From: Greg Myers Date: Fri, 20 Apr 2018 19:58:20 +0100 Subject: [PATCH 35/37] Update README, FactoryGirl -> FactoryBot (#550) https://github.com/thoughtbot/factory_bot/blob/master/NAME.md --- README.rdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rdoc b/README.rdoc index c282c82b3..5e4582df0 100644 --- a/README.rdoc +++ b/README.rdoc @@ -17,7 +17,7 @@ your... - Object Daddy exemplars - Machinist blueprints - Fabrication fabricators -- Thoughtbot's factory_girl factories, i.e. the (spec|test)/factories/_factory.rb files +- Thoughtbot's factory_bot factories, i.e. the (spec|test)/factories/_factory.rb files - routes.rb file (for Rails projects) The schema comment looks like this: @@ -258,7 +258,7 @@ extra carefully, and consider using one. == Links -- Factory Girl: http://github.com/thoughtbot/factory_girl +- Factory Bot: http://github.com/thoughtbot/factory_bot - Object Daddy: http://github.com/flogic/object_daddy - Machinist: http://github.com/notahat/machinist - Fabrication: http://github.com/paulelliott/fabrication From a9a31217566fbf1645a1af5ea6755e1ccb978589 Mon Sep 17 00:00:00 2001 From: Cuong Tran Date: Fri, 20 Apr 2018 14:04:55 -0700 Subject: [PATCH 36/37] Auto deploy to rubygems on tags (#554) --- .travis.yml | 24 ++++++++++++++++-------- Gemfile | 1 + 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index c1b3e3826..dd62bb17b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,15 +1,23 @@ sudo: false language: ruby rvm: - - 2.2.7 - - 2.3.4 - - 2.4.1 - - ruby-head +- 2.2.7 +- 2.3.4 +- 2.4.1 +- ruby-head matrix: allow_failures: - - rvm: ruby-head + - rvm: ruby-head before_install: - - gem update --system - - gem update bundler +- gem update --system +- gem update bundler script: - - bundle exec rubocop && bundle exec rspec +- bundle exec rubocop && bundle exec rspec +deploy: + provider: rubygems + api_key: + secure: Y7DUitak26kcRAAkgph/7m6Y1wHeObD0BelSSJbmCfjkRd/qaVy7fz9VvHL9zxlRJtLGVHInyCnwcfzinibY6OFd3MoMYHKv8GFa2LxLJNEVSY46KQYFxfH5JTg1ejh6ldoJRRBoeOx9dcWS80pRNjYMKPGnpSz7yDBl1azibFs= + gem: annotate + on: + tags: true + repo: ctran/annotate_models diff --git a/Gemfile b/Gemfile index f6e85c21e..8b466be60 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ gem 'rake', require: false group :development do gem 'bump' gem 'mg', require: false + gem 'travis', require: false platforms :mri, :mingw do gem 'yard', require: false end From db3eb14c40b51eb65ad3e972ebb0d096ad61bff0 Mon Sep 17 00:00:00 2001 From: "cuong.tran" Date: Sat, 21 Apr 2018 10:26:32 -0700 Subject: [PATCH 37/37] v2.7.3 --- CHANGELOG.rdoc | 3 +++ lib/annotate/version.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index bd6ce32ff..2db610007 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,3 +1,6 @@ +== 2.7.3 +See https://github.com/ctran/annotate_models/releases/tag/v2.7.3 + == 2.7.2 See https://github.com/ctran/annotate_models/releases/tag/v2.7.2 diff --git a/lib/annotate/version.rb b/lib/annotate/version.rb index fd53beb47..39df253f0 100644 --- a/lib/annotate/version.rb +++ b/lib/annotate/version.rb @@ -1,5 +1,5 @@ module Annotate def self.version - '2.7.2' + '2.7.3' end end