Merge branch 'rc/enable-PreferredHashMethods-cop' into 'master'

Enable the Style/PreferredHashMethods cop

See merge request !11883

.rubocop.yml

@@ -390,6 +390,15 @@ Style/OpMethod:
 Style/ParenthesesAroundCondition:
   Enabled: true
 
+# This cop (by default) checks for uses of methods Hash#has_key? and
+# Hash#has_value? where it enforces Hash#key? and Hash#value?
+# It is configurable to enforce the inverse, using `verbose` method
+# names also.
+# Configuration parameters: EnforcedStyle, SupportedStyles.
+# SupportedStyles: short, verbose
+Style/PreferredHashMethods:
+  Enabled: true
+
 # Checks for an obsolete RuntimeException argument in raise/fail.
 Style/RedundantException:
   Enabled: true

.rubocop_todo.yml

@@ -236,13 +236,6 @@ Style/PerlBackrefs:
 Style/PredicateName:
   Enabled: false
 
-# Offense count: 45
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: short, verbose
-Style/PreferredHashMethods:
-  Enabled: false
-
 # Offense count: 65
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.

app/finders/todos_finder.rb

@@ -39,7 +39,7 @@ class TodosFinder
   private
 
   def action_id?
-    action_id.present? && Todo::ACTION_NAMES.has_key?(action_id.to_i)
+    action_id.present? && Todo::ACTION_NAMES.key?(action_id.to_i)
   end
 
   def action_id

app/helpers/dropdowns_helper.rb

 module DropdownsHelper
   def dropdown_tag(toggle_text, options: {}, &block)
-    content_tag :div, class: "dropdown #{options[:wrapper_class] if options.has_key?(:wrapper_class)}" do
+    content_tag :div, class: "dropdown #{options[:wrapper_class] if options.key?(:wrapper_class)}" do
       data_attr = { toggle: "dropdown" }
 
-      if options.has_key?(:data)
+      if options.key?(:data)
         data_attr = options[:data].merge(data_attr)
       end
 
       dropdown_output = dropdown_toggle(toggle_text, data_attr, options)
 
-      dropdown_output << content_tag(:div, class: "dropdown-menu dropdown-select #{options[:dropdown_class] if options.has_key?(:dropdown_class)}") do
+      dropdown_output << content_tag(:div, class: "dropdown-menu dropdown-select #{options[:dropdown_class] if options.key?(:dropdown_class)}") do
         output = ""
 
-        if options.has_key?(:title)
+        if options.key?(:title)
           output << dropdown_title(options[:title])
         end
 
-        if options.has_key?(:filter)
+        if options.key?(:filter)
           output << dropdown_filter(options[:placeholder])
         end
 
-        output << content_tag(:div, class: "dropdown-content #{options[:content_class] if options.has_key?(:content_class)}") do
-          capture(&block) if block && !options.has_key?(:footer_content)
+        output << content_tag(:div, class: "dropdown-content #{options[:content_class] if options.key?(:content_class)}") do
+          capture(&block) if block && !options.key?(:footer_content)
         end
 
         if block && options[:footer_content]
@@ -41,7 +41,7 @@ module DropdownsHelper
 
   def dropdown_toggle(toggle_text, data_attr, options = {})
     default_label = data_attr[:default_label]
-    content_tag(:button, class: "dropdown-menu-toggle #{options[:toggle_class] if options.has_key?(:toggle_class)}", id: (options[:id] if options.has_key?(:id)), type: "button", data: data_attr) do
+    content_tag(:button, class: "dropdown-menu-toggle #{options[:toggle_class] if options.key?(:toggle_class)}", id: (options[:id] if options.key?(:id)), type: "button", data: data_attr) do
       output = content_tag(:span, toggle_text, class: "dropdown-toggle-text #{'is-default' if toggle_text == default_label}")
       output << icon('chevron-down')
       output.html_safe

app/models/application_setting.rb

@@ -143,7 +143,7 @@ class ApplicationSetting < ActiveRecord::Base
 
   validates_each :restricted_visibility_levels do |record, attr, value|
     value&.each do |level|
-      unless Gitlab::VisibilityLevel.options.has_value?(level)
+      unless Gitlab::VisibilityLevel.options.value?(level)
         record.errors.add(attr, "'#{level}' is not a valid visibility level")
       end
     end
@@ -151,7 +151,7 @@ class ApplicationSetting < ActiveRecord::Base
 
   validates_each :import_sources do |record, attr, value|
     value&.each do |source|
-      unless Gitlab::ImportSources.options.has_value?(source)
+      unless Gitlab::ImportSources.options.value?(source)
         record.errors.add(attr, "'#{source}' is not a import source")
       end
     end

app/models/commit.rb

@@ -177,7 +177,7 @@ class Commit
     if RequestStore.active?
       key = "commit_author:#{author_email.downcase}"
       # nil is a valid value since no author may exist in the system
-      if RequestStore.store.has_key?(key)
+      if RequestStore.store.key?(key)
         @author = RequestStore.store[key]
       else
         @author = find_author_by_any_email

app/models/issue.rb

@@ -251,9 +251,9 @@ class Issue < ActiveRecord::Base
 
   def as_json(options = {})
     super(options).tap do |json|
-      json[:subscribed] = subscribed?(options[:user], project) if options.has_key?(:user) && options[:user]
+      json[:subscribed] = subscribed?(options[:user], project) if options.key?(:user) && options[:user]
 
-      if options.has_key?(:labels)
+      if options.key?(:labels)
         json[:labels] = labels.as_json(
           project: project,
           only: [:id, :title, :description, :color, :priority],

app/models/label.rb

@@ -172,7 +172,7 @@ class Label < ActiveRecord::Base
 
   def as_json(options = {})
     super(options).tap do |json|
-      json[:priority] = priority(options[:project]) if options.has_key?(:project)
+      json[:priority] = priority(options[:project]) if options.key?(:project)
     end
   end
 

app/models/list.rb

@@ -28,7 +28,7 @@ class List < ActiveRecord::Base
 
   def as_json(options = {})
     super(options).tap do |json|
-      if options.has_key?(:label)
+      if options.key?(:label)
         json[:label] = label.as_json(
           project: board.project,
           only: [:id, :title, :description, :color]

app/services/issuable_base_service.rb

@@ -148,7 +148,7 @@ class IssuableBaseService < BaseService
         execute(params[:description], issuable)
 
     # Avoid a description already set on an issuable to be overwritten by a nil
-    params[:description] = description if params.has_key?(:description)
+    params[:description] = description if params.key?(:description)
 
     params.merge!(command_params)
   end

lib/api/helpers.rb

@@ -158,7 +158,7 @@ module API
       params_hash = custom_params || params
       attrs = {}
       keys.each do |key|
-        if params_hash[key].present? || (params_hash.has_key?(key) && params_hash[key] == false)
+        if params_hash[key].present? || (params_hash.key?(key) && params_hash[key] == false)
           attrs[key] = params_hash[key]
         end
       end

lib/api/projects.rb

@@ -109,7 +109,7 @@ module API
       end
       post do
         attrs = declared_params(include_missing: false)
-        attrs[:builds_enabled] = attrs.delete(:jobs_enabled) if attrs.has_key?(:jobs_enabled)
+        attrs[:builds_enabled] = attrs.delete(:jobs_enabled) if attrs.key?(:jobs_enabled)
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
@@ -248,7 +248,7 @@ module API
         authorize! :rename_project, user_project if attrs[:name].present?
         authorize! :change_visibility_level, user_project if attrs[:visibility].present?
 
-        attrs[:builds_enabled] = attrs.delete(:jobs_enabled) if attrs.has_key?(:jobs_enabled)
+        attrs[:builds_enabled] = attrs.delete(:jobs_enabled) if attrs.key?(:jobs_enabled)
 
         result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute
 

lib/api/runner.rb

@@ -141,7 +141,7 @@ module API
       patch '/:id/trace' do
         job = authenticate_job!
 
-        error!('400 Missing header Content-Range', 400) unless request.headers.has_key?('Content-Range')
+        error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
         content_range = request.headers['Content-Range']
         content_range = content_range.split('-')
 

lib/api/time_tracking_endpoints.rb

@@ -5,7 +5,7 @@ module API
     included do
       helpers do
         def issuable_name
-          declared_params.has_key?(:issue_iid) ? 'issue' : 'merge_request'
+          declared_params.key?(:issue_iid) ? 'issue' : 'merge_request'
         end
 
         def issuable_key

lib/api/v3/projects.rb

@@ -44,7 +44,7 @@ module API
         end
 
         def set_only_allow_merge_if_pipeline_succeeds!
-          if params.has_key?(:only_allow_merge_if_build_succeeds)
+          if params.key?(:only_allow_merge_if_build_succeeds)
             params[:only_allow_merge_if_pipeline_succeeds] = params.delete(:only_allow_merge_if_build_succeeds)
           end
         end

lib/api/v3/time_tracking_endpoints.rb

@@ -6,7 +6,7 @@ module API
       included do
         helpers do
           def issuable_name
-            declared_params.has_key?(:issue_id) ? 'issue' : 'merge_request'
+            declared_params.key?(:issue_id) ? 'issue' : 'merge_request'
           end
 
           def issuable_key

lib/bitbucket/representation/pull_request_comment.rb

@@ -22,11 +22,11 @@ module Bitbucket
       end
 
       def inline?
-        raw.has_key?('inline')
+        raw.key?('inline')
       end
 
       def has_parent?
-        raw.has_key?('parent')
+        raw.key?('parent')
       end
 
       private

lib/ci/api/builds.rb

@@ -88,7 +88,7 @@ module Ci
         patch ":id/trace.txt" do
           build = authenticate_build!
 
-          error!('400 Missing header Content-Range', 400) unless request.headers.has_key?('Content-Range')
+          error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
           content_range = request.headers['Content-Range']
           content_range = content_range.split('-')
 

lib/gitlab/chat_commands/presenters/base.rb

@@ -45,9 +45,9 @@ module Gitlab
         end
 
         def format_response(response)
-          response[:text] = format(response[:text]) if response.has_key?(:text)
+          response[:text] = format(response[:text]) if response.key?(:text)
 
-          if response.has_key?(:attachments)
+          if response.key?(:attachments)
             response[:attachments].each do |attachment|
               attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext]
               attachment[:text] = format(attachment[:text]) if attachment[:text]

lib/gitlab/google_code_import/client.rb

@@ -14,7 +14,7 @@ module Gitlab
       end
 
       def valid?
-        raw_data.is_a?(Hash) && raw_data["kind"] == "projecthosting#user" && raw_data.has_key?("projects")
+        raw_data.is_a?(Hash) && raw_data["kind"] == "projecthosting#user" && raw_data.key?("projects")
       end
 
       def repos

lib/gitlab/google_code_import/importer.rb

@@ -95,7 +95,7 @@ module Gitlab
           labels = import_issue_labels(raw_issue)
 
           assignee_id = nil
-          if raw_issue.has_key?("owner")
+          if raw_issue.key?("owner")
             username = user_map[raw_issue["owner"]["name"]]
 
             if username.start_with?("@")
@@ -144,7 +144,7 @@ module Gitlab
       def import_issue_comments(issue, comments)
         Note.transaction do
           while raw_comment = comments.shift
-            next if raw_comment.has_key?("deletedBy")
+            next if raw_comment.key?("deletedBy")
 
             content     = format_content(raw_comment["content"])
             updates     = format_updates(raw_comment["updates"])
@@ -235,15 +235,15 @@ module Gitlab
       def format_updates(raw_updates)
         updates = []
 
-        if raw_updates.has_key?("status")
+        if raw_updates.key?("status")
           updates << "*Status: #{raw_updates["status"]}*"
         end
 
-        if raw_updates.has_key?("owner")
+        if raw_updates.key?("owner")
           updates << "*Owner: #{user_map[raw_updates["owner"]]}*"
         end
 
-        if raw_updates.has_key?("cc")
+        if raw_updates.key?("cc")
           cc = raw_updates["cc"].map do |l|
             deleted = l.start_with?("-")
             l = l[1..-1] if deleted
@@ -255,7 +255,7 @@ module Gitlab
           updates << "*Cc: #{cc.join(", ")}*"
         end
 
-        if raw_updates.has_key?("labels")
+        if raw_updates.key?("labels")
           labels = raw_updates["labels"].map do |l|
             deleted = l.start_with?("-")
             l = l[1..-1] if deleted
@@ -267,11 +267,11 @@ module Gitlab
           updates << "*Labels: #{labels.join(", ")}*"
         end
 
-        if raw_updates.has_key?("mergedInto")
+        if raw_updates.key?("mergedInto")
           updates << "*Merged into: ##{raw_updates["mergedInto"]}*"
         end
 
-        if raw_updates.has_key?("blockedOn")
+        if raw_updates.key?("blockedOn")
           blocked_ons = raw_updates["blockedOn"].map do |raw_blocked_on|
             format_blocking_updates(raw_blocked_on)
           end
@@ -279,7 +279,7 @@ module Gitlab
           updates << "*Blocked on: #{blocked_ons.join(", ")}*"
         end
 
-        if raw_updates.has_key?("blocking")
+        if raw_updates.key?("blocking")
           blockings = raw_updates["blocking"].map do |raw_blocked_on|
             format_blocking_updates(raw_blocked_on)
           end

lib/gitlab/route_map.rb

@@ -25,8 +25,8 @@ module Gitlab
 
     def parse_entry(entry)
       raise FormatError, 'Route map entry is not a hash' unless entry.is_a?(Hash)
-      raise FormatError, 'Route map entry does not have a source key' unless entry.has_key?('source')
-      raise FormatError, 'Route map entry does not have a public key' unless entry.has_key?('public')
+      raise FormatError, 'Route map entry does not have a source key' unless entry.key?('source')
+      raise FormatError, 'Route map entry does not have a public key' unless entry.key?('public')
 
       source_pattern = entry['source']
       public_path = entry['public']

lib/gitlab/visibility_level.rb

@@ -83,7 +83,7 @@ module Gitlab
       end
 
       def valid_level?(level)
-        options.has_value?(level)
+        options.value?(level)
       end
 
       def level_name(level)

lib/gitlab/workhorse.rb

@@ -129,7 +129,7 @@ module Gitlab
             'MaxSessionTime' => terminal[:max_session_time]
           }
         }
-        details['Terminal']['CAPem'] = terminal[:ca_pem] if terminal.has_key?(:ca_pem)
+        details['Terminal']['CAPem'] = terminal[:ca_pem] if terminal.key?(:ca_pem)
 
         details
       end

spec/requests/api/v3/projects_spec.rb

@@ -165,7 +165,7 @@ describe API::V3::Projects do
 
         expect(json_response).to satisfy do |response|
           response.one? do |entry|
-            entry.has_key?('permissions') &&
+            entry.key?('permissions') &&
               entry['name'] == project.name &&
               entry['owner']['username'] == user.username
           end