Some Rubocop fixes

.rubocop_todo.yml

@@ -140,13 +140,6 @@ Layout/SpaceInsideHashLiteralBraces:
 Layout/SpaceInsidePercentLiteralDelimiters:
   Enabled: false
 
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: space, no_space
-Layout/SpaceInsideReferenceBrackets:
-  Enabled: false
-
 # Offense count: 55
 Lint/AmbiguousBlockAssociation:
   Enabled: false
@@ -457,11 +450,6 @@ Style/ConditionalAssignment:
 Style/DoubleNegation:
   Enabled: false
 
-# Offense count: 10
-# Cop supports --auto-correct.
-Style/EachWithObject:
-  Enabled: false
-
 # Offense count: 146
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle.
@@ -538,13 +526,6 @@ Style/MixinUsage:
 Style/MultilineIfModifier:
   Enabled: false
 
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: keyword, braces
-Style/MultilineMemoization:
-  Enabled: false
-
 # Offense count: 3
 Style/MultipleComparison:
   Enabled: false
@@ -653,13 +634,6 @@ Style/RescueStandardError:
 Style/SignalException:
   Enabled: false
 
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: use_perl_names, use_english_names
-Style/SpecialGlobalVars:
-  Enabled: false
-
 # Offense count: 1
 Style/StructInheritance:
   Enabled: false
@@ -671,11 +645,6 @@ Style/StructInheritance:
 Style/SymbolArray:
   EnforcedStyle: brackets
 
-# Offense count: 9
-# Cop supports --auto-correct.
-Style/SymbolLiteral:
-  Enabled: false
-
 # Offense count: 77
 # Cop supports --auto-correct.
 # Configuration parameters: IgnoredMethods.
@@ -696,8 +665,3 @@ Style/TrailingCommaInLiteral:
 # Whitelist: to_ary, to_a, to_c, to_enum, to_h, to_hash, to_i, to_int, to_io, to_open, to_path, to_proc, to_r, to_regexp, to_str, to_s, to_sym
 Style/TrivialAccessors:
   Enabled: false
-
-# Offense count: 2
-# Cop supports --auto-correct.
-Style/UnlessElse:
-  Enabled: false

app/controllers/concerns/api/v2/lookup_keys_common_controller.rb

@@ -50,9 +50,10 @@ def smart_class_parameter_id?
   def find_smart_variable
     id = params.keys.include?('smart_variable_id') ? params['smart_variable_id'] : params['id']
     @smart_variable   = VariableLookupKey.authorized(:view_external_variables).smart_variables.find_by_id(id.to_i) if id.to_i > 0
-    @smart_variable ||= (puppet_cond = { :puppetclass_id => @puppetclass.id } if @puppetclass
-                         VariableLookupKey.authorized(:view_external_variables).smart_variables.where(puppet_cond).find_by_key(id.to_s)
-                        )
+    @smart_variable ||= begin
+                          puppet_cond = { :puppetclass_id => @puppetclass.id } if @puppetclass
+                          VariableLookupKey.authorized(:view_external_variables).smart_variables.where(puppet_cond).find_by_key(id.to_s)
+                        end
     @smart_variable
   end
 
@@ -71,10 +72,11 @@ def smart_variables_resource_scope
   def find_smart_class_parameter
     id = params.keys.include?('smart_class_parameter_id') ? params['smart_class_parameter_id'] : params['id']
     @smart_class_parameter = PuppetclassLookupKey.authorized(:view_external_parameters).smart_class_parameters.find_by_id(id.to_i) if id.to_i > 0
-    @smart_class_parameter ||= (puppet_cond = { 'environment_classes.puppetclass_id'=> @puppetclass.id } if @puppetclass
-                                env_cond = { 'environment_classes.environment_id' => @environment.id } if @environment
-                                PuppetclassLookupKey.authorized(:view_external_parameters).smart_class_parameters.where(puppet_cond).where(env_cond).where(:key => id).first
-                               )
+    @smart_class_parameter ||= begin
+                                 puppet_cond = { 'environment_classes.puppetclass_id'=> @puppetclass.id } if @puppetclass
+                                 env_cond = { 'environment_classes.environment_id' => @environment.id } if @environment
+                                 PuppetclassLookupKey.authorized(:view_external_parameters).smart_class_parameters.where(puppet_cond).where(env_cond).where(:key => id).first
+                               end
     @smart_class_parameter
   end
 

app/controllers/concerns/foreman/controller/parameters/keep_param.rb

@@ -19,12 +19,11 @@ def keep_param(params, top_level_hash, *keys)
 
   def detect_old_params(params, top_level_hash, keys)
     # Delete keys being kept from the `params` hash, so the block yielded to filters the others
-    keys.inject({}) do |op,(key,val)|
+    keys.each_with_object({}) do |(key,val),op|
       if params[top_level_hash].try!(:has_key?, key)
         op[key] = params[top_level_hash].delete(key)
         op[key].permit! if op[key].is_a?(ActionController::Parameters)
       end
-      op
     end
   end
 end

app/helpers/application_helper.rb

@@ -273,9 +273,9 @@ def flot_pie_chart(name, title, data, options = {})
                     { :id    => name,
                       :class => 'statistics-pie',
                       :data  => {
-                        :'title'  => title,
-                        :'series' => data,
-                        :'url'    => options[:search] ? "#{request.script_name}/hosts?search=#{URI.encode(options.delete(:search))}" : "#"
+                        :title  => title,
+                        :series => data,
+                        :url    => options[:search] ? "#{request.script_name}/hosts?search=#{URI.encode(options.delete(:search))}" : "#"
                       }
                     }.merge(options))
   end
@@ -289,7 +289,7 @@ def flot_chart(name, xaxis_label, yaxis_label, data, options = {})
                     :'legend-options' => options.delete(:legend),
                     :'xaxis-label'    => xaxis_label,
                     :'yaxis-label'    => yaxis_label,
-                    :'series'         => data
+                    :series         => data
                   }
                 }.merge(options))
   end
@@ -316,8 +316,8 @@ def flot_bar_chart(name, xaxis_label, yaxis_label, data, options = {})
                   :data => {
                     :'xaxis-label' => xaxis_label,
                     :'yaxis-label' => yaxis_label,
-                    :'chart'   => data,
-                    :'ticks'   => ticks
+                    :chart   => data,
+                    :ticks   => ticks
                   }
                 }.merge(options))
   end

app/helpers/compute_resources_vms_helper.rb

@@ -87,16 +87,14 @@ def datastore_stats(datastore)
   end
 
   def vsphere_datastores(compute)
-    compute.datastores.inject({}) do |hsh, datastore|
+    compute.datastores.each_with_object({}) do |datastore, hsh|
       hsh[datastore.name] = datastore_stats(datastore)
-      hsh
     end
   end
 
   def vsphere_storage_pods(compute)
-    compute.storage_pods.inject({}) do |hsh, pod|
+    compute.storage_pods.each_with_object({}) do |pod, hsh|
       hsh[pod.name] = storage_pod_stats(pod)
-      hsh
     end
   end
 

app/helpers/smart_proxies_helper.rb

@@ -32,7 +32,7 @@ def feature_actions(proxy, authorizer)
       actions << link_to_function_if_authorized(_('Expire logs'), "expireLogs(this, (new Date).getTime() / 1000);",
         hash_for_expire_logs_smart_proxy_path(:id => proxy), {
           :data => {
-            :"url" => expire_logs_smart_proxy_path(:id => proxy),
+            :url => expire_logs_smart_proxy_path(:id => proxy),
             :"url-errors" => errors_card_smart_proxy_path(:id => proxy),
             :"url-modules" => modules_card_smart_proxy_path(:id => proxy)
           }

app/models/compute_resource.rb

@@ -51,9 +51,8 @@ def self.supported_providers
   end
 
   def self.registered_providers
-    Foreman::Plugin.all.map(&:compute_resources).inject({}) do |prov_hash, providers|
+    Foreman::Plugin.all.map(&:compute_resources).each_with_object({}) do |providers, prov_hash|
       providers.each { |provider| prov_hash.update(provider.split('::').last => provider) }
-      prov_hash
     end
   end
 
@@ -389,7 +388,7 @@ def nested_attributes_for(type, opts)
     # convert our options hash into a sorted array (e.g. to preserve nic / disks order)
     opts = opts.sort { |l, r| l[0].to_s.sub('new_','').to_i <=> r[0].to_s.sub('new_','').to_i }.map { |e| Hash[e[1]] }
     opts.map do |v|
-      if v[:"_delete"] == '1' && v[:id].blank?
+      if v[:_delete] == '1' && v[:id].blank?
         nil
       else
         v.deep_symbolize_keys # convert to symbols deeper hashes

app/models/compute_resources/foreman/model/vmware.rb

@@ -582,7 +582,7 @@ def vm_compute_attributes(vm)
       vm_attrs = super
       dc_networks = networks
       interfaces = vm.interfaces || []
-      vm_attrs[:interfaces_attributes] = interfaces.each_with_index.inject({}) do |hsh, (interface, index)|
+      vm_attrs[:interfaces_attributes] = interfaces.each_with_index.each_with_object({}) do |(interface, index), hsh|
         network = dc_networks.detect { |n| [n.id, n.name].include?(interface.network) }
         raise Foreman::Exception.new(N_("Could not find network %s on VMWare compute resource"), interface.network) unless network
         interface_attrs = {}
@@ -591,7 +591,6 @@ def vm_compute_attributes(vm)
         interface_attrs[:compute_attributes][:network] = network.name
         interface_attrs[:compute_attributes][:type] = interface.type.to_s.split('::').last
         hsh[index.to_s] = interface_attrs
-        hsh
       end
       vm_attrs[:scsi_controllers] = vm.scsi_controllers.map do |controller|
         controller.attributes

app/models/concerns/audit_associations.rb

@@ -20,7 +20,7 @@ def find_association_class(name)
 
     def associated_changes
       associations = Array.wrap(audited_options[:associations])
-      associations.inject({}) do |changes_hash, association_name|
+      associations.each_with_object({}) do |association_name, changes_hash|
         association_ids = "#{association_name.to_s.singularize}_ids"
         if send("#{association_ids}_changed?")
           association_class = find_association_class(association_name)
@@ -29,16 +29,12 @@ def associated_changes
 
           id_name_map = association_class.where(id: change_ids).inject({}) { |r,p| r.merge(p.id => p.to_label) }
 
-          changes_hash[association_name] = change.inject([]) do |humaized_associations, ids|
-            humaized_associations << ids.inject([]) do |humanized_ids, id|
+          changes_hash[association_name] = change.each_with_object([]) do |ids, humaized_associations|
+            humaized_associations << ids.each_with_object([]) do |id, humanized_ids|
               humanized_ids << id_name_map[id]
-              humanized_ids
             end.join(', ')
-            humaized_associations
           end
         end
-
-        changes_hash
       end
     end
   end

app/models/feature.rb

@@ -6,9 +6,8 @@ class Feature < ApplicationRecord
   validates :name, :presence => true
 
   def self.name_map
-    Feature.all.inject({}) do |ret_val, feature|
+    Feature.all.each_with_object({}) do |feature, ret_val|
       ret_val[feature.name.downcase.gsub(/\s+/, "")] = feature.name
-      ret_val
     end
   end
 end

app/registries/menu/item.rb

@@ -28,9 +28,8 @@ def url
 
     def url_hash
       @url_hash ||= @context.routes.url_helpers.send("hash_for_#{name}_path")
-      @url_hash.inject({}) do |h,(key,value)|
+      @url_hash.each_with_object({}) do |(key,value),h|
         h[key] = (value.respond_to?(:call) ? value.call : value)
-        h
       end
     end
 

app/services/puppet_fact_parser.rb

@@ -130,9 +130,8 @@ def get_interfaces
   end
 
   def get_facts_for_interface(interface)
-    iface_facts = @facts.inject([]) do |facts, (name, value)|
+    iface_facts = @facts.each_with_object([]) do |(name, value), facts|
       facts << [name.chomp("_#{interface}"), value] if name.end_with?("_#{interface}")
-      facts
     end
     iface_facts = HashWithIndifferentAccess[iface_facts]
     logger.debug { "Interface #{interface} facts: #{iface_facts.inspect}" }

app/views/api/v2/hosts/show.json.rabl

@@ -26,8 +26,7 @@ end
 
 node :permissions do |host|
   authorizer = Authorizer.new(User.current)
-  Permission.where(:resource_type => "Host").all.inject({}) do |hash, permission|
+  Permission.where(:resource_type => "Host").all.each_with_object({}) do |permission, hash|
     hash[permission.name] = authorizer.can?(permission.name, host, false)
-    hash
   end
 end

config/application.rb

@@ -85,7 +85,7 @@ class Application < Rails::Application
     # config.autoload_paths += %W(#{config.root}/extras)
     config.autoload_paths += Dir["#{config.root}/lib"]
     config.autoload_paths += Dir["#{config.root}/app/controllers/concerns"]
-    config.autoload_paths += Dir[ Rails.root.join('app', 'models', 'power_manager') ]
+    config.autoload_paths += Dir[Rails.root.join('app', 'models', 'power_manager')]
     config.autoload_paths += Dir["#{config.root}/app/models/concerns"]
     config.autoload_paths += Dir["#{config.root}/app/services"]
     config.autoload_paths += Dir["#{config.root}/app/mailers"]

lib/tasks/jenkins.rake

@@ -20,7 +20,7 @@ begin
         --require rubocop/formatter/checkstyle_formatter \
         --format RuboCop::Formatter::CheckstyleFormatter \
         --no-color --out rubocop.xml")
-      exit($?.exitstatus)
+      exit($CHILD_STATUS.exitstatus)
     end
   end
 rescue LoadError

lib/tasks/pkg.rake

@@ -9,7 +9,7 @@ namespace :pkg do
     # run 'debuild'
     system 'debuild'
 
-    if $? == 0
+    if $CHILD_STATUS == 0
       # remove 'debian' directory
       FileUtils.rm_r Rake.application.original_dir + '/debian', :force => true
     else

lib/tasks/puppet.rake

@@ -117,18 +117,18 @@ namespace :puppet do
           rescue => e
             errors = e.message + "\n" + e.backtrace.join("\n")
           end
-          unless args.batch
-            unless errors.empty?
+          if args.batch
+            Rails.logger.warn "Failed to refresh puppet classes: #{errors}"
+          else
+            if errors.empty?
+              puts "Import complete"
+            else
               puts "Problems were detected during the execution phase"
               puts
               puts errors.each { |error| error.gsub(/<br\/>/, "\n") } << "\n"
               puts
               puts "Import failed"
-            else
-              puts "Import complete"
             end
-          else
-            Rails.logger.warn "Failed to refresh puppet classes: #{errors}"
           end
         end
       end

test/integration/compute_profile_js_test.rb

@@ -16,7 +16,7 @@ class ComputeProfileJSTest < IntegrationTestWithJavascript
     visit compute_profile_path(compute_profiles(:one))
     # amazon123 exists in fixture compute_attributes.yml
     click_link("amazon123 (eu-west-1-EC2)")
-  
+
     assert page.has_selector?('#breadcrumb .active', :text => compute_profiles(:one).name), "#{compute_profiles(:one).name} was expected in the breadcrumb active, but was not found"
     selected_profile = find("#s2id_compute_attribute_compute_profile_id .select2-chosen").text
     assert_equal compute_profiles(:one).name, selected_profile

test/integration/fact_value_test.rb

@@ -59,6 +59,6 @@ def setup
       first(:xpath, "//td[1]//a").click
     end
 
-    assert_equal "Show all #{@parent_name.name} children fact values", first(:xpath, "//td[2]//a")[:'title']
+    assert_equal "Show all #{@parent_name.name} children fact values", first(:xpath, "//td[2]//a")[:title]
   end
 end

test/unit/shared/access_permissions_test_base.rb

@@ -7,9 +7,8 @@ module AccessPermissionsTestBase
   module ClassMethods
     def check_routes(app_routes, skipped_actions)
       # For each controller action, verify it has a permission that grants access
-      app_routes = app_routes.routes.inject({}) do |routes, r|
+      app_routes = app_routes.routes.each_with_object({}) do |r, routes|
         routes["#{r.defaults[:controller].gsub(/::/, '_').underscore}/#{r.defaults[:action]}"] = r if r.defaults[:controller]
-        routes
       end
 
       app_routes.each do |path, r|