Skip to content

Commit

Permalink
Merge pull request #15785 from eduardoj/refactoring/rubocop_offenses_…
Browse files Browse the repository at this point in the history
…part_7

Autocorrect some Style/StringConcatenation RuboCop offenses, part II
  • Loading branch information
eduardoj committed Mar 14, 2024
2 parents 6053ff1 + c0333fe commit 4b876bb
Show file tree
Hide file tree
Showing 21 changed files with 89 additions and 112 deletions.
65 changes: 0 additions & 65 deletions src/api/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -228,71 +228,6 @@ Style/StringLiterals:
# due to changes from single to double quotes... we have better things to do!
- 'config/**/*'

# Exclude files for this cop here since we want to tackle them one by one. Our exclusion limit is too low, so
# it gets disabled in the todo's file. Since we don't want to raise the exclusion limit globally we simply
# exclude the affected files here manually, and remove them one by one after the cop is resolved.
Style/StringConcatenation:
Exclude:
- 'app/components/notification_action_bar_component.rb'
- 'app/helpers/webui/elisions_helper.rb'
- 'app/helpers/webui/group_helper.rb'
- 'app/helpers/webui/monitor_helper.rb'
- 'app/helpers/webui/project_helper.rb'
- 'app/helpers/webui/repository_helper.rb'
- 'app/helpers/webui/user_helper.rb'
- 'app/helpers/webui/webui_helper.rb'
- 'app/lib/gitea_api/v1/client.rb'
- 'app/lib/suse/validator.rb'
- 'app/mailers/admin_mailer.rb'
- 'app/mixins/parse_package_diff.rb'
- 'app/queries/packages_finder.rb'
- 'app/services/trigger_controller_service/token_extractor.rb'
- 'app/validators/workflow_steps_validator.rb'
- 'app/views/models/_project.xml.builder'
- 'app/views/models/_remote_project.xml.builder'
- 'app/views/models/staging/_staging_project.xml.builder'
- 'config/initializers/nokogiri_builder.rb'
- 'lib/memory_debugger.rb'
- 'lib/tasks/dev/rake_support.rb'
- 'lib/tasks/extract.rake'
- 'lib/tasks/statistics/github/code_frequency.rake'
- 'lib/tasks/statistics/github/commit_activity.rake'
- 'lib/tasks/watchlist_migration.rake'
- 'lib/xpath_engine.rb'
- 'script/reformat_memprof'
- 'script/start_test_backend'
- 'test/functional/aaa_pre_consistency_test.rb'
- 'test/functional/about_controller_test.rb'
- 'test/functional/architectures_controller_test.rb'
- 'test/functional/attributes_test.rb'
- 'test/functional/backend_test.rb'
- 'test/functional/binary_release_test.rb'
- 'test/functional/branch_publish_flag_test.rb'
- 'test/functional/build_controller_test.rb'
- 'test/functional/channel_maintenance_test.rb'
- 'test/functional/configurations_controller_test.rb'
- 'test/functional/distributions_controller_test.rb'
- 'test/functional/group_test.rb'
- 'test/functional/interconnect_test.rb'
- 'test/functional/issue_controller_test.rb'
- 'test/functional/issue_trackers_controller_test.rb'
- 'test/functional/kgraft_maintenance_test.rb'
- 'test/functional/person_controller_test.rb'
- 'test/functional/product_test.rb'
- 'test/functional/public_controller_test.rb'
- 'test/functional/published_controller_test.rb'
- 'test/functional/read_permission_test.rb'
- 'test/functional/release_management_test.rb'
- 'test/functional/request_controller_test.rb'
- 'test/functional/search_controller_test.rb'
- 'test/functional/source_controller_test.rb'
- 'test/functional/source_services_test.rb'
- 'test/functional/statistics_controller_test.rb'
- 'test/functional/status_controller_test.rb'
- 'test/functional/trigger_controller_test.rb'
- 'test/functional/zzz_post_consistency_test.rb'
- 'test/integration/last_events_test.rb'

##################### Metrics ##################################

Metrics/ParameterLists:
Expand Down
42 changes: 42 additions & 0 deletions src/api/.rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,48 @@ Style/SlicingWithRange:
- 'app/services/package_service/schema_verifier.rb'
- 'test/node_matcher.rb'

