Upgrade rubocop and address lints

Author: petergoldstein

.rubocop.yml

@@ -13,6 +13,9 @@ AllCops:
     - 'spec/integration/**/*'
   NewCops: enable
 
+Lint/FormatParameterMismatch:
+  Enabled: false
+
 Metrics/BlockLength:
   Exclude:
     - 'spec/**/*.rb'

Gemfile

@@ -18,13 +18,12 @@ group :development, :test do
   gem 'guard-rspec', require: false
   gem 'rspec', require: false
 
-  gem 'rubocop', '~> 1.12.0', require: false
+  gem 'rubocop', '~> 1.48.0', require: false
   gem 'rubocop-rake', require: false
-  gem 'rubocop-rspec', '~> 2.2.0', require: false
+  gem 'rubocop-rspec', '~> 2.19.0', require: false
   gem 'simplecov', require: false
   gem 'terminal-notifier-guard', require: false
 
-
   gem 'overcommit'
 
   platforms :mri, :mingw do

annotate.gemspec

@@ -18,15 +18,14 @@ Gem::Specification.new do |s|
   s.homepage = 'https://github.com/ctran/annotate_models'
   s.licenses = ['Ruby']
   s.require_paths = ['lib']
-  s.rubygems_version = '2.1.11'
   s.summary = 'Annotates Rails Models, routes, fixtures, and others based on the database schema.'
 
-  s.specification_version = 4 if s.respond_to? :specification_version
   s.add_runtime_dependency(%q<rake>, '>= 10.4', '< 14.0')
   s.add_runtime_dependency(%q<activerecord>, ['>= 3.2', '< 8.0'])
 
   s.metadata = {
     "bug_tracker_uri" => "https://github.com/ctran/annotate_models/issues/",
-    "source_code_uri" => "https://github.com/ctran/annotate_models.git"
+    "source_code_uri" => "https://github.com/ctran/annotate_models.git",
+    'rubygems_mfa_required' => 'true'
   }
 end

bin/annotate

@@ -24,7 +24,7 @@ options_result = Annotate::Parser.parse(ARGV)
 exit if options_result[:exit]
 
 options = Annotate.setup_options(
-  is_rake: ENV['is_rake'] && !ENV['is_rake'].empty?
+  is_rake: ENV.fetch('is_rake', nil) && !ENV['is_rake'].empty?
 )
 Annotate.eager_load(options) if Annotate::Helpers.include_models?
 

lib/annotate.rb

@@ -44,13 +44,13 @@ def self.set_defaults(options = {})
   #
   def self.setup_options(options = {})
     Constants::POSITION_OPTIONS.each do |key|
-      options[key] = Annotate::Helpers.fallback(ENV[key.to_s], ENV['position'], 'before')
+      options[key] = Annotate::Helpers.fallback(ENV.fetch(key.to_s, nil), ENV.fetch('position', nil), 'before')
     end
     Constants::FLAG_OPTIONS.each do |key|
-      options[key] = Annotate::Helpers.true?(ENV[key.to_s])
+      options[key] = Annotate::Helpers.true?(ENV.fetch(key.to_s, nil))
     end
     Constants::OTHER_OPTIONS.each do |key|
-      options[key] = !ENV[key.to_s].blank? ? ENV[key.to_s] : nil
+      options[key] = !ENV[key.to_s].blank? ? ENV.fetch(key.to_s, nil) : nil
     end
     Constants::PATH_OPTIONS.each do |key|
       options[key] = !ENV[key.to_s].blank? ? ENV[key.to_s].split(',') : []

lib/annotate/annotate_models.rb

@@ -155,7 +155,7 @@ def get_schema_info(klass, header, options = {}) # rubocop:disable Metrics/Metho
       with_comments_column = with_comments_column?(klass, options)
 
       # Precalculate Values
-      cols_meta = cols.map do |col|
+      cols_meta = cols.to_h do |col|
         col_comment = with_comments || with_comments_column ? col.comment&.gsub(/\n/, "\\n") : nil
         col_type = get_col_type(col)
         attrs = get_attributes(col, col_type, klass, options)
