Merge branch 'rubocop/enable-access-modifiers-cops' into 'master'

Enable Rubocop cops that check access modifiers

## What does this MR do?

This MR enables Rubocop cops that detect methods that should be restricted but are the part of public API because of access modifiers used improperly.

This also fixes existing offenses.

## Why was this MR needed?

Some method in our codebase are public instead of being private because it is sometimes difficult to get it right without static analysis.

## What are the relevant issue numbers?

See #17478  
Closes #17372 

See merge request !5014

8 files changed, 76 insertions, 66 deletions

diff --git a/lib/banzai/filter/emoji_filter.rb b/lib/banzai/filter/emoji_filter.rb
index ae7d31cf191..2492b5213ac 100644
--- a/lib/banzai/filter/emoji_filter.rb
+++ b/lib/banzai/filter/emoji_filter.rb
@@ -38,6 +38,11 @@ module Banzai
         end
       end
 
+      # Build a regexp that matches all valid :emoji: names.
+      def self.emoji_pattern
+        @emoji_pattern ||= /:(#{Gitlab::Emoji.emojis_names.map { |name| Regexp.escape(name) }.join('|')}):/
+      end
+
       private
 
       def emoji_url(name)
@@ -59,11 +64,6 @@ module Banzai
         ActionController::Base.helpers.url_to_image(image)
       end
 
-      # Build a regexp that matches all valid :emoji: names.
-      def self.emoji_pattern
-        @emoji_pattern ||= /:(#{Gitlab::Emoji.emojis_names.map { |name| Regexp.escape(name) }.join('|')}):/
-      end
-
       def emoji_pattern
         self.class.emoji_pattern
       end
diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb
index 9b209533a89..ff580ec68f8 100644
--- a/lib/banzai/filter/markdown_filter.rb
+++ b/lib/banzai/filter/markdown_filter.rb
@@ -12,7 +12,12 @@ module Banzai
         html
       end
 
-      private
+      def self.renderer
+        @renderer ||= begin
+          renderer = Redcarpet::Render::HTML.new
+          Redcarpet::Markdown.new(renderer, redcarpet_options)
+        end
+      end
 
       def self.redcarpet_options
         # https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
@@ -28,12 +33,7 @@ module Banzai
         }.freeze
       end
 
-      def self.renderer
-        @renderer ||= begin
-          renderer = Redcarpet::Render::HTML.new
-          Redcarpet::Markdown.new(renderer, redcarpet_options)
-        end
-      end
+      private_class_method :redcarpet_options
     end
   end
 end
diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb
index 910687a7b6a..a4ae27eefd8 100644
--- a/lib/banzai/renderer.rb
+++ b/lib/banzai/renderer.rb
@@ -1,5 +1,7 @@
 module Banzai
   module Renderer
+    extend self
+
     # Convert a Markdown String into an HTML-safe String of HTML
     #
     # Note that while the returned HTML will have been sanitized of dangerous
@@ -14,7 +16,7 @@ module Banzai
     # context  - Hash of context options passed to our HTML Pipeline
     #
     # Returns an HTML-safe String
-    def self.render(text, context = {})
+    def render(text, context = {})
       cache_key = context.delete(:cache_key)
       cache_key = full_cache_key(cache_key, context[:pipeline])
 
@@ -52,7 +54,7 @@ module Banzai
     #    texts_and_contexts
     #    => [{ text: '### Hello',
     #          context: { cache_key: [note, :note] } }]
-    def self.cache_collection_render(texts_and_contexts)
+    def cache_collection_render(texts_and_contexts)
       items_collection = texts_and_contexts.each_with_index do |item, index|
         context = item[:context]
         cache_key = full_cache_multi_key(context.delete(:cache_key), context[:pipeline])
@@ -81,7 +83,7 @@ module Banzai
       items_collection.map { |item| item[:rendered] }
     end
 
-    def self.render_result(text, context = {})
+    def render_result(text, context = {})
       text = Pipeline[:pre_process].to_html(text, context) if text
 
       Pipeline[context[:pipeline]].call(text, context)
@@ -100,7 +102,7 @@ module Banzai
     #            :user      - User object
     #
     # Returns an HTML-safe String
-    def self.post_process(html, context)
+    def post_process(html, context)
       context = Pipeline[context[:pipeline]].transform_context(context)
 
       pipeline = Pipeline[:post_process]
@@ -113,7 +115,7 @@ module Banzai
 
     private
 
-    def self.cacheless_render(text, context = {})
+    def cacheless_render(text, context = {})
       Gitlab::Metrics.measure(:banzai_cacheless_render) do
         result = render_result(text, context)
 
@@ -126,7 +128,7 @@ module Banzai
       end
     end
 
-    def self.full_cache_key(cache_key, pipeline_name)
+    def full_cache_key(cache_key, pipeline_name)
       return unless cache_key
       ["banzai", *cache_key, pipeline_name || :full]
     end
@@ -134,7 +136,7 @@ module Banzai
     # To map Rails.cache.read_multi results we need to know the Rails.cache.expanded_key.
     # Other option will be to generate stringified keys on our side and don't delegate to Rails.cache.expanded_key
     # method.
-    def self.full_cache_multi_key(cache_key, pipeline_name)
+    def full_cache_multi_key(cache_key, pipeline_name)
       return unless cache_key
       Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name))
     end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index aedc28fe1d0..2de82d40c9d 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -40,7 +40,7 @@ module Gitlab
               Hash[(@nodes || {}).map { |key, factory| [key, factory.dup] }]
             end
 
