DEV: Enable unless cops

We discussed the use of `unless` internally and decided to enforce
available rules from rubocop to restrict its most problematic uses.

.rubocop.yml

@@ -7,3 +7,7 @@ Discourse/NoAddReferenceOrAliasesActiveRecordMigration:
 
 Discourse/NoResetColumnInformationInMigrations:
   Enabled: true
+
+Lint/Debugger:
+  Exclude:
+    - script/**/*

Gemfile.lock

@@ -411,7 +411,7 @@ GEM
       rspec-core (>= 2.14)
     rtlcss (0.2.0)
       mini_racer (~> 0.6.3)
-    rubocop (1.44.1)
+    rubocop (1.45.1)
       json (~> 2.3)
       parallel (~> 1.10)
       parser (>= 3.2.0.0)
@@ -423,9 +423,9 @@ GEM
       unicode-display_width (>= 2.4.0, < 3.0)
     rubocop-ast (1.26.0)
       parser (>= 3.2.1.0)
-    rubocop-capybara (2.17.0)
+    rubocop-capybara (2.17.1)
       rubocop (~> 1.41)
-    rubocop-discourse (3.0.3)
+    rubocop-discourse (3.1.0)
       rubocop (>= 1.1.0)
       rubocop-rspec (>= 2.0.0)
     rubocop-rspec (2.18.1)

app/controllers/edit_directory_columns_controller.rb

@@ -48,7 +48,7 @@ class EditDirectoryColumnsController < ApplicationController
         .where(directory_column: { user_field_id: nil })
         .where("show_on_profile=? OR show_on_user_card=?", true, true)
 
-    return unless user_fields_without_column.count > 0
+    return if user_fields_without_column.count <= 0
 
     next_position = DirectoryColumn.maximum("position") + 1
 

app/controllers/posts_controller.rb

@@ -541,7 +541,7 @@ class PostsController < ApplicationController
       ] if post_revision.modifications["category_id"].present? &&
         post_revision.modifications["category_id"][0] != topic.category.id
     end
-    return render_json_error(I18n.t("revert_version_same")) unless changes.length > 0
+    return render_json_error(I18n.t("revert_version_same")) if changes.length <= 0
     changes[:edit_reason] = I18n.with_locale(SiteSetting.default_locale) do
       I18n.t("reverted_to_version", version: post_revision.number.to_i - 1)
     end

app/controllers/topics_controller.rb

@@ -89,7 +89,7 @@ class TopicsController < ApplicationController
     end
 
     if opts[:print]