# Offense count: 83
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: Mode.
Style/StringConcatenation:
Exclude:
- 'app/helpers/webui/group_helper.rb'
- 'app/helpers/webui/monitor_helper.rb'
- 'app/helpers/webui/project_helper.rb'
- 'app/helpers/webui/user_helper.rb'
- 'app/helpers/webui/webui_helper.rb'
- 'app/validators/workflow_steps_validator.rb'
- 'config/initializers/nokogiri_builder.rb'
- 'test/functional/aaa_pre_consistency_test.rb'
- 'test/functional/about_controller_test.rb'
- 'test/functional/architectures_controller_test.rb'
- 'test/functional/attributes_test.rb'
- 'test/functional/backend_test.rb'
- 'test/functional/branch_publish_flag_test.rb'
- 'test/functional/build_controller_test.rb'
- 'test/functional/channel_maintenance_test.rb'
- 'test/functional/configurations_controller_test.rb'
- 'test/functional/distributions_controller_test.rb'
- 'test/functional/group_test.rb'
- 'test/functional/interconnect_test.rb'
- 'test/functional/issue_controller_test.rb'
- 'test/functional/issue_trackers_controller_test.rb'
- 'test/functional/kgraft_maintenance_test.rb'
- 'test/functional/person_controller_test.rb'
- 'test/functional/product_test.rb'
- 'test/functional/public_controller_test.rb'
- 'test/functional/published_controller_test.rb'
- 'test/functional/read_permission_test.rb'
- 'test/functional/release_management_test.rb'
- 'test/functional/request_controller_test.rb'
- 'test/functional/search_controller_test.rb'
- 'test/functional/source_controller_test.rb'
- 'test/functional/source_services_test.rb'
- 'test/functional/statistics_controller_test.rb'
- 'test/functional/status_controller_test.rb'
- 'test/functional/zzz_post_consistency_test.rb'
- 'test/integration/last_events_test.rb'

# Offense count: 10
ViewComponent/AvoidGlobalState:
Exclude:
Expand Down
4 changes: 2 additions & 2 deletions src/api/app/components/notification_action_bar_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ def disable_with_content(all: false)
private

def add_params(path)
return path + '&update_all=true' if path.include?('?')
return "#{path}&update_all=true" if path.include?('?')

path + '?update_all=true'
"#{path}?update_all=true"
end
end
6 changes: 3 additions & 3 deletions src/api/app/helpers/webui/elisions_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ def elide_two(text1, text2, overall_length = 40, mode = :middle)
def perform_elision(text, length, mode)
case mode
when :left # shorten at the beginning
'...' + text[text.length - length + 3..text.length]
"...#{text[text.length - length + 3..text.length]}"
when :middle # shorten in the middle
pre = text[0..(length / 2) - 2]
offset = 2 # depends if (shortened) length is even or odd
offset = 1 if length.odd?
post = text[text.length - (length / 2) + offset..text.length]
pre + '...' + post
"#{pre}...#{post}"
when :right # shorten at the end
text[0..length - 4] + '...'
"#{text[0..length - 4]}..."
end
end
end
4 changes: 2 additions & 2 deletions src/api/app/helpers/webui/repository_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ def icon_class(flag, is_flag_set_by_user)
def title(flag, is_flag_set_by_user)
title = flag.status.capitalize
if is_flag_set_by_user
title + ' (set by user)'
"#{title} (set by user)"
else
title + ' (calculated)'
"#{title} (calculated)"
end
end

Expand Down
4 changes: 2 additions & 2 deletions src/api/app/helpers/webui/webui_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def bugzilla_url(email_list = '', desc = '')
return '' if @configuration['bugzilla_url'].blank?

assignee = email_list.first if email_list
cc = ('&cc=' + email_list[1..-1].join('&cc=')) if email_list.length > 1 && email_list
cc = "&cc=#{email_list[1..-1].join('&cc=')}" if email_list.length > 1 && email_list