@@ -166,7 +166,7 @@ def get_schema_info(klass, header, options = {}) # rubocop:disable Metrics/Metho
                    end
         simple_formatted_attrs = attrs.join(", ")
         [col.name, { col_type: col_type, attrs: attrs, col_name: col_name, simple_formatted_attrs: simple_formatted_attrs, col_comment: col_comment }]
-      end.to_h
+      end
 
       # Output annotation
       bare_max_attrs_length = cols_meta.map { |_, m| m[:simple_formatted_attrs].length }.max
@@ -179,15 +179,15 @@ def get_schema_info(klass, header, options = {}) # rubocop:disable Metrics/Metho
         col_comment = cols_meta[col.name][:col_comment]
 
         if options[:format_rdoc]
-          info << sprintf("# %-#{max_size}.#{max_size}s<tt>%s</tt>", "*#{col_name}*::", attrs.unshift(col_type).join(", ")).rstrip + "\n"
+          info << (sprintf("# %-#{max_size}.#{max_size}s<tt>%s</tt>", "*#{col_name}*::", attrs.unshift(col_type).join(", ")).rstrip + "\n")
         elsif options[:format_yard]
-          info << sprintf("# @!attribute #{col_name}") + "\n"
+          info << (sprintf("# @!attribute #{col_name}") + "\n")
           ruby_class = col.respond_to?(:array) && col.array ? "Array<#{map_col_type_to_ruby_classes(col_type)}>": map_col_type_to_ruby_classes(col_type)
-          info << sprintf("#   @return [#{ruby_class}]") + "\n"
+          info << (sprintf("#   @return [#{ruby_class}]") + "\n")
         elsif options[:format_markdown]
           name_remainder = max_size - col_name.length - non_ascii_length(col_name)
           type_remainder = (md_type_allowance - 2) - col_type.length
-          info << (sprintf("# **`%s`**%#{name_remainder}s | `%s`%#{type_remainder}s | `%s`", col_name, " ", col_type, " ", attrs.join(", ").rstrip)).gsub('``', '  ').rstrip + "\n"
+          info << ((sprintf("# **`%s`**%#{name_remainder}s | `%s`%#{type_remainder}s | `%s`", col_name, " ", col_type, " ", attrs.join(", ").rstrip)).gsub('``', '  ').rstrip + "\n")
         elsif with_comments_column
           info << format_default(col_name, max_size, col_type, bare_type_allowance, simple_formatted_attrs, bare_max_attrs_length, col_comment)
         else

lib/annotate/annotate_routes/header_generator.rb

@@ -29,7 +29,7 @@ def routes_map(options)
         # Skip routes which match given regex
         # Note: it matches the complete line (route_name, path, controller/action)
         if regexp_for_ignoring_routes
-          result.reject { |line| line =~ regexp_for_ignoring_routes }
+          result.grep_v(regexp_for_ignoring_routes)
         else
           result
         end
@@ -53,9 +53,9 @@ def generate
 
       out << comment(options[:wrapper_open]) if options[:wrapper_open]
 
-      out << comment(markdown? ? PREFIX_MD : PREFIX) + timestamp_if_required
+      out << (comment(markdown? ? PREFIX_MD : PREFIX) + timestamp_if_required)
       out << comment
-      return out if contents_without_magic_comments.size.zero?
+      return out if contents_without_magic_comments.empty?
 
       maxs = [HEADER_ROW.map(&:size)] + contents_without_magic_comments[1..-1].map { |line| line.split.map(&:size) }
 

lib/annotate/helpers.rb

@@ -3,15 +3,15 @@ module Annotate
   class Helpers
     class << self
       def skip_on_migration?
-        ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ Constants::TRUE_RE || ENV['skip_on_db_migrate'] =~ Constants::TRUE_RE
+        ENV.fetch('ANNOTATE_SKIP_ON_DB_MIGRATE', nil) =~ Constants::TRUE_RE || ENV.fetch('skip_on_db_migrate', nil) =~ Constants::TRUE_RE
       end
 
       def include_routes?
