Update files to pass modern rubocop checks. Disable some of them

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

lib/gitlab_access_status.rb

@@ -14,12 +14,12 @@ class GitAccessStatus
 
   def self.create_from_json(json)
     values = JSON.parse(json)
-    self.new(values["status"],
-             values["message"],
-             gl_repository: values["gl_repository"],
-             gl_username: values["gl_username"],
-             repository_path: values["repository_path"],
-             gitaly: values["gitaly"])
+    new(values["status"],
+        values["message"],
+        gl_repository: values["gl_repository"],
+        gl_username: values["gl_username"],
+        repository_path: values["repository_path"],
+        gitaly: values["gitaly"])
   end
 
   def allowed?

lib/gitlab_custom_hook.rb

@@ -47,7 +47,7 @@ class GitlabCustomHook
     # Submit changes to the hook via its stdin.
     begin
       IO.copy_stream(StringIO.new(changes), stdin_writer)
-    rescue Errno::EPIPE
+    rescue Errno::EPIPE # rubocop:disable Lint/HandleExceptions
       # It is not an error if the hook does not consume all of its input.
     end
 

lib/gitlab_keys.rb

@@ -4,19 +4,26 @@ require_relative 'gitlab_config'
 require_relative 'gitlab_logger'
 require_relative 'gitlab_metrics'
 
-class GitlabKeys
-  class KeyError < StandardError ; end
+class GitlabKeys # rubocop:disable Metrics/ClassLength
+  class KeyError < StandardError; end
 
   attr_accessor :auth_file, :key
 
   def self.command(key_id)
-    raise KeyError.new("Invalid key_id: #{key_id.inspect}") unless /\A[a-z0-9-]+\z/ =~ key_id
+    unless /\A[a-z0-9-]+\z/ =~ key_id
+      raise KeyError, "Invalid key_id: #{key_id.inspect}"
+    end
+
     "#{ROOT_PATH}/bin/gitlab-shell #{key_id}"
   end
 
   def self.key_line(key_id, public_key)
     public_key.chomp!
-    raise KeyError.new("Invalid public_key: #{public_key.inspect}") if public_key.include?("\n")
+
+    if public_key.include?("\n")
+      raise KeyError, "Invalid public_key: #{public_key.inspect}"
+    end
+
     "command=\"#{command(key_id)}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}"
   end
 
@@ -31,19 +38,19 @@ class GitlabKeys
   def exec
     GitlabMetrics.measure("command-#{@command}") do
       case @command
-      when 'add-key';
+      when 'add-key'
         add_key
-      when 'batch-add-keys';
+      when 'batch-add-keys'
         batch_add_keys
-      when 'rm-key';
+      when 'rm-key'
         rm_key
-      when 'list-keys';
+      when 'list-keys'
         list_keys
-      when 'list-key-ids';
+      when 'list-key-ids'
         list_key_ids
-      when 'clear';
+      when 'clear'
         clear
-      when 'check-permissions';
+      when 'check-permissions'
         check_permissions
       else
         $logger.warn "Attempt to execute invalid gitlab-keys command #{@command.inspect}."
@@ -111,7 +118,7 @@ class GitlabKeys
     lock do
       $logger.info "Removing key #{@key_id}"
       open_auth_file('r+') do |f|
-        while line = f.gets do
+        while line = f.gets # rubocop:disable Style/AssignmentInCondition
           next unless line.start_with?("command=\"#{self.class.command(@key_id)}\"")
           f.seek(-line.length, IO::SEEK_CUR)
           # Overwrite the line with #'s. Because the 'line' variable contains
@@ -145,7 +152,7 @@ class GitlabKeys
     File.open(lock_file, "w+") do |f|
       begin
         f.flock File::LOCK_EX
-        Timeout::timeout(timeout) { yield }
+        Timeout.timeout(timeout) { yield }
       ensure
         f.flock File::LOCK_UN
       end

lib/gitlab_lfs_authentication.rb

@@ -11,12 +11,10 @@ class GitlabLfsAuthentication
   end
 
   def self.build_from_json(json)
-    begin
-      values = JSON.parse(json)
-      self.new(values['username'], values['lfs_token'], values['repository_http_path'])
-    rescue
-      nil
-    end
+    values = JSON.parse(json)
+    new(values['username'], values['lfs_token'], values['repository_http_path'])
+  rescue
+    nil
   end
 
   def authentication_payload

