make the library work with globally enabled frozen-string-literals (#…

…1598)

more refacts to make tests pass with frozen strings

fix linter errors

clean up some code

enable frozen string literals on CI

.github/workflows/ci.yml

@@ -61,6 +61,6 @@ jobs:
       - name: Install dependencies
         run: bundle install --jobs=3 --retry=3
       - name: Run Unit Tests
-        run: bundle exec rake spec:unit --trace
+        run: RUBYOPT='--enable=frozen-string-literal' bundle exec rake spec:unit --trace
       - name: Run Acceptance Tests
-        run: bundle exec rake spec:acceptance --trace
+        run: RUBYOPT='--enable=frozen-string-literal' bundle exec rake spec:acceptance --trace

lib/shoulda/matchers/action_controller/permit_matcher.rb

@@ -276,16 +276,14 @@ def failure_message_when_negated
           :context, :subparameter_name, :parameters_double_registry
 
         def expectation
-          message = 'restrict parameters '
+          String.new('restrict parameters ').tap do |message|
+            if subparameter_name
+              message << "on #{subparameter_name.inspect} "
+            end
 
-          if subparameter_name
-            message << "on #{subparameter_name.inspect} "
+            message << 'to '\
+              "#{format_parameter_names(expected_permitted_parameter_names)}"
           end
-
-          message << 'to '\
-            "#{format_parameter_names(expected_permitted_parameter_names)}"
-
-          message
         end
 
         def reality

lib/shoulda/matchers/action_controller/set_session_or_flash_matcher.rb

@@ -83,25 +83,23 @@ def expected_value_matches?
         end
 
         def expectation_description
-          string = 'set'
-
-          string <<
-            if key_set?
-              " #{store.name}[#{key.inspect}]"
-            else
-              " any key in #{store.name}"
-            end
-
-          if expected_value_set?
+          String.new('set').tap do |string|
             string <<
-              if expected_value.is_a?(Regexp)
-                " to a value matching #{expected_value.inspect}"
+              if key_set?
+                " #{store.name}[#{key.inspect}]"
               else
-                " to #{expected_value.inspect}"
+                " any key in #{store.name}"
               end
-          end
 
-          string
+            if expected_value_set?
+              string <<
+                if expected_value.is_a?(Regexp)
+                  " to a value matching #{expected_value.inspect}"
+                else
+                  " to #{expected_value.inspect}"
+                end
+            end
+          end
         end
       end
     end

lib/shoulda/matchers/active_model/allow_value_matcher.rb

@@ -550,7 +550,7 @@ def failure_message_preface
         end
 
         def default_failure_message_preface
-          ''.tap do |preface|
+          String.new.tap do |preface|
             if descriptions_for_preset_values.any?
               preface << 'After setting '
               preface << descriptions_for_preset_values.to_sentence

lib/shoulda/matchers/active_model/comparison_matcher.rb

@@ -50,7 +50,7 @@ def simple_description
           description = ''
 
           if expects_strict?
-            description << ' strictly'
+            description = ' strictly'
           end
 
           description +

lib/shoulda/matchers/active_model/validate_comparison_of_matcher.rb

@@ -353,16 +353,14 @@ def does_not_match?(subject)
         end
 
         def simple_description
-          description = ''
+          String.new.tap do |description|
+            description << "validate that :#{attribute} looks like "
+            description << Shoulda::Matchers::Util.a_or_an(allowed_type_name)
 
-          description << "validate that :#{attribute} looks like "
-          description << Shoulda::Matchers::Util.a_or_an(allowed_type_name)
-
-          if comparison_descriptions.present?
-            description << " #{comparison_descriptions}"
+            if comparison_descriptions.present?
+              description << " #{comparison_descriptions}"
+            end
           end
-
-          description
         end
 
         def failure_message

lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb

@@ -453,20 +453,18 @@ def does_not_match?(subject)
         end
 
         def simple_description
-          description = ''
+          String.new.tap do |description|
+            description << "validate that :#{attribute} looks like "
+            description << Shoulda::Matchers::Util.a_or_an(full_allowed_type)
 
-          description << "validate that :#{attribute} looks like "
-          description << Shoulda::Matchers::Util.a_or_an(full_allowed_type)
-
-          if range_description.present?
-            description << " #{range_description}"
-          end
+            if range_description.present?
+              description << " #{range_description}"
+            end
 
-          if comparison_descriptions.present?
-            description << " #{comparison_descriptions}"
+            if comparison_descriptions.present?
+              description << " #{comparison_descriptions}"
+            end
           end
-
-          description
         end
 
         def failure_message

lib/shoulda/matchers/active_model/validation_matcher/build_description.rb