-        ENV['routes'] =~ Constants::TRUE_RE
+        ENV.fetch('routes', nil) =~ Constants::TRUE_RE
       end
 
       def include_models?
-        ENV['models'] =~ Constants::TRUE_RE
+        ENV.fetch('models', nil) =~ Constants::TRUE_RE
       end
 
       def true?(val)

lib/tasks/annotate_models.rake

@@ -11,48 +11,48 @@ task annotate_models: :environment do
   require "#{annotate_lib}/annotate/active_record_patch"
 
   options = {is_rake: true}
-  ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV['position'], 'before')
+  ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV.fetch('position', nil), 'before')
   options[:additional_file_patterns] = ENV['additional_file_patterns'] ? ENV['additional_file_patterns'].split(',') : []
-  options[:position_in_class] = Annotate::Helpers.fallback(ENV['position_in_class'], ENV['position'])
-  options[:position_in_fixture] = Annotate::Helpers.fallback(ENV['position_in_fixture'], ENV['position'])
-  options[:position_in_factory] = Annotate::Helpers.fallback(ENV['position_in_factory'], ENV['position'])
-  options[:position_in_test] = Annotate::Helpers.fallback(ENV['position_in_test'], ENV['position'])
-  options[:position_in_serializer] = Annotate::Helpers.fallback(ENV['position_in_serializer'], ENV['position'])
-  options[:show_check_constraints] = Annotate::Helpers.true?(ENV['show_check_constraints'])
-  options[:show_foreign_keys] = Annotate::Helpers.true?(ENV['show_foreign_keys'])
-  options[:show_complete_foreign_keys] = Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])
-  options[:show_indexes] = Annotate::Helpers.true?(ENV['show_indexes'])
-  options[:simple_indexes] = Annotate::Helpers.true?(ENV['simple_indexes'])
+  options[:position_in_class] = Annotate::Helpers.fallback(ENV.fetch('position_in_class', nil), ENV.fetch('position', nil))
+  options[:position_in_fixture] = Annotate::Helpers.fallback(ENV.fetch('position_in_fixture', nil), ENV.fetch('position', nil))
+  options[:position_in_factory] = Annotate::Helpers.fallback(ENV.fetch('position_in_factory', nil), ENV.fetch('position', nil))
+  options[:position_in_test] = Annotate::Helpers.fallback(ENV.fetch('position_in_test', nil), ENV.fetch('position', nil))
+  options[:position_in_serializer] = Annotate::Helpers.fallback(ENV.fetch('position_in_serializer', nil), ENV.fetch('position', nil))
+  options[:show_check_constraints] = Annotate::Helpers.true?(ENV.fetch('show_check_constraints', nil))
+  options[:show_foreign_keys] = Annotate::Helpers.true?(ENV.fetch('show_foreign_keys', nil))
+  options[:show_complete_foreign_keys] = Annotate::Helpers.true?(ENV.fetch('show_complete_foreign_keys', nil))
+  options[:show_indexes] = Annotate::Helpers.true?(ENV.fetch('show_indexes', nil))
+  options[:simple_indexes] = Annotate::Helpers.true?(ENV.fetch('simple_indexes', nil))
   options[:model_dir] = ENV['model_dir'] ? ENV['model_dir'].split(',') : ['app/models']
-  options[:root_dir] = ENV['root_dir']
-  options[:include_version] = Annotate::Helpers.true?(ENV['include_version'])
+  options[:root_dir] = ENV.fetch('root_dir', nil)
+  options[:include_version] = Annotate::Helpers.true?(ENV.fetch('include_version', nil))
   options[:require] = ENV['require'] ? ENV['require'].split(',') : []
