Fix offences and added test (#425)

bin/annotate

@@ -8,11 +8,11 @@ require 'rubygems'
 begin
   require 'bundler'
   Bundler.setup
-rescue Exception
+rescue StandardError
 end
 
 here = File.expand_path(File.dirname __FILE__)
-$:<< "#{here}/../lib"
+$LOAD_PATH << "#{here}/../lib"
 
 require 'optparse'
 require 'annotate'
@@ -33,7 +33,7 @@ OptionParser.new do |opts|
           'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p|
     ENV['position'] = p
     %w(position_in_class position_in_factory position_in_fixture position_in_test position_in_routes position_in_serializer).each do |key|
-      ENV[key] = p unless (has_set_position[key])
+      ENV[key] = p unless has_set_position[key]
     end
   end
 
@@ -192,10 +192,11 @@ 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
-
 end.parse!
 
-options = Annotate.setup_options({is_rake: ENV['is_rake'] && !ENV['is_rake'].empty?})
+options = Annotate.setup_options(
+  is_rake: ENV['is_rake'] && !ENV['is_rake'].empty?
+)
 Annotate.eager_load(options)
 
 AnnotateModels.send(target_action, options) if Annotate.include_models?

lib/annotate.rb

@@ -9,7 +9,7 @@ begin
   # ActiveSupport 3.x...
   require 'active_support/hash_with_indifferent_access'
   require 'active_support/core_ext/object/blank'
-rescue Exception
+rescue StandardError
   # ActiveSupport 2.x...
   require 'active_support/core_ext/hash/indifferent_access'
   require 'active_support/core_ext/blank'
@@ -35,13 +35,18 @@ module Annotate
     :exclude_sti_subclasses, :ignore_unknown_models
   ].freeze
   OTHER_OPTIONS = [
-    :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close, :wrapper, :routes,
-    :hide_limit_column_types, :hide_default_column_types, :ignore_routes, :active_admin
+    :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close,
+    :wrapper, :routes, :hide_limit_column_types, :hide_default_column_types,
+    :ignore_routes, :active_admin
   ].freeze
   PATH_OPTIONS = [
     :require, :model_dir, :root_dir
   ].freeze
 
+  def self.all_options
+    [POSITION_OPTIONS, FLAG_OPTIONS, PATH_OPTIONS, OTHER_OPTIONS]
+  end
+
   ##
   # Set default values that can be overridden via environment variables.
   #
@@ -51,13 +56,13 @@ module Annotate
 
     options = HashWithIndifferentAccess.new(options)
 
-    [POSITION_OPTIONS, FLAG_OPTIONS, PATH_OPTIONS, OTHER_OPTIONS].flatten.each do |key|
+    all_options.flatten.each do |key|
       if options.key?(key)
         default_value = if options[key].is_a?(Array)
                           options[key].join(',')
                         else
                           options[key]
-        end
+                        end
       end
 
       default_value = ENV[key.to_s] unless ENV[key.to_s].blank?
@@ -97,9 +102,7 @@ module Annotate
   end
 
   def self.reset_options
-    [POSITION_OPTIONS, FLAG_OPTIONS, PATH_OPTIONS, OTHER_OPTIONS].flatten.each do |key|
-      ENV[key.to_s] = nil
-    end
+    all_options.flatten.each { |key| ENV[key.to_s] = nil }
   end
 
   def self.skip_on_migration?
@@ -126,11 +129,14 @@ module Annotate
     return if loaded_tasks
     self.loaded_tasks = true
 
-    Dir[File.join(File.dirname(__FILE__), 'tasks', '**/*.rake')].each { |rake| load rake }
+    Dir[File.join(File.dirname(__FILE__), 'tasks', '**/*.rake')].each do |rake|
+      load rake
+    end
   end
 
   def self.load_requires(options)
-    options[:require].each { |path| require path } if options[:require].count > 0
+    options[:require].count > 0 &&
+      options[:require].each { |path| require path }
   end
 
   def self.eager_load(options)
@@ -161,7 +167,7 @@ module Annotate
   def self.bootstrap_rake
     begin
       require 'rake/dsl_definition'
-    rescue Exception => e
+    rescue StandardError => e
       # We might just be on an old version of Rake...
       puts e.message
       exit e.status_code

spec/integration/common_validation.rb

@@ -2,10 +2,7 @@ module Annotate
   module Validations
     module Common
       def self.test_commands
-        return %q{
-          bin/annotate &&
-          bin/annotate --routes
-        }
+        'bin/annotate && bin/annotate --routes'
       end
 
       def self.verify_output(output)
@@ -13,49 +10,47 @@ module Annotate
         output.should =~ /Route file annotated./
       end
 
-      def self.verify_files(which_files, test_rig, schema_annotation, routes_annotation, place_before=true)
-        check_task_model(test_rig, schema_annotation, place_before) if(which_files[:model])
-        check_task_unittest(test_rig, schema_annotation, place_before) if(which_files[:test])
-        check_task_fixture(test_rig, schema_annotation, place_before) if(which_files[:fixture])
-        check_task_factory(test_rig, schema_annotation, place_before) if(which_files[:factory])
-        check_routes(test_rig, routes_annotation, place_before) if(which_files[:routes])
+      def self.verify_files(which_files, test_rig, schema_annotation, routes_annotation, place_before = true)
+        check_task_model(test_rig, schema_annotation, place_before) if which_files[:model]
+        check_task_unittest(test_rig, schema_annotation, place_before) if which_files[:test]
+        check_task_fixture(test_rig, schema_annotation, place_before) if which_files[:fixture]
+        check_task_factory(test_rig, schema_annotation, place_before) if which_files[:factory]
+        check_routes(test_rig, routes_annotation, place_before) if which_files[:routes]
       end
 
