Merge pull request #2498 from rspec/upgrade-rubocop

Upgrade rubocop, motivated by CVE.

Author: xaviershay

.rubocop.yml

@@ -1,4 +1,5 @@
-inherit_from: .rubocop_rspec_base.yml
+inherit_from:
+  - .rubocop_rspec_base.yml
 
 AllCops:
   Exclude:
@@ -10,7 +11,6 @@ Metrics/ClassLength:
   Max: 330
 
 Encoding:
-  EnforcedStyle: when_needed
   Exclude:
     - lib/rspec/core/formatters/exception_presenter.rb
 
@@ -53,23 +53,17 @@ Style/StringLiteralsInInterpolation:
 Style/SymbolProc:
   Enabled: false
 
-Style/SpaceAroundOperators:
-  MultiSpaceAllowedForOperators:
-      - '='
-      - '||='
-      - '==='
-      - '=>'
-      - '<<'
-      - '||'
+Layout/SpaceAroundOperators:
+  AllowForAlignment: true
 
-Style/AccessModifierIndentation:
+Layout/AccessModifierIndentation:
   Enabled: false
 
 Style/RegexpLiteral:
   Enabled: false
 
 # This could change depending of the style used
-Style/MultilineOperationIndentation:
+Layout/MultilineOperationIndentation:
   Enabled: false
 
 Style/BarePercentLiterals:
@@ -95,3 +89,22 @@ Style/RaiseArgs:
     - lib/rspec/core/hooks.rb
     - lib/rspec/core/option_parser.rb
     - lib/rspec/core/pending.rb
+
+Performance/RedundantBlockCall:
+  Exclude:
+    - lib/rspec/core/rake_task.rb
+
+Lint/IneffectiveAccessModifier:
+  Exclude:
+    - lib/rspec/core/memoized_helpers.rb # Fixing this file was too much of a diff
+
+Performance/Caller:
+  Exclude:
+    # The suggested fix for this cop only works on more recent rubies. The
+    # speed up can be substantial (previous optimization work I've done on
+    # RSpec involved caller optimization) but I haven't benchmarked this
+    # particular case to see if it's worth introducing version branching.
+    - 'lib/rspec/core/example_group.rb'
+
+Metrics/BlockLength:
+  Max: 193

.rubocop_rspec_base.yml

@@ -1,11 +1,8 @@
-# This file was generated on 2018-01-02T08:14:15-08:00 from the rspec-dev repo.
-# DO NOT modify it by hand as your changes will get lost the next time it is generated.
-
 # This file contains defaults for RSpec projects. Individual projects
 # can customize by inheriting this file and overriding particular settings.
 
 AccessModifierIndentation:
-  EnforcedStyle: outdent
+  Enabled: false
 
 # "Use alias_method instead of alias"
 # We're fine with `alias`.
@@ -50,9 +47,6 @@ DoubleNegation:
 EachWithObject:
   Enabled: false
 
-Encoding:
-  EnforcedStyle: when_needed
-
 FormatString:
   EnforcedStyle: percent
 
@@ -73,7 +67,7 @@ MethodLength:
   Max: 15
 
 # Who cares what we call the argument for binary operator methods?
-OpMethod:
+BinaryOperatorParameterName:
   Enabled: false
 
 PercentLiteralDelimiters:
@@ -98,9 +92,6 @@ PredicateName:
 Proc:
   Enabled: false
 
-RedundantReturn:
-  AllowMultipleReturnValues: true
-
 # Exceptions should be rescued with `Support::AllExceptionsExceptOnesWeMustNotRescue`
 RescueException:
   Enabled: true
@@ -121,10 +112,196 @@ StringLiterals:
 Style/SpecialGlobalVars:
   Enabled: false
 
-Style/TrailingComma:
+Style/TrailingCommaInLiteral:
+  Enabled: false
+
+Style/TrailingCommaInArguments:
   Enabled: false
 
 TrivialAccessors:
   AllowDSLWriters: true
   AllowPredicates: true
   ExactNameMatch: true