Addressable::URI.escape(
"#{@configuration['bugzilla_url']}/enter_bug.cgi?classification=7340&product=openSUSE.org" \
Expand Down Expand Up @@ -134,7 +134,7 @@ def codemirror_style(opts = {})
style += "border-width: 0 0 0 0;\n" if opts[:no_border] || opts[:read_only]
style += "height: #{opts[:height]};\n" unless opts[:height] == 'auto'
style += "width: #{opts[:width]}; \n" unless opts[:width] == 'auto'
style + "}\n"
"#{style}}\n"
end

def package_link(pack, opts = {})
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/lib/gitea_api/v1/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Client
ApiError = Class.new(GiteaApiError)

def initialize(api_endpoint:, token:)
@api_endpoint = api_endpoint + '/api/v1/'
@api_endpoint = "#{api_endpoint}/api/v1/"
@token = token
end

Expand Down
24 changes: 12 additions & 12 deletions src/api/app/lib/suse/validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,21 @@ def add_schema_mapping(controller, action, opt)
@schema_map ||= {}
@schema_map[controller] ||= {}
key = if opt.key?(:method)
action.to_s + '-' + opt[:method].to_s
"#{action}-#{opt[:method]}"
else
action.to_s
end
@schema_map[controller][key + '-request'] = opt[:request].to_s if opt[:request] # have a request validation schema?
@schema_map[controller][key + '-response'] = opt[:response].to_s if opt[:response] # have a reponse validate schema?
@schema_map[controller]["#{key}-request"] = opt[:request].to_s if opt[:request] # have a request validation schema?
@schema_map[controller]["#{key}-response"] = opt[:response].to_s if opt[:response] # have a reponse validate schema?
end

# Retrieves the schema filename from the action to schema mapping.
def get_schema(opt)
raise 'option hash needs keys :controller and :action' unless opt.key?(:controller) && opt.key?(:action) && opt.key?(:method) && opt.key?(:type)

c = opt[:controller].to_s
key = opt[:action].to_s + '-' + opt[:method].to_s.downcase + '-' + opt[:type].to_s
key2 = opt[:action].to_s + '-' + opt[:type].to_s
key = "#{opt[:action]}-#{opt[:method].to_s.downcase}-#{opt[:type]}"
key2 = "#{opt[:action]}-#{opt[:type]}"

# logger.debug "checking schema map for controller '#{c}', key: '#{key}'"
return if @schema_map.nil?
Expand All @@ -78,14 +78,14 @@ def validate(opt, content)
raise "illegal option; need Hash/Symbol/String, seen: #{opt.class.name}"
end

schema_base_filename = schema_location + '/' + schema_file
schema_base_filename = "#{schema_location}/#{schema_file}"
schema = nil
if File.exist?(schema_base_filename + '.rng')
schema = Nokogiri::XML::RelaxNG(File.open(schema_base_filename + '.rng'))
logger.debug "validating against #{schema_base_filename + '.rng'}"
elsif File.exist?(schema_base_filename + '.xsd')
schema = Nokogiri::XML::Schema(File.open(schema_base_filename + '.xsd'))
logger.debug "validating against #{schema_base_filename + '.xsd'}"
if File.exist?("#{schema_base_filename}.rng")
schema = Nokogiri::XML::RelaxNG(File.open("#{schema_base_filename}.rng"))
logger.debug "validating against #{"#{schema_base_filename}.rng"}"
elsif File.exist?("#{schema_base_filename}.xsd")
schema = Nokogiri::XML::Schema(File.open("#{schema_base_filename}.xsd"))
logger.debug "validating against #{"#{schema_base_filename}.xsd"}"
else
logger.debug "no schema found, skipping validation for #{opt.inspect}"
return true
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/mailers/admin_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def set_headers
end

def mail_sender
'OBS Admin Notification <' + ::Configuration.admin_email + '>'
"OBS Admin Notification <#{::Configuration.admin_email}>"
end

def error(message)
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/mixins/parse_package_diff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def sorted_filenames_from_sourcediff(sd)

parsed_sourcediff = []

sd = '<diffs>' + sd + '</diffs>'
sd = "<diffs>#{sd}</diffs>"
Xmlhash.parse(sd).elements('sourcediff').each do |sourcediff|
parsed_sourcediff << parse_one_diff(sourcediff)
end
Expand Down
8 changes: 4 additions & 4 deletions src/api/app/queries/packages_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ def find_by_attribute

def build_sql_find_by_attribute(package = nil)
if package
base_query + find_by_attribute + ' AND pack.name = ? GROUP by pack.id'
"#{base_query}#{find_by_attribute} AND pack.name = ? GROUP by pack.id"
else
base_query + find_by_attribute + ' GROUP by pack.id'
"#{base_query}#{find_by_attribute} GROUP by pack.id"
end
end

Expand All @@ -76,9 +76,9 @@ def find_by_attribute_and_value

def build_sql_find_by_attribute_and_value(package = nil)
if package
base_query + find_by_attribute_and_value + ' AND pack.name = ?'
"#{base_query}#{find_by_attribute_and_value} AND pack.name = ?"
else
base_query + find_by_attribute_and_value + ' GROUP by pack.id'
"#{base_query}#{find_by_attribute_and_value} GROUP by pack.id"
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def valid_signature?(token_string)
end

def signature_of(token_string)
'sha256=' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), token_string, @body)
"sha256=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), token_string, @body)}"
end