@@ -42,13 +42,12 @@ def description_clauses_for_qualifiers
             description_clauses = []
 
             if matcher.try(:expects_strict?)
-              description_clauses << 'raising a validation exception'
-
-              if matcher.try(:expects_custom_validation_message?)
-                description_clauses.last << ' with a custom message'
-              end
-
-              description_clauses.last << ' on failure'
+              description_clauses <<
+                if matcher.try(:expects_custom_validation_message?)
+                  'raising a validation exception with a custom message on failure'
+                else
+                  'raising a validation exception on failure'
+                end
             elsif matcher.try(:expects_custom_validation_message?)
               description_clauses <<
                 'producing a custom validation error on failure'

lib/shoulda/matchers/active_record/association_matchers/optional_matcher.rb

@@ -22,44 +22,48 @@ def matches?(subject)
             if submatcher_passes?(subject)
               true
             else
-              @missing_option = 'and for the record '
+              @missing_option = build_missing_option
 
-              missing_option <<
+              false
+            end
+          end
+
+          private
+
+          attr_reader :attribute_name, :optional, :submatcher
+
+          def submatcher_passes?(subject)
+            if optional
+              submatcher.matches?(subject)
+            else
+              submatcher.does_not_match?(subject)
+            end
+          end
+
+          def build_missing_option
+            String.new('and for the record ').tap do |missing_option_string|
+              missing_option_string <<
                 if optional
                   'not to '
                 else
                   'to '
                 end
 
-              missing_option << (
+              missing_option_string << (
                 'fail validation if '\
                 ":#{attribute_name} is unset; i.e., either the association "\
                 'should have been defined with `optional: '\
                 "#{optional.inspect}`, or there "
               )
 
-              missing_option <<
+              missing_option_string <<
                 if optional
                   'should not '
                 else
                   'should '
                 end
 
-              missing_option << "be a presence validation on :#{attribute_name}"
-
-              false
-            end
-          end
-
-          private
-
-          attr_reader :attribute_name, :optional, :submatcher
-
-          def submatcher_passes?(subject)
-            if optional
-              submatcher.matches?(subject)
-            else
-              submatcher.does_not_match?(subject)
+              missing_option_string << "be a presence validation on :#{attribute_name}"
             end
           end
         end

lib/shoulda/matchers/active_record/association_matchers/required_matcher.rb

@@ -23,50 +23,54 @@ def matches?(subject)
             if submatcher_passes?(subject)
               true
             else
-              @missing_option = 'and for the record '
+              @missing_option = build_missing_option
 
-              missing_option <<
+              false
+            end
+          end
+
+          private
+
+          attr_reader :attribute_name, :required, :submatcher
+
+          def submatcher_passes?(subject)
+            if required
+              submatcher.matches?(subject)
+            else
+              submatcher.does_not_match?(subject)
+            end
+          end
+
+          def validation_message_key
+            :required
+          end
+
+          def build_missing_option
+            String.new('and for the record ').tap do |missing_option_string|
+              missing_option_string <<
                 if required
                   'to '
                 else
                   'not to '
                 end
 
-              missing_option << (
+              missing_option_string << (
                 'fail validation if '\
                 ":#{attribute_name} is unset; i.e., either the association "\
                 'should have been defined with `required: '\
                 "#{required.inspect}`, or there "
               )
 
-              missing_option <<
+              missing_option_string <<
                 if required
                   'should '
                 else
                   'should not '
                 end
 
-              missing_option << "be a presence validation on :#{attribute_name}"
-
-              false
+              missing_option_string << "be a presence validation on :#{attribute_name}"
             end
           end
-
-          private
-
-          attr_reader :attribute_name, :required, :submatcher
-
-          def submatcher_passes?(subject)
-            if required
-              submatcher.matches?(subject)
-            else
-              submatcher.does_not_match?(subject)
-            end
-          end
-
-          def validation_message_key
-            :required
-          end
         end
       end
     end

lib/shoulda/matchers/active_record/have_db_index_matcher.rb

@@ -152,18 +152,18 @@ def failure_message_when_negated
         end
 
         def description
-          description = 'have '
-
-          description <<
-            if qualifiers.include?(:unique)
-              "#{Shoulda::Matchers::Util.a_or_an(index_type)} "
-            else
-              'an '
-            end
+          String.new('have ').tap do |description|
+            description <<
+              if qualifiers.include?(:unique)
+                "#{Shoulda::Matchers::Util.a_or_an(index_type)} "
+              else
+                'an '
+              end
 
-          description << 'index on '
+            description << 'index on '
 
-          description << inspected_expected_columns
+            description << inspected_expected_columns
+          end
         end
 
         private