Update RuboCop support; use Bixby style guide

Bixby is a RuboCop style guide that decouples (to the degree possible) RuboCop software updates from unrelated style guide updates. Bixby disables all rules by default, and re-enables rules that are adopted in the community in general. Bixby's promise is to avoid adding rules without bumping major version. Pinning to the current v1.0.0 will allow us to update software versions with reduced changes (ideally none) to code style. Enforcement of existing rules may still change to become more strict based on updates to rule enforcement code. Several rule exceptions and other configurations that I could identify as being inherited from Bixby are removed from the local `.rubocop.yml`. Closes #2568.
samvera · Feb 18, 2018 · 6ddae60 · 6ddae60
1 parent 0a4ac8b
commit 6ddae60
Show file tree

Hide file tree

Showing 33 changed files with 44 additions and 147 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,38 +1,22 @@
-require: rubocop-rspec
+inherit_gem:
+  bixby: bixby_default.yml
+
 inherit_from: .rubocop_fixme.yml
 
 AllCops:
   TargetRubyVersion: 2.1
   DisplayCopNames: true
-  Include:
-    - '**/Rakefile'
-    - '**/config.ru'
   Exclude:
     - 'db/**/*'
     - 'script/**/*'
     - 'spec/test_app_templates/**/*'
     - 'vendor/**/*'
     - 'lib/hyrax/specs/**/*'
 
-Bundler/DuplicatedGem:  # This doesn't work with engine_cart
-  Enabled: false
-
-Layout/IndentationConsistency:
-  EnforcedStyle: rails
-
 Lint/ImplicitStringConcatenation:
   Exclude:
     - 'lib/generators/hyrax/**/*'
 
-Lint/AmbiguousBlockAssociation:
-  Enabled: false
-
-Metrics/LineLength:
-  Max: 200
-
-Metrics/AbcSize:
-  Max: 26
-
 Metrics/BlockLength:
   ExcludedMethods: ['included']
   Exclude:
@@ -47,12 +31,9 @@ Metrics/BlockLength:
     - 'lib/tasks/*.rake'
     - 'spec/**/*.rb'
 
-Metrics/MethodLength:
-  Max: 13
-
-Naming/FileName: # https://github.com/bbatsov/rubocop/issues/2973
-  Exclude:
-    - 'Gemfile'
+# Naming/FileName: # https://github.com/bbatsov/rubocop/issues/2973
+#   Exclude:
+#     - 'Gemfile'
 
 Style/AsciiComments:
   Enabled: false
@@ -76,31 +57,16 @@ Style/NumericPredicate:
 Style/SymbolArray:
   Enabled: false
 
-Style/WordArray:
-  Enabled: false
-
-Style/RegexpLiteral:
-  Enabled: false
-
-Style/StringLiterals:
-  Enabled: false
-
 Style/ClassAndModuleChildren:
   Enabled: false
 
-Style/Documentation:
-  Enabled: false
-
 Style/SingleLineBlockParams:
   Enabled: false
 
 Style/ZeroLengthPredicate:
   Exclude:
     - 'app/controllers/hyrax/file_sets_controller.rb'
 
-Rails:
-  Enabled: true
-
 Rails/ApplicationJob:
   Enabled: false
 
@@ -141,12 +107,9 @@ RSpec/DescribeClass:
     - 'spec/tasks/rake_spec.rb'
     - 'spec/views/**/*'
 
-RSpec/ContextWording:
-  Enabled: false
-
-# By default RSpec/MessageSpies has the following:
-#   Prefer have_received for setting message expectations. Setup form as a spy using allow or instance_spy.
-# The default assumes EnforcedStyle is 'have_received'. Most of our specs are 'receive'
+# # By default RSpec/MessageSpies has the following:
+# #   Prefer have_received for setting message expectations. Setup form as a spy using allow or instance_spy.
+# # The default assumes EnforcedStyle is 'have_received'. Most of our specs are 'receive'
 RSpec/MessageSpies:
   Enabled: false
 
@@ -163,9 +126,6 @@ RSpec/LetSetup:
 RSpec/MessageExpectation:
   Enabled: false
 
-RSpec/MultipleExpectations:
-  Enabled: false
-
 RSpec/NestedGroups:
   Enabled: false
 

diff --git a/Gemfile b/Gemfile
@@ -10,6 +10,7 @@ group :development, :test do
   gem "simplecov", require: false
 end
 
+# rubocop:disable Bundler/DuplicatedGem
 # BEGIN ENGINE_CART BLOCK
 # engine_cart: 1.1.0
 # engine_cart stanza: 0.10.0
@@ -44,7 +45,6 @@ else
   end
 end
 # END ENGINE_CART BLOCK
+# rubocop:enable Bundler/DuplicatedGem
 
-unless File.exist?(file)
-  eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__))
-end
+eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__)) unless File.exist?(file)
diff --git a/app/actors/hyrax/actors/file_set_actor.rb b/app/actors/hyrax/actors/file_set_actor.rb
@@ -147,7 +147,6 @@ def assign_visibility?(file_set_params = {})
         # Must clear the fileset from the thumbnail_id, representative_id and rendering_ids fields on the work
         #   and force it to be re-solrized.
         # Although ActiveFedora clears the children nodes it leaves those fields in Solr populated.
-        # rubocop:disable Metrics/AbcSize
         # rubocop:disable Metrics/CyclomaticComplexity
         def unlink_from_work
           work = file_set.parent

diff --git a/app/actors/hyrax/actors/interpret_visibility_actor.rb b/app/actors/hyrax/actors/interpret_visibility_actor.rb
@@ -114,9 +114,7 @@ def validate(env, intention, attributes)
 
         def apply_visibility(env, intention)
           result = apply_lease(env, intention) && apply_embargo(env, intention)
-          if env.attributes[:visibility]
-            env.curation_concern.visibility = env.attributes[:visibility]
-          end
+          env.curation_concern.visibility = env.attributes[:visibility] if env.attributes[:visibility]
           result
         end
 

diff --git a/app/actors/hyrax/actors/model_actor.rb b/app/actors/hyrax/actors/model_actor.rb
@@ -2,6 +2,9 @@ module Hyrax
   module Actors
     # This is a proxy for the model specific actor
     class ModelActor < AbstractActor
+      # See: https://github.com/bbatsov/rubocop/issues/5393
+      # rubocop:disable Rails/Delegate
+
       # @param [Hyrax::Actors::Environment] env
       # @return [Boolean] true if update was successful
       def update(env)
@@ -19,6 +22,7 @@ def create(env)
       def destroy(env)
         model_actor(env).destroy(env)
       end
+      # rubocop:enable Rails/Delegate
 
       private
 

diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb
@@ -156,9 +156,7 @@ def presenter
       def parent_presenter
         @parent_presenter ||=
           begin
-            if params[:parent_id]
-              @parent_presenter ||= show_presenter.new(search_result_document(id: params[:parent_id]), current_ability, request)
-            end
+            @parent_presenter ||= show_presenter.new(search_result_document(id: params[:parent_id]), current_ability, request) if params[:parent_id]
           end
       end
 

diff --git a/app/controllers/hyrax/api/items_controller.rb b/app/controllers/hyrax/api/items_controller.rb
@@ -34,13 +34,9 @@ def show
         def my_load_and_authorize_resource
           @work = Hyrax::WorkRelation.new.find(params[:id])
 
-          unless user.can? :edit, @work
-            return render plain: "#{user} lacks access to #{@work}", status: :unauthorized
-          end
+          return render plain: "#{user} lacks access to #{@work}", status: :unauthorized unless user.can? :edit, @work
 
-          if @work.arkivo_checksum.nil?
-            return render plain: "Forbidden: #{@work} not deposited via Arkivo", status: :forbidden
-          end
+          return render plain: "Forbidden: #{@work} not deposited via Arkivo", status: :forbidden if @work.arkivo_checksum.nil?
         rescue ActiveFedora::ObjectNotFoundError
           return render plain: "id '#{params[:id]}' not found", status: :not_found
         end

diff --git a/app/controllers/hyrax/transfers_controller.rb b/app/controllers/hyrax/transfers_controller.rb
@@ -46,9 +46,7 @@ def index
     # any existing edit permissions on the work.
     def accept
       @proxy_deposit_request.transfer!(params[:reset])
-      if params[:sticky]
-        current_user.can_receive_deposits_from << @proxy_deposit_request.sending_user
-      end
+      current_user.can_receive_deposits_from << @proxy_deposit_request.sending_user if params[:sticky]
       redirect_to hyrax.transfers_path, notice: "Transfer complete"
     end
 

diff --git a/app/controllers/hyrax/users_controller.rb b/app/controllers/hyrax/users_controller.rb
@@ -24,9 +24,7 @@ def show
       def search(query)
         clause = query.blank? ? nil : "%" + query.downcase + "%"
         base = ::User.where(*base_query)
-        if clause.present?
-          base = base.where("#{Hydra.config.user_key_field} like lower(?) OR display_name like lower(?)", clause, clause)
-        end
+        base = base.where("#{Hydra.config.user_key_field} like lower(?) OR display_name like lower(?)", clause, clause) if clause.present?
         base.registered
             .where("#{Hydra.config.user_key_field} not in (?)",
                    [::User.batch_user_key, ::User.audit_user_key])

diff --git a/app/forms/hyrax/forms/permission_template_form.rb b/app/forms/hyrax/forms/permission_template_form.rb
@@ -45,7 +45,6 @@ def initialize(model)
       # @return [Hash{Symbol => String, Boolean}] { :content_tab (for confirmation message),
       #                                             :updated (true/false),
       #                                               :error_code (for flash error lookup) }
-      # rubocop:disable Metrics/MethodLength
       def update(attributes)
         @attributes = attributes
         return_info = { content_tab: tab_to_update }
@@ -219,9 +218,7 @@ def managers_updated?
         def grants_as_collection
           return [] unless attributes[:access_grants_attributes]
           attributes_collection = attributes[:access_grants_attributes]
-          if attributes_collection.respond_to?(:permitted?)
-            attributes_collection = attributes_collection.to_h
-          end
+          attributes_collection = attributes_collection.to_h if attributes_collection.respond_to?(:permitted?)
           if attributes_collection.is_a? Hash
             attributes_collection = attributes_collection
                                     .sort_by { |i, _| i.to_i }

diff --git a/app/forms/hyrax/forms/work_form.rb b/app/forms/hyrax/forms/work_form.rb
@@ -160,9 +160,7 @@ def collections_for_select
       # are going into a mediated deposit workflow.
       def self.sanitize_params(form_params)
         admin_set_id = form_params[:admin_set_id]
-        if admin_set_id && workflow_for(admin_set_id: admin_set_id).allows_access_grant?
-          return super
-        end
+        return super if admin_set_id && workflow_for(admin_set_id: admin_set_id).allows_access_grant?
         params_without_permissions = permitted_params.reject { |arg| arg.respond_to?(:key?) && arg.key?(:permissions_attributes) }
         form_params.permit(*params_without_permissions)
       end

diff --git a/app/helpers/hyrax/citations_behaviors/common_behavior.rb b/app/helpers/hyrax/citations_behaviors/common_behavior.rb
@@ -6,9 +6,7 @@ def persistent_url(work)
       end
 
       def clean_end_punctuation(text)
-        if text && ([".", ",", ":", ";", "/"].include? text[-1, 1])
-          return text[0, text.length - 1]
-        end
+        return text[0, text.length - 1] if text && ([".", ",", ":", ";", "/"].include? text[-1, 1])
         text
       end
     end

diff --git a/app/helpers/hyrax/citations_behaviors/formatters/chicago_formatter.rb b/app/helpers/hyrax/citations_behaviors/formatters/chicago_formatter.rb
@@ -4,17 +4,13 @@ module Formatters
       class ChicagoFormatter < BaseFormatter
         include Hyrax::CitationsBehaviors::PublicationBehavior
         include Hyrax::CitationsBehaviors::TitleBehavior
-
-        # rubocop:disable Metrics/MethodLength
         def format(work)
           text = ""
 
           # setup formatted author list
           authors_list = all_authors(work)
           text << format_authors(authors_list)
-          if text.present?
-            text = "<span class=\"citation-author\">#{text}</span>"
-          end
+          text = "<span class=\"citation-author\">#{text}</span>" if text.present?
           # Get Pub Date
           pub_date = setup_pub_date(work)
           text << " #{pub_date}." unless pub_date.nil?

diff --git a/app/helpers/hyrax/file_set_helper.rb b/app/helpers/hyrax/file_set_helper.rb
@@ -15,7 +15,6 @@ def media_display(presenter, locals = {})
            locals.merge(file_set: presenter)
   end
 
-  # rubocop:disable Metrics/MethodLength
   def media_display_partial(file_set)
     'hyrax/file_sets/media_display/' +
       if file_set.image?

diff --git a/app/helpers/hyrax/hyrax_helper_behavior.rb b/app/helpers/hyrax/hyrax_helper_behavior.rb
@@ -273,7 +273,6 @@ def count_classes_for(unread_count)
         end
       end
 
-      # rubocop:disable Metrics/MethodLength
       def search_action_for_dashboard
         case params[:controller]
         when "hyrax/my/collections"

diff --git a/app/helpers/hyrax/trophy_helper.rb b/app/helpers/hyrax/trophy_helper.rb
@@ -1,6 +1,5 @@
 module Hyrax
   module TrophyHelper
-    # rubocop:disable Metrics/MethodLength
     def display_trophy_link(user, id, args = {}, &_block)
       return unless user
       trophy = user.trophies.where(work_id: id).exists?

diff --git a/app/inputs/multi_value_select_input.rb b/app/inputs/multi_value_select_input.rb
@@ -39,9 +39,7 @@ def build_field_options(value)
     def build_field(value, index)
       render_options = select_options
       html_options = build_field_options(value)
-      if options[:item_helper]
-        (render_options, html_options) = options[:item_helper].call(value, index, render_options, html_options)
-      end
+      (render_options, html_options) = options[:item_helper].call(value, index, render_options, html_options) if options[:item_helper]
       template.select_tag(attribute_name, template.options_for_select(render_options, value), html_options)
     end
 end
diff --git a/app/models/admin_set.rb b/app/models/admin_set.rb
@@ -38,8 +38,6 @@ class AdminSet < ActiveFedora::Base
   property :creator, predicate: ::RDF::Vocab::DC11.creator do |index|
     index.as :symbol
   end
-
-  # rubocop:disable Rails/HasManyOrHasOneDependent
   has_many :members,
            predicate:  Hyrax.config.admin_set_predicate,
            class_name: 'ActiveFedora::Base'
@@ -58,9 +56,7 @@ def default_set?
 
   # Creates the default AdminSet and an associated PermissionTemplate with workflow
   def self.find_or_create_default_admin_set_id
-    unless exists?(DEFAULT_ID)
-      Hyrax::AdminSetCreateService.create_default_admin_set(admin_set_id: DEFAULT_ID, title: DEFAULT_TITLE)
-    end
+    Hyrax::AdminSetCreateService.create_default_admin_set(admin_set_id: DEFAULT_ID, title: DEFAULT_TITLE) unless exists?(DEFAULT_ID)
     DEFAULT_ID
   end
 

diff --git a/app/models/concerns/hyrax/ability.rb b/app/models/concerns/hyrax/ability.rb
@@ -112,9 +112,7 @@ def proxy_deposit_abilities
           end
         end
 
-        if Flipflop.proxy_deposit? && registered_user?
-          can :create, ProxyDepositRequest
-        end
+        can :create, ProxyDepositRequest if Flipflop.proxy_deposit? && registered_user?
 
         can :accept, ProxyDepositRequest, receiving_user_id: current_user.id, status: 'pending'
         can :reject, ProxyDepositRequest, receiving_user_id: current_user.id, status: 'pending'

diff --git a/app/models/concerns/hyrax/suppressible.rb b/app/models/concerns/hyrax/suppressible.rb
@@ -17,9 +17,7 @@ def suppressed?
       return false if state.nil?
       # This is a clumsy check only needed under RDF < 2.0 where there
       # is a bug with `AT::Resource#==`
-      if RDF::VERSION.to_s < '2.0'
-        return state.rdf_subject == Vocab::FedoraResourceStatus.inactive
-      end
+      return state.rdf_subject == Vocab::FedoraResourceStatus.inactive if RDF::VERSION.to_s < '2.0'
       state == Vocab::FedoraResourceStatus.inactive
     end
 

diff --git a/app/models/featured_work_list.rb b/app/models/featured_work_list.rb
@@ -3,9 +3,7 @@ class FeaturedWorkList
 
   # @param [ActionController::Parameters] a collection of nested perameters
   def featured_works_attributes=(attributes_collection)
-    if attributes_collection.respond_to?(:permitted?)
-      attributes_collection = attributes_collection.to_h
-    end
+    attributes_collection = attributes_collection.to_h if attributes_collection.respond_to?(:permitted?)
     attributes_collection = attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes } if attributes_collection.is_a? Hash
     attributes_collection.each do |attributes|
       raise "Missing id" if attributes['id'].blank?

diff --git a/app/presenters/hyrax/work_show_presenter.rb b/app/presenters/hyrax/work_show_presenter.rb
@@ -194,9 +194,7 @@ def manifest_helper
       end
 
       def featured?
-        if @featured.nil?
-          @featured = FeaturedWork.where(work_id: solr_document.id).exists?
-        end
+        @featured = FeaturedWork.where(work_id: solr_document.id).exists? if @featured.nil?
         @featured
       end
 

diff --git a/app/services/hyrax/admin_set_create_service.rb b/app/services/hyrax/admin_set_create_service.rb
@@ -72,9 +72,7 @@ def access_grants_attributes
           { agent_type: 'group', agent_id: admin_group_name, access: Hyrax::PermissionTemplateAccess::MANAGE }
         ].tap do |attribute_list|
           # Grant manage access to the creating_user if it exists. Should exist for all but default Admin Set
-          if creating_user
-            attribute_list << { agent_type: 'user', agent_id: creating_user.user_key, access: Hyrax::PermissionTemplateAccess::MANAGE }
-          end
+          attribute_list << { agent_type: 'user', agent_id: creating_user.user_key, access: Hyrax::PermissionTemplateAccess::MANAGE } if creating_user
         end
       end
 

diff --git a/app/services/hyrax/analytics.rb b/app/services/hyrax/analytics.rb
@@ -57,9 +57,7 @@ def self.oauth_client
     end
 
     def self.auth_client(scope)
-      unless File.exist?(config.privkey_path)
-        raise "Private key file for Google analytics was expected at '#{config.privkey_path}', but no file was found."
-      end
+      raise "Private key file for Google analytics was expected at '#{config.privkey_path}', but no file was found." unless File.exist?(config.privkey_path)
       private_key = File.read(config.privkey_path)
       Signet::OAuth2::Client.new token_credential_uri: 'https://accounts.google.com/o/oauth2/token',
                                  audience: 'https://accounts.google.com/o/oauth2/token',