Remove all Ruby-emitted warnings

Run RSpec tests with warnings enabled so we stay on top of this better
in the future.

NEWS.md

@@ -15,6 +15,12 @@
   from `validate_uniqueness_of`, your best bet continues to be creating a record
   manually and calling `validate_uniqueness_of` on that instead.
 
+### Other changes
+
+* The majority of warnings that the gem produced have been removed. The gem
+  still produces warnings under Ruby 1.9.3; we will address this in a future
+  release.
+
 # 2.6.1
 
 ### Features

Rakefile

@@ -9,6 +9,7 @@ require_relative 'lib/shoulda/matchers/version'
 CURRENT_VERSION = Shoulda::Matchers::VERSION
 
 RSpec::Core::RakeTask.new do |t|
+  t.ruby_opts = '-w -r ./spec/report_warnings'
   t.pattern = "spec/**/*_spec.rb"
   t.rspec_opts = '--color --format progress'
   t.verbose = false

lib/shoulda/matchers/action_controller/redirect_to_matcher.rb

@@ -47,14 +47,18 @@ module Shoulda
         alias failure_message_for_should_not failure_message_when_negated
 
         def initialize(url_or_description, context, &block)
+          @url_block = nil
+          @url = nil
+          @context = context
+          @failure_message = nil
+          @failure_message_when_negated = nil
+
           if block
             @url_block = block
             @location = url_or_description
           else
-            @url = url_or_description
-            @location = @url
+            @location = @url = url_or_description
           end
-          @context = context
         end
 
         def in_context(context)

lib/shoulda/matchers/action_controller/render_template_matcher.rb

@@ -53,7 +53,10 @@ module Shoulda
           @options = options
           @message = message
           @template = options.is_a?(Hash) ? options[:partial] : options
-          @context  = context
+          @context = context
+          @controller = nil
+          @failure_message = nil
+          @failure_message_when_negated = nil
         end
 
         def matches?(controller)

lib/shoulda/matchers/action_controller/render_with_layout_matcher.rb

@@ -64,9 +64,13 @@ module Shoulda
       # @private
       class RenderWithLayoutMatcher
         def initialize(expected_layout)
-          unless expected_layout.nil?
+          if expected_layout
             @expected_layout = expected_layout.to_s
+          else
+            @expected_layout = nil
           end
+
+          @controller = nil
         end
 
         # Used to provide access to layouts recorded by

lib/shoulda/matchers/action_controller/rescue_from_matcher.rb

@@ -39,6 +39,8 @@ module Shoulda
       class RescueFromMatcher
         def initialize(exception)
           @exception = exception
+          @expected_method = nil
+          @controller = nil
         end
 
         def with(method)

lib/shoulda/matchers/action_controller/set_session_matcher.rb

@@ -73,6 +73,7 @@ module Shoulda
       class SetSessionMatcher
         def initialize(key)
           @key = key.to_s
+          @value_block = nil
         end
 
         def to(value = nil, &block)
@@ -114,6 +115,10 @@ module Shoulda
 
         private
 
+        def value_or_default_value
+          defined?(@value) && @value
+        end
+
         def assigned_value?
           !assigned_value.nil?
         end
@@ -124,10 +129,10 @@ module Shoulda
 
         def assigned_correct_value?
           if assigned_value?
-            if @value.nil?
+            if value_or_default_value.nil?
               true
             else
-              assigned_value == @value
+              assigned_value == value_or_default_value
             end
           end
         end
@@ -138,8 +143,9 @@ module Shoulda
 
         def expectation
           expectation = "session variable #{@key} to be set"
-          if @value
-            expectation << " to #{@value.inspect}"
+
+          if value_or_default_value
+            expectation << " to #{value_or_default_value.inspect}"
           end
         end
 

lib/shoulda/matchers/action_controller/set_the_flash_matcher.rb

@@ -153,6 +153,7 @@ module Shoulda
       class SetTheFlashMatcher
         def initialize
           @options = {}
+          @value = nil
         end
 
         def to(value)

lib/shoulda/matchers/active_model/ensure_exclusion_of_matcher.rb

