Merge branch 'ce/merge-rubocop-changes' into 'master'

Merge Rubocop changes introduced in CE

This MR merges rubocop changes introduced in EE in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4261 and also fixes offenses detected after enabling these cops.

See merge request !414

.rubocop.yml

@@ -754,19 +754,19 @@ Lint/BlockAlignment:
 # Default values in optional keyword arguments and optional ordinal arguments
 # should not refer back to the name of the argument.
 Lint/CircularArgumentReference:
-  Enabled: false
+  Enabled: true
 
 # Checks for condition placed in a confusing position relative to the keyword.
 Lint/ConditionPosition:
-  Enabled: false
+  Enabled: true
 
 # Check for debugger calls.
 Lint/Debugger:
-  Enabled: false
+  Enabled: true
 
 # Align ends corresponding to defs correctly.
 Lint/DefEndAlignment:
-  Enabled: false
+  Enabled: true
 
 # Check for deprecated class method calls.
 Lint/DeprecatedClassMethods:
@@ -782,15 +782,15 @@ Lint/DuplicatedKey:
 
 # Check for immutable argument given to each_with_object.
 Lint/EachWithObjectArgument:
-  Enabled: false
+  Enabled: true
 
 # Check for odd code arrangement in an else block.
 Lint/ElseLayout:
-  Enabled: false
+  Enabled: true
 
 # Checks for empty ensure block.
 Lint/EmptyEnsure:
-  Enabled: false
+  Enabled: true
 
 # Checks for empty string interpolation.
 Lint/EmptyInterpolation:
@@ -798,37 +798,36 @@ Lint/EmptyInterpolation:
 
 # Align ends correctly.
 Lint/EndAlignment:
-  Enabled: false
+  Enabled: true
 
 # END blocks should not be placed inside method definitions.
 Lint/EndInMethod:
-  Enabled: false
+  Enabled: true
 
 # Do not use return in an ensure block.
 Lint/EnsureReturn:
-  Enabled: false
+  Enabled: true
 
 # The use of eval represents a serious security risk.
 Lint/Eval:
-  Enabled: false
+  Enabled: true
 
 # Catches floating-point literals too large or small for Ruby to represent.
 Lint/FloatOutOfRange:
-  Enabled: false
+  Enabled: true
 
 # The number of parameters to format/sprint must match the fields.
 Lint/FormatParameterMismatch:
-  Enabled: false
+  Enabled: true
 
 # Don't suppress exception.
 Lint/HandleExceptions:
   Enabled: false
 
-# TODO: Enable ImplicitStringConcatenation Cop.
 # Checks for adjacent string literals on the same line, which could better be
 # represented as a single string literal.
 Lint/ImplicitStringConcatenation:
-  Enabled: false
+  Enabled: true
 
 # TODO: Enable IneffectiveAccessModifier Cop.
 # Checks for attempts to use `private` or `protected` to set the visibility
@@ -839,7 +838,7 @@ Lint/IneffectiveAccessModifier:
 # Checks for invalid character literals with a non-escaped whitespace
 # character.
 Lint/InvalidCharacterLiteral:
-  Enabled: false
+  Enabled: true
 
 # Checks of literals used in conditions.
 Lint/LiteralInCondition:
@@ -847,7 +846,7 @@ Lint/LiteralInCondition:
 
 # Checks for literals used in interpolation.
 Lint/LiteralInInterpolation:
-  Enabled: false
+  Enabled: true
 
 # Use Kernel#loop with break rather than begin/end/until or begin/end/while
 # for post-loop tests.
@@ -856,11 +855,11 @@ Lint/Loop:
 
 # Do not use nested method definitions.
 Lint/NestedMethodDefinition:
-  Enabled: false
+  Enabled: true
 
 # Do not omit the accumulator when calling `next` in a `reduce`/`inject` block.
 Lint/NextWithoutAccumulator:
-  Enabled: false
+  Enabled: true
 
 # Checks for method calls with a space before the opening parenthesis.
 Lint/ParenthesesAsGroupedExpression:
@@ -869,11 +868,11 @@ Lint/ParenthesesAsGroupedExpression:
 # Checks for `rand(1)` calls. Such calls always return `0` and most likely
 # a mistake.
 Lint/RandOne:
