RuboCop: Style/NilComparison, Style/NonNilCheck

auto-corrected

Change-Id: If8c067e5b9bd4d21131c6699900a7a14a988efeb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279000
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

.rubocop.common.yml

@@ -250,6 +250,10 @@ Style/NegatedWhile:
   Severity: error
 Style/NestedParenthesizedCalls:
   Severity: error
+Style/NilComparison:
+  Severity: error
+Style/NonNilCheck:
+  Severity: error
 Style/Not:
   Severity: error
 Style/NumericPredicate:

app/controllers/application_controller.rb

@@ -930,7 +930,7 @@ class ApplicationController < ActionController::Base
         raise ActiveRecord::RecordNotFound, "Context is required, but none found"
       end
     end
-    @context != nil
+    !@context.nil?
   end
 
   def require_context_and_read_access
@@ -2137,7 +2137,7 @@ class ApplicationController < ActionController::Base
   def feature_enabled?(feature)
     @features_enabled ||= {}
     feature = feature.to_sym
-    return @features_enabled[feature] if @features_enabled[feature] != nil
+    return @features_enabled[feature] unless @features_enabled[feature].nil?
 
     @features_enabled[feature] ||= if [:question_banks].include?(feature)
                                      true

app/helpers/application_helper.rb

@@ -537,9 +537,9 @@ module ApplicationHelper
 
   def hash_get(hash, key, default = nil)
     if hash
-      if hash[key.to_s] != nil
+      if !hash[key.to_s].nil?
         hash[key.to_s]
-      elsif hash[key.to_sym] != nil
+      elsif !hash[key.to_sym].nil?
         hash[key.to_sym]
       else
         default

app/models/learning_outcome_result.rb

@@ -162,7 +162,7 @@ class LearningOutcomeResult < ActiveRecord::Base
     self.context_code = "#{self.context_type.underscore}_#{self.context_id}" rescue nil
     self.original_score ||= self.score
     self.original_possible ||= self.possible
-    self.original_mastery = self.mastery if self.original_mastery == nil
+    self.original_mastery = self.mastery if self.original_mastery.nil?
     calculate_percent!
     true
   end

app/models/quizzes/quiz.rb

@@ -126,11 +126,11 @@ class Quizzes::Quiz < ActiveRecord::Base
       self.show_correct_answers_at = nil
       self.hide_correct_answers_at = nil
     end
-    self.allowed_attempts = 1 if self.allowed_attempts == nil
+    self.allowed_attempts = 1 if self.allowed_attempts.nil?
     if self.allowed_attempts <= 1
       self.show_correct_answers_last_attempt = false
     end
-    self.scoring_policy = "keep_highest" if self.scoring_policy == nil
+    self.scoring_policy = "keep_highest" if self.scoring_policy.nil?
     self.ip_filter = nil if self.ip_filter && self.ip_filter.strip.empty?
     if !self.available? && !self.survey?
       self.points_possible = self.current_points_possible

app/models/quizzes/quiz_submission.rb

@@ -255,7 +255,7 @@ class Quizzes::QuizSubmission < ActiveRecord::Base
   alias_method :overdue_and_needs_submission, :overdue_and_needs_submission?
 
   def end_date_needs_recalculated?
-    self.end_at == nil && !!quiz.time_limit
+    self.end_at.nil? && !!quiz.time_limit
   end
 
   def end_date_is_valid?

app/models/rubric.rb

@@ -259,7 +259,7 @@ class Rubric < ActiveRecord::Base
   def update_criteria(params)
     self.without_versioning(&:save) if self.new_record?
     data = generate_criteria(params)
-    self.hide_score_total = params[:hide_score_total] if self.hide_score_total == nil || (self.association_count || 0) < 2
+    self.hide_score_total = params[:hide_score_total] if self.hide_score_total.nil? || (self.association_count || 0) < 2
     self.data = data.criteria
     self.title = data.title
     self.points_possible = data.points_possible

app/models/user.rb

