bump rubocop-performance

and apply new fixes [skip-stages=Flakey] Change-Id: Ie2f9057a435076c4b872175651cedf737b2c5696 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315800 Tested-by: Service Cloud Jenkins <[email protected]> Reviewed-by: Jacob Burroughs <[email protected]> QA-Review: Cody Cutrer <[email protected]> Product-Review: Cody Cutrer <[email protected]> Build-Review: Cody Cutrer <[email protected]>
Yehuda-Binik · Apr 12, 2023 · 480e29c · 480e29c
1 parent 4d96f5e
commit 480e29c
Show file tree

Hide file tree

Showing 48 changed files with 89 additions and 89 deletions.
diff --git a/Gemfile.d/rubocop.rb b/Gemfile.d/rubocop.rb
@@ -32,7 +32,7 @@
 
   gem "rubocop", "1.49.0", require: false
   gem "rubocop-canvas", require: false, path: "gems/rubocop-canvas"
-  gem "rubocop-performance", "1.12.0", require: false
+  gem "rubocop-performance", "1.17.1", require: false
   gem "rubocop-rails", "2.12.4", require: false
   gem "rubocop-rake", "0.6.0", require: false
   gem "rubocop-rspec", "2.6.0", require: false

diff --git a/Gemfile.rails70.lock.partial b/Gemfile.rails70.lock.partial
@@ -974,7 +974,7 @@ GEM
       unicode-display_width (>= 2.4.0, < 3.0)
     rubocop-ast (1.28.0)
       parser (>= 3.2.1.0)
-    rubocop-performance (1.12.0)
+    rubocop-performance (1.17.1)
       rubocop (>= 1.7.0, < 2.0)
       rubocop-ast (>= 0.4.0)
     rubocop-rails (2.12.4)
@@ -1356,7 +1356,7 @@ DEPENDENCIES
   rss (= 0.2.9)
   rubocop (= 1.49.0)
   rubocop-canvas!
-  rubocop-performance (= 1.12.0)
+  rubocop-performance (= 1.17.1)
   rubocop-rails (= 2.12.4)
   rubocop-rake (= 0.6.0)
   rubocop-rspec (= 2.6.0)

diff --git a/app/controllers/announcements_controller.rb b/app/controllers/announcements_controller.rb
@@ -57,7 +57,7 @@ def load_announcements
 
   def index
     return unless authorized_action(@context, @current_user, :read)
-    return if @context.class.const_defined?("TAB_ANNOUNCEMENTS") && !tab_enabled?(@context.class::TAB_ANNOUNCEMENTS)
+    return if @context.class.const_defined?(:TAB_ANNOUNCEMENTS) && !tab_enabled?(@context.class::TAB_ANNOUNCEMENTS)
 
     redirect_to named_context_url(@context, :context_url) if @context.is_a?(Course) && @context.elementary_homeroom_course?
 

diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb
@@ -38,7 +38,7 @@ class AssignmentsController < ApplicationController
   add_crumb(
     proc { t "#crumbs.assignments", "Assignments" },
     except: %i[destroy syllabus index new edit]
-  ) { |c| c.send :course_assignments_path, c.instance_variable_get("@context") }
+  ) { |c| c.send :course_assignments_path, c.instance_variable_get(:@context) }
   before_action(except: [:new, :edit]) { |c| c.active_tab = "assignments" }
   before_action(only: [:new, :edit]) { |c| setup_active_tab(c) }
   before_action :normalize_title_param, only: [:new, :edit]

diff --git a/app/controllers/calendar_events_api_controller.rb b/app/controllers/calendar_events_api_controller.rb
@@ -1117,7 +1117,7 @@ def public_feed
       get_all_pertinent_contexts
       GuardRail.activate(:secondary) do
         @contexts.each do |context|
-          @assignments = context.assignments.active.to_a if context.respond_to?("assignments")
+          @assignments = context.assignments.active.to_a if context.respond_to?(:assignments)
           # no overrides to apply without a current user
           @events.concat context.calendar_events.active.to_a
           @events.concat @assignments || []