-            private
+            private # rubocop:disable Lint/UselessAccessModifier
 
             def node(key, node, metadata)
               factory = Node::Factory.new(node)
diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb
index 078609c86f1..55b8f888d53 100644
--- a/lib/gitlab/database.rb
+++ b/lib/gitlab/database.rb
@@ -55,12 +55,12 @@ module Gitlab
       end
     end
 
-    private
-
     def self.connection
       ActiveRecord::Base.connection
     end
 
+    private_class_method :connection
+
     def self.database_version
       row = connection.execute("SELECT VERSION()").first
 
@@ -70,5 +70,7 @@ module Gitlab
         row.first
       end
     end
+
+    private_class_method :database_version
   end
 end
diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb
index 28ad637fda4..55708d42161 100644
--- a/lib/gitlab/diff/inline_diff.rb
+++ b/lib/gitlab/diff/inline_diff.rb
@@ -19,24 +19,6 @@ module Gitlab
 
       attr_accessor :old_line, :new_line, :offset
 
-      def self.for_lines(lines)
-        changed_line_pairs = self.find_changed_line_pairs(lines)
-
-        inline_diffs = []
-
-        changed_line_pairs.each do |old_index, new_index|
-          old_line = lines[old_index]
-          new_line = lines[new_index]
-
-          old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
-
-          inline_diffs[old_index] = old_diffs
-          inline_diffs[new_index] = new_diffs
-        end
-
-        inline_diffs
-      end
-
       def initialize(old_line, new_line, offset: 0)
         @old_line = old_line[offset..-1]
         @new_line = new_line[offset..-1]
@@ -63,32 +45,54 @@ module Gitlab
         [old_diffs, new_diffs]
       end
 
-      private
+      class << self
+        def for_lines(lines)
+          changed_line_pairs = find_changed_line_pairs(lines)
 
-      # Finds pairs of old/new line pairs that represent the same line that changed
-      def self.find_changed_line_pairs(lines)
-        # Prefixes of all diff lines, indicating their types
-        # For example: `" - +  -+  ---+++ --+  -++"`
-        line_prefixes = lines.each_with_object("") { |line, s| s << line[0] }.gsub(/[^ +-]/, ' ')
+          inline_diffs = []
 
-        changed_line_pairs = []
-        line_prefixes.scan(LINE_PAIRS_PATTERN) do
-          # For `"---+++"`, `begin_index == 0`, `end_index == 6`
-          begin_index, end_index = Regexp.last_match.offset(:del_ins)
+          changed_line_pairs.each do |old_index, new_index|
+            old_line = lines[old_index]
+            new_line = lines[new_index]
 