-      raise Discourse::InvalidAccess unless SiteSetting.max_prints_per_hour_per_user > 0
+      raise Discourse::InvalidAccess if SiteSetting.max_prints_per_hour_per_user.zero?
       begin
         unless @guardian.is_admin?
           RateLimiter.new(

app/controllers/user_avatars_controller.rb

@@ -104,7 +104,7 @@ class UserAvatarsController < ApplicationController
     return render_blank if version > OptimizedImage::VERSION
 
     upload_id = upload_id.to_i
-    return render_blank unless upload_id > 0
+    return render_blank if upload_id <= 0
 
     size = params[:size].to_i
     return render_blank if size < 8 || size > 1000

app/jobs/scheduled/auto_queue_handler.rb

@@ -7,7 +7,7 @@ module Jobs
     every 1.day
 
     def execute(args)
-      return unless SiteSetting.auto_handle_queued_age.to_i > 0
+      return if SiteSetting.auto_handle_queued_age.to_i.zero?
 
       Reviewable
         .pending

app/jobs/scheduled/pending_queued_posts_reminder.rb

@@ -5,7 +5,7 @@ module Jobs
     every 15.minutes
 
     def execute(args)
-      return true unless SiteSetting.notify_about_queued_posts_after > 0
+      return true if SiteSetting.notify_about_queued_posts_after.zero?
 
       queued_post_ids = should_notify_ids
 

app/models/admin_dashboard_data.rb

@@ -295,7 +295,7 @@ class AdminDashboardData
 
   def queue_size_check
     queue_size = Jobs.queued
-    I18n.t("dashboard.queue_size_warning", queue_size: queue_size) unless queue_size < 100_000
+    I18n.t("dashboard.queue_size_warning", queue_size: queue_size) if queue_size >= 100_000
   end
 
   def ram_check

app/models/global_setting.rb

@@ -63,14 +63,12 @@ class GlobalSetting
 
         define_singleton_method(key) do
           val = instance_variable_get("@#{key}_cache")
-          unless val.nil?
-            val == :missing ? nil : val
-          else
+          if val.nil?
             val = provider.lookup(key, default)
             val = :missing if val.nil?
             instance_variable_set("@#{key}_cache", val)
-            val == :missing ? nil : val
           end
+          val == :missing ? nil : val
         end
       end
   end

app/models/optimized_image.rb

@@ -24,7 +24,7 @@ class OptimizedImage < ActiveRecord::Base
   end
 
   def self.create_for(upload, width, height, opts = {})
-    return unless width > 0 && height > 0
+    return if width <= 0 || height <= 0
     return if upload.try(:sha1).blank?
 
     # no extension so try to guess it

app/models/post.rb

@@ -1085,8 +1085,8 @@ class Post < ActiveRecord::Base
       next if Rails.configuration.multisite && src.exclude?(current_db)
 
       src = "#{SiteSetting.force_https ? "https" : "http"}:#{src}" if src.start_with?("//")
-      unless Discourse.store.has_been_uploaded?(src) || Upload.secure_uploads_url?(src) ||
-               (include_local_upload && src =~ %r{\A/[^/]}i)
+      if !Discourse.store.has_been_uploaded?(src) && !Upload.secure_uploads_url?(src) &&
+           !(include_local_upload && src =~ %r{\A/[^/]}i)
         next
       end
 

app/models/topic.rb

@@ -58,7 +58,7 @@ class Topic < ActiveRecord::Base
 
   def thumbnail_info(enqueue_if_missing: false, extra_sizes: [])
     return nil unless original = image_upload
-    return nil unless original.filesize < SiteSetting.max_image_size_kb.kilobytes
+    return nil if original.filesize >= SiteSetting.max_image_size_kb.kilobytes
     return nil unless original.read_attribute(:width) && original.read_attribute(:height)
 
     infos = []
@@ -99,7 +99,7 @@ class Topic < ActiveRecord::Base
   def generate_thumbnails!(extra_sizes: [])
     return nil unless SiteSetting.create_thumbnails
     return nil unless original = image_upload
-    return nil unless original.filesize < SiteSetting.max_image_size_kb.kilobytes
+    return nil if original.filesize >= SiteSetting.max_image_size_kb.kilobytes
     return nil unless original.width && original.height
     extra_sizes = [] unless extra_sizes.kind_of?(Array)
 

app/models/topic_group.rb

@@ -51,8 +51,7 @@ class TopicGroup < ActiveRecord::Base
         AND tag.topic_id = :topic_id
     SQL
 
-    query +=
-      "AND NOT(tag.group_id IN (:already_updated_groups))" unless updated_group_ids.length.zero?
+    query += "AND NOT(tag.group_id IN (:already_updated_groups))" if updated_group_ids.present?
 
     query += <<~SQL
       ON CONFLICT(topic_id, group_id)

app/models/user_action.rb

@@ -411,9 +411,7 @@ class UserAction < ActiveRecord::Base
       visible_post_types: visible_post_types,
     )
 
-    unless (guardian.user && guardian.user.id == user_id) || guardian.is_staff?
-      builder.where("t.visible")
-    end
+    builder.where("t.visible") if guardian.user&.id != user_id && !guardian.is_staff?
 
     filter_private_messages(builder, user_id, guardian, ignore_private_messages)
     filter_categories(builder, guardian)

app/services/user_silencer.rb

@@ -23,42 +23,39 @@ class UserSilencer
 
   def silence
     hide_posts unless @opts[:keep_posts]
-    unless @user.silenced_till.present?
-      @user.silenced_till = @opts[:silenced_till] || 1000.years.from_now
-      if @user.save
-        message_type = @opts[:message] || :silenced_by_staff
+    return false if @user.silenced_till.present?
+    @user.silenced_till = @opts[:silenced_till] || 1000.years.from_now
+    if @user.save
+      message_type = @opts[:message] || :silenced_by_staff
 
-        details = StaffMessageFormat.new(:silence, @opts[:reason], @opts[:message_body]).format
+      details = StaffMessageFormat.new(:silence, @opts[:reason], @opts[:message_body]).format
 
-        context = "#{message_type}: #{@opts[:reason]}"
+      context = "#{message_type}: #{@opts[:reason]}"
 
-        if @by_user
-          log_params = { context: context, details: details }
-          log_params[:post_id] = @opts[:post_id].to_i if @opts[:post_id]
+      if @by_user
+        log_params = { context: context, details: details }
+        log_params[:post_id] = @opts[:post_id].to_i if @opts[:post_id]
 
-          @user_history = StaffActionLogger.new(@by_user).log_silence_user(@user, log_params)
-        end
-
-        silence_message_params = {}
-        DiscourseEvent.trigger(
-          :user_silenced,
-          user: @user,
-          silenced_by: @by_user,
-          reason: @opts[:reason],
-          message: @opts[:message_body],
-          user_history: @user_history,
-          post_id: @opts[:post_id],
-          silenced_till: @user.silenced_till,
-          silenced_at: DateTime.now,
-          silence_message_params: silence_message_params,
-        )
-
-        silence_message_params.merge!(post_alert_options: { skip_send_email: true })
-        SystemMessage.create(@user, message_type, silence_message_params)
-        true
+        @user_history = StaffActionLogger.new(@by_user).log_silence_user(@user, log_params)
       end
-    else
-      false
+
+      silence_message_params = {}
+      DiscourseEvent.trigger(
+        :user_silenced,
+        user: @user,
+        silenced_by: @by_user,
+        reason: @opts[:reason],
+        message: @opts[:message_body],
+        user_history: @user_history,
+        post_id: @opts[:post_id],
+        silenced_till: @user.silenced_till,
+        silenced_at: DateTime.now,
+        silence_message_params: silence_message_params,
+      )
+
+      silence_message_params.merge!(post_alert_options: { skip_send_email: true })
+      SystemMessage.create(@user, message_type, silence_message_params)
+      true
     end
   end
 

bin/bundle

@@ -73,7 +73,7 @@ m = Module.new do
 
     requirement = bundler_gem_version.approximate_recommendation
 
-    return requirement unless Gem::Version.new(Gem::VERSION) < Gem::Version.new("2.7.0")
+    return requirement if Gem::Version.new(Gem::VERSION) >= Gem::Version.new("2.7.0")
 
     requirement += ".a" if bundler_gem_version.prerelease?
 

config/site_settings.yml

@@ -1027,6 +1027,7 @@ posting:
   notify_about_queued_posts_after:
     type: float
     default: 24
+    min: 0
   auto_close_messages_post_count:
     default: 500
   auto_close_topics_post_count:
@@ -1955,6 +1956,7 @@ rate_limits:
   max_prints_per_hour_per_user:
     default: 5
     client: true
+    min: 0
   max_logins_per_ip_per_hour:
     min: 1
     default: 30

db/migrate/20210224162050_remove_emoji_one_from_emoji_set_site_setting.rb

@@ -3,7 +3,7 @@
 class RemoveEmojiOneFromEmojiSetSiteSetting < ActiveRecord::Migration[6.0]
   def up
     result = execute("SELECT value FROM site_settings WHERE name='emoji_set' and value='emoji_one'")
-    return unless result.count > 0
+    return if result.count.zero?
 
     execute "DELETE FROM site_settings where name='emoji_set' and value='emoji_one'"
     execute "UPDATE posts SET baked_version = 0 WHERE cooked LIKE '%/images/emoji/emoji_one%'"

lib/common_passwords.rb

@@ -31,7 +31,7 @@ class CommonPasswords
   end
 
   def self.password_list
-    @mutex.synchronize { load_passwords unless redis.scard(LIST_KEY) > 0 }
+    @mutex.synchronize { load_passwords if redis.scard(LIST_KEY) <= 0 }
     RedisPasswordList.new
   end
 

lib/composer_messages_finder.rb

@@ -197,8 +197,8 @@ class ComposerMessagesFinder
         .pluck(:reply_to_user_id)
         .find_all { |uid| uid != @user.id && uid == reply_to_user_id }
 
-    return unless last_x_replies.size == SiteSetting.get_a_room_threshold
-    return unless @topic.posts.count("distinct user_id") >= min_users_posted
+    return if last_x_replies.size != SiteSetting.get_a_room_threshold
+    return if @topic.posts.count("distinct user_id") < min_users_posted
 
     UserHistory.create!(
       action: UserHistory.actions[:notified_about_get_a_room],

lib/content_buffer.rb

@@ -48,7 +48,7 @@ class ContentBuffer
           @lines.insert(start_row + i, line)
           i += 1
         end
-        @lines.insert(i, "") unless @lines.length > i
+        @lines.insert(i, "") if @lines.length <= i
         @lines[i] = split[-1] + @lines[i]
       end
     end

lib/cooked_processor_mixin.rb

@@ -135,7 +135,7 @@ module CookedProcessorMixin
 
   def get_size_from_attributes(img)
     w, h = img["width"].to_i, img["height"].to_i
-    return w, h unless w <= 0 || h <= 0
+    return w, h if w > 0 && h > 0
     # if only width or height are specified attempt to scale image
     if w > 0 || h > 0
       w = w.to_f

lib/excerpt_parser.rb

@@ -98,7 +98,7 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
       end
     when "aside"
       attributes = Hash[*attributes.flatten]
-      unless (@keep_onebox_source || @keep_onebox_body) && attributes["class"]&.include?("onebox")
+      if !(@keep_onebox_source || @keep_onebox_body) || !attributes["class"]&.include?("onebox")
         @in_quote = true
       end
 

lib/file_store/to_s3_migration.rb

@@ -31,13 +31,13 @@ module FileStore
     end
 
     def self.s3_options_from_env
-      unless ENV["DISCOURSE_S3_BUCKET"].present? && ENV["DISCOURSE_S3_REGION"].present? &&
-               (
-                 (
-                   ENV["DISCOURSE_S3_ACCESS_KEY_ID"].present? &&
-                     ENV["DISCOURSE_S3_SECRET_ACCESS_KEY"].present?
-                 ) || ENV["DISCOURSE_S3_USE_IAM_PROFILE"].present?
-               )
+      if ENV["DISCOURSE_S3_BUCKET"].blank? || ENV["DISCOURSE_S3_REGION"].blank? ||
+           !(
+             (
+               ENV["DISCOURSE_S3_ACCESS_KEY_ID"].present? &&
+                 ENV["DISCOURSE_S3_SECRET_ACCESS_KEY"].present?
+             ) || ENV["DISCOURSE_S3_USE_IAM_PROFILE"].present?
+           )
         raise ToS3MigrationError.new(<<~TEXT)
           Please provide the following environment variables:
             - DISCOURSE_S3_BUCKET

lib/guardian/post_guardian.rb

@@ -25,7 +25,7 @@ module PostGuardian
   # Can the user act on the post in a particular way.
   #  taken_actions = the list of actions the user has already taken
   def post_can_act?(post, action_key, opts: {}, can_see_post: nil)
-    return false unless (can_see_post.nil? && can_see_post?(post)) || can_see_post
+    return false if !(can_see_post.nil? && can_see_post?(post)) && !can_see_post
 
     # no warnings except for staff
     if action_key == :notify_user &&

lib/guardian/topic_guardian.rb

@@ -308,7 +308,7 @@ module TopicGuardian
 
   def can_edit_featured_link?(category_id)
     return false unless SiteSetting.topic_featured_link_enabled
-    return false unless @user.trust_level >= TrustLevel.levels[:basic]
+    return false if @user.trust_level == TrustLevel.levels[:newuser]
     Category.where(
       id: category_id || SiteSetting.uncategorized_category_id,
       topic_featured_link_allowed: true,

lib/onebox/engine/allowlisted_generic_onebox.rb

@@ -145,12 +145,12 @@ module Onebox
                  !!AllowlistedGenericOnebox.allowed_twitter_labels.find { |l|
                    d[:label2] =~ /#{l}/i
                  }
-              unless Onebox::Helpers.blank?(d[:label_1])
-                d[:label_2] = Onebox::Helpers.truncate(d[:label2])
-                d[:data_2] = Onebox::Helpers.truncate(d[:data2])
-              else
+              if Onebox::Helpers.blank?(d[:label_1])
                 d[:label_1] = Onebox::Helpers.truncate(d[:label2])
                 d[:data_1] = Onebox::Helpers.truncate(d[:data2])
+              else
+                d[:label_2] = Onebox::Helpers.truncate(d[:label2])
+                d[:data_2] = Onebox::Helpers.truncate(d[:data2])
               end
             end
 

lib/onebox/engine/wikipedia_onebox.rb

@@ -23,7 +23,9 @@ module Onebox
           m_url_hash_name = m_url_hash[1]
         end
 
-        unless m_url_hash.nil?
+        if m_url_hash.nil? # no hash found in url
+          paras = raw.search("p") # default get all the paras
+        else
           section_header_title = raw.xpath("//span[@id='#{CGI.unescape(m_url_hash_name)}']")
 
           if section_header_title.empty?
@@ -49,8 +51,6 @@ module Onebox
               end
             end
           end
-        else # no hash found in url
-          paras = raw.search("p") # default get all the paras
         end
 
         unless paras.empty?

lib/onebox/helpers.rb

@@ -40,8 +40,8 @@ module Onebox
       should_ignore_canonical =
         IGNORE_CANONICAL_DOMAINS.map { |hostname| uri.hostname.match?(hostname) }.any?
 
-      unless (ignore_canonical_tag && ignore_canonical_tag["content"].to_s == "true") ||
-               should_ignore_canonical
+      if !(ignore_canonical_tag && ignore_canonical_tag["content"].to_s == "true") &&
+           !should_ignore_canonical
         # prefer canonical link
         canonical_link = doc.at('//link[@rel="canonical"]/@href')
         canonical_uri = Addressable::URI.parse(canonical_link)

lib/onebox/mixins/git_blob_onebox.rb

@@ -120,15 +120,15 @@ module Onebox
           a_lines.each do |l|
             l = l.chomp("\n") # remove new line
             m = l.match(/\A[ ]*/) # find leading spaces 0 or more
-            unless m.nil? || l.size == m[0].size || l.size == 0 # no match | only spaces in line | empty line
+            if m.nil? || l.size == m[0].size || l.size == 0
+              next # SKIP no match or line is only spaces
+            else # no match | only spaces in line | empty line
               m_str_length = m[0].size
               if m_str_length <= 1 # minimum space is 1 or nothing we can break we found our minimum
                 min_space = m_str_length
                 break #stop iteration
               end
               min_space = m_str_length if m_str_length < min_space
-            else
-              next # SKIP no match or line is only spaces
             end
           end
           a_lines.each do |l|

lib/post_creator.rb

@@ -148,7 +148,7 @@ class PostCreator
         end
       end
 
-      unless @topic.present? && (@opts[:skip_guardian] || guardian.can_create?(Post, @topic))
+      if @topic.blank? || !(@opts[:skip_guardian] || guardian.can_create?(Post, @topic))
         errors.add(:base, I18n.t(:topic_not_found))
         return false
       end

lib/post_destroyer.rb

@@ -430,8 +430,8 @@ class PostDestroyer
 
   def update_associated_category_latest_topic
     return unless @post.topic && @post.topic.category
-    unless @post.id == @post.topic.category.latest_post_id ||
-             (@post.is_first_post? && @post.topic_id == @post.topic.category.latest_topic_id)
+    if @post.id != @post.topic.category.latest_post_id &&
+         !(@post.is_first_post? && @post.topic_id == @post.topic.category.latest_topic_id)
       return
     end
 

lib/post_jobs_enqueuer.rb

@@ -44,7 +44,7 @@ class PostJobsEnqueuer
   def make_visible
     return if @topic.private_message?
     return unless SiteSetting.embed_unlisted?
-    return unless @post.post_number > 1
+    return if @post.post_number == 1
     return if @topic.visible?
     return if @post.post_type != Post.types[:regular]
 

lib/pretty_text/helpers.rb

@@ -9,13 +9,10 @@ module PrettyText
     # functions here are available to v8
     def t(key, opts)
       key = "js." + key
-      unless opts
-        I18n.t(key)
-      else
-        str = I18n.t(key, Hash[opts.entries].symbolize_keys).dup
-        opts.each { |k, v| str.gsub!("{{#{k.to_s}}}", v.to_s) }
-        str
-      end
+      return I18n.t(key) if opts.blank?
+      str = I18n.t(key, Hash[opts.entries].symbolize_keys).dup
+      opts.each { |k, v| str.gsub!("{{#{k.to_s}}}", v.to_s) }
+      str
     end
 
     def avatar_template(username)

lib/site_settings/validations.rb

@@ -243,7 +243,7 @@ module SiteSettings::Validations
 
   def validate_cors_origins(new_val)
     return if new_val.blank?
-    return unless new_val.split("|").any?(%r{/\z})
+    return if new_val.split("|").none?(%r{/\z})
     validate_error :cors_origins_should_not_have_trailing_slash
   end
 

lib/tasks/javascript.rake

@@ -213,10 +213,10 @@ task "javascript:update" => "clean_up" do
   dependencies.each do |f|
     src = "#{library_src}/#{f[:source]}"
 
-    unless f[:destination]
-      filename = f[:source].split("/").last
-    else
+    if f[:destination]
       filename = f[:destination]
+    else
+      filename = f[:source].split("/").last
     end
 
     if src.include? "highlightjs"

lib/tasks/qunit.rake

@@ -90,7 +90,7 @@ task "qunit:test", %i[timeout qunit_path filter] do |_, args|
       Net::HTTP.get(uri)
     rescue Errno::ECONNREFUSED, Errno::EADDRNOTAVAIL, Net::ReadTimeout, EOFError
       sleep 1
-      retry unless elapsed() > 60
+      retry if elapsed() <= 60
       puts "Timed out. Can not connect to forked server!"
       exit 1
     end

lib/tasks/svg_icons.rake

@@ -19,10 +19,10 @@ task "svgicons:update" do
   dependencies.each do |f|
     src = "#{library_src}/#{f[:source]}/."
 
-    unless f[:destination]
-      filename = f[:source].split("/").last
-    else
+    if f[:destination]
       filename = f[:destination]
+    else
+      filename = f[:source].split("/").last
     end
 
     dest = "#{vendor_svgs}/#{filename}"

lib/topic_query.rb

@@ -430,7 +430,7 @@ class TopicQuery
       (pinned_topics + unpinned_topics)[0...limit] if limit
     else
       offset = (page * per_page) - pinned_topics.length
-      offset = 0 unless offset > 0
+      offset = 0 if offset <= 0
       unpinned_topics.offset(offset).to_a
     end
   end

plugins/chat/app/services/base.rb

@@ -199,7 +199,7 @@ module Chat
         def call(instance, context)
           method = instance.method(method_name)
           args = {}
-          args = context.to_h if !method.arity.zero?
+          args = context.to_h if method.arity.nonzero?
           context[result_key] = Context.build
           instance.instance_exec(**args, &method)
         end

plugins/chat/lib/chat_message_rate_limiter.rb

@@ -37,7 +37,7 @@ class Chat::ChatMessageRateLimiter
 
   def silence_user
     silenced_for_minutes = SiteSetting.chat_auto_silence_duration
-    return unless silenced_for_minutes > 0
+    return if silenced_for_minutes.zero?
 
     UserSilencer.silence(
       @user,

plugins/chat/spec/support/api_schema_matcher.rb

@@ -9,7 +9,7 @@ RSpec::Matchers.define :match_response_schema do |schema|
       JSON::Validator.validate!(schema_path, object, strict: true)
     rescue JSON::Schema::ValidationError => e
       puts "-- Printing response body after validation error\n"
-      pp object
+      pp object # rubocop:disable Lint/Debugger
       raise e
     end
   end

plugins/discourse-narrative-bot/lib/discourse_narrative_bot/actions.rb

@@ -49,7 +49,7 @@ module DiscourseNarrativeBot
           SiteSetting.rate_limit_create_post
         end
 
-      return unless duration > 0
+      return if duration <= 0
 
       data = DiscourseNarrativeBot::Store.get(user.id.to_s)
       return unless data

script/import_scripts/base.rb

@@ -116,7 +116,7 @@ class ImportScripts::Base
   def reset_site_settings
     @old_site_settings.each do |key, value|
       current_value = SiteSetting.get(key)
-      SiteSetting.set(key, value) unless current_value != @site_settings_during_import[key]
+      SiteSetting.set(key, value) if current_value == @site_settings_during_import[key]
     end
 
     RateLimiter.enable

script/import_scripts/bespoke_1.rb

@@ -178,11 +178,7 @@ class ImportScripts::Bespoke < ImportScripts::Base
 
       topic = topics[post[:topic_id]]
 
-      unless topic[:post_id]
-        mapped[:category] = category_id_from_imported_category_id(topic[:category_id])
-        mapped[:title] = post[:title]
-        topic[:post_id] = post[:id]
-      else
+      if topic[:post_id]
         parent = topic_lookup_from_imported_post_id(topic[:post_id])
         next unless parent
 
@@ -195,6 +191,10 @@ class ImportScripts::Bespoke < ImportScripts::Base
             mapped[:reply_to_post_number] = reply_to_post_number
           end
         end
+      else
+        mapped[:category] = category_id_from_imported_category_id(topic[:category_id])
+        mapped[:title] = post[:title]
+        topic[:post_id] = post[:id]
       end
 
       next if topic[:deleted] || post[:deleted]

script/import_scripts/discuz_x.rb

@@ -887,7 +887,7 @@ class ImportScripts::DiscuzX < ImportScripts::Base
                               LIMIT 1",
         )
 
-      return discuzx_link unless results.size > 0
+      return discuzx_link if results.size.zero?
 
       linked_post_id = results.first["pid"]
       lookup = topic_lookup_from_imported_post_id(linked_post_id)

script/import_scripts/jive.rb

@@ -225,11 +225,7 @@ class ImportScripts::Jive < ImportScripts::Base
         next
       end
 
-      unless topic[:post_id]
-        mapped[:category] = category_id_from_imported_category_id(topic[:category_id])
-        mapped[:title] = post[:title]
-        topic[:post_id] = post[:id]
-      else
+      if topic[:post_id]
         parent = topic_lookup_from_imported_post_id(topic[:post_id])
         next unless parent
 
@@ -242,6 +238,10 @@ class ImportScripts::Jive < ImportScripts::Base
             mapped[:reply_to_post_number] = reply_to_post_number
           end
         end
+      else
+        mapped[:category] = category_id_from_imported_category_id(topic[:category_id])
+        mapped[:title] = post[:title]
+        topic[:post_id] = post[:id]
       end
 
       next if topic[:deleted] || post[:deleted]

script/import_scripts/kunena.rb

@@ -95,7 +95,7 @@ class ImportScripts::Kunena < ImportScripts::Base
         cache_rows: false,
       )
     results.each do |u|
-      next unless u["userid"].to_i > 0
+      next if u["userid"].to_i <= 0
       user = @users[u["userid"].to_i]
       if user
         user[:bio] = u["signature"]

script/import_scripts/kunena3.rb

@@ -114,7 +114,7 @@ class ImportScripts::Kunena < ImportScripts::Base
         cache_rows: false,
       )
     results.each do |u|
