[tech] rubocop cleanups

.rubocop.yml

@@ -27,6 +27,12 @@ Metrics/BlockLength:
 Metrics/CyclomaticComplexity:
   Enabled: false
 
+Metrics/ParameterLists:
+  Max: 8
+
+Metrics/PerceivedComplexity:
+  Max: 10
+
 Metrics/MethodLength:
   Enabled: false
 

Rakefile

@@ -22,8 +22,10 @@
 
 require "bundler/gem_tasks"
 require "rspec/core/rake_task"
+require 'rubocop/rake_task'
 
 RSpec::Core::RakeTask.new(:spec)
+RuboCop::RakeTask.new
 
 task :test_all do
   formatter = "--format progress"
@@ -39,4 +41,4 @@ task :test_all do
   sh "rspec #{rspec_args}"
 end
 
-task default: :test_all
+task default: [:rubocop, :test_all]

examples/Example1/Gemfile

@@ -5,4 +5,6 @@ REPO_ROOT = File.expand_path(File.join('..', '..'))
 gem 'u3d', path: REPO_ROOT
 
 plugins_path = File.join(File.dirname(__FILE__), 'fastlane', 'Pluginfile')
+# rubocop:disable Eval
 eval(File.read(plugins_path), binding) if File.exist?(plugins_path)
+# rubocop:enable Eval

examples/Example2/Gemfile

@@ -5,4 +5,6 @@ REPO_ROOT = File.expand_path(File.join('..', '..'))
 gem 'u3d', path: REPO_ROOT
 
 plugins_path = File.join(File.dirname(__FILE__), 'fastlane', 'Pluginfile')
+# rubocop:disable Eval
 eval(File.read(plugins_path), binding) if File.exist?(plugins_path)
+# rubocop:enable Eval

lib/u3d/commands.rb

@@ -33,6 +33,7 @@
 
 module U3d
   # API for U3d, redirecting calls to class they concern
+  # rubocop:disable ClassLength
   class Commands
     class << self
       def list_installed(options: {})
@@ -49,7 +50,7 @@ def list_installed(options: {})
         sorted_keys = vcomparators.sort.map { |v| v.version.to_s }
         sorted_keys.each do |k|
           u = map[k]
-          UI.message "%-*s%s" % [30, "Version #{u.version}", " (#{u.path})"]
+          UI.message "Version #{u.version.ljust(30)}(#{u.path})"
           packages = u.packages
           next unless options[:packages] && packages && !packages.empty?
           UI.message 'Packages:'
@@ -59,17 +60,9 @@ def list_installed(options: {})
 
       def list_available(options: {})
         ver = options[:unity_version]
-        os = options[:operating_system]
+        os = valid_os_or_current(options[:operating_system])
         rl = options[:release_level]
-        if os
-          os = os.to_sym
-          oses = U3dCore::Helper.operating_systems
-          raise "Specified OS (#{os}) isn't valid [#{oses.join(', ')}]" unless oses.include?(os)
-        else
-          os = U3dCore::Helper.operating_system
-        end
         cache = Cache.new(force_os: os, force_refresh: options[:force])
-        versions = {}
 
         return UI.error "Version #{ver} is not in cache" if ver && cache[os.id2name]['versions'][ver].nil?
 
@@ -107,9 +100,10 @@ def download(args: [], options: {})
         version = args[0]
         UI.user_error!('Please specify a Unity version to download') unless version
 
-        packages = packages_with_unity_first(options)
-
         os = U3dCore::Helper.operating_system
+
+        packages = packages_with_unity_first(os, options)
+
         cache = Cache.new(force_os: os)
         versions = cache[os.id2name]['versions']
         version = interpret_latest(version, versions)
@@ -124,37 +118,20 @@ def download(args: [], options: {})
           raise 'Could not get administrative privileges' unless U3dCore::CommandExecutor.has_admin_privileges?
         end
 
-        files = []
-        if os == :linux
-          UI.important 'Option -a | --all not available for Linux' if options[:all]
-          UI.important 'Option -p | --packages not available for Linux' if options[:packages]
-          downloader = Downloader::LinuxDownloader
-          files << ["Unity #{version}", downloader.download(version, versions), {}]
-        else
-          downloader = Downloader::MacDownloader if os == :mac
-          downloader = Downloader::WindowsDownloader if os == :win
-          if options[:all]
-            files = downloader.download_all(version, versions)
-          else
-            packages.each do |package|
-              result = downloader.download_specific(package, version, versions)
-              files << [package, result[0], result[1]] unless result.nil?
-            end
-          end
-        end
+        files = Downloader.download_modules(version, versions, os, packages)
 
         return if options[:no_install]
         Installer.install_modules(files, version, installation_path: options[:installation_path])
       end
 
       def local_install(args: [], options: {})
-        UI.user_error!('Please specify a version') if args.empty?
         version = args[0]
-
-        packages = packages_with_unity_first(options)
+        UI.user_error!('Please specify a Unity version to download') unless version
 
         os = U3dCore::Helper.operating_system
 
+        packages = packages_with_unity_first(os, options)
+
         unity = check_unity_presence(version: version)
         return unless enforce_setup_coherence(packages, options, unity)
 
@@ -163,24 +140,7 @@ def local_install(args: [], options: {})
         UI.important 'Root privileges are required'
         raise 'Could not get administrative privileges' unless U3dCore::CommandExecutor.has_admin_privileges?
 
-        files = []
-        if os == :linux
-          UI.important 'Option -a | --all not available for Linux' if options[:all]
-          UI.important 'Option -p | --packages not available for Linux' if options[:packages]
-          downloader = Downloader::LinuxDownloader
-          files << ["Unity #{version}", downloader.local_file(version), {}]
-        else
-          downloader = Downloader::MacDownloader if os == :mac
-          downloader = Downloader::WindowsDownloader if os == :win
-          if options[:all]
-            files = downloader.all_local_files(version)
-          else
-            packages.each do |package|
-              result = downloader.local_file(package, version)
-              files << [package, result[0], result[1]] unless result.nil?
-            end
-          end
-        end
+        files = Downloader.local_files(version, os, packages)
 
         Installer.install_modules(files, version, installation_path: options[:installation_path])
       end
@@ -226,20 +186,7 @@ def credentials(args: [], _options: {})
           U3dCore::Globals.use_keychain = true
           U3dCore::Credentials.new(user: ENV['USER']).forget_credentials(force: true)
         else
-          U3dCore::Globals.use_keychain = true
-          credentials = U3dCore::Credentials.new(user: ENV['USER'])
-          U3dCore::Globals.with_do_not_login(true) do
-            if credentials.password.to_s.empty?
-              UI.message "No credentials stored"
-            else
-              if U3dCore::CommandExecutor.has_admin_privileges?
-                UI.success "Stored credentials are valid"
-              else
-                UI.error "Stored credentials are not valid"
-              end
-            end
-          end
-          # FIXME: return value
+          credentials_check
         end
       end
 
@@ -268,6 +215,35 @@ def release_letter_mapping
 
       private
 
+      def credentials_check
+        U3dCore::Globals.use_keychain = true
+        credentials = U3dCore::Credentials.new(user: ENV['USER'])
+        U3dCore::Globals.with_do_not_login(true) do
+          if credentials.password.to_s.empty?
+            UI.message "No credentials stored"
+          elsif U3dCore::CommandExecutor.has_admin_privileges?
+            UI.success "Stored credentials are valid"
+          else
+            UI.error "Stored credentials are not valid"
+          end
+        end
+        # FIXME: return value
+      end
+
+      # if the specified string representatio of `os` is non nil
+      # convert the it to a symbol and checks it against the valid ones
+      # or return the current OS
+      def valid_os_or_current(os)
+        if os
+          os = os.to_sym
+          oses = U3dCore::Helper.operating_systems
+          raise "Specified OS (#{os}) isn't valid [#{oses.join(', ')}]" unless oses.include?(os)
+        else
+          os = U3dCore::Helper.operating_system
+        end
+        os
+      end
+
       def interpret_latest(version, versions)
         return version unless release_letter_mapping.keys.include? version.to_sym
 
@@ -282,7 +258,12 @@ def interpret_latest(version, versions)
         iversion
       end
 
-      def packages_with_unity_first(options)
+      def packages_with_unity_first(os, options)
+        if os == :linux
+          UI.important 'Option -a | --all not available for Linux' if options[:all]
+          UI.important 'Option -p | --packages not available for Linux' if options[:packages]
+        end
+        return [] if options[:all]
         temp = options[:packages] || ['Unity']
         temp.insert(0, 'Unity') if temp.delete('Unity')
         temp
@@ -324,4 +305,5 @@ def enforce_setup_coherence(packages, options, unity)
       end
     end
   end
+  # rubocop:enable ClassLength
 end

lib/u3d/commands_generator.rb

@@ -28,6 +28,7 @@
 
 module U3d
   # CLI using commander gem for u3d
+  # rubocop:disable ClassLength
   class CommandsGenerator
     include Commander::Methods
     UI = U3dCore::UI
@@ -160,4 +161,5 @@ def run
       run!
     end
   end
+  # rubocop:enable ClassLength
 end

lib/u3d/downloader.rb

@@ -26,6 +26,7 @@
 
 module U3d
   # Take care of downloading files and packages
+  # rubocop:disable ModuleLength
   module Downloader
     # Name of the directory for the package downloading
     DOWNLOAD_DIRECTORY = 'Unity_Packages'.freeze
@@ -61,6 +62,48 @@ def size_validation(expected: nil, actual: nil)
         true
       end
 
+      # download packages or all if none specified
+      def download_modules(version, versions, os, packages: [])
+        files = []
+        if os == :linux
+          downloader = Downloader::LinuxDownloader
+          files << ["Unity #{version}", downloader.download(version, versions), {}]
+        else
+          downloader = Downloader::MacDownloader if os == :mac
+          downloader = Downloader::WindowsDownloader if os == :win
+          if packages.count.zero?
+            files = downloader.download_all(version, versions)
+          else
+            packages.each do |package|
+              result = downloader.download_specific(package, version, versions)
+              files << [package, result[0], result[1]] unless result.nil?
+            end
+          end
+        end
+        files
+      end
+
+      # find already downloaded packages or all if none specified
+      def local_files(version, os, packages: [])
+        files = []
+        if os == :linux
+          downloader = Downloader::LinuxDownloader
+          files << ["Unity #{version}", downloader.local_file(version), {}]
+        else
+          downloader = Downloader::MacDownloader if os == :mac
+          downloader = Downloader::WindowsDownloader if os == :win
+          if packages.count.zero?
+            files = downloader.all_local_files(version)
+          else
+            packages.each do |package|
+              result = downloader.local_file(package, version)
+              files << [package, result[0], result[1]] unless result.nil?
+            end
+          end
+        end
+        files
+      end
+
       def download_package(path, url, size: nil)
         File.open(path, 'wb') do |f|
           uri = URI(url)
@@ -365,4 +408,5 @@ def get_package(name, ini_file, main_dir, base_url)
       end
     end
   end
+  # rubocop:enable ModuleLength
 end

lib/u3d/installer.rb

@@ -102,7 +102,7 @@ def version
       raise "Couldn't find file #{fpath}" unless File.exist? fpath
       doc = REXML::Document.new(File.read(fpath))
       version = REXML::XPath.first(doc, 'ivy-module/info/@e:unityVersion').value
-      if m = version.match(/^(.*)x(.*)Linux$/)
+      if (m = version.match(/^(.*)x(.*)Linux$/))
         version = "#{m[1]}#{m[2]}"
       end
       version
@@ -234,7 +234,7 @@ def installed
       versions.sort! { |x, y| x.version <=> y.version }
     end
 
-    def install(file_path, version, installation_path: nil, info: {})
+    def install(file_path, version, installation_path: nil, _info: {})
       extension = File.extname(file_path)
       raise "Installation of #{extension} files is not supported on Mac" if extension != '.pkg'
       path = installation_path || DEFAULT_MAC_INSTALL
@@ -296,7 +296,7 @@ def installed
       versions.sort! { |x, y| x.version <=> y.version }
     end
 
-    def install(file_path, version, installation_path: nil, info: {})
+    def install(file_path, version, installation_path: nil, _info: {})
       extension = File.extname(file_path)
       raise "Installation of #{extension} files is not supported on Linux" if extension != '.sh'
       path = installation_path || DEFAULT_LINUX_INSTALL
@@ -374,7 +374,7 @@ def install_exe(file_path, installation_path: nil, info: {})
           command.sub!(/{INSTDIR}/, final_path)
           command.sub!(/{DOCDIR}/, final_path)
           command.sub!(/{MODULEDIR}/, final_path)
-          command.sub!(/\/D=/, '/S /D=') unless /\/S/ =~ command
+          command.sub!(%r{\/D=}, '/S /D=') unless %r{\/S} =~ command
         end
         command ||= file_path.to_s
         U3dCore::CommandExecutor.execute(command: command, admin: true)

lib/u3d/log_analyzer.rb

@@ -24,6 +24,7 @@
 
 module U3d
   # Analyzes log by filtering output along a set of rules
+  # rubocop:disable ClassLength, PerceivedComplexity, BlockNesting
   class LogAnalyzer
     RULES_PATH = File.expand_path('../../../config/log_rules.json', __FILE__)
     MEMORY_SIZE = 10
@@ -227,4 +228,5 @@ def parse_rule(r)
       true
     end
   end
+  # rubocop:enable ClassLength, PerceivedComplexity, BlockNesting
 end