-  options[:exclude_tests] = Annotate::Helpers.true?(ENV['exclude_tests'])
-  options[:exclude_factories] = Annotate::Helpers.true?(ENV['exclude_factories'])
-  options[:exclude_fixtures] = Annotate::Helpers.true?(ENV['exclude_fixtures'])
-  options[:exclude_serializers] = Annotate::Helpers.true?(ENV['exclude_serializers'])
-  options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV['exclude_scaffolds'])
+  options[:exclude_tests] = Annotate::Helpers.true?(ENV.fetch('exclude_tests', nil))
+  options[:exclude_factories] = Annotate::Helpers.true?(ENV.fetch('exclude_factories', nil))
+  options[:exclude_fixtures] = Annotate::Helpers.true?(ENV.fetch('exclude_fixtures', nil))
+  options[:exclude_serializers] = Annotate::Helpers.true?(ENV.fetch('exclude_serializers', nil))
+  options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV.fetch('exclude_scaffolds', nil))
   options[:exclude_controllers] = Annotate::Helpers.true?(ENV.fetch('exclude_controllers', 'true'))
   options[:exclude_helpers] = Annotate::Helpers.true?(ENV.fetch('exclude_helpers', 'true'))
-  options[:exclude_sti_subclasses] = Annotate::Helpers.true?(ENV['exclude_sti_subclasses'])
-  options[:ignore_model_sub_dir] = Annotate::Helpers.true?(ENV['ignore_model_sub_dir'])
-  options[:format_bare] = Annotate::Helpers.true?(ENV['format_bare'])
-  options[:format_rdoc] = Annotate::Helpers.true?(ENV['format_rdoc'])
-  options[:format_yard] = Annotate::Helpers.true?(ENV['format_yard'])
-  options[:format_markdown] = Annotate::Helpers.true?(ENV['format_markdown'])
-  options[:sort] = Annotate::Helpers.true?(ENV['sort'])
-  options[:force] = Annotate::Helpers.true?(ENV['force'])
-  options[:frozen] = Annotate::Helpers.true?(ENV['frozen'])
-  options[:classified_sort] = Annotate::Helpers.true?(ENV['classified_sort'])
-  options[:trace] = Annotate::Helpers.true?(ENV['trace'])
-  options[:wrapper_open] = Annotate::Helpers.fallback(ENV['wrapper_open'], ENV['wrapper'])
-  options[:wrapper_close] = Annotate::Helpers.fallback(ENV['wrapper_close'], ENV['wrapper'])
+  options[:exclude_sti_subclasses] = Annotate::Helpers.true?(ENV.fetch('exclude_sti_subclasses', nil))
+  options[:ignore_model_sub_dir] = Annotate::Helpers.true?(ENV.fetch('ignore_model_sub_dir', nil))
+  options[:format_bare] = Annotate::Helpers.true?(ENV.fetch('format_bare', nil))
+  options[:format_rdoc] = Annotate::Helpers.true?(ENV.fetch('format_rdoc', nil))
+  options[:format_yard] = Annotate::Helpers.true?(ENV.fetch('format_yard', nil))
+  options[:format_markdown] = Annotate::Helpers.true?(ENV.fetch('format_markdown', nil))
+  options[:sort] = Annotate::Helpers.true?(ENV.fetch('sort', nil))
+  options[:force] = Annotate::Helpers.true?(ENV.fetch('force', nil))
+  options[:frozen] = Annotate::Helpers.true?(ENV.fetch('frozen', nil))
+  options[:classified_sort] = Annotate::Helpers.true?(ENV.fetch('classified_sort', nil))
+  options[:trace] = Annotate::Helpers.true?(ENV.fetch('trace', nil))
+  options[:wrapper_open] = Annotate::Helpers.fallback(ENV.fetch('wrapper_open', nil), ENV.fetch('wrapper', nil))
+  options[:wrapper_close] = Annotate::Helpers.fallback(ENV.fetch('wrapper_close', nil), ENV.fetch('wrapper', nil))
   options[:ignore_columns] = ENV.fetch('ignore_columns', nil)
   options[:ignore_routes] = ENV.fetch('ignore_routes', nil)