lib/gitlab_logger.rb

@@ -2,7 +2,7 @@ require 'logger'
 
 require_relative 'gitlab_config'
 
-def convert_log_level log_level
+def convert_log_level(log_level)
   Logger.const_get(log_level.upcase)
 rescue NameError
   $stderr.puts "WARNING: Unrecognized log level #{log_level.inspect}."

lib/gitlab_net.rb

@@ -8,7 +8,7 @@ require_relative 'gitlab_access'
 require_relative 'gitlab_lfs_authentication'
 require_relative 'httpunix'
 
-class GitlabNet
+class GitlabNet # rubocop:disable Metrics/ClassLength
   class ApiUnreachableError < StandardError; end
   class NotFound < StandardError; end
 
@@ -16,7 +16,7 @@ class GitlabNet
   READ_TIMEOUT = 300
 
   def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {})
-    changes = changes.join("\n") unless changes.kind_of?(String)
+    changes = changes.join("\n") unless changes.is_a?(String)
 
     params = {
       action: cmd,
@@ -73,7 +73,7 @@ class GitlabNet
   end
 
   def merge_request_urls(gl_repository, repo_path, changes)
-    changes = changes.join("\n") unless changes.kind_of?(String)
+    changes = changes.join("\n") unless changes.is_a?(String)
     changes = changes.encode('UTF-8', 'ASCII', invalid: :replace, replace: '')
     url = "#{host}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
     url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
@@ -152,7 +152,7 @@ class GitlabNet
     "#{config.gitlab_url}/api/v4/internal"
   end
 
-  def http_client_for(uri, options={})
+  def http_client_for(uri, options = {})
     http = if uri.is_a?(URI::HTTPUNIX)
              Net::HTTPUNIX.new(uri.hostname)
            else
@@ -189,7 +189,7 @@ class GitlabNet
     request
   end
 
-  def request(method, url, params = {}, options={})
+  def request(method, url, params = {}, options = {})
     $logger.debug "Performing #{method.to_s.upcase} #{url}"
 
     uri = URI.parse(url)
@@ -205,7 +205,7 @@ class GitlabNet
       raise ApiUnreachableError
     ensure
       $logger.info do
-        sprintf('%s %s %0.5f', method.to_s.upcase, url, Time.new - start_time)
+        sprintf('%s %s %0.5f', method.to_s.upcase, url, Time.new - start_time) # rubocop:disable Style/FormatString
       end
     end
 
@@ -218,7 +218,7 @@ class GitlabNet
     response
   end
 
-  def get(url, options={})
+  def get(url, options = {})
     request(:get, url, {}, options)
   end
 
@@ -231,13 +231,11 @@ class GitlabNet
       store = OpenSSL::X509::Store.new
       store.set_default_paths
 
-      if ca_file = config.http_settings['ca_file']
-        store.add_file(ca_file)
-      end
+      ca_file = config.http_settings['ca_file']
+      store.add_file(ca_file) if ca_file
 
-      if ca_path = config.http_settings['ca_path']
-        store.add_path(ca_path)
-      end
+      ca_path = config.http_settings['ca_path']
+      store.add_path(ca_path) if ca_path
 
       store
     end

lib/gitlab_post_receive.rb

@@ -50,9 +50,9 @@ class GitlabPostReceive
   def print_merge_request_link(merge_request)
     message =
       if merge_request["new_merge_request"]
-        "To create a merge request for #{merge_request["branch_name"]}, visit:"
+        "To create a merge request for #{merge_request['branch_name']}, visit:"
       else
-        "View merge request for #{merge_request["branch_name"]}:"
+        "View merge request for #{merge_request['branch_name']}:"
       end
 
     puts message

lib/gitlab_shell.rb

@@ -4,7 +4,7 @@ require 'pathname'
 require_relative 'gitlab_net'
 require_relative 'gitlab_metrics'
 
-class GitlabShell
+class GitlabShell # rubocop:disable Metrics/ClassLength
   class AccessDeniedError < StandardError; end
   class DisallowedCommandError < StandardError; end
   class InvalidRepositoryPathError < StandardError; end
@@ -12,7 +12,7 @@ class GitlabShell
   GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-lfs-authenticate).freeze
   GITALY_MIGRATED_COMMANDS = {
     'git-upload-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-pack'),