-  Enabled: false
+  Enabled: true
 
 # Use parentheses in the method call to avoid confusion about precedence.
 Lint/RequireParentheses:
-  Enabled: false
+  Enabled: true
 
 # Avoid rescuing the Exception class.
 Lint/RescueException:
@@ -908,7 +907,7 @@ Lint/UnusedMethodArgument:
 
 # Unreachable code.
 Lint/UnreachableCode:
-  Enabled: false
+  Enabled: true
 
 # Checks for useless access modifiers.
 Lint/UselessAccessModifier:
@@ -920,19 +919,19 @@ Lint/UselessAssignment:
 
 # Checks for comparison of something with itself.
 Lint/UselessComparison:
-  Enabled: false
+  Enabled: true
 
 # Checks for useless `else` in `begin..end` without `rescue`.
 Lint/UselessElseWithoutRescue:
-  Enabled: false
+  Enabled: true
 
 # Checks for useless setter call to a local variable.
 Lint/UselessSetterCall:
-  Enabled: false
+  Enabled: true
 
 # Possible use of operator/literal/variable in void context.
 Lint/Void:
-  Enabled: false
+  Enabled: true
 
 
 ##################### Performance ############################
@@ -941,11 +940,10 @@ Lint/Void:
 Performance/Casecmp:
   Enabled: true
 
-# TODO: Enable DoubleStartEndWith Cop.
 # Use `str.{start,end}_with?(x, ..., y, ...)` instead of
 # `str.{start,end}_with?(x, ...) || str.{start,end}_with?(y, ...)`.
 Performance/DoubleStartEndWith:
-  Enabled: false
+  Enabled: true
 
 # TODO: Enable EndWith Cop.
 # Use `end_with?` instead of a regex match anchored to the end of a string.
@@ -979,10 +977,9 @@ Performance/RedundantMerge:
   MaxKeyValuePairs: 2
   Enabled: false
 
-# TODO: Enable RedundantSortBy Cop.
 # Use `sort` instead of `sort_by { |x| x }`.
 Performance/RedundantSortBy:
-  Enabled: false
+  Enabled: true
 
 # TODO: Enable StartWith Cop.
 # Use `start_with?` instead of a regex match anchored to the beginning of a
@@ -1024,11 +1021,11 @@ Rails/Delegate:
 
 # Prefer `find_by` over `where.first`.
 Rails/FindBy:
-  Enabled: false
+  Enabled: true
 
 # Prefer `all.find_each` over `all.find`.
 Rails/FindEach:
-  Enabled: false
+  Enabled: true
 
 # Prefer has_many :through to has_and_belongs_to_many.
 Rails/HasAndBelongsToMany:
@@ -1040,7 +1037,7 @@ Rails/Output:
 
 # Checks for incorrect grammar when using methods like `3.day.ago`.
 Rails/PluralizationGrammar:
-  Enabled: false
+  Enabled: true
 
 # Checks for `read_attribute(:attr)` and `write_attribute(:attr, val)`.
 Rails/ReadWriteAttribute:
@@ -1048,7 +1045,7 @@ Rails/ReadWriteAttribute:
 
 # Checks the arguments of ActiveRecord scopes.
 Rails/ScopeArgs:
-  Enabled: false
+  Enabled: true
 
 # Checks the correct usage of time zone aware methods.
 # http://danilenko.org/2012/7/6/rails_timezones

lib/api/users.rb

@@ -24,7 +24,7 @@ module API
           @users = @users.non_ldap if skip_ldap
           @users = @users.search(params[:search]) if params[:search].present?
           @users = paginate @users
-       end
+        end
 
         if current_user.is_admin?
           present @users, with: Entities::UserFull

spec/controllers/oauth/geo_auth_controller_spec.rb

@@ -106,7 +106,7 @@ describe Oauth::GeoAuthController do
         allow_any_instance_of(Oauth2::LogoutTokenValidationService).to receive(:execute) { { status: :error, message: :expired } }
         get :logout, state: logout_state
 
-        expect(response.body).to include("There is a problem with the OAuth access_token: #{:expired}")
+        expect(response.body).to include("There is a problem with the OAuth access_token: expired")
       end
     end
   end