# To trigger the webhook, the sender needs to
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/views/models/_project.xml.builder
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ xml.project(project_attributes) do
end
repo.path_elements.includes(:link).order(kind: :desc).each do |pe|
project_name = if pe.link.remote_project_name.present?
pe.link.project.name + ':' + pe.link.remote_project_name
"#{pe.link.project.name}:#{pe.link.remote_project_name}"
else
pe.link.project.name
end
Expand Down
2 changes: 1 addition & 1 deletion src/api/lib/memory_debugger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def check_up(line)
def log_line(logger, d, prefix = '')
logger.debug prefix + d.line.inspect
d.lines.each do |l|
log_line(logger, l, prefix + ' ')
log_line(logger, l, "#{prefix} ")
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/api/lib/tasks/dev/rake_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def self.find_or_create_project(project_name, user)

def self.copy_example_file(example_file)
if File.exist?(example_file) && !ENV['FORCE_EXAMPLE_FILES']
example_file = File.join(File.expand_path(File.dirname(__FILE__) + '/../..'), example_file)
example_file = File.join(File.expand_path("#{File.dirname(__FILE__)}/../.."), example_file)
puts "WARNING: You already have the config file #{example_file}, make sure it works with docker"
else
puts "Creating config/#{example_file} from config/#{example_file}.example"
Expand Down
8 changes: 4 additions & 4 deletions src/api/lib/tasks/extract.rake
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,15 @@ namespace :db do
record['static_permission'] = perm.title
end
project_prefixes.each do |prefix|
next unless record.key?(prefix + '_id')
next unless record.key?("#{prefix}_id")

p = Project.find(record.delete(prefix + '_id'))
p = Project.find(record.delete("#{prefix}_id"))
prefix = 'project' if prefix == 'db_project'
record[prefix] = p.name.tr(':', '_')
end
package_prefixes.each do |prefix|
if record.key?(prefix + '_id')
p = Package.find(record.delete(prefix + '_id'))
if record.key?("#{prefix}_id")
p = Package.find(record.delete("#{prefix}_id"))
record[prefix] = p.fixtures_name
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/api/lib/tasks/statistics/github/code_frequency.rake
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace :statistics do

weeks.map do |timestamp, additions, deletions|
line = Time.at(timestamp).strftime('%Y-%m-%d') + ", #{additions}, #{deletions}"
file.write(line + "\n")
file.write("#{line}\n")
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/api/lib/tasks/statistics/github/commit_activity.rake
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace :statistics do
commits = week['total']

line = Time.at(timestamp).strftime('%Y-%m-%d') + ", #{commits}"
file.write(line + "\n")
file.write("#{line}\n")
end
end

Expand Down
6 changes: 3 additions & 3 deletions src/api/lib/xpath_engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def parse_predicate(root, stack)
case token
when :function
fname = stack.shift
fname_int = 'xpath_func_' + fname.tr('-', '_')
fname_int = "xpath_func_#{fname.tr('-', '_')}"
raise IllegalXpathError, "unknown xpath function '#{fname}'" unless respond_to?(fname_int)

__send__(fname_int, root, *stack.shift)
Expand Down Expand Up @@ -405,7 +405,7 @@ def parse_predicate(root, stack)
end
when *@operators
opname = token.to_s
opname_int = 'xpath_op_' + opname
opname_int = "xpath_op_#{opname}"
raise IllegalXpathError, "unhandled xpath operator '#{opname}'" unless respond_to?(opname_int)

__send__(opname_int, root, *stack)
Expand Down Expand Up @@ -433,7 +433,7 @@ def evaluate_expr(expr, root, escape: false)
when :attribute
expr.shift # :qname token
expr.shift # namespace
a << ('@' + expr.shift)
a << ("@#{expr.shift}")
when :literal
value = (escape ? escape_for_like(expr.shift) : expr.shift)
return '' if @last_key && @attribs[table][@last_key][:empty]
Expand Down
2 changes: 1 addition & 1 deletion src/api/script/reformat_memprof
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ end
def log_line(file, d, prefix = '')
file.write prefix + d.line.inspect
d.lines.each do |l|
log_line(file, l, prefix + ' ')
log_line(file, l, "#{prefix} ")
end
end

Expand Down
Loading

0 comments on commit 4b876bb

Please sign in to comment.