-          # For `"---+++"`, `changed_line_count == 3`
-          changed_line_count = (end_index - begin_index) / 2
+            old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
 
-          halfway_index = begin_index + changed_line_count
-          (begin_index...halfway_index).each do |i|
-            # For `"---+++"`, index 1 maps to 1 + 3 = 4
-            changed_line_pairs << [i, i + changed_line_count]
+            inline_diffs[old_index] = old_diffs
+            inline_diffs[new_index] = new_diffs
           end
+
+          inline_diffs
         end
 
-        changed_line_pairs
+        private
+
+        # Finds pairs of old/new line pairs that represent the same line that changed
+        def find_changed_line_pairs(lines)
+          # Prefixes of all diff lines, indicating their types
+          # For example: `" - +  -+  ---+++ --+  -++"`
+          line_prefixes = lines.each_with_object("") { |line, s| s << line[0] }.gsub(/[^ +-]/, ' ')
+
+          changed_line_pairs = []
+          line_prefixes.scan(LINE_PAIRS_PATTERN) do
+            # For `"---+++"`, `begin_index == 0`, `end_index == 6`
+            begin_index, end_index = Regexp.last_match.offset(:del_ins)
+
+            # For `"---+++"`, `changed_line_count == 3`
+            changed_line_count = (end_index - begin_index) / 2
+
+            halfway_index = begin_index + changed_line_count
+            (begin_index...halfway_index).each do |i|
+              # For `"---+++"`, index 1 maps to 1 + 3 = 4
+              changed_line_pairs << [i, i + changed_line_count]
+            end
+          end
+
+          changed_line_pairs
+        end
       end
 
+      private
+
       def longest_common_prefix(a, b)
         max_length = [a.length, b.length].max
 
diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 86a5b9a201a..081576a440c 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -141,10 +141,10 @@ module Gitlab
       end
     end
 
-    private
-
     def self.current_transaction
       Transaction.current
     end
+
+    private_class_method :current_transaction
   end
 end
diff --git a/lib/gitlab/themes.rb b/lib/gitlab/themes.rb
index 83f91de810c..d4020af76f9 100644
--- a/lib/gitlab/themes.rb
+++ b/lib/gitlab/themes.rb
@@ -2,6 +2,8 @@ module Gitlab
   # Module containing GitLab's application theme definitions and helper methods
   # for accessing them.
   module Themes
+    extend self
+
     # Theme ID used when no `default_theme` configuration setting is provided.
     APPLICATION_DEFAULT = 2
 
@@ -22,7 +24,7 @@ module Gitlab
     # classes that might be applied to the `body` element
     #
     # Returns a String
-    def self.body_classes
+    def body_classes
       THEMES.collect(&:css_class).uniq.join(' ')
     end
 
@@ -33,26 +35,26 @@ module Gitlab
     # id - Integer ID
     #
     # Returns a Theme
-    def self.by_id(id)
+    def by_id(id)
       THEMES.detect { |t| t.id == id } || default
     end
 
     # Returns the number of defined Themes
-    def self.count
+    def count
       THEMES.size
     end
 
     # Get the default Theme
     #
     # Returns a Theme
-    def self.default
+    def default
       by_id(default_id)
     end
 
     # Iterate through each Theme
     #
     # Yields the Theme object
-    def self.each(&block)
+    def each(&block)
       THEMES.each(&block)
     end
 
@@ -61,7 +63,7 @@ module Gitlab
     # user - User record
     #
     # Returns a Theme
-    def self.for_user(user)
+    def for_user(user)
       if user
         by_id(user.theme_id)
       else
@@ -71,7 +73,7 @@ module Gitlab
 
     private
 
-    def self.default_id
+    def default_id
       id = Gitlab.config.gitlab.default_theme.to_i
 
       # Prevent an invalid configuration setting from causing an infinite loop