@@ -91,6 +91,13 @@ module Shoulda
 
       # @private
       class EnsureExclusionOfMatcher < ValidationMatcher
+        def initialize(attribute)
+          super(attribute)
+          @array = nil
+          @range = nil
+          @expected_message = nil
+        end
+
         def in_array(array)
           @array = array
           self

lib/shoulda/matchers/active_model/ensure_inclusion_of_matcher.rb

@@ -241,6 +241,12 @@ EOT
         def initialize(attribute)
           super(attribute)
           @options = {}
+          @array = nil
+          @range = nil
+          @minimum = nil
+          @maximum = nil
+          @low_message = nil
+          @high_message = nil
         end
 
         def in_array(array)

lib/shoulda/matchers/active_model/ensure_length_of_matcher.rb

@@ -204,6 +204,8 @@ module Shoulda
         def initialize(attribute)
           super(attribute)
           @options = {}
+          @short_message = nil
+          @long_message = nil
         end
 
         def is_at_least(length)

lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb

@@ -21,6 +21,7 @@ module Shoulda
             @operator = operator
             @message = ERROR_MESSAGES[operator]
             @comparison_combos = comparison_combos
+            @strict = false
           end
 
           def for(attribute)

lib/shoulda/matchers/active_model/validate_confirmation_of_matcher.rb

@@ -62,7 +62,7 @@ module Shoulda
         attr_reader :attribute, :confirmation_attribute
 
         def initialize(attribute)
-          @attribute = attribute
+          super(attribute)
           @confirmation_attribute = "#{attribute}_confirmation"
         end
 

lib/shoulda/matchers/active_model/validation_matcher.rb

@@ -10,6 +10,8 @@ module Shoulda
         def initialize(attribute)
           @attribute = attribute
           @strict = false
+          @failure_message = nil
+          @failure_message_when_negated = nil
         end
 
         def on(context)
@@ -61,31 +63,33 @@ module Shoulda
         end
 
         def allow_value_matcher(value, message)
-          matcher = AllowValueMatcher.
-            new(value).
-            for(@attribute).
-            on(@context).
+          matcher = AllowValueMatcher.new(value).for(@attribute).
             with_message(message)
 
+          if defined?(@context)
+            matcher.on(@context)
+          end
+
           if strict?
             matcher.strict
-          else
-            matcher
           end
+
+          matcher
         end
 
         def disallow_value_matcher(value, message)
-          matcher = DisallowValueMatcher.
-            new(value).
-            for(@attribute).
-            on(@context).
+          matcher = DisallowValueMatcher.new(value).for(@attribute).
             with_message(message)
 
+          if defined?(@context)
+            matcher.on(@context)
+          end
+
           if strict?
             matcher.strict
-          else
-            matcher
           end
+
+          matcher
         end
 
         def strict?

lib/shoulda/matchers/doublespeak/double.rb

@@ -65,6 +65,7 @@ module Shoulda
 
         def restore_original_method
           original_method = @original_method
+          klass.__send__(:remove_method, method_name)
           klass.__send__(:define_method, method_name) do |*args, &block|
             original_method.bind(self).call(*args, &block)
           end

spec/report_warnings.rb

@@ -0,0 +1,7 @@
+require File.expand_path('../warnings_spy', __FILE__)
+
+# Adapted from <http://myronmars.to/n/dev-blog/2011/08/making-your-gem-warning-free>
+
+warnings_spy = WarningsSpy.new('shoulda-matchers')
+warnings_spy.capture_warnings
+warnings_spy.report_warnings_at_exit

spec/shoulda/matchers/action_controller/route_matcher_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Shoulda::Matchers::ActionController::RouteMatcher, type: :controller do
+describe 'Shoulda::Matchers::ActionController::RouteMatcher', type: :controller do
   context 'given a controller with a defined glob url' do
     it 'accepts glob route' do
       controller = define_controller('Examples').new

spec/shoulda/matchers/doublespeak/double_spec.rb

@@ -103,7 +103,7 @@ module Shoulda::Matchers::Doublespeak
 
     describe '#call_original_method' do
       it 'binds the stored method object to the class and calls it with the given args and block' do
-        klass = create_class(a_method: nil)
+        klass = create_class
         instance = klass.new
         actual_args = actual_block = method_called = nil
         expected_args = [:one, :two, :three]

spec/spec_helper.rb

@@ -39,3 +39,5 @@ RSpec.configure do |config|
   config.include Shoulda::Matchers::ActionController,
                  example_group: { file_path: /action_controller/ }
 end
+
+$VERBOSE = true

spec/support/class_builder.rb

@@ -10,6 +10,10 @@ module ClassBuilder
   def define_class(class_name, base = Object, &block)
     class_name = class_name.to_s.camelize
 
+    if Object.const_defined?(class_name)
+      Object.__send__(:remove_const, class_name)
+    end
+
     # FIXME: ActionMailer 3.2 calls `name.underscore` immediately upon
     # subclassing. Class.new.name == nil. So, Class.new(ActionMailer::Base)
     # errors out since it's trying to do `nil.underscore`. This is very ugly but

spec/warnings_spy.rb

@@ -0,0 +1,64 @@
+require 'forwardable'
+
+require File.expand_path('../warnings_spy/filesystem', __FILE__)
+require File.expand_path('../warnings_spy/reader', __FILE__)
+require File.expand_path('../warnings_spy/partitioner', __FILE__)
+require File.expand_path('../warnings_spy/reporter', __FILE__)
+
+class WarningsSpy
+  extend Forwardable
+
+  def initialize(project_name)
+    filesystem = Filesystem.new
+    @warnings_file = filesystem.warnings_file
+    @reader = Reader.new(filesystem)
+    @partitioner = Partitioner.new(reader, filesystem)
+    @reporter = Reporter.new(partitioner, filesystem, project_name)
+  end
+
+  def capture_warnings
+    $stderr.reopen(warnings_file.path)
+  end
+
+  def report_warnings_at_exit
+    at_exit do
+      printing_exceptions do
+        report_and_exit
+      end
+    end
+  end
+
+  protected
+
+  attr_reader :warnings_file, :reader, :partitioner, :reporter
+
+  private
+
+  def_delegators :partitioner, :relevant_warning_groups,
+    :irrelevant_warning_groups
+
+  def report_and_exit
+    reader.read
+    partitioner.partition
+    reporter.report
+    #fail_build_if_there_are_any_warnings
+  end
+
+  def fail_build_if_there_are_any_warnings
+    if relevant_warning_groups.any?
+      exit(1)
+    end
+  end
+
+  def printing_exceptions
+    begin
+      yield
+    rescue => error
+      puts "\n--- ERROR IN AT_EXIT --------------------------------"
+      puts "#{error.class}: #{error.message}"
+      puts error.backtrace.join("\n")
+      puts "-----------------------------------------------------"
+      raise error
+    end
+  end
+end

spec/warnings_spy/filesystem.rb

@@ -0,0 +1,45 @@
+require 'fileutils'
+
+class WarningsSpy
+  class Filesystem
+    PROJECT_DIR = File.expand_path('../../..', __FILE__)
+    TEMP_DIR = File.join(PROJECT_DIR, 'tmp')
+
+    def initialize
+      @files_by_name = Hash.new do |hash, name|
+        FileUtils.mkdir_p(TEMP_DIR)
+        hash[name] = file_for(name)
+      end
+    end
+
+    def warnings_file
+      files_by_name['all_warnings']
+    end
+
+    def irrelevant_warnings_file
+      files_by_name['irrelevant_warnings']
+    end
+
+    def relevant_warnings_file
+      files_by_name['relevant_warnings']
+    end
+
+    def project_dir
+      PROJECT_DIR
+    end
+
+    protected
+
+    attr_reader :files_by_name
+
+    private
+
+    def path_for(name)
+      File.join(TEMP_DIR, "#{name}.txt")
+    end
+
+    def file_for(name)
+      File.open(path_for(name), 'w+')
+    end
+  end
+end

spec/warnings_spy/partitioner.rb