diff --git a/app/controllers/calendar_events_controller.rb b/app/controllers/calendar_events_controller.rb
@@ -24,7 +24,7 @@ class CalendarEventsController < ApplicationController
   before_action :require_context
   before_action :rce_js_env, only: [:new, :edit]
 
-  add_crumb(proc { t(:"#crumbs.calendar_events", "Calendar Events") }, only: %i[show new edit]) { |c| c.send :calendar_url_for, c.instance_variable_get("@context") }
+  add_crumb(proc { t(:"#crumbs.calendar_events", "Calendar Events") }, only: %i[show new edit]) { |c| c.send :calendar_url_for, c.instance_variable_get(:@context) }
 
   def show
     @event = @context.calendar_events.find(params[:id])

diff --git a/app/controllers/calendars_controller.rb b/app/controllers/calendars_controller.rb
@@ -63,18 +63,18 @@ def show
         id: context.id,
         type: context.class.to_s.downcase,
         url: named_context_url(context, :context_url),
-        create_calendar_event_url: context.respond_to?("calendar_events") ? named_context_url(context, :context_calendar_events_url) : "",
-        create_assignment_url: context.respond_to?("assignments") ? named_context_url(context, :api_v1_context_assignments_url) : "",
-        create_appointment_group_url: context.respond_to?("appointment_groups") ? api_v1_appointment_groups_url : "",
-        new_calendar_event_url: context.respond_to?("calendar_events") ? named_context_url(context, :new_context_calendar_event_url) : "",
-        new_assignment_url: context.respond_to?("assignments") ? named_context_url(context, :new_context_assignment_url) : "",
-        calendar_event_url: context.respond_to?("calendar_events") ? named_context_url(context, :context_calendar_event_url, "{{ id }}") : "",
-        assignment_url: context.respond_to?("assignments") ? named_context_url(context, :api_v1_context_assignment_url, "{{ id }}") : "",
+        create_calendar_event_url: context.respond_to?(:calendar_events) ? named_context_url(context, :context_calendar_events_url) : "",
+        create_assignment_url: context.respond_to?(:assignments) ? named_context_url(context, :api_v1_context_assignments_url) : "",
+        create_appointment_group_url: context.respond_to?(:appointment_groups) ? api_v1_appointment_groups_url : "",
+        new_calendar_event_url: context.respond_to?(:calendar_events) ? named_context_url(context, :new_context_calendar_event_url) : "",
+        new_assignment_url: context.respond_to?(:assignments) ? named_context_url(context, :new_context_assignment_url) : "",
+        calendar_event_url: context.respond_to?(:calendar_events) ? named_context_url(context, :context_calendar_event_url, "{{ id }}") : "",
+        assignment_url: context.respond_to?(:assignments) ? named_context_url(context, :api_v1_context_assignment_url, "{{ id }}") : "",
         assignment_override_url: context.respond_to?(:assignments) ? api_v1_assignment_override_url(course_id: context.id, assignment_id: "{{ assignment_id }}", id: "{{ id }}") : "",
-        appointment_group_url: context.respond_to?("appointment_groups") ? api_v1_appointment_groups_url(id: "{{ id }}") : "",
-        can_create_calendar_events: context.respond_to?("calendar_events") && CalendarEvent.new.tap { |e| e.context = context }.grants_right?(@current_user, session, :create),
-        can_create_assignments: context.respond_to?("assignments") && Assignment.new.tap { |a| a.context = context }.grants_right?(@current_user, session, :create),
-        assignment_groups: context.respond_to?("assignments") ? context.assignment_groups.active.pluck(:id, :name).map { |id, name| { id: id, name: name } } : [],
+        appointment_group_url: context.respond_to?(:appointment_groups) ? api_v1_appointment_groups_url(id: "{{ id }}") : "",
+        can_create_calendar_events: context.respond_to?(:calendar_events) && CalendarEvent.new.tap { |e| e.context = context }.grants_right?(@current_user, session, :create),
+        can_create_assignments: context.respond_to?(:assignments) && Assignment.new.tap { |a| a.context = context }.grants_right?(@current_user, session, :create),
+        assignment_groups: context.respond_to?(:assignments) ? context.assignment_groups.active.pluck(:id, :name).map { |id, name| { id: id, name: name } } : [],
         can_create_appointment_groups: ag_permission,
         can_make_reservation: context.grants_right?(@current_user, :participate_as_student),
         can_update_todo_date: context.grants_any_right?(@current_user, session, :manage_content, :manage_course_content_edit),
@@ -87,7 +87,7 @@ def show
         default_due_time: context.is_a?(Course) && context.default_due_time,
         can_view_context: context.grants_right?(@current_user, session, :read)
       }
-      if context.respond_to?("course_sections")
+      if context.respond_to?(:course_sections)
         info[:course_sections] = context.course_sections.active.pluck(:id, :name).map do |id, name|
           hash = { id: id, asset_string: "course_section_#{id}", name: name }
           if ag_permission
@@ -111,7 +111,7 @@ def show
           DUE_DATE_REQUIRED_FOR_ACCOUNT: due_date_required_for_account
         }
       end
-      if ag_permission && ag_permission[:all_sections] && context.respond_to?("group_categories")
+      if ag_permission && ag_permission[:all_sections] && context.respond_to?(:group_categories)
         info[:group_categories] = context.group_categories.active.pluck(:id, :name).map { |id, name| { id: id, asset_string: "group_category_#{id}", name: name } }
       end
       info

diff --git a/app/controllers/context_modules_controller.rb b/app/controllers/context_modules_controller.rb
@@ -23,7 +23,7 @@ class ContextModulesController < ApplicationController
   include WebZipExportHelper
 
   before_action :require_context
-  add_crumb(proc { t("#crumbs.modules", "Modules") }) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_context_modules_url }
+  add_crumb(proc { t("#crumbs.modules", "Modules") }) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_context_modules_url }
   before_action { |c| c.active_tab = "modules" }
 
   include K5Mode

diff --git a/app/controllers/gradebooks_controller.rb b/app/controllers/gradebooks_controller.rb
@@ -39,7 +39,7 @@ class GradebooksController < ApplicationController
 
   batch_jobs_in_actions only: :update_submission, batch: { priority: Delayed::LOW_PRIORITY }
 
-  add_crumb(proc { t "#crumbs.grades", "Grades" }) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_grades_url }
+  add_crumb(proc { t "#crumbs.grades", "Grades" }) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_grades_url }
   before_action { |c| c.active_tab = "grades" }
 
   MAX_POST_GRADES_TOOLS = 10

diff --git a/app/controllers/grading_standards_controller.rb b/app/controllers/grading_standards_controller.rb
@@ -23,7 +23,7 @@ class GradingStandardsController < ApplicationController
     %i[display_name context_code assessed_assignment? context_name].freeze
 
   before_action :require_context
-  add_crumb(proc { t "#crumbs.grading_standards", "Grading" }) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_grading_standards_url }
+  add_crumb(proc { t "#crumbs.grading_standards", "Grading" }) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_grading_standards_url }
   before_action { |c| c.active_tab = "grading_standards" }
 
   def index

diff --git a/app/controllers/outcomes_controller.rb b/app/controllers/outcomes_controller.rb
@@ -23,7 +23,7 @@ class OutcomesController < ApplicationController
   include Api::V1::Role
 
   before_action :require_context, except: [:build_outcomes]
-  add_crumb(proc { t "#crumbs.outcomes", "Outcomes" }, except: [:destroy, :build_outcomes]) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_outcomes_path }
+  add_crumb(proc { t "#crumbs.outcomes", "Outcomes" }, except: [:destroy, :build_outcomes]) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_outcomes_path }
   before_action { |c| c.active_tab = "outcomes" }
   before_action :rce_js_env, only: [:show, :index]
 

diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb
@@ -525,6 +525,6 @@ def qr_mobile_login
 end
 
 def instructure_misc_plugin_available?
-  Object.const_defined?("InstructureMiscPlugin")
+  Object.const_defined?(:InstructureMiscPlugin)
 end
 private :instructure_misc_plugin_available?
diff --git a/app/controllers/question_banks_controller.rb b/app/controllers/question_banks_controller.rb
@@ -20,7 +20,7 @@
 
 class QuestionBanksController < ApplicationController
   before_action :require_context, except: :bookmark
-  add_crumb(proc { t("#crumbs.question_banks", "Question Banks") }, except: :bookmark) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_question_banks_url }
+  add_crumb(proc { t("#crumbs.question_banks", "Question Banks") }, except: :bookmark) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_question_banks_url }
 
   include Api::V1::Outcome
   include QuizMathDataFixup

diff --git a/app/controllers/quizzes/quizzes_controller.rb b/app/controllers/quizzes/quizzes_controller.rb
@@ -37,7 +37,7 @@ class Quizzes::QuizzesController < ApplicationController
 
   include K5Mode
 
-  add_crumb(proc { t("#crumbs.quizzes", "Quizzes") }) { |c| c.send :named_context_url, c.instance_variable_get("@context"), :context_quizzes_url }
+  add_crumb(proc { t("#crumbs.quizzes", "Quizzes") }) { |c| c.send :named_context_url, c.instance_variable_get(:@context), :context_quizzes_url }
   before_action { |c| c.active_tab = "quizzes" }
   before_action :require_quiz, only: %i[
     statistics

diff --git a/app/controllers/wiki_pages_controller.rb b/app/controllers/wiki_pages_controller.rb
@@ -33,8 +33,8 @@ class WikiPagesController < ApplicationController
   include K5Mode
 
   add_crumb(proc { t "#crumbs.wiki_pages", "Pages" }) do |c|
-    context = c.instance_variable_get("@context")
-    current_user = c.instance_variable_get("@current_user")
+    context = c.instance_variable_get(:@context)
+    current_user = c.instance_variable_get(:@current_user)
     if context.grants_right?(current_user, :read)
       c.send :polymorphic_path, [context, :wiki_pages]
     end

diff --git a/app/models/assignment.rb b/app/models/assignment.rb
@@ -1520,7 +1520,7 @@ def interpret_grade(grade, prefer_points_over_scheme: false)
     case grade.to_s
     when /^[+-]?\d*\.?\d+%$/
       # interpret as a percentage
-      percentage = grade.to_f / 100.0.to_d
+      percentage = grade.to_f / BigDecimal("100.0")
       points_possible.to_f * percentage
     when /^[+-]?\d*\.?\d+$/
       if !prefer_points_over_scheme && uses_grading_standard && (standard_based_score = grading_standard_or_default.grade_to_score(grade))
@@ -1535,7 +1535,7 @@ def interpret_grade(grade, prefer_points_over_scheme: false)
     else
       # try to treat it as a letter grade
       if uses_grading_standard && (standard_based_score = grading_standard_or_default.grade_to_score(grade))
-        ((points_possible || 0.0).to_d * standard_based_score.to_d / 100.0.to_d).to_f
+        ((points_possible || 0.0).to_d * standard_based_score.to_d / BigDecimal("100.0")).to_f
       else
         nil
       end

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
@@ -748,7 +748,7 @@ def self.get_quota(context)
       GuardRail.activate(:secondary) do
         context.shard.activate do
           quota = Setting.get("context_default_quota", 50.megabytes.to_s).to_i
-          quota = context.quota if context.respond_to?("quota") && context.quota
+          quota = context.quota if context.respond_to?(:quota) && context.quota
 
           attachment_scope = context.attachments.active.where(root_attachment_id: nil)
 

diff --git a/app/models/content_tag.rb b/app/models/content_tag.rb
@@ -329,11 +329,11 @@ def update_asset_name!(user = nil)
     return unless asset_context_matches?
 
     # Assignment proxies name= and name to title= and title, which breaks the asset_safe_title logic
-    if content.respond_to?("name=") && content.respond_to?("name") && !content.is_a?(Assignment)
+    if content.respond_to?(:name=) && content.respond_to?(:name) && !content.is_a?(Assignment)
       content.name = asset_safe_title("name")
-    elsif content.respond_to?("title=")
+    elsif content.respond_to?(:title=)
       content.title = asset_safe_title("title")
-    elsif content.respond_to?("display_name=")
+    elsif content.respond_to?(:display_name=)
       content.display_name = asset_safe_title("display_name")
     end
     if content.changed?

diff --git a/app/models/grading_standard.rb b/app/models/grading_standard.rb
@@ -112,10 +112,10 @@ def grade_to_score(grade)
     # grade cutoffs.
     # otherwise, we step down just 1/10th of a point, which is the
     # granularity we support right now
-    elsif idx && (ordered_scheme[idx].last - ordered_scheme[idx - 1].last).abs >= 0.01.to_d
-      (ordered_scheme[idx - 1].last * 100.0.to_d) - 1.0.to_d
+    elsif idx && (ordered_scheme[idx].last - ordered_scheme[idx - 1].last).abs >= BigDecimal("0.01")
+      (ordered_scheme[idx - 1].last * BigDecimal("100.0")) - BigDecimal("1.0")
     elsif idx
-      (ordered_scheme[idx - 1].last * 100.0.to_d) - 0.1.to_d
+      (ordered_scheme[idx - 1].last * BigDecimal("100.0")) - BigDecimal("0.1")
     else
       nil
     end
@@ -127,7 +127,7 @@ def score_to_grade(score)
     # assign the highest grade whose min cutoff is less than the score
     # if score is less than all scheme cutoffs, assign the lowest grade
     score = BigDecimal(score.to_s) # Cast this to a BigDecimal too or comparisons get wonky
-    ordered_scheme.max_by { |_, lower_bound| (score >= lower_bound * 100.0.to_d) ? lower_bound : -lower_bound }[0]
+    ordered_scheme.max_by { |_, lower_bound| (score >= lower_bound * BigDecimal("100.0")) ? lower_bound : -lower_bound }[0]
   end
 
   def data=(new_val)

diff --git a/app/models/group.rb b/app/models/group.rb
@@ -136,13 +136,13 @@ def participating_users_in_context(user_ids = nil, sort: false, include_inactive
   end
 
   def all_real_students
-    return context.all_real_students.where(users: { id: group_memberships.select(:user_id) }) if context.respond_to? "all_real_students"
+    return context.all_real_students.where(users: { id: group_memberships.select(:user_id) }) if context.respond_to? :all_real_students
 
     users
   end
 
   def all_real_student_enrollments
-    return context.all_real_student_enrollments.where(user_id: group_memberships.select(:user_id)) if context.respond_to? "all_real_student_enrollments"
+    return context.all_real_student_enrollments.where(user_id: group_memberships.select(:user_id)) if context.respond_to? :all_real_student_enrollments
 
     group_memberships
   end

diff --git a/app/models/message.rb b/app/models/message.rb
@@ -255,7 +255,7 @@ def save_using_update_all
       tap do
         @table = klass.arel_table_from_key_values(attrs)
         @predicate_builder = predicate_builder.dup
-        @predicate_builder.instance_variable_set("@table", ActiveRecord::TableMetadata.new(klass, @table))
+        @predicate_builder.instance_variable_set(:@table, ActiveRecord::TableMetadata.new(klass, @table))
       end
     end
   }

diff --git a/app/models/pseudonym.rb b/app/models/pseudonym.rb
@@ -89,7 +89,7 @@ class Pseudonym < ActiveRecord::Base
             if: :require_password?
 
   validates_each :password, if: :require_password?,
-                 &Canvas::PasswordPolicy.method("validate")
+                 &Canvas::PasswordPolicy.method(:validate)
   validates :password_confirmation,
             presence: true,
             if: :require_password?

diff --git a/app/models/setting.rb b/app/models/setting.rb
@@ -72,7 +72,7 @@ def self.set(name, value, secret: nil)
     @all_settings = nil
     MultiCache.delete("all_settings")
 
-    if Rails.const_defined? "Console"
+    if Rails.const_defined? :Console
       message = Setting.get("setting_set_sighup_required_message", "** NOTE: After calling `Setting.set`, SIGHUP must be sent to all Canvas processes **")
       Rails.logger.info(message)
     end

diff --git a/app/models/submission.rb b/app/models/submission.rb
@@ -321,11 +321,11 @@ def needs_grading_changed?
   end
 
   def submitted_changed?
-    submitted? != %w[submitted pending_review graded].include?(send("workflow_state_before_last_save"))
+    submitted? != %w[submitted pending_review graded].include?(send(:workflow_state_before_last_save))
   end
 
   def graded_changed?
-    graded? != (send("workflow_state_before_last_save") == "graded")
+    graded? != (send(:workflow_state_before_last_save) == "graded")
   end
 
   scope :needs_grading, lambda {

diff --git a/app/models/submission_draft.rb b/app/models/submission_draft.rb
@@ -44,7 +44,7 @@ def validate_url
         # also updates the url with a scheme if missing and is a valid url
         # otherwise leaves the url as whatever the user submitted as thier draft
         value, = CanvasHttp.validate_url(url)
-        send("url=", value)
+        send(:url=, value)
       rescue
         nil
       end
@@ -55,7 +55,7 @@ def validate_lti_launch_url
     return if lti_launch_url.blank?
 
     value, = CanvasHttp.validate_url(lti_launch_url)
-    send("lti_launch_url=", value) if value
+    send(:lti_launch_url=, value) if value
   rescue
     # we couldn't validate, just leave it
   end

diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb
@@ -197,6 +197,6 @@ def insert_past_global_announcements(tabs, user, _opts)
 end
 
 def instructure_misc_plugin_available?
-  Object.const_defined?("InstructureMiscPlugin")
+  Object.const_defined?(:InstructureMiscPlugin)
 end
 private :instructure_misc_plugin_available?
diff --git a/app/serializers/canvas/api_serializer.rb b/app/serializers/canvas/api_serializer.rb
@@ -167,7 +167,7 @@ def build_serializer(association)
       options = { controller: @controller, scope: scope }
       association.build_serializer(object, options).tap do |serializer|
         if association.options.key?(:wrap_in_array)
-          serializer.instance_variable_set("@wrap_in_array",
+          serializer.instance_variable_set(:@wrap_in_array,
                                            association.options[:wrap_in_array])
         end
       end

diff --git a/gems/plugins/academic_benchmark/spec_canvas/academic_benchmark_spec.rb b/gems/plugins/academic_benchmark/spec_canvas/academic_benchmark_spec.rb
@@ -145,7 +145,7 @@ def verify_full_import
   end
 
   def check_for_parent_num_duplication(outcome)
-    parent = outcome.instance_variable_get("@parent")
+    parent = outcome.instance_variable_get(:@parent)
     if outcome.resolve_number && parent && parent.build_title && outcome.resolve_number.include?(parent.build_title)
       outcome.title == "#{parent.title}.#{outcome.resolve_number}"
     else
@@ -155,7 +155,7 @@ def check_for_parent_num_duplication(outcome)
 
   def check_built_outcome(outcome)
     expect(check_for_parent_num_duplication(outcome)).to be_falsey
-    outcome.instance_variable_get("@children").each { |o| check_built_outcome(o) }
+    outcome.instance_variable_get(:@children).each { |o| check_built_outcome(o) }
   end
 
   it "imports the standards successfully" do

diff --git a/gems/plugins/account_reports/lib/account_reports.rb b/gems/plugins/account_reports/lib/account_reports.rb
@@ -166,7 +166,7 @@ def self.report_attachment(account_report, csv = nil)
       end
       filetype = "application/zip"
     elsif csv
-      ext = csv !~ /\n/ && File.extname(csv)
+      ext = !csv.include?("\n") && File.extname(csv)
       case ext
       when ".csv"
         filename = File.basename(csv)