-    'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack'),
+    'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack')
   }.freeze
   API_COMMANDS = %w(2fa_recovery_codes).freeze
   GL_PROTOCOL = 'ssh'.freeze
@@ -44,7 +44,7 @@ class GitlabShell
     process_cmd(args)
 
     true
-  rescue GitlabNet::ApiUnreachableError => ex
+  rescue GitlabNet::ApiUnreachableError
     $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
     false
   rescue AccessDeniedError => ex
@@ -53,13 +53,13 @@ class GitlabShell
 
     $stderr.puts "GitLab: #{ex.message}"
     false
-  rescue DisallowedCommandError => ex
+  rescue DisallowedCommandError
     message = "gitlab-shell: Attempt to execute disallowed command <#{origin_cmd}> by #{log_username}."
     $logger.warn message
 
     $stderr.puts "GitLab: Disallowed command"
     false
-  rescue InvalidRepositoryPathError => ex
+  rescue InvalidRepositoryPathError
     $stderr.puts "GitLab: Invalid repository path"
     false
   end
@@ -113,7 +113,7 @@ class GitlabShell
   end
 
   def process_cmd(args)
-    return self.send("api_#{@command}") if API_COMMANDS.include?(@command)
+    return send("api_#{@command}") if API_COMMANDS.include?(@command)
 
     if @command == 'git-lfs-authenticate'
       GitlabMetrics.measure('lfs-authenticate') do
@@ -126,7 +126,7 @@ class GitlabShell
     executable = @command
     args = [repo_path]
 
-    if GITALY_MIGRATED_COMMANDS.has_key?(executable) && @gitaly
+    if GITALY_MIGRATED_COMMANDS.key?(executable) && @gitaly
       executable = GITALY_MIGRATED_COMMANDS[executable]
 
       gitaly_address = @gitaly['address']
@@ -172,15 +172,15 @@ class GitlabShell
     end
 
     if git_trace_available?
-      env.merge!({
+      env.merge!(
         'GIT_TRACE' => @config.git_trace_log_file,
         'GIT_TRACE_PACKET' => @config.git_trace_log_file,
-        'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file,
-      })
+        'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file
+      )
     end
 
     # We use 'chdir: ROOT_PATH' to let the next executable know where config.yml is.
-    Kernel::exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
+    Kernel.exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
   end
 
   def api

lib/httpunix.rb

@@ -11,7 +11,7 @@ module URI
   class HTTPUNIX < HTTP
     def hostname
       # decode %XX from path to file
-      v = self.host
+      v = host
       URI.decode(v)
     end
 
@@ -30,7 +30,7 @@ end
 # - Net::HTTP::connect
 module Net
   class HTTPUNIX < HTTP
-    def initialize(socketpath, port=nil)
+    def initialize(socketpath, port = nil)
       super(socketpath, port)
       @port = nil # HTTP will set it to default - override back -> set DEFAULT_PORT
     end

support/go-format

@@ -5,7 +5,7 @@ def main(check)
   cmd = %w[gofmt -s -l]
   cmd << '-w' unless check
   cmd += go_files
-  output = IO.popen(cmd, 'r') { |io| io.read }
+  output = IO.popen(cmd, 'r', &:read)
   $stdout.write(output)
   abort 'gofmt failed' unless $?.success?
   if check && output.lines.any? { |l| l != "\n" }

support/go_build.rb

@@ -12,7 +12,7 @@ module GoBuild
 
   GO_ENV = {
     'GOPATH' => BUILD_DIR,
-    'GO15VENDOREXPERIMENT' => '1',
+    'GO15VENDOREXPERIMENT' => '1'
   }.freeze
 
   def create_fresh_build_dir
@@ -26,7 +26,7 @@ module GoBuild
     raise "env must be a hash" unless env.is_a?(Hash)
     raise "cmd must be an array" unless cmd.is_a?(Array)
 
-    if !system(env, *cmd)
+    unless system(env, *cmd)
       abort "command failed: #{env.inspect} #{cmd.join(' ')}"
     end
   end