From 8dc1463ab35519d7e3f160d322b63ff27751e13d Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 4 Sep 2018 10:16:21 +0800 Subject: [PATCH] Enable `Lint/ShadowingOuterLocalVariable` for Rubocop. --- .rubocop.yml | 3 +++ app/models/topic_tracking_state.rb | 4 +-- config/initializers/101-lograge.rb | 4 +-- lib/email/sender.rb | 10 ++++---- lib/final_destination.rb | 15 ++++++++--- lib/freedom_patches/inflector_backport.rb | 11 +++++---- lib/freedom_patches/translate_accelerator.rb | 6 +++-- lib/import_export/base_exporter.rb | 11 ++++++--- lib/tasks/typepad.thor | 11 +++++---- script/discourse | 2 +- script/import_scripts/discuz_x.rb | 8 +++--- script/import_scripts/smf1.rb | 4 +-- script/import_scripts/smf2.rb | 8 +++--- script/import_scripts/telligent.rb | 4 +-- spec/models/topic_tracking_state_spec.rb | 26 ++++++-------------- spec/services/topic_status_updater_spec.rb | 2 +- 16 files changed, 70 insertions(+), 59 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8472c89a91f..e28a0876aa1 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -102,6 +102,9 @@ Layout/EndAlignment: Lint/RequireParentheses: Enabled: true +Lint/ShadowingOuterLocalVariable: + Enabled: true + Layout/MultilineMethodCallIndentation: Enabled: true EnforcedStyle: indented diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index fe71d76bad2..27965ecf4f5 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -292,11 +292,11 @@ SQL topic_id: topic.id } - channels.each do |channel, user_ids| + channels.each do |channel, ids| MessageBus.publish( channel, message.as_json, - user_ids: user_ids + user_ids: ids ) end end diff --git a/config/initializers/101-lograge.rb b/config/initializers/101-lograge.rb index d24b84877ff..c9a4eb92753 100644 --- a/config/initializers/101-lograge.rb +++ b/config/initializers/101-lograge.rb @@ -54,8 +54,8 @@ if (Rails.env.production? && SiteSetting.logging_provider == 'lograge') || ENV[" end if (files = params[:files]) && files.respond_to?(:map) - params[:files] = files.map do |file| - file.respond_to?(:headers) ? file.headers : file + params[:files] = files.map do |f| + f.respond_to?(:headers) ? f.headers : f end end diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 46d0ca84e82..c4daf17ec44 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -106,14 +106,14 @@ module Email .where(id: PostReply.where(reply_id: post_id).select(:post_id)) .order(id: :desc) - referenced_post_message_ids = referenced_posts.map do |post| - if post.incoming_email&.message_id.present? - "<#{post.incoming_email.message_id}>" + referenced_post_message_ids = referenced_posts.map do |referenced_post| + if referenced_post.incoming_email&.message_id.present? + "<#{referenced_post.incoming_email.message_id}>" else - if post.post_number == 1 + if referenced_post.post_number == 1 "" else - "" + "" end end end diff --git a/lib/final_destination.rb b/lib/final_destination.rb index b69b2e0ebd7..d863344af7f 100644 --- a/lib/final_destination.rb +++ b/lib/final_destination.rb @@ -38,10 +38,17 @@ class FinalDestination @opts[:lookup_ip] ||= lambda { |host| FinalDestination.lookup_ip(host) } @ignored = @opts[:ignore_hostnames] || [] - [Discourse.base_url_no_prefix].concat(@opts[:ignore_redirects] || []).each do |url| - url = uri(url) - if url.present? && url.hostname - @ignored << url.hostname + + ignore_redirects = [Discourse.base_url_no_prefix] + + if @opts[:ignore_redirects] + ignore_redirects.concat(@opts[:ignore_redirects]) + end + + ignore_redirects.each do |ignore_redirect| + ignore_redirect = uri(ignore_redirect) + if ignore_redirect.present? && ignore_redirect.hostname + @ignored << ignore_redirect.hostname end end diff --git a/lib/freedom_patches/inflector_backport.rb b/lib/freedom_patches/inflector_backport.rb index 1e192b0033e..cc976591557 100644 --- a/lib/freedom_patches/inflector_backport.rb +++ b/lib/freedom_patches/inflector_backport.rb @@ -20,12 +20,12 @@ module ActiveSupport uncached = "#{method_name}_without_cache" alias_method uncached, method_name - define_method(method_name) do |*args| + define_method(method_name) do |*arguments| # this avoids recursive locks found = true - data = cache.fetch(args) { found = false } + data = cache.fetch(arguments) { found = false } unless found - cache[args] = data = send(uncached, *args) + cache[arguments] = data = send(uncached, *arguments) end # so cache is never corrupted data.dup @@ -45,9 +45,10 @@ module ActiveSupport args.each do |method_name| orig = "#{method_name}_without_clear_memoize" alias_method orig, method_name - define_method(method_name) do |*args| + + define_method(method_name) do |*arguments| ActiveSupport::Inflector.clear_memoize! - send(orig, *args) + send(orig, *arguments) end end end diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index d2ca7a30c31..9d1c55d92ca 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -41,9 +41,11 @@ module I18n I18n.backend.load_translations(I18n.load_path.grep(/\.rb$/)) # load plural rules from plugins - DiscoursePluginRegistry.locales.each do |locale, options| + DiscoursePluginRegistry.locales.each do |plugin_locale, options| if options[:plural] - I18n.backend.store_translations(locale, i18n: { plural: options[:plural] }) + I18n.backend.store_translations(plugin_locale, + i18n: { plural: options[:plural] } + ) end end end diff --git a/lib/import_export/base_exporter.rb b/lib/import_export/base_exporter.rb index 58d3b77e4e8..77057406ce3 100644 --- a/lib/import_export/base_exporter.rb +++ b/lib/import_export/base_exporter.rb @@ -97,9 +97,14 @@ module ImportExport topic_data[:posts] = [] topic.ordered_posts.find_each do |post| - h = POST_ATTRS.inject({}) { |h, a| h[a] = post.send(a); h; } - h[:raw] = h[:raw].gsub('src="/uploads', "src=\"#{Discourse.base_url_no_prefix}/uploads") - topic_data[:posts] << h + attributes = POST_ATTRS.inject({}) { |h, a| h[a] = post.send(a); h; } + + attributes[:raw] = attributes[:raw].gsub( + 'src="/uploads', + "src=\"#{Discourse.base_url_no_prefix}/uploads" + ) + + topic_data[:posts] << attributes end data << topic_data diff --git a/lib/tasks/typepad.thor b/lib/tasks/typepad.thor index 7db75d415ed..8197f7f0f57 100644 --- a/lib/tasks/typepad.thor +++ b/lib/tasks/typepad.thor @@ -27,21 +27,21 @@ class Typepad < Thor end inside_block = true - entry = "" + input = "" entries = [] File.open(options[:file]).each_line do |l| l = l.scrub if l =~ /^--------$/ - parsed_entry = process_entry(entry) + parsed_entry = process_entry(input) if parsed_entry puts "Parsed #{parsed_entry[:title]}" entries << parsed_entry end - entry = "" + input = "" else - entry << l + input << l end end @@ -55,6 +55,7 @@ class Typepad < Thor SiteSetting.email_domains_blacklist = "" puts "Importing #{entries.size} entries" + entries.each_with_index do |entry, idx| puts "Importing (#{idx + 1}/#{entries.size})" next if entry[:body].blank? @@ -219,7 +220,7 @@ class Typepad < Thor current << c end end - segments.delete_if { |s| s.nil? || s.size < 2 } + segments.delete_if { |segment| segment.nil? || segment.size < 2 } segments << current comment[:author] = segments[0] diff --git a/script/discourse b/script/discourse index 3cf57078fe2..d0fccded04c 100755 --- a/script/discourse +++ b/script/discourse @@ -95,7 +95,7 @@ class DiscourseCLI < Thor if !filename puts "You must provide a filename to restore. Did you mean one of the following?\n\n" - Dir["public/backups/default/*"].sort_by { |filename| File.mtime(filename) }.reverse.each do |f| + Dir["public/backups/default/*"].sort_by { |path| File.mtime(path) }.reverse.each do |f| puts "#{discourse} restore #{File.basename(f)}" end diff --git a/script/import_scripts/discuz_x.rb b/script/import_scripts/discuz_x.rb index 8f99ae49113..54c50719171 100644 --- a/script/import_scripts/discuz_x.rb +++ b/script/import_scripts/discuz_x.rb @@ -393,12 +393,12 @@ class ImportScripts::DiscuzX < ImportScripts::Base end if m['status'] & 1 == 1 || mapped[:raw].blank? - mapped[:post_create_action] = lambda do |post| - PostDestroyer.new(Discourse.system_user, post).perform_delete + mapped[:post_create_action] = lambda do |action_post| + PostDestroyer.new(Discourse.system_user, action_post).perform_delete end elsif (m['status'] & 2) >> 1 == 1 # waiting for approve - mapped[:post_create_action] = lambda do |post| - PostAction.act(Discourse.system_user, post, 6, take_action: false) + mapped[:post_create_action] = lambda do |action_post| + PostAction.act(Discourse.system_user, action_post, 6, take_action: false) end end skip ? nil : mapped diff --git a/script/import_scripts/smf1.rb b/script/import_scripts/smf1.rb index 8b17ce6815e..7230450e0c1 100644 --- a/script/import_scripts/smf1.rb +++ b/script/import_scripts/smf1.rb @@ -355,10 +355,10 @@ class ImportScripts::Smf1 < ImportScripts::Base post[:archetype] = Archetype.private_message post[:title] = title post[:target_usernames] = User.where(id: recipients).pluck(:username) - post[:post_create_action] = proc do |p| + post[:post_create_action] = proc do |action_post| @pm_mapping[users] ||= {} @pm_mapping[users][title] ||= [] - @pm_mapping[users][title] << p.topic_id + @pm_mapping[users][title] << action_post.topic_id end end diff --git a/script/import_scripts/smf2.rb b/script/import_scripts/smf2.rb index 5dc36d44533..354a69ce48e 100644 --- a/script/import_scripts/smf2.rb +++ b/script/import_scripts/smf2.rb @@ -201,15 +201,17 @@ class ImportScripts::Smf2 < ImportScripts::Base SQL skip = false ignore_quotes = false + post = { id: message[:id_msg], user_id: user_id_from_imported_user_id(message[:id_member]) || -1, created_at: Time.zone.at(message[:poster_time]), - post_create_action: ignore_quotes && proc do |post| - post.custom_fields['import_rebake'] = 't' - post.save + post_create_action: ignore_quotes && proc do |p| + p.custom_fields['import_rebake'] = 't' + p.save end } + if message[:id_msg] == message[:id_first_msg] post[:category] = category_id_from_imported_category_id(message[:id_board]) post[:title] = decode_entities(message[:subject]) diff --git a/script/import_scripts/telligent.rb b/script/import_scripts/telligent.rb index 7594affcec8..7981e1d5ca7 100644 --- a/script/import_scripts/telligent.rb +++ b/script/import_scripts/telligent.rb @@ -273,8 +273,8 @@ class ImportScripts::Telligent < ImportScripts::Base user_id: user_id, created_at: row["DateCreated"], closed: row["IsLocked"], - post_create_action: proc do |post| - topic = post.topic + post_create_action: proc do |action_post| + topic = action_post.topic Jobs.enqueue_at(topic.pinned_until, :unpin_topic, topic_id: topic.id) if topic.pinned_until url = "f/#{row['ForumId']}/t/#{row['ThreadId']}" Permalink.create(url: url, topic_id: topic.id) unless Permalink.exists?(url: url) diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index bbeefa368f1..73a124814aa 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -114,16 +114,16 @@ describe TopicTrackingState do "/private-messages/group/#{group2.name}" ) - message = messages.find do |message| - message.channel == '/private-messages/inbox' + message = messages.find do |m| + m.channel == '/private-messages/inbox' end expect(message.data["topic_id"]).to eq(private_message_topic.id) expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id)) [group1, group2].each do |group| - message = messages.find do |message| - message.channel == "/private-messages/group/#{group.name}" + message = messages.find do |m| + m.channel == "/private-messages/group/#{group.name}" end expect(message.data["topic_id"]).to eq(private_message_topic.id) @@ -148,9 +148,7 @@ describe TopicTrackingState do "/private-messages/group/#{group2.name}/archive", ) - message = messages.find do |message| - message.channel == '/private-messages/inbox' - end + message = messages.find { |m| m.channel == '/private-messages/inbox' } expect(message.data["topic_id"]).to eq(private_message_topic.id) expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id)) @@ -162,10 +160,7 @@ describe TopicTrackingState do group_channel, "#{group_channel}/archive" ].each do |channel| - message = messages.find do |message| - message.channel == channel - end - + message = messages.find { |m| m.channel == channel } expect(message.data["topic_id"]).to eq(private_message_topic.id) expect(message.user_ids).to eq(group.users.map(&:id)) end @@ -211,9 +206,7 @@ describe TopicTrackingState do [user.id], [group.users.first.id] ]).each do |channel, user_ids| - message = messages.find do |message| - message.channel == channel - end + message = messages.find { |m| m.channel == channel } expect(message.data["topic_id"]).to eq(private_message_topic.id) expect(message.user_ids).to eq(user_ids) @@ -239,10 +232,7 @@ describe TopicTrackingState do expect(messages.map(&:channel)).to eq(expected_channels) expected_channels.each do |channel| - message = messages.find do |message| - message.channel = channel - end - + message = messages.find { |m| m.channel = channel } expect(message.data["topic_id"]).to eq(private_message_topic.id) expect(message.user_ids).to eq([private_message_post.user_id]) end diff --git a/spec/services/topic_status_updater_spec.rb b/spec/services/topic_status_updater_spec.rb index 14106b10f1f..70ce71a74d4 100644 --- a/spec/services/topic_status_updater_spec.rb +++ b/spec/services/topic_status_updater_spec.rb @@ -44,7 +44,7 @@ describe TopicStatusUpdater do topic = create_topic called = false - updater = -> (topic) { called = true } + updater = -> (_) { called = true } DiscourseEvent.on(:topic_closed, &updater) TopicStatusUpdater.new(topic, admin).update!("closed", true)