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 @@ def ensure_user_fields_have_columns
         .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 @@ def revert
       ] 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 @@ def show
     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 @@ def show_in_site(hostname)
     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 @@ class AutoQueueHandler < ::Jobs::Scheduled
     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 @@ class PendingQueuedPostsReminder < ::Jobs::Scheduled
     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 @@ def sidekiq_check
 
   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 @@ def self.load_defaults
 
         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 @@ def self.lock(upload_id, width, height)
   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 @@ def each_upload_url(fragments: nil, include_local_upload: true)
       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 @@ def filtered_topic_thumbnails(extra_sizes: [])
 
   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 @@ def thumbnail_info(enqueue_if_missing: false, extra_sizes: [])
   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 @@ def self.create_topic_group(user, topic_id, post_number, updated_group_ids)
         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 @@ def self.apply_common_filters(builder, user_id, guardian, ignore_private_message
       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 @@ def self.was_silenced_for?(post)
 
   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
-
-        details = StaffMessageFormat.new(:silence, @opts[:reason], @opts[:message_body]).format
-
-        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]
-
-          @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
+    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
+
+      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]
+
+        @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 @@ def include?(password)
   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 @@ def check_get_a_room(min_users_posted: 5)
         .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 @@ def apply_transform!(transform)
           @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 @@ def limit_size!(img)
 
   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 @@ def start_element(name, attributes = [])
       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 @@ def self.s3_options_from_site_settings
     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 @@ def can_post_link?(host: nil)
   # 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 @@ def filter_allowed_categories(records)
 
   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 @@ def data
                  !!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 @@ def data
           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 @@ def data
               end
             end
           end
-        else # no hash found in url
-          paras = raw.search("p") # default get all the paras
         end
 
         unless paras.empty?