-  options[:hide_limit_column_types] = Annotate::Helpers.fallback(ENV['hide_limit_column_types'], '')
-  options[:hide_default_column_types] = Annotate::Helpers.fallback(ENV['hide_default_column_types'], '')
-  options[:with_comment] = Annotate::Helpers.true?(ENV['with_comment'])
-  options[:with_comment_column] = Annotate::Helpers.true?(ENV['with_comment_column'])
+  options[:hide_limit_column_types] = Annotate::Helpers.fallback(ENV.fetch('hide_limit_column_types', nil), '')
+  options[:hide_default_column_types] = Annotate::Helpers.fallback(ENV.fetch('hide_default_column_types', nil), '')
+  options[:with_comment] = Annotate::Helpers.true?(ENV.fetch('with_comment', nil))
+  options[:with_comment_column] = Annotate::Helpers.true?(ENV.fetch('with_comment_column', nil))
   options[:ignore_unknown_models] = Annotate::Helpers.true?(ENV.fetch('ignore_unknown_models', 'false'))
 
   AnnotateModels.do_annotations(options)
@@ -64,9 +64,9 @@ task remove_annotation: :environment do
   require "#{annotate_lib}/annotate/active_record_patch"
 
   options = {is_rake: true}
-  options[:model_dir] = ENV['model_dir']
-  options[:root_dir] = ENV['root_dir']
+  options[:model_dir] = ENV.fetch('model_dir', nil)
+  options[:root_dir] = ENV.fetch('root_dir', nil)
   options[:require] = ENV['require'] ? ENV['require'].split(',') : []
-  options[:trace] = Annotate::Helpers.true?(ENV['trace'])
+  options[:trace] = Annotate::Helpers.true?(ENV.fetch('trace', nil))
   AnnotateModels.remove_annotations(options)
 end

lib/tasks/annotate_routes.rake

@@ -10,13 +10,13 @@ task :annotate_routes => :environment do
   require "#{annotate_lib}/annotate/annotate_routes"
 
   options={}
-  ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV['position'], 'before')
-  options[:position_in_routes] = Annotate::Helpers.fallback(ENV['position_in_routes'], ENV['position'])
-  options[:ignore_routes] = Annotate::Helpers.fallback(ENV['ignore_routes'],  nil)
+  ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV.fetch('position', nil), 'before')
+  options[:position_in_routes] = Annotate::Helpers.fallback(ENV.fetch('position_in_routes', nil), ENV.fetch('position', nil))
+  options[:ignore_routes] = Annotate::Helpers.fallback(ENV.fetch('ignore_routes', nil),  nil)
   options[:require] = ENV['require'] ? ENV['require'].split(',') : []
-  options[:frozen] = Annotate::Helpers.true?(ENV['frozen'])
-  options[:wrapper_open] = Annotate::Helpers.fallback(ENV['wrapper_open'], ENV['wrapper'])
-  options[:wrapper_close] = Annotate::Helpers.fallback(ENV['wrapper_close'], ENV['wrapper'])
+  options[:frozen] = Annotate::Helpers.true?(ENV.fetch('frozen', nil))
+  options[:wrapper_open] = Annotate::Helpers.fallback(ENV.fetch('wrapper_open', nil), ENV.fetch('wrapper', nil))
+  options[:wrapper_close] = Annotate::Helpers.fallback(ENV.fetch('wrapper_close', nil), ENV.fetch('wrapper', nil))
   AnnotateRoutes.do_annotations(options)
 end
 

spec/lib/annotate/annotate_models_spec.rb

@@ -205,7 +205,7 @@ def mock_column(name, type, options = {})
         end
 
         it 'sets skip_subdirectory_model_load to true' do
-          is_expected.to eq(true)
+          is_expected.to be(true)
         end
       end
 
@@ -219,7 +219,7 @@ def mock_column(name, type, options = {})
         end
 
         it 'sets skip_subdirectory_model_load to false' do