@@ -296,7 +296,7 @@ class User < ActiveRecord::Base
     active_observer_scope = joins(:enrollments).where(enrollments: { type: 'ObserverEnrollment', course_id: course_ids, workflow_state: 'active' })
     users_observing_students = active_observer_scope.where.not(enrollments: { associated_user_id: nil }).pluck(:id)
 
-    if users_observing_students == [] || users_observing_students == nil
+    if users_observing_students == [] || users_observing_students.nil?
       active_observer_scope
     else
       active_observer_scope.where.not(users: { id: users_observing_students })
@@ -885,7 +885,7 @@ class User < ActiveRecord::Base
   end
 
   def preserve_lti_id
-    errors.add(:lti_id, 'Cannot change lti_id!') if lti_id_changed? && lti_id_was != nil
+    errors.add(:lti_id, 'Cannot change lti_id!') if lti_id_changed? && !lti_id_was.nil?
   end
 
   def ensure_lti_id

lib/canvas/migration/helpers/selective_content_formatter.rb

@@ -147,7 +147,7 @@ module Canvas::Migration::Helpers
         content_list << process_group(group, outcomes)
       end
       content_list.concat(
-        outcomes.select { |outcome| outcome['parent_migration_id'] == nil }.map { |outcome| item_hash('learning_outcomes', outcome) }
+        outcomes.select { |outcome| outcome['parent_migration_id'].nil? }.map { |outcome| item_hash('learning_outcomes', outcome) }
       )
     end
 

lib/notification_message_creator.rb

@@ -131,7 +131,7 @@ class NotificationMessageCreator
     fallback_policy = nil
     NotificationPolicy.unique_constraint_retry do
       # notification_policies are already loaded, so use find instead of generating a query
-      fallback_policy = channel.notification_policies.find { |np| np.frequency == 'daily' && np.notification_id == nil }
+      fallback_policy = channel.notification_policies.find { |np| np.frequency == 'daily' && np.notification_id.nil? }
       fallback_policy ||= channel.notification_policies.create!(frequency: 'daily')
     end
 

lib/sorts_assignments.rb

@@ -49,12 +49,12 @@ class SortsAssignments
 
     def dated(assignments)
       assignments ||= []
-      assignments.reject { |assignment| assignment.due_at == nil }
+      assignments.reject { |assignment| assignment.due_at.nil? }
     end
 
     def undated(assignments)
       assignments ||= []
-      assignments.select { |assignment| assignment.due_at == nil }
+      assignments.select { |assignment| assignment.due_at.nil? }
     end
 
     def unsubmitted_for_user_and_session(course, assignments, user, current_user, session)

lib/vericite.rb

@@ -135,7 +135,7 @@ module VeriCite
             paper_title = File.basename(a.display_name, ".*")
             paper_ext = a.extension
             paper_type = a.content_type
-            if paper_ext == nil
+            if paper_ext.nil?
               paper_ext = ""
             end
             paper_size = 100 # File.size(
@@ -182,7 +182,7 @@ module VeriCite
       course = assignment.context
       object_id = submission.vericite_data_hash[asset_string][:object_id] rescue nil
       response = sendRequest(:generate_report, :oid => object_id, :utp => '2', :current_user => current_user, :user => user, :course => course, :assignment => assignment)
-      if response != nil
+      if !response.nil?
         response[:report_url]
       else
         nil
@@ -195,7 +195,7 @@ module VeriCite
       course = assignment.context
       object_id = submission.vericite_data_hash[asset_string][:object_id] rescue nil
       response = sendRequest(:generate_report, :oid => object_id, :utp => '1', :current_user => current_user, :user => user, :course => course, :assignment => assignment, :tem => email(course))
-      if response != nil
+      if !response.nil?
         response[:report_url]
       else
         nil
@@ -223,17 +223,17 @@ module VeriCite
           context_id = course.id
           assignment_id = assignment.id
           assignment_data = VeriCiteClient::AssignmentData.new
-          assignment_data.assignment_title = assignment.title != nil ? assignment.title : assignment_id
-          assignment_data.assignment_instructions = assignment.description != nil ? assignment.description : ""
+          assignment_data.assignment_title = assignment.title || assignment_id
+          assignment_data.assignment_instructions = assignment.description || ""
           assignment_data.assignment_exclude_quotes = args["exclude_quoted"] == '1'
           assignment_data.assignment_exclude_self_plag = args["exclude_self_plag"] == '1'
           assignment_data.assignment_store_in_index = args["store_in_index"] == '1'
           assignment_data.assignment_due_date = 0
-          if assignment.due_at != nil
+          unless assignment.due_at.nil?
             # convert to epoch time in milli
             assignment_data.assignment_due_date = assignment.due_at.to_time.utc.to_i * 1000
           end
-          assignment_data.assignment_grade = assignment.points_possible != nil ? assignment.points_possible : -1
+          assignment_data.assignment_grade = assignment.points_possible || -1
           _data, status_code, _headers = vericite_client.assignments_context_id_assignment_id_post(context_id, assignment_id, consumer, consumer_secret, assignment_data)
           # check status code
           response[:return_code] = status_code
@@ -254,10 +254,10 @@ module VeriCite
           report_meta_data.user_email = email(user)
           report_meta_data.user_role = args[:role]
           if assignment
-            report_meta_data.assignment_title = assignment.title != nil ? assignment.title : assignment_id
+            report_meta_data.assignment_title = assignment.title || assignment_id
           end
           if course
-            report_meta_data.context_title = course.name != nil ? course.name : context_id
+            report_meta_data.context_title = course.name || context_id
           end
           external_content_data = VeriCiteClient::ExternalContentData.new
           external_content_data.external_content_id = "#{consumer}/#{context_id}/#{assignment_id}/#{user_id}/#{args[:pid]}"
@@ -288,7 +288,7 @@ module VeriCite
           users_score_map = {}
           # first check if the cache already has the user's score and if we haven't looked up this assignment lately:
           users_score_map[user_id.to_s] = Rails.cache.read("#{user_score_cache_key_prefix}#{user_id}")
-          if users_score_map[user_id.to_s].nil? && Rails.cache.read(user_score_cache_key_prefix) == nil
+          if users_score_map[user_id.to_s].nil? && Rails.cache.read(user_score_cache_key_prefix).nil?
             # we already looked up this user in Redis, don't bother again (by setting {})
             users_score_map[user_id.to_s] ||= {}
             # we need to look up the user scores in VeriCite for this course

script/render_json_lint

@@ -16,7 +16,7 @@ def bad_to_json_in_render?(expr)
   # look for that structure, but skip if the value is a literal. known not to catch
   # "render status: 200, json: value.to_json(args)"
   # but that's ok for now
-  expr[0] == :call && expr[1] == nil && expr[2] == :render &&
+  expr[0] == :call && expr[1].nil? && expr[2] == :render &&
     expr[3].is_a?(Sexp) && expr[3].first == :hash &&
     expr[3][1].is_a?(Sexp) && expr[3][1].first == :lit && expr[3][1].last == :json &&
     expr[3][2].is_a?(Sexp) && expr[3][2].first == :call && expr[3][2][2] == :to_json &&

spec/selenium/courses/course_settings_spec.rb

@@ -205,7 +205,7 @@ describe "course settings" do
       wait.until do
         el = f('.self_enrollment_message')
         el.present? &&
-          el.text != nil &&
+          !el.text.nil? &&
           el.text != ""
       end
       message = f('.self_enrollment_message')

spec/selenium/test_setup/common_helper_methods/custom_validators.rb

@@ -20,7 +20,7 @@
 module CustomValidators
   def validate_breadcrumb_link(link_element, breadcrumb_text)
     expect_new_page_load { link_element.click }
-    if breadcrumb_text != nil
+    unless breadcrumb_text.nil?
       breadcrumb = f('#breadcrumbs')
       expect(breadcrumb).to include_text(breadcrumb_text)
     end