Maintenance: Custom Rubocop matcher for making sure "unless" conditional is not used in Ruby

.rubocop/cop/zammad/prefer_negated_if_over_unless.rb

@@ -0,0 +1,30 @@
+module RuboCop
+  module Cop
+    module Zammad
+      # This cop is used to identify usages of `unless` conditionals
+      #
+      # @example
+      #   # bad
+      #   unless statement
+      #   return unless statement
+      #
+      #   # good
+      #   if !statement
+      #   return if !statement
+      class PreferNegatedIfOverUnless < Cop
+        include ConfigurableEnforcedStyle
+        include NegativeConditional
+        extend AutoCorrector
+
+        MSG = 'Favor `if !foobar` over `unless foobar` for ' \
+              'negative conditions.'.freeze
+
+        def on_if(node)
+          return unless node.unless?
+
+          add_offense(node, message: MSG)
+        end
+      end
+    end
+  end
+end

.rubocop/default.yml

@@ -291,3 +291,7 @@ Layout/MultilineMethodCallIndentation:
   EnforcedStyle: indented
   Include:
     - "**/*_spec.rb"
+
+Zammad/PreferNegatedIfOverUnless:
+  Exclude:
+    - 'bin/rspec'

.rubocop/rubocop_zammad.rb

@@ -1 +1,2 @@
 require_relative 'cop/zammad/exists_condition'
+require_relative 'cop/zammad/prefer_negated_if_over_unless'

app/helpers/knowledge_base_helper.rb

@@ -6,19 +6,15 @@ module KnowledgeBaseHelper
   end
 
   def custom_path_if_needed(path, knowledge_base, full: false)
-    return path unless knowledge_base.custom_address_matches? request
+    return path if !knowledge_base.custom_address_matches?(request)
 
     custom_address = knowledge_base.custom_address_uri
-    return path unless custom_address
+    return path if !custom_address
 
-    output = path.gsub(%r{^/help}, custom_address.path || '').presence || '/'
+    custom_path = path.gsub(%r{^/help}, custom_address.path || '').presence || '/'
+    prefix      = full ? knowledge_base.custom_path_prefix(request) : ''
 
-    if full
-      fqdn = request.headers.env['SERVER_NAME']
-      output = "#{custom_address.scheme}://#{custom_address.host || fqdn}#{output}"
-    end
-
-    output
+    "#{prefix}#{custom_path}"
   end
 
   def translation_locale_code(translation)

app/helpers/knowledge_base_rich_text_helper.rb

@@ -42,7 +42,7 @@ module KnowledgeBaseRichTextHelper
               "https://player.vimeo.com/video/#{settings[:id]}"
             end
 
-      return match unless url
+      return match if !url
 
       "<div class='videoWrapper'><iframe id='#{settings[:provider]}#{settings[:id]}' type='text/html' src='#{url}' frameborder='0'></iframe></div>"
     end

app/models/channel/driver/imap.rb

@@ -378,7 +378,7 @@ returns
   def self.extract_rfc822_headers(message_meta)
     blob = message_meta&.attr&.dig 'RFC822.HEADER'
 
-    return unless blob
+    return if !blob
 
     parse_rfc822_headers blob
   end

app/models/knowledge_base.rb

@@ -93,6 +93,11 @@ class KnowledgeBase < ApplicationModel
     false
   end
 
+  def custom_address_prefix(request)
+    host = custom_address.host || request.headers.env['SERVER_NAME']
+    "#{custom_address.scheme}://#{host}"
+  end
+
   def full_destroy!
     ChecksKbClientNotification.disable_in_all_classes!
 

lib/knowledge_base/menu_item_update_action.rb

@@ -11,7 +11,7 @@ class KnowledgeBase
     end
 
     def perform!
-      raise_unprocessable unless all_ids_present?
+      raise_unprocessable if !all_ids_present?
 
       KnowledgeBase::MenuItem.transaction do
         KnowledgeBase::MenuItem.acts_as_list_no_update do

lib/tasks/zammad/setup/db_config.rake

@@ -9,7 +9,7 @@ namespace :zammad do
       template    = config_dir.join('database', 'database.yml')
       destination = config_dir.join('database.yml')
 
-      raise Errno::ENOENT, "#{template} not found" unless File.exist?(template)
+      raise Errno::ENOENT, "#{template} not found" if !File.exist?(template)
 
       if File.exist?(destination)
         next if FileUtils.identical?(template, destination)

script/scheduler.rb

@@ -3,7 +3,7 @@
 begin
   load File.expand_path('../bin/spring', __dir__)
 rescue LoadError => e
-  raise unless e.message.include?('spring')
+  raise if !e.message.include?('spring')
 end
 
 dir = File.expand_path(File.join(File.dirname(__FILE__), '..'))

script/websocket-server.rb

@@ -3,7 +3,7 @@
 begin
   load File.expand_path('../bin/spring', __dir__)
 rescue LoadError => e
-  raise unless e.message.include?('spring')
+  raise if !e.message.include?('spring')
 end
 
 dir = File.expand_path(File.join(File.dirname(__FILE__), '..'))

spec/support/db_migration.rb

@@ -67,7 +67,7 @@ module DbMigrationHelper
   def without_column(from_table, column:)
     suppress_messages do
       Array(column).each do |elem|
-        next unless column_exists?(from_table, elem)
+        next if !column_exists?(from_table, elem)
 
         remove_column(from_table, elem)
       end

spec/system/ticket/zoom_spec.rb

@@ -859,7 +859,7 @@ RSpec.describe 'Ticket zoom', type: :system do
 
       image_a.height.times do |y|
         image_a.row(y).each_with_index do |pixel, x|
-          return false unless pixel == image_b[x, y]
+          return false if pixel != image_b[x, y]
         end
       end
 

test/browser_test_helper.rb

@@ -514,7 +514,7 @@ class TestCase < ActiveSupport::TestCase
 
     begin
       elements = instance.find_elements(find_element_key => params[param_key])
-                         .tap { |e| e.slice!(1..-1) unless params[:all] }
+                         .tap { |e| e.slice!(1..-1) if !params[:all] }
 
       if elements.empty?
         return if params[:only_if_exists] == true