+
+Style/ParallelAssignment:
+  Enabled: false
+
+Layout/EmptyLineBetweenDefs:
+  Enabled: false
+
+Layout/FirstParameterIndentation:
+  Enabled: false
+
+Naming/ConstantName:
+  Enabled: false
+
+Style/ClassCheck:
+  Enabled: false
+
+Style/ConditionalAssignment:
+  Enabled: false
+
+Style/EmptyMethod:
+  Enabled: false
+
+Style/FormatStringToken:
+  Enabled: false
+
+Style/GuardClause:
+  Enabled: false
+
+Style/IdenticalConditionalBranches:
+  Enabled: false
+
+Style/IfUnlessModifier:
+  Enabled: false
+
+Style/IfUnlessModifierOfIfUnless:
+  Enabled: false
+
+Style/MethodMissing:
+  Enabled: false
+
+Style/MixinUsage:
+  Enabled: false
+
+Style/MultipleComparison:
+  Enabled: false
+
+Style/MutableConstant:
+  Enabled: false
+
+Style/NestedModifier:
+  Enabled: false
+
+Style/NestedParenthesizedCalls:
+  Enabled: false
+
+Style/NumericPredicate:
+  Enabled: false
+
+Style/RedundantParentheses:
+  Enabled: false
+
+Style/StringLiteralsInInterpolation:
+  Enabled: false
+
+Style/SymbolArray:
+  Enabled: false
+
+Style/SymbolProc:
+  Enabled: false
+
+Style/YodaCondition:
+  Enabled: false
+
+Style/ZeroLengthPredicate:
+  Enabled: false
+
+Layout/ClosingParenthesisIndentation:
+  Enabled: false
+
+Layout/ExtraSpacing:
+  Enabled: false
+
+Layout/MultilineMethodCallBraceLayout:
+  Enabled: false
+
+Layout/MultilineMethodCallIndentation:
+  Enabled: false
+
+Layout/MultilineOperationIndentation:
+  Enabled: false
+
+Layout/SpaceAroundBlockParameters:
+  Enabled: false
+
+Layout/SpaceAroundOperators:
+  Enabled: false
+
+Layout/SpaceBeforeComma:
+  Enabled: false
+
+Style/BlockDelimiters:
+  Enabled: false
+
+Style/EmptyCaseCondition:
+  Enabled: false
+
+Style/MultilineIfModifier:
+  Enabled: false
+
+Style/RescueStandardError:
+  Enabled: false
+
+Style/StderrPuts:
+  Enabled: false
+
+Style/TernaryParentheses:
+  Enabled: false
+
+# This could likely be enabled, but it had a false positive on rspec-mocks
+# (suggested change was not behaviour preserving) so I don't trust it.
+Performance/HashEachMethods:
+  Enabled: false
+
+Naming/HeredocDelimiterNaming:
+  Enabled: false
+
+Layout/EmptyLineAfterMagicComment:
+  Enabled: false
+
+Layout/IndentArray:
+  Enabled: false
+
+Layout/IndentAssignment:
+  Enabled: false
+
+Layout/IndentHeredoc:
+  Enabled: false
+
+Layout/SpaceInsidePercentLiteralDelimiters:
+  Enabled: false
+
+Style/EmptyElse:
+  Enabled: false
+
+Style/IfInsideElse:
+  Enabled: false
+
+Style/RedundantReturn:
+  Enabled: false
+
+Style/StructInheritance:
+  Enabled: false
+
+Naming/VariableNumber:
+  Enabled: false
+
+Layout/SpaceInsideStringInterpolation:
+  Enabled: false
+
+Style/DateTime:
+  Enabled: false
+
+Style/ParenthesesAroundCondition:
+  Enabled: false
+
+Layout/EmptyLinesAroundBlockBody:
+  Enabled: false
+
+Lint/ImplicitStringConcatenation:
+  Enabled: false
+
+Lint/NestedMethodDefinition:
+  Enabled: false
+
+Style/RegexpLiteral:
+  Enabled: false
+
+Style/TrailingUnderscoreVariable:
+  Enabled: false
+
+Layout/EmptyLinesAroundAccessModifier:
+  Enabled: false
+

Gemfile

@@ -42,8 +42,9 @@ end
 
 gem 'simplecov', '~> 0.8'
 
-if RUBY_VERSION >= '1.9.3' && RUBY_VERSION < '2.4.0' && !(%w[java jruby].include? RUBY_ENGINE)
-  gem "rubocop", "~> 0.32.1"
+# No need to run rubocop on earlier versions
+if RUBY_VERSION >= '2.4' && RUBY_ENGINE == 'ruby'
+  gem "rubocop", "~> 0.52.1"
 end
 
 gem 'test-unit', '~> 3.0' if RUBY_VERSION.to_f >= 2.2

lib/rspec/core/bisect/runner.rb

@@ -29,8 +29,8 @@ def command_for(locations)
           parts << "--format"   << "bisect"
           parts << "--drb-port" << @server.drb_port
 
-          parts.concat reusable_cli_options
-          parts.concat locations.map { |l| open3_safe_escape(l) }
+          parts.concat(reusable_cli_options)
+          parts.concat(locations.map { |l| open3_safe_escape(l) })
 
           parts.join(" ")
         end

lib/rspec/core/configuration.rb

@@ -1771,7 +1771,7 @@ def define_derived_metadata(*filters, &block)
       #       require 'support/db'
       #     end
       #   end
-      def when_first_matching_example_defined(*filters, &block)
+      def when_first_matching_example_defined(*filters)
         specified_meta = Metadata.build_hash_from(filters, :warn_about_example_group_filtering)
 
         callback = lambda do |example_or_group_meta|
@@ -1782,7 +1782,7 @@ def when_first_matching_example_defined(*filters, &block)
           # Ensure the callback only fires once.
           @derived_metadata_blocks.delete(callback, specified_meta)
 
-          block.call
+          yield
         end
 
         @derived_metadata_blocks.append(callback, specified_meta)

lib/rspec/core/example.rb

@@ -393,7 +393,7 @@ def display_exception=(ex)
         end
       end
 
-      # rubocop:disable Style/AccessorMethodName
+      # rubocop:disable Naming/AccessorMethodName
 
       # @private
       #
@@ -420,7 +420,7 @@ def set_aggregate_failures_exception(exception)
         self.display_exception = exception
       end
 
-      # rubocop:enable Style/AccessorMethodName
+      # rubocop:enable Naming/AccessorMethodName
 
       # @private
       #
@@ -642,12 +642,12 @@ def initialize(hook_description, reporter)
         @reporter = reporter
       end
 
-      # rubocop:disable Style/AccessorMethodName
+      # rubocop:disable Naming/AccessorMethodName
       def set_exception(exception)
         reporter.notify_non_example_exception(exception, "An error occurred in #{description}.")
         RSpec.world.wants_to_quit = true
       end
-      # rubocop:enable Style/AccessorMethodName
+      # rubocop:enable Naming/AccessorMethodName
     end
   end
 end

lib/rspec/core/formatters/deprecation_formatter.rb

@@ -81,7 +81,7 @@ def too_many_warnings_message
 
           def output_formatted(str)
             return str unless str.lines.count > 1
-            separator = "#{'-' * 80}"
+            separator = '-' * 80
             "#{separator}\n#{str.chomp}\n#{separator}"
           end