@@ -0,0 +1,29 @@
+require 'forwardable'
+
+class WarningsSpy
+  class Partitioner
+    extend Forwardable
+
+    attr_reader :relevant_warning_groups, :irrelevant_warning_groups
+
+    def initialize(reader, filesystem)
+      @reader = reader
+      @search_text = filesystem.project_dir
+    end
+
+    def partition
+      @relevant_warning_groups, @irrelevant_warning_groups =
+        warning_groups.partition do |group|
+          group.any? { |line| line.include?(search_text) }
+        end
+    end
+
+    protected
+
+    attr_reader :reader, :search_text
+
+    private
+
+    def_delegators :reader, :warning_groups
+  end
+end

spec/warnings_spy/reader.rb

@@ -0,0 +1,64 @@
+class WarningsSpy
+  class Reader
+    attr_reader :warning_groups
+
+    def initialize(filesystem)
+      @warnings_file = filesystem.warnings_file
+
+      @recording = false
+      @current_group = []
+      @warning_groups = []
+    end
+
+    def read
+      warnings_file.rewind
+
+      warnings_file.each_line do |line|
+        process_line(line)
+      end
+
+      add_group(current_group)
+    end
+
+    protected
+
+    attr_reader :warnings_file, :current_group
+
+    private
+
+    def process_line(line)
+      if backtrace_line?(line) && recording?
+        current_group << line
+      else
+        unless current_group.empty?
+          add_group(current_group)
+          current_group.clear
+        end
+
+        current_group << line
+
+        @recording = true
+      end
+    end
+
+    def add_group(group)
+      unless group_already_added?(group)
+        warning_groups << group
+      end
+    end
+
+    def group_already_added?(group_to_be_added)
+      warning_groups.any? do |group|
+        group == group_to_be_added
+      end
+    end
+
+    def backtrace_line?(line)
+      line =~ /^\s+/
+    end
+
+    def recording?
+      @recording
+    end
+  end
+end

spec/warnings_spy/reporter.rb

@@ -0,0 +1,87 @@
+require 'forwardable'
+
+class WarningsSpy
+  class Reporter
+    extend Forwardable
+
+    def initialize(partitioner, filesystem, project_name)
+      @partitioner = partitioner
+      @warnings_file = filesystem.warnings_file
+      @relevant_warnings_file = filesystem.relevant_warnings_file
+      @irrelevant_warnings_file = filesystem.irrelevant_warnings_file
+      @project_name = project_name
+    end
+
+    def report
+      reporting_all_groups do
+        report_relevant_warning_groups
+        report_irrelevant_warning_groups
+      end
+    end
+
+    protected
+
+    attr_reader :partitioner, :warnings_file, :relevant_warnings_file,
+      :irrelevant_warnings_file, :project_name
+
+    private
+
+    def_delegators :partitioner, :relevant_warning_groups, :irrelevant_warning_groups
+
+    def report_relevant_warning_groups
+      if relevant_warning_groups.any?
+        print_divider('-', 75, header: " #{project_name} warnings:")
+        relevant_warning_groups.each do |group|
+          group.each do |line|
+            relevant_warnings_file.puts(line)
+            puts line
+          end
+        end
+        print_divider('-', 75)
+        puts "#{project_name} warnings written to #{relevant_warnings_file.path}."
+      end
+    end
+
+    def report_irrelevant_warning_groups
+      if irrelevant_warning_groups.any?
+        irrelevant_warning_groups.each do |group|
+          group.each do |line|
+            irrelevant_warnings_file.puts(line)
+          end
+        end
+        puts "Non #{project_name} warnings were raised during the test run. These have been written to #{irrelevant_warnings_file.path}."
+      end
+    end
+
+    def reporting_all_groups
+      if relevant_warning_groups.any? || irrelevant_warning_groups.any?
+        puts
+        yield
+        puts "All warnings were written to #{warnings_file.path}."
+        puts
+      end
+    end
+
+    def print_divider(character, count, options = {})
+      puts
+
+      if options[:header]
+        first_count = 10
+        second_count = options[:header].length - first_count
+        string =
+          horizontal_rule(character, first_count) +
+          options[:header] +
+          horizontal_rule(character, second_count)
+        puts string
+      else
+        puts horizontal_rule(character, count)
+      end
+
+      puts
+    end
+
+    def horizontal_rule(character, count)
+      character * count
+    end
+  end
+end