-          is_expected.to eq(false)
+          is_expected.to be(false)
         end
       end
     end
@@ -1943,7 +1943,7 @@ def mock_column(name, type, options = {})
 
   describe '.set_defaults' do
     subject do
-      Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])
+      Annotate::Helpers.true?(ENV.fetch('show_complete_foreign_keys', nil))
     end
 
     after :each do
@@ -2838,7 +2838,7 @@ class User < ActiveRecord::Base
     def write_model(file_name, file_content)
       fname = File.join(@model_dir, file_name)
       FileUtils.mkdir_p(File.dirname(fname))
-      File.open(fname, 'wb') { |f| f.write file_content }
+      File.binwrite(fname, file_content)
 
       [fname, file_content]
     end
@@ -3089,19 +3089,19 @@ class User < ActiveRecord::Base
     end
 
     describe 'frozen option' do
-      it "should abort without existing annotation when frozen: true " do
+      it "should abort without existing annotation when frozen: true" do
         expect { annotate_one_file frozen: true }.to raise_error SystemExit, /user.rb needs to be updated, but annotate was run with `--frozen`./
       end
 
-      it "should abort with different annotation when frozen: true " do
+      it "should abort with different annotation when frozen: true" do
         annotate_one_file
         another_schema_info = AnnotateModels.get_schema_info(mock_class(:users, :id, [mock_column(:id, :integer)]), '== Schema Info')
         @schema_info = another_schema_info
 
         expect { annotate_one_file frozen: true }.to raise_error SystemExit, /user.rb needs to be updated, but annotate was run with `--frozen`./
       end
 
-      it "should NOT abort with same annotation when frozen: true " do
+      it "should NOT abort with same annotation when frozen: true" do
         annotate_one_file
         expect { annotate_one_file frozen: true }.not_to raise_error
       end
@@ -3122,7 +3122,7 @@ class Foo < ActiveRecord::Base; end
     after { Object.send :remove_const, 'Foo' }
 
     it 'skips attempt to annotate if no table exists for model' do
-      is_expected.to eq nil
+      is_expected.to be_nil
     end
 
     context 'with a non-class' do

spec/lib/annotate/helpers_spec.rb

@@ -5,7 +5,7 @@
     subject { described_class.skip_on_migration? }
 
     before do
-      allow(ENV).to receive(:[]).and_return(nil)
+      allow(ENV).to receive(:fetch).with(anything, nil).and_return(nil)
     end
 
     it { is_expected.to be_falsy }
@@ -15,7 +15,7 @@
       let(:env_value) { '1' }
 
       before do
-        allow(ENV).to receive(:[]).with(key).and_return(env_value)
+        allow(ENV).to receive(:fetch).with(key, nil).and_return(env_value)
       end
 
       it { is_expected.to be_truthy }
@@ -26,7 +26,7 @@
       let(:env_value) { '1' }
 
       before do
-        allow(ENV).to receive(:[]).with(key).and_return(env_value)
+        allow(ENV).to receive(:fetch).with(key, nil).and_return(env_value)
       end
 
       it { is_expected.to be_truthy }
@@ -37,7 +37,7 @@
     subject { described_class.include_routes? }
 
     before do
-      allow(ENV).to receive(:[]).and_return(nil)
+      allow(ENV).to receive(:fetch).with(anything, nil).and_return(nil)
     end
 
     it { is_expected.to be_falsy }
@@ -47,7 +47,7 @@
       let(:env_value) { '1' }
 
       before do
-        allow(ENV).to receive(:[]).with(key).and_return(env_value)
+        allow(ENV).to receive(:fetch).with(key, nil).and_return(env_value)
       end
 
       it { is_expected.to be_truthy }
@@ -68,7 +68,7 @@
       let(:env_value) { '1' }
 
       before do
-        allow(ENV).to receive(:[]).with(key).and_return(env_value)
+        allow(ENV).to receive(:fetch).with(key, nil).and_return(env_value)
       end
 
       it { is_expected.to be_truthy }