-      def self.check_task_model(test_rig, annotation, place_before=true)
-        model = apply_annotation(test_rig, "app/models/task.rb", annotation, place_before)
-        File.read("app/models/task.rb").should == model
+      def self.check_task_model(test_rig, annotation, place_before = true)
+        model = apply_annotation(test_rig, 'app/models/task.rb', annotation, place_before)
+        File.read('app/models/task.rb').should == model
       end
 
-      def self.check_routes(test_rig, annotation, place_before=true)
-        routes = apply_annotation(test_rig, "config/routes.rb", annotation, place_before)
-        File.read("config/routes.rb").
-          sub(/\d{4}-\d{2}-\d{2} \d{2}:\d{2}/, 'YYYY-MM-DD HH:MM').
-          should == routes
+      def self.check_routes(test_rig, annotation, place_before = true)
+        routes = apply_annotation(test_rig, 'config/routes.rb', annotation, place_before)
+        File.read('config/routes.rb')
+            .sub(/\d{4}-\d{2}-\d{2} \d{2}:\d{2}/, 'YYYY-MM-DD HH:MM')
+            .should == routes
       end
 
-      def self.check_task_unittest(test_rig, annotation, place_before=true)
-        unittest = apply_annotation(test_rig, "test/unit/task_test.rb", annotation, place_before)
-        File.read("test/unit/task_test.rb").should == unittest
+      def self.check_task_unittest(test_rig, annotation, place_before = true)
+        unittest = apply_annotation(test_rig, 'test/unit/task_test.rb', annotation, place_before)
+        File.read('test/unit/task_test.rb').should == unittest
       end
 
       def self.check_task_modeltest(test_rig, annotation, place_before=true)
-        unittest = apply_annotation(test_rig, "test/models/task_test.rb", annotation, place_before)
-        File.read("test/models/task_test.rb").should == unittest
+        unittest = apply_annotation(test_rig, 'test/models/task_test.rb', annotation, place_before)
+        File.read('test/models/task_test.rb').should == unittest
       end
 
       def self.check_task_factory(test_rig, annotation, place_before=true)
-        fixture = apply_annotation(test_rig, "test/factories/tasks.rb", annotation, place_before)
-        File.read("test/factories/tasks.rb").should == fixture
+        fixture = apply_annotation(test_rig, 'test/factories/tasks.rb', annotation, place_before)
+        File.read('test/factories/tasks.rb').should == fixture
       end
 
-      def self.check_task_fixture(test_rig, annotation, place_before=true)
-        fixture = apply_annotation(test_rig, "test/fixtures/tasks.yml", annotation, place_before)
-        File.read("test/fixtures/tasks.yml").should == fixture
+      def self.check_task_fixture(test_rig, annotation, place_before = true)
+        fixture = apply_annotation(test_rig, 'test/fixtures/tasks.yml', annotation, place_before)
+        File.read('test/fixtures/tasks.yml').should == fixture
       end
 
-      protected
-
-      def self.apply_annotation(test_rig, fname, annotation, place_before=true)
+      def self.apply_annotation(test_rig, fname, annotation, place_before = true)
         corpus = File.read(File.join(test_rig, fname))
         if place_before
           annotation + "\n" + corpus

spec/integration/integration_spec.rb

@@ -17,12 +17,13 @@ ENV['BUNDLE_GEMFILE'] = './Gemfile'
 
 describe "annotate inside Rails, using #{CURRENT_RUBY}" do
   chosen_scenario = nil
-  if !ENV['SCENARIO'].blank?
+  unless ENV['SCENARIO'].blank?
     chosen_scenario = File.expand_path(ENV['SCENARIO'])
-    raise "Can't find specified scenario '#{chosen_scenario}'!" unless(File.directory?(chosen_scenario))
+    raise "Can't find specified scenario '#{chosen_scenario}'!" unless File.directory?(chosen_scenario)
   end
+
   Annotate::Integration::SCENARIOS.each do |test_rig, base_dir, test_name|
-    next if(chosen_scenario && chosen_scenario != test_rig)
+    next if chosen_scenario && chosen_scenario != test_rig
     it "works under #{test_name}" do
       unless USING_RVM
         skip 'Must have RVM installed.'
@@ -51,7 +52,7 @@ describe "annotate inside Rails, using #{CURRENT_RUBY}" do
 
           # By default, rvm_ruby_string isn't inherited over properly, so let's
           # make sure it's there so our .rvmrc will work.
-          ENV['rvm_ruby_string']=CURRENT_RUBY
+          ENV['rvm_ruby_string'] = CURRENT_RUBY
 
           require base_dir.to_s # Will get "#{base_dir}.rb"...
           klass = "Annotate::Validations::#{base_dir.tr('.', '_').classify}".constantize

spec/integration/rails_3.2.8.rb

@@ -4,7 +4,7 @@ module Annotate
   module Validations
     class Rails328 < Base
       def self.schema_annotation
-        return <<-RUBY
+        <<-RUBY
 # == Schema Information
 #
 # Table name: tasks
@@ -18,7 +18,7 @@ RUBY
       end
 
       def self.route_annotation
-        return <<-RUBY
+        <<-RUBY
 # == Route Map (Updated YYYY-MM-DD HH:MM)
 #
 #     tasks GET    /tasks(.:format)          tasks#index