-      next unless u["userid"].to_i > 0
+      next if u["userid"].to_i <= 0
       user = @users[u["userid"].to_i]
       if user
         user[:bio] = u["signature"]

script/import_scripts/lithium.rb

@@ -803,12 +803,12 @@ class ImportScripts::Lithium < ImportScripts::Base
             import_mode: true,
           }
 
-          unless topic_id
+          if topic_id
+            msg[:topic_id] = topic_id
+          else
             msg[:title] = @htmlentities.decode(topic["subject"]).strip[0...255]
             msg[:archetype] = Archetype.private_message
             msg[:target_usernames] = usernames.join(",")
-          else
-            msg[:topic_id] = topic_id
           end
 
           msg

script/import_scripts/modx.rb

@@ -382,7 +382,9 @@ FROM #{TABLE_PREFIX}discuss_users
           title_username_of_pm_first_post[[title, participants]] ||= m["pmtextid"]
         end
 
-        unless topic_id
+        if topic_id
+          mapped[:topic_id] = topic_id
+        else
           mapped[:title] = title
           mapped[:archetype] = Archetype.private_message
           mapped[:target_usernames] = target_usernames.join(",")
@@ -392,8 +394,6 @@ FROM #{TABLE_PREFIX}discuss_users
             mapped[:target_usernames] = "system"
             puts "pm-#{m["pmtextid"]} has no target (#{m["touserarray"]})"
           end
-        else
-          mapped[:topic_id] = topic_id
         end
 
         skip ? nil : mapped

script/import_scripts/smf2.rb

@@ -378,7 +378,7 @@ class ImportScripts::Smf2 < ImportScripts::Base
       AttachmentPatterns.each do |p|
         pattern, emitter = *p
         body.gsub!(pattern) do |s|
-          next s unless (num = $~[:num].to_i - 1) >= 0
+          next s if (num = $~[:num].to_i - 1) < 0
           next s unless (upload = attachments[num]).present?
           use_count[num] += 1
           instance_exec(upload, &emitter)

script/import_scripts/vbulletin.rb

@@ -586,7 +586,9 @@ class ImportScripts::VBulletin < ImportScripts::Base
           title_username_of_pm_first_post[[title, participants]] ||= m["pmtextid"]
         end
 
-        unless topic_id
+        if topic_id
+          mapped[:topic_id] = topic_id
+        else
           mapped[:title] = title
           mapped[:archetype] = Archetype.private_message
           mapped[:target_usernames] = target_usernames.join(",")
@@ -596,8 +598,6 @@ class ImportScripts::VBulletin < ImportScripts::Base
             mapped[:target_usernames] = "system"
             puts "pm-#{m["pmtextid"]} has no target (#{m["touserarray"]})"
           end
-        else
-          mapped[:topic_id] = topic_id
         end
 
         skip ? nil : mapped

script/import_scripts/xenforo.rb

@@ -335,7 +335,15 @@ class ImportScripts::XenForo < ImportScripts::Base
             created_at: Time.zone.at(post["message_date"].to_i),
             import_mode: true,
           }
-          unless post["topic_id"] > 0
+          if post["topic_id"] <= 0
+            topic_id = post["topic_id"]
+            if t = topic_lookup_from_imported_post_id("pm_#{topic_id}")
+              msg[:topic_id] = t[:topic_id]
+            else
+              puts "Topic ID #{topic_id} not found, skipping post #{post["message_id"]} from #{post["user_id"]}"
+              next
+            end
+          else
             msg[:title] = post["title"]
             msg[:archetype] = Archetype.private_message
             to_user_array = PHP.unserialize(post["recipients"])
@@ -344,14 +352,6 @@ class ImportScripts::XenForo < ImportScripts::Base
               usernames = User.where(id: [discourse_user_ids]).pluck(:username)
               msg[:target_usernames] = usernames.join(",")
             end
-          else
-            topic_id = post["topic_id"]
-            if t = topic_lookup_from_imported_post_id("pm_#{topic_id}")
-              msg[:topic_id] = t[:topic_id]
-            else
-              puts "Topic ID #{topic_id} not found, skipping post #{post["message_id"]} from #{post["user_id"]}"
-              next
-            end
           end
           msg
         else

spec/support/concurrency.rb

@@ -21,7 +21,7 @@ module Concurrency
         def choose(*options)
           raise DeadEnd if options.empty?
 
-          @path << [options.size, 0] unless @index < @path.size
+          @path << [options.size, 0] if @index >= @path.size
 
           pair = @path[@index]
           raise "non-determinism" unless pair[0] == options.size