Commit 9a7ac054 authored by Robert Speicher's avatar Robert Speicher

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

[EE] Enable the Style/PreferredHashMethods cop

See merge request !2023
parents fc939c83 2b81d060
...@@ -392,6 +392,15 @@ Style/OpMethod: ...@@ -392,6 +392,15 @@ Style/OpMethod:
Style/ParenthesesAroundCondition: Style/ParenthesesAroundCondition:
Enabled: true 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. # Checks for an obsolete RuntimeException argument in raise/fail.
Style/RedundantException: Style/RedundantException:
Enabled: true Enabled: true
......
...@@ -236,13 +236,6 @@ Style/PerlBackrefs: ...@@ -236,13 +236,6 @@ Style/PerlBackrefs:
Style/PredicateName: Style/PredicateName:
Enabled: false Enabled: false
# Offense count: 45
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# SupportedStyles: short, verbose
Style/PreferredHashMethods:
Enabled: false
# Offense count: 65 # Offense count: 65
# Cop supports --auto-correct. # Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles. # Configuration parameters: EnforcedStyle, SupportedStyles.
......
...@@ -39,7 +39,7 @@ class TodosFinder ...@@ -39,7 +39,7 @@ class TodosFinder
private private
def action_id? 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 end
def action_id def action_id
......
module DropdownsHelper module DropdownsHelper
def dropdown_tag(toggle_text, options: {}, &block) 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" } data_attr = { toggle: "dropdown" }
if options.has_key?(:data) if options.key?(:data)
data_attr = options[:data].merge(data_attr) data_attr = options[:data].merge(data_attr)
end end
dropdown_output = dropdown_toggle(toggle_text, data_attr, options) dropdown_output = dropdown_toggle(toggle_text, data_attr, options)
if options.has_key?(:toggle_link) if options.key?(:toggle_link)
dropdown_output = dropdown_toggle_link(toggle_text, data_attr, options) dropdown_output = dropdown_toggle_link(toggle_text, data_attr, options)
end end
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 = "" output = ""
if options.has_key?(:title) if options.key?(:title)
output << dropdown_title(options[:title]) output << dropdown_title(options[:title])
end end
if options.has_key?(:filter) if options.key?(:filter)
output << dropdown_filter(options[:placeholder]) output << dropdown_filter(options[:placeholder])
end end
output << content_tag(:div, class: "dropdown-content #{options[:content_class] if options.has_key?(:content_class)}") do output << content_tag(:div, class: "dropdown-content #{options[:content_class] if options.key?(:content_class)}") do
capture(&block) if block && !options.has_key?(:footer_content) capture(&block) if block && !options.key?(:footer_content)
end end
if block && options[:footer_content] if block && options[:footer_content]
...@@ -45,7 +45,7 @@ module DropdownsHelper ...@@ -45,7 +45,7 @@ module DropdownsHelper
def dropdown_toggle(toggle_text, data_attr, options = {}) def dropdown_toggle(toggle_text, data_attr, options = {})
default_label = data_attr[:default_label] 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 = content_tag(:span, toggle_text, class: "dropdown-toggle-text #{'is-default' if toggle_text == default_label}")
output << icon('chevron-down') output << icon('chevron-down')
output.html_safe output.html_safe
...@@ -53,7 +53,7 @@ module DropdownsHelper ...@@ -53,7 +53,7 @@ module DropdownsHelper
end end
def dropdown_toggle_link(toggle_text, data_attr, options = {}) def dropdown_toggle_link(toggle_text, data_attr, options = {})
output = content_tag(:a, toggle_text, class: "dropdown-toggle-text #{options[:toggle_class] if options.has_key?(:toggle_class)}", id: (options[:id] if options.has_key?(:id)), data: data_attr) output = content_tag(:a, toggle_text, class: "dropdown-toggle-text #{options[:toggle_class] if options.key?(:toggle_class)}", id: (options[:id] if options.key?(:id)), data: data_attr)
output.html_safe output.html_safe
end end
......
...@@ -160,7 +160,7 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -160,7 +160,7 @@ class ApplicationSetting < ActiveRecord::Base
validates_each :restricted_visibility_levels do |record, attr, value| validates_each :restricted_visibility_levels do |record, attr, value|
value&.each do |level| 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") record.errors.add(attr, "'#{level}' is not a valid visibility level")
end end
end end
...@@ -168,7 +168,7 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -168,7 +168,7 @@ class ApplicationSetting < ActiveRecord::Base
validates_each :import_sources do |record, attr, value| validates_each :import_sources do |record, attr, value|
value&.each do |source| 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") record.errors.add(attr, "'#{source}' is not a import source")
end end
end end
......
...@@ -177,7 +177,7 @@ class Commit ...@@ -177,7 +177,7 @@ class Commit
if RequestStore.active? if RequestStore.active?
key = "commit_author:#{author_email.downcase}" key = "commit_author:#{author_email.downcase}"
# nil is a valid value since no author may exist in the system # 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] @author = RequestStore.store[key]
else else
@author = find_author_by_any_email @author = find_author_by_any_email
......
...@@ -271,9 +271,9 @@ class Issue < ActiveRecord::Base ...@@ -271,9 +271,9 @@ class Issue < ActiveRecord::Base
def as_json(options = {}) def as_json(options = {})
super(options).tap do |json| 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( json[:labels] = labels.as_json(
project: project, project: project,
only: [:id, :title, :description, :color, :priority], only: [:id, :title, :description, :color, :priority],
......
...@@ -172,7 +172,7 @@ class Label < ActiveRecord::Base ...@@ -172,7 +172,7 @@ class Label < ActiveRecord::Base
def as_json(options = {}) def as_json(options = {})
super(options).tap do |json| 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
end end
......
...@@ -28,7 +28,7 @@ class List < ActiveRecord::Base ...@@ -28,7 +28,7 @@ class List < ActiveRecord::Base
def as_json(options = {}) def as_json(options = {})
super(options).tap do |json| super(options).tap do |json|
if options.has_key?(:label) if options.key?(:label)
json[:label] = label.as_json( json[:label] = label.as_json(
project: board.project, project: board.project,
only: [:id, :title, :description, :color] only: [:id, :title, :description, :color]
......
...@@ -148,7 +148,7 @@ class IssuableBaseService < BaseService ...@@ -148,7 +148,7 @@ class IssuableBaseService < BaseService
execute(params[:description], issuable) execute(params[:description], issuable)
# Avoid a description already set on an issuable to be overwritten by a nil # 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) params.merge!(command_params)
end end
......
...@@ -175,7 +175,7 @@ module API ...@@ -175,7 +175,7 @@ module API
params_hash = custom_params || params params_hash = custom_params || params
attrs = {} attrs = {}
keys.each do |key| 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] attrs[key] = params_hash[key]
end end
end end
......
...@@ -110,7 +110,7 @@ module API ...@@ -110,7 +110,7 @@ module API
end end
post do post do
attrs = declared_params(include_missing: false) 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 project = ::Projects::CreateService.new(current_user, attrs).execute
if project.saved? if project.saved?
...@@ -253,7 +253,7 @@ module API ...@@ -253,7 +253,7 @@ module API
authorize! :rename_project, user_project if attrs[:name].present? authorize! :rename_project, user_project if attrs[:name].present?
authorize! :change_visibility_level, user_project if attrs[:visibility].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 result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute
......
...@@ -141,7 +141,7 @@ module API ...@@ -141,7 +141,7 @@ module API
patch '/:id/trace' do patch '/:id/trace' do
job = authenticate_job! 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 = request.headers['Content-Range']
content_range = content_range.split('-') content_range = content_range.split('-')
......
...@@ -5,7 +5,7 @@ module API ...@@ -5,7 +5,7 @@ module API
included do included do
helpers do helpers do
def issuable_name def issuable_name
declared_params.has_key?(:issue_iid) ? 'issue' : 'merge_request' declared_params.key?(:issue_iid) ? 'issue' : 'merge_request'
end end
def issuable_key def issuable_key
......
...@@ -48,7 +48,7 @@ module API ...@@ -48,7 +48,7 @@ module API
end end
def set_only_allow_merge_if_pipeline_succeeds! 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) params[:only_allow_merge_if_pipeline_succeeds] = params.delete(:only_allow_merge_if_build_succeeds)
end end
end end
......
...@@ -6,7 +6,7 @@ module API ...@@ -6,7 +6,7 @@ module API
included do included do
helpers do helpers do
def issuable_name def issuable_name
declared_params.has_key?(:issue_id) ? 'issue' : 'merge_request' declared_params.key?(:issue_id) ? 'issue' : 'merge_request'
end end
def issuable_key def issuable_key
......
...@@ -22,11 +22,11 @@ module Bitbucket ...@@ -22,11 +22,11 @@ module Bitbucket
end end
def inline? def inline?
raw.has_key?('inline') raw.key?('inline')
end end
def has_parent? def has_parent?
raw.has_key?('parent') raw.key?('parent')
end end
private private
......
...@@ -88,7 +88,7 @@ module Ci ...@@ -88,7 +88,7 @@ module Ci
patch ":id/trace.txt" do patch ":id/trace.txt" do
build = authenticate_build! 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 = request.headers['Content-Range']
content_range = content_range.split('-') content_range = content_range.split('-')
......
...@@ -45,9 +45,9 @@ module Gitlab ...@@ -45,9 +45,9 @@ module Gitlab
end end
def format_response(response) 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| response[:attachments].each do |attachment|
attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext] attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext]
attachment[:text] = format(attachment[:text]) if attachment[:text] attachment[:text] = format(attachment[:text]) if attachment[:text]
......
...@@ -14,7 +14,7 @@ module Gitlab ...@@ -14,7 +14,7 @@ module Gitlab
end end
def valid? 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 end
def repos def repos
......
...@@ -95,7 +95,7 @@ module Gitlab ...@@ -95,7 +95,7 @@ module Gitlab
labels = import_issue_labels(raw_issue) labels = import_issue_labels(raw_issue)
assignee_id = nil assignee_id = nil
if raw_issue.has_key?("owner") if raw_issue.key?("owner")
username = user_map[raw_issue["owner"]["name"]] username = user_map[raw_issue["owner"]["name"]]
if username.start_with?("@") if username.start_with?("@")
...@@ -144,7 +144,7 @@ module Gitlab ...@@ -144,7 +144,7 @@ module Gitlab
def import_issue_comments(issue, comments) def import_issue_comments(issue, comments)
Note.transaction do Note.transaction do
while raw_comment = comments.shift while raw_comment = comments.shift
next if raw_comment.has_key?("deletedBy") next if raw_comment.key?("deletedBy")
content = format_content(raw_comment["content"]) content = format_content(raw_comment["content"])
updates = format_updates(raw_comment["updates"]) updates = format_updates(raw_comment["updates"])
...@@ -235,15 +235,15 @@ module Gitlab ...@@ -235,15 +235,15 @@ module Gitlab
def format_updates(raw_updates) def format_updates(raw_updates)
updates = [] updates = []
if raw_updates.has_key?("status") if raw_updates.key?("status")
updates << "*Status: #{raw_updates["status"]}*" updates << "*Status: #{raw_updates["status"]}*"
end end
if raw_updates.has_key?("owner") if raw_updates.key?("owner")
updates << "*Owner: #{user_map[raw_updates["owner"]]}*" updates << "*Owner: #{user_map[raw_updates["owner"]]}*"
end end
if raw_updates.has_key?("cc") if raw_updates.key?("cc")
cc = raw_updates["cc"].map do |l| cc = raw_updates["cc"].map do |l|
deleted = l.start_with?("-") deleted = l.start_with?("-")
l = l[1..-1] if deleted l = l[1..-1] if deleted
...@@ -255,7 +255,7 @@ module Gitlab ...@@ -255,7 +255,7 @@ module Gitlab
updates << "*Cc: #{cc.join(", ")}*" updates << "*Cc: #{cc.join(", ")}*"
end end
if raw_updates.has_key?("labels") if raw_updates.key?("labels")
labels = raw_updates["labels"].map do |l| labels = raw_updates["labels"].map do |l|
deleted = l.start_with?("-") deleted = l.start_with?("-")
l = l[1..-1] if deleted l = l[1..-1] if deleted
...@@ -267,11 +267,11 @@ module Gitlab ...@@ -267,11 +267,11 @@ module Gitlab
updates << "*Labels: #{labels.join(", ")}*" updates << "*Labels: #{labels.join(", ")}*"
end end
if raw_updates.has_key?("mergedInto") if raw_updates.key?("mergedInto")
updates << "*Merged into: ##{raw_updates["mergedInto"]}*" updates << "*Merged into: ##{raw_updates["mergedInto"]}*"
end end
if raw_updates.has_key?("blockedOn") if raw_updates.key?("blockedOn")
blocked_ons = raw_updates["blockedOn"].map do |raw_blocked_on| blocked_ons = raw_updates["blockedOn"].map do |raw_blocked_on|
format_blocking_updates(raw_blocked_on) format_blocking_updates(raw_blocked_on)
end end
...@@ -279,7 +279,7 @@ module Gitlab ...@@ -279,7 +279,7 @@ module Gitlab
updates << "*Blocked on: #{blocked_ons.join(", ")}*" updates << "*Blocked on: #{blocked_ons.join(", ")}*"
end end
if raw_updates.has_key?("blocking") if raw_updates.key?("blocking")
blockings = raw_updates["blocking"].map do |raw_blocked_on| blockings = raw_updates["blocking"].map do |raw_blocked_on|
format_blocking_updates(raw_blocked_on) format_blocking_updates(raw_blocked_on)
end end
......
...@@ -25,8 +25,8 @@ module Gitlab ...@@ -25,8 +25,8 @@ module Gitlab
def parse_entry(entry) def parse_entry(entry)
raise FormatError, 'Route map entry is not a hash' unless entry.is_a?(Hash) 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 source key' unless entry.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 public key' unless entry.key?('public')
source_pattern = entry['source'] source_pattern = entry['source']
public_path = entry['public'] public_path = entry['public']
......
...@@ -83,7 +83,7 @@ module Gitlab ...@@ -83,7 +83,7 @@ module Gitlab
end end
def valid_level?(level) def valid_level?(level)
options.has_value?(level) options.value?(level)
end end
def level_name(level) def level_name(level)
......
...@@ -130,7 +130,7 @@ module Gitlab ...@@ -130,7 +130,7 @@ module Gitlab
'MaxSessionTime' => terminal[:max_session_time] '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 details
end end
......
...@@ -166,7 +166,7 @@ describe API::V3::Projects do ...@@ -166,7 +166,7 @@ describe API::V3::Projects do
expect(json_response).to satisfy do |response| expect(json_response).to satisfy do |response|
response.one? do |entry| response.one? do |entry|
entry.has_key?('permissions') && entry.key?('permissions') &&
entry['name'] == project.name && entry['name'] == project.name &&
entry['owner']['username'] == user.username entry['owner']['username'] == user.username
end end
......
...@@ -4,7 +4,7 @@ require 'webmock/rspec' ...@@ -4,7 +4,7 @@ require 'webmock/rspec'
def webmock_setup_defaults def webmock_setup_defaults
allowed = %w[elasticsearch registry.gitlab.com-gitlab-org-test-elastic-image] allowed = %w[elasticsearch registry.gitlab.com-gitlab-org-test-elastic-image]
if ENV.has_key?('ELASTIC_URL') if ENV.key?('ELASTIC_URL')
url = URI.parse(ENV['ELASTIC_URL']) url = URI.parse(ENV['ELASTIC_URL'])
allowed << url.host allowed << url.host
allowed.uniq! allowed.uniq!
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment