RuboCop: Style grab bag

[skip-stages=Flakey] see .rubocop.common.yml changes for enabled cops auto-corrected Change-Id: Ia63a1c597c58646394b251ce81b707f32828bd4c Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279010 Reviewed-by: Jacob Burroughs <[email protected]> Tested-by: Service Cloud Jenkins <[email protected]> QA-Review: Cody Cutrer <[email protected]> Product-Review: Cody Cutrer <[email protected]> Migration-Review: Cody Cutrer <[email protected]>
zwily · Nov 20, 2021 · ecd0798 · ecd0798
1 parent 877ca06
commit ecd0798
Show file tree

Hide file tree

Showing 167 changed files with 771 additions and 446 deletions.
diff --git a/.rubocop.common.yml b/.rubocop.common.yml
@@ -204,17 +204,24 @@ RSpec/StubbedMock:
 Specs/EnsureSpecExtension:
   Exclude:
     - spec/shared_examples/**/*
+
 Style/Alias:
   EnforcedStyle: prefer_alias_method # https://github.com/rubocop/ruby-style-guide/issues/821
   Severity: error
 Style/AndOr:
   Severity: error
+Style/ArrayJoin:
+  Severity: error
 Style/BarePercentLiterals:
   Severity: error
+Style/BlockComments:
+  Severity: error
 Style/CaseEquality:
   Severity: error
 Style/CaseLikeIf:
   Severity: error
+Style/CharacterLiteral:
+  Severity: error
 Style/ClassCheck:
   Severity: error
 Style/ClassEqualityComparison:
@@ -229,6 +236,10 @@ Style/Dir:
   Severity: error
 Style/Documentation:
   Enabled: false # most things don't need to be documented
+Style/EmptyLambdaParameter:
+  Severity: error
+Style/EachForSimpleLoop:
+  Severity: error
 Style/EachWithObject:
   Severity: error
 Style/EmptyCaseCondition:
@@ -241,12 +252,20 @@ Style/ExpandPathArguments:
   Severity: error
 Style/ExplicitBlockArgument:
   Severity: error
+Style/EvalWithLocation:
+  Severity: error
+Style/EvenOdd:
+  Severity: error
+Style/For:
+  Severity: error
 Style/FrozenStringLiteralComment:
   Severity: error
 Style/HashConversion:
   Severity: error
 Style/HashEachMethods:
   Severity: error
+Style/HashLikeCase:
+  Severity: error
 Style/HashTransformKeys:
   Severity: error
 Style/HashTransformValues:
@@ -260,8 +279,18 @@ Style/IfUnlessModifier:
   Enabled: false # can obscure important decisions or put too much code in a line
 Style/IfWithBooleanLiteralBranches:
   Severity: error
+Style/IfUnlessModifierOfIfUnless:
+  Severity: error
+Style/LambdaCall:
+  Severity: error
 Style/MethodCallWithoutArgsParentheses:
   Severity: error
+Style/MethodDefParentheses:
+  Severity: error
+Style/MultilineMemoization:
+  Severity: error
+Style/MultilineTernaryOperator:
+  Severity: error
 Style/MultilineWhenThen:
   Severity: error
 Style/MutableConstant:
@@ -272,16 +301,26 @@ Style/NegatedUnless:
   Severity: error
 Style/NegatedWhile:
   Severity: error
+Style/NestedModifier:
+  Severity: error
 Style/NestedParenthesizedCalls:
   Severity: error
+Style/NestedTernaryOperator:
+  Severity: error
 Style/NilComparison:
   Severity: error
+Style/NilLambda:
+  Severity: error
 Style/NonNilCheck:
   Severity: error
 Style/Not:
   Severity: error
+Style/NumericLiteralPrefix:
+  Severity: error
 Style/NumericPredicate:
   Enabled: false # `> 0` can be easier to read than `.positive?`
+Style/OneLineConditional:
+  Severity: error
 Style/OrAssignment:
   Severity: error
 Style/ParenthesesAroundCondition:
@@ -295,25 +334,33 @@ Style/PerlBackrefs:
   Enabled: false # Regexp.last_match(1) is far worse than $1
 Style/PreferredHashMethods:
   Severity: error
+Style/Proc:
+  Severity: error
 Style/QuotedSymbols:
   EnforcedStyle: double_quotes # once Style/StringLiterals is enabled, we can remove this since it will inherit
   Severity: error
-Style/RedundantArgument:
-  Severity: error
 Style/RaiseArgs:
   Severity: error
+Style/RandomWithOffset:
+  Severity: error
+Style/RedundantArgument:
+  Severity: error
 Style/RedundantAssignment:
   Severity: error
 Style/RedundantBegin:
   Severity: error
 Style/RedundantCondition:
   Severity: error
-Style/RedundantInterpolation:
+Style/RedundantException:
+  Severity: error
+Style/RedundantFetchBlock:
   Severity: error
 Style/RedundantFileExtensionInRequire:
   Severity: error
 Style/RedundantFreeze:
   Severity: error
+Style/RedundantInterpolation:
+  Severity: error
 Style/RedundantParentheses:
   Severity: error
 Style/RedundantPercentQ:
@@ -324,6 +371,10 @@ Style/RedundantRegexpEscape:
   Severity: error
 Style/RedundantReturn:
   Severity: error
+Style/RedundantSelfAssignment:
+  Severity: error
+Style/RedundantSelfAssignmentBranch:
+  Severity: error
 Style/RedundantSort:
   Severity: error
 Style/RegexpLiteral:
@@ -333,6 +384,8 @@ Style/RescueStandardError:
   Severity: error
 Style/SafeNavigation:
   Severity: error
+Style/SelectByRegexp:
+  Severity: error
 Style/SelfAssignment:
   Severity: error
 Style/Semicolon:
@@ -347,6 +400,8 @@ Style/SlicingWithRange:
   Severity: error
 Style/SpecialGlobalVars:
   Enabled: false # $! and $? are fine
+Style/StringChars:
+  Severity: error
 Style/StructInheritance:
   Severity: error
 Style/SymbolProc:

diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
@@ -1434,10 +1434,11 @@ def statistics_graph
   end
 
   def avatars
-    # multi-line ternary is not ideal, but is a clean solution for temp granular check
-    is_authorized = @domain_root_account.feature_enabled?(:granular_permissions_manage_users) ?
-      authorized_action(@account, @current_user, :allow_course_admin_actions) :
-      authorized_action(@account, @current_user, :manage_admin_users)
+    is_authorized = if @domain_root_account.feature_enabled?(:granular_permissions_manage_users)
+                      authorized_action(@account, @current_user, :allow_course_admin_actions)
+                    else
+                      authorized_action(@account, @current_user, :manage_admin_users)
+                    end
 
     if is_authorized
       @users = @account.all_users(nil)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -1855,9 +1855,11 @@ def content_tag_redirect(context, tag, error_redirect_symbol, tag_type = nil)
         @assignment = @tag.context
 
         @resource_title = @assignment.title
-        @module_tag = params[:module_item_id] ?
-          @context.context_module_tags.not_deleted.find(params[:module_item_id]) :
-          @assignment.context_module_tags.first
+        @module_tag = if params[:module_item_id]
+                        @context.context_module_tags.not_deleted.find(params[:module_item_id])
+                      else
+                        @assignment.context_module_tags.first
+                      end
       else
         @module_tag = @tag
         @resource_title = @tag.title
@@ -2058,9 +2060,13 @@ def files_url_for(contexts_to_link_to = nil, options = {})
       options[:query].merge({
         :controller => 'files',
         :action => "full_index",
-      }.merge(options[:anchor].empty? ? {} : {
-                :anchor => options[:anchor]
-              }))
+      }.merge(if options[:anchor].empty?
+                {}
+              else
+                {
+                  :anchor => options[:anchor]
+                }
+              end))
     )
   end
   helper_method :calendar_url_for, :files_url_for
@@ -2280,13 +2286,17 @@ def public_user_content(str, context = @context, user = @current_user, is_public
   def find_bank(id, check_context_chain = true)
     bank = @context.assessment_question_banks.active.where(id: id).first || @current_user.assessment_question_banks.active.where(id: id).first
     if bank
-      (block_given? ?
-        authorized_action(bank, @current_user, :read) :
-        bank.grants_right?(@current_user, session, :read)) or return nil
+      (if block_given?
+         authorized_action(bank, @current_user, :read)
+       else
+         bank.grants_right?(@current_user, session, :read)
+       end) or return nil
     elsif check_context_chain
-      (block_given? ?
-        authorized_action(@context, @current_user, :read_question_banks) :
-        @context.grants_right?(@current_user, session, :read_question_banks)) or return nil
+      (if block_given?
+         authorized_action(@context, @current_user, :read_question_banks)
+       else
+         @context.grants_right?(@current_user, session, :read_question_banks)
+       end) or return nil
       bank = @context.inherited_assessment_question_banks.where(id: id).first
     end
 
@@ -2681,10 +2691,16 @@ def set_js_assignment_data
              :HAS_GRADING_PERIODS => @context.grading_periods?,
              :VALID_DATE_RANGE => CourseDateRange.new(@context),
              :assignment_menu_tools => external_tools_display_hashes(:assignment_menu),
-             :assignment_index_menu_tools => (@domain_root_account&.feature_enabled?(:commons_favorites) ?
-        external_tools_display_hashes(:assignment_index_menu) : []),
-             :assignment_group_menu_tools => (@domain_root_account&.feature_enabled?(:commons_favorites) ?
-        external_tools_display_hashes(:assignment_group_menu) : []),
+             :assignment_index_menu_tools => (if @domain_root_account&.feature_enabled?(:commons_favorites)
+                                                external_tools_display_hashes(:assignment_index_menu)
+                                              else
+                                                []
+                                              end),
+             :assignment_group_menu_tools => (if @domain_root_account&.feature_enabled?(:commons_favorites)
+                                                external_tools_display_hashes(:assignment_group_menu)
+                                              else
+                                                []
+                                              end),
              :discussion_topic_menu_tools => external_tools_display_hashes(:discussion_topic_menu),
              :quiz_menu_tools => external_tools_display_hashes(:quiz_menu),
              :current_user_has_been_observer_in_this_course => current_user_has_been_observer_in_this_course,

diff --git a/app/controllers/assignments_api_controller.rb b/app/controllers/assignments_api_controller.rb
@@ -882,8 +882,11 @@ def get_assignments(user)
       assignments.map do |assignment|
         visibility_array = assignment_visibilities[assignment.id] if assignment_visibilities
         submission = submissions[assignment.id]
-        needs_grading_course_proxy = @context.grants_right?(user, session, :manage_grades) ?
-          Assignments::NeedsGradingCountQuery::CourseProxy.new(@context, user) : nil
+        needs_grading_course_proxy = if @context.grants_right?(user, session, :manage_grades)
+                                       Assignments::NeedsGradingCountQuery::CourseProxy.new(@context, user)
+                                     else
+                                       nil
+                                     end
 
         assignment_json(assignment, user, session,
                         submission: submission, override_dates: override_dates,

diff --git a/app/controllers/conferences_controller.rb b/app/controllers/conferences_controller.rb
@@ -177,9 +177,11 @@ def index
     return unless @current_user
 
     log_api_asset_access(["conferences", @context], "conferences", "other")
-    conferences = @context.grants_right?(@current_user, :manage_content) ?
-      @context.web_conferences.active :
-      @current_user.web_conferences.active.shard(@context.shard).where(context_type: @context.class.to_s, context_id: @context.id)
+    conferences = if @context.grants_right?(@current_user, :manage_content)
+                    @context.web_conferences.active
+                  else
+                    @current_user.web_conferences.active.shard(@context.shard).where(context_type: @context.class.to_s, context_id: @context.id)
+                  end
     conferences = conferences.with_config_for(context: @context).order("created_at DESC, id DESC")
     api_request? ? api_index(conferences, polymorphic_url([:api_v1, @context, :conferences])) : web_index(conferences)
   end

diff --git a/app/controllers/content_imports_controller.rb b/app/controllers/content_imports_controller.rb
@@ -137,7 +137,11 @@ def copy_course_content
                                     :source_course => @source_course,
                                     :copy_options => copy_params,
                                     :migration_type => 'course_copy_importer',
-                                    :initiated_source => api_request? ? (in_app? ? :api_in_app : :api) : :manual)
+                                    :initiated_source => if api_request?
+                                                           in_app? ? :api_in_app : :api
+                                                         else
+                                                           :manual
+                                                         end)
       cm.queue_migration
       cm.workflow_state = 'created'
       render :json => copy_status_json(cm, @context, @current_user, session)

diff --git a/app/controllers/context_controller.rb b/app/controllers/context_controller.rb
@@ -146,9 +146,11 @@ def roster
   end
 
   def prior_users
-    manage_admins = @context.root_account.feature_enabled?(:granular_permissions_manage_users) ?
-      :allow_course_admin_actions :
-      :manage_admin_users
+    manage_admins = if @context.root_account.feature_enabled?(:granular_permissions_manage_users)
+                      :allow_course_admin_actions
+                    else
+                      :manage_admin_users
+                    end
     if authorized_action(@context, @current_user, [:manage_students, manage_admins, :read_prior_roster])
       @prior_users = @context.prior_users
                              .by_top_enrollment.merge(Enrollment.not_fake)

diff --git a/app/controllers/context_modules_controller.rb b/app/controllers/context_modules_controller.rb
@@ -406,9 +406,11 @@ def content_tag_master_course_data
         end
         restriction_info = {}
         if tag_ids.any?
-          restriction_info = is_child_course ?
-            MasterCourses::MasterContentTag.fetch_module_item_restrictions_for_child(tag_ids) :
-            MasterCourses::MasterContentTag.fetch_module_item_restrictions_for_master(tag_ids)
+          restriction_info = if is_child_course
+                               MasterCourses::MasterContentTag.fetch_module_item_restrictions_for_child(tag_ids)
+                             else
+                               MasterCourses::MasterContentTag.fetch_module_item_restrictions_for_master(tag_ids)
+                             end
         end
         info[:tag_restrictions] = restriction_info
       end

diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
@@ -2594,7 +2594,11 @@ def copy_course
       @content_migration = @course.content_migrations.build(
         :user => @current_user, :source_course => @context,
         :context => @course, :migration_type => 'course_copy_importer',
-        :initiated_source => api_request? ? (in_app? ? :api_in_app : :api) : :manual
+        :initiated_source => if api_request?
+                               in_app? ? :api_in_app : :api
+                             else
+                               :manual
+                             end
       )
       @content_migration.migration_settings[:source_course_id] = @context.id
       @content_migration.migration_settings[:import_quizzes_next] = true if params.dig(:settings, :import_quizzes_next)
@@ -3849,9 +3853,11 @@ def effective_due_dates_params
   end
 
   def manage_admin_users_perm
-    @context.root_account.feature_enabled?(:granular_permissions_manage_users) ?
-      :allow_course_admin_actions :
+    if @context.root_account.feature_enabled?(:granular_permissions_manage_users)
+      :allow_course_admin_actions
+    else
       :manage_admin_users
+    end
   end
 
   def course_params

diff --git a/app/controllers/crocodoc_sessions_controller.rb b/app/controllers/crocodoc_sessions_controller.rb
@@ -29,9 +29,11 @@ def show
     attachment = Attachment.find(blob["attachment_id"])
 
     if attachment.crocodoc_available?
-      annotations = params[:annotations] ?
-        value_to_boolean(params[:annotations]) :
-        true
+      annotations = if params[:annotations]
+                      value_to_boolean(params[:annotations])
+                    else
+                      true
+                    end
 
       crocodoc = attachment.crocodoc_document
       url = crocodoc.session_url(:user => @current_user,

diff --git a/app/controllers/custom_gradebook_columns_api_controller.rb b/app/controllers/custom_gradebook_columns_api_controller.rb
@@ -77,9 +77,11 @@ class CustomGradebookColumnsApiController < ApplicationController
   def index
     if authorized_action? @context.custom_gradebook_columns.build,
                           @current_user, :read
-      scope = value_to_boolean(params[:include_hidden]) ?
-        @context.custom_gradebook_columns.not_deleted :
-        @context.custom_gradebook_columns.active
+      scope = if value_to_boolean(params[:include_hidden])
+                @context.custom_gradebook_columns.not_deleted
+              else
+                @context.custom_gradebook_columns.active
+              end
       columns = Api.paginate(scope, self,
                              api_v1_course_custom_gradebook_columns_url(@context))