RuboCop: Performance/StringInclude

[skip-stages=Flakey]

auto-corrected, with manual review to identify possible nilness

Change-Id: I205436e5c3cb37aae99ea552c7d14e6d1a04ef06
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277893
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Simon Williams <[email protected]>
QA-Review: Cody Cutrer <[email protected]>
Product-Review: Cody Cutrer <[email protected]>

.rubocop.common.yml

@@ -64,6 +64,9 @@ Naming/FileName:
   Exclude:
     - "**/Gemfile.d/~after.rb"
 
+Performance/StringInclude:
+  Severity: error
+
 Rails/ApplicationRecord:
   Enabled: false # we never bothered creating an ApplicationRecord
 Rails/HasManyOrHasOneDependent:

app/controllers/application_controller.rb

@@ -1547,7 +1547,7 @@ def log_participation(user)
   end
 
   def log_gets
-    if @page_view && !request.xhr? && request.get? && ((response.media_type || "").to_s.match(/html/) ||
+    if @page_view && !request.xhr? && request.get? && ((response.media_type || "").to_s.include?('html') ||
       ((Setting.get('create_get_api_page_views', 'true') == 'true') && api_request?))
       @page_view.render_time ||= (Time.now.utc - @page_before_render) rescue nil
       @page_view_update = true
@@ -2302,7 +2302,7 @@ def in_app?
   end
 
   def json_as_text?
-    (request.headers['CONTENT_TYPE'].to_s =~ %r{multipart/form-data}) &&
+    request.headers['CONTENT_TYPE'].to_s.include?('multipart/form-data') &&
       (params[:format].to_s != 'json' || in_app?)
   end
 
@@ -2334,7 +2334,7 @@ def set_layout_options
   end
 
   def stringify_json_ids?
-    request.headers['Accept'] =~ %r{application/json\+canvas-string-ids}
+    request.headers['Accept']&.include?('application/json+canvas-string-ids')
   end
 
   def json_cast(obj)
@@ -2529,7 +2529,7 @@ def mobile_device?
   end
 
   def ms_office?
-    !!(request.user_agent.to_s =~ /ms-office/) ||
+    !!request.user_agent.to_s.include?('ms-office') ||
       !!(request.user_agent.to_s =~ %r{Word/\d+\.\d+})
   end
 

app/controllers/content_migrations_controller.rb

@@ -515,7 +515,7 @@ def require_auth
   end
 
   def find_migration_plugin(name)
-    if name =~ /context_external_tool/
+    if name.include?('context_external_tool')
       plugin = Canvas::Plugin.new(name)
       plugin.meta[:settings] = { requires_file_upload: true, worker: 'CCWorker', valid_contexts: %w{Course} }.with_indifferent_access
       plugin

app/controllers/enrollments_api_controller.rb

@@ -428,7 +428,7 @@ def index
       enrollments = enrollments.joins(:user).select("enrollments.*")
 
       has_courses = enrollments.where_clause.instance_variable_get(:@predicates)
-                               .any? { |cond| cond.is_a?(String) && cond =~ /courses\./ }
+                               .any? { |cond| cond.is_a?(String) && cond.include?('courses.') }
       enrollments = enrollments.joins(:course) if has_courses
       enrollments = enrollments.shard(@shard_scope) if @shard_scope
 

app/controllers/lti/concerns/oembed.rb

@@ -61,7 +61,7 @@ def oembed_url
     end
 
     def oembed_object_uri
-      URI.parse(oembed_endpoint + (oembed_endpoint.match(/\?/) ? '&url=' : '?url=') + CGI.escape(oembed_url) + '&format=json')
+      URI.parse(oembed_endpoint + (oembed_endpoint.include?('?') ? '&url=' : '?url=') + CGI.escape(oembed_url) + '&format=json')
     end
 
     def uri_source

app/controllers/services_api_controller.rb

@@ -80,7 +80,7 @@ def start_kaltura_session
     client = CanvasKaltura::ClientV3.new
     uid = "#{@user.id}_#{@domain_root_account.id}"
     res = client.startSession(CanvasKaltura::SessionType::USER, uid)
-    raise "Kaltura session failed to generate" if res =~ /START_SESSION_ERROR/
+    raise "Kaltura session failed to generate" if res.include?('START_SESSION_ERROR')
 
     hash = {
       :ks => res,

app/helpers/attachment_helper.rb

@@ -54,7 +54,7 @@ def doc_preview_attributes(attachment, attrs = {})
   end
 
   def media_preview_attributes(attachment, attrs = {})
-    attrs[:type] = attachment.content_type.match(/video/) ? 'video' : 'audio'
+    attrs[:type] = attachment.content_type&.include?('video') ? 'video' : 'audio'
     attrs[:download_url] = context_url(attachment.context, :context_file_download_url, attachment.id)
     attrs[:media_entry_id] = attachment.media_entry_id if attachment.media_entry_id
     attrs.inject(+"") { |s, (attr, val)| s << "data-#{attr}=#{val} " }

app/helpers/calendar_events_helper.rb

@@ -36,7 +36,7 @@ def return_to_calendar(options = {})
     end
     # Use a explicit "return_to" option first, absent that, use calendar_url_for
     clean_return_to(
-      params[:return_to] && params[:return_to].match(/calendar/) && params[:return_to]
+      params[:return_to]&.include?('calendar') && params[:return_to]
     ) ||
       calendar_url_for(options[:context], cal_options)
   end

app/helpers/quizzes_helper.rb

@@ -68,9 +68,9 @@ def display_save_button?(quiz = @quiz)
 
   def render_number(num)
     # if the string representation of this number uses scientific notation,
-    return format('%g', num) if num.to_s =~ /e/ # short circuit if scientific notation
+    return format('%g', num) if num.to_s.include?('e') # short circuit if scientific notation
 
-    if num.to_s =~ /%/
+    if num.to_s.include?('%')
       I18n.n(round_if_whole(num.delete('%'))) + '%'
     else
       I18n.n(round_if_whole(num))

app/models/asset_user_access.rb

@@ -94,7 +94,7 @@ def context_code
   end
 
   def readable_name(include_group_name: true)
-    if self.asset_code && self.asset_code.match(/:/)
+    if self.asset_code&.include?(':')
       split = self.asset_code.split(/:/)
 
       if split[1].match(/course_\d+/)

app/models/assignment.rb

@@ -3071,8 +3071,7 @@ def non_digital_submission?
   end
 
   def allow_google_docs_submission?
-    self.submission_types &&
-      self.submission_types.match(/online_upload/)
+    submission_types&.include?('online_upload')
   end
 
   def <=>(comparable)

app/models/attachment.rb

@@ -238,7 +238,7 @@ def run_after_attachment_saved
     end
 
     # try an infer encoding if it would be useful to do so
-    delay.infer_encoding if self.encoding.nil? && self.content_type =~ /text/ && self.context_type != 'SisBatch'
+    delay.infer_encoding if self.encoding.nil? && self.content_type&.include?('text') && self.context_type != 'SisBatch'
     if respond_to?(:process_attachment, true)
       automatic_thumbnail_sizes.each do |suffix|
         delay_if_production(singleton: "attachment_thumbnail_#{global_id}_#{suffix}")
@@ -470,12 +470,12 @@ def after_extension
   end
 
   def assert_file_extension
-    self.content_type = nil if self.content_type && (self.content_type == 'application/x-unknown' || self.content_type.match(/ERROR/))
+    self.content_type = nil if content_type == 'application/x-unknown' || content_type&.include?('ERROR')
     self.content_type ||= self.mimetype(self.filename)
     if self.filename && self.filename.split(".").length < 2
       # we actually have better luck assuming zip files without extensions
       # are docx files than assuming they're zip files
-      self.content_type = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' if self.content_type.match(/zip/)
+      self.content_type = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' if content_type&.include?('zip')
       ext = self.extension
       self.write_attribute(:filename, self.filename + ext) unless ext == '.unknown'
     end

app/models/context_external_tool.rb

@@ -707,7 +707,7 @@ def self.standardize_url(url)
     return "" if url.blank?
 
     url = url.gsub(/[[:space:]]/, '')
-    url = "http://" + url unless url.match(/:\/\//)
+    url = "http://" + url unless url.include?('://')
     res = Addressable::URI.parse(url).normalize
     res.query = res.query.split(/&/).sort.join('&') if !res.query.blank?
     res.to_s

app/models/importers/link_parser.rb

@@ -63,8 +63,9 @@ def placeholder(old_value)
     def convert_link(node, attr, item_type, mig_id, field)
       return unless node[attr].present?
 
-      if attr == 'value'
-        return unless (node[attr] && node[attr] =~ %r{IMS(?:-|_)CC(?:-|_)FILEBASE}) || node[attr] =~ %r{CANVAS_COURSE_REFERENCE}
+      if attr == 'value' &&
+         !(node[attr] =~ %r{IMS(?:-|_)CC(?:-|_)FILEBASE} || node[attr].include?('CANVAS_COURSE_REFERENCE'))
+        return
       end
 
       url = node[attr].dup
@@ -144,13 +145,13 @@ def parse_url(url, node, attr)
 
       elsif url =~ %r{\$IMS(?:-|_)CC(?:-|_)FILEBASE\$/(.*)}
         rel_path = URI.unescape($1)
-        if (attr == 'href' && node['class'] && node['class'] =~ /instructure_inline_media_comment/) ||
+        if (attr == 'href' && node['class']&.include?('instructure_inline_media_comment')) ||
            (attr == 'src' && node.name == 'iframe' && node['data-media-id'])
           unresolved(:media_object, :rel_path => rel_path)
         else
           unresolved(:file, :rel_path => rel_path)
         end
-      elsif (attr == 'href' && node['class'] && node['class'] =~ /instructure_inline_media_comment/) ||
+      elsif (attr == 'href' && node['class']&.include?('instructure_inline_media_comment')) ||
             (attr == 'src' && node.name == 'iframe' && node['data-media-id'])
         # Course copy media reference, leave it alone
         resolved

app/models/media_object.rb

@@ -216,7 +216,7 @@ def process_retrieved_details(entry, media_type, assets)
       self.data[:plays] = entry[:plays].to_i
       self.data[:download_url] = entry[:downloadUrl]
       tags = (entry[:tags] || "").split(/,/).map(&:strip)
-      old_id = tags.detect { |t| t.match(/old_id_/) }
+      old_id = tags.detect { |t| t.include?('old_id_') }
       self.old_media_id = old_id.sub(/old_id_/, '') if old_id
     end
     self.data[:extensions] ||= {}

app/models/quizzes/quiz.rb

@@ -323,7 +323,7 @@ def lock_at=(val)
     val = val.in_time_zone.end_of_day if val.is_a?(Date)
     if val.is_a?(String)
       super(Time.zone.parse(val))
-      self.lock_at = CanvasTime.fancy_midnight(self.lock_at) unless val =~ /:/
+      self.lock_at = CanvasTime.fancy_midnight(self.lock_at) unless val.include?(':')
     else
       super(val)
     end
@@ -333,7 +333,7 @@ def due_at=(val)
     val = val.in_time_zone.end_of_day if val.is_a?(Date)
     if val.is_a?(String)
       super(Time.zone.parse(val))
-      infer_times unless val =~ /:/
+      infer_times unless val.include?(':')
     else
       super(val)
     end

app/models/quizzes/quiz_regrader/answer.rb

@@ -99,7 +99,7 @@ def regrade_and_merge_answer!
 
     fake_submission_data = if question_data[:question_type] == 'multiple_answers_question'
                              hash = {}
-                             answer.each { |k, v| hash["question_#{question_id}_#{k}"] = v if /answer/ =~ k.to_s }
+                             answer.each { |k, v| hash["question_#{question_id}_#{k}"] = v if k.to_s.include?('answer') }
                              answer.merge(hash)
                            else
                              answer.merge("question_#{question_id}" => answer[:text])

app/models/submission.rb

@@ -2072,7 +2072,7 @@ def quiz_submission_version
 
   def processed?
     if submission_type == "online_url"
-      return attachment && attachment.content_type.match(/image/)
+      return attachment&.content_type&.include?('image')
     end
 
     false

config/initializers/postgresql_adapter.rb

@@ -317,7 +317,7 @@ def create_icu_collations
     end
 
     I18n.available_locales.each do |locale|
-      next if locale =~ /-x-/
+      next if locale.to_s.include?('-x-')
 
       I18n.locale = locale
       next if Canvas::ICU.collator.rules.empty?

gems/attachment_fu/lib/attachment_fu/processors/mini_magick_processor.rb

@@ -70,7 +70,7 @@ def resize_image(img, size)
           commands.limit("disk", "1000MB") # because arbitrary numbers are arbitrary
 
           # gif are not handled correct, this is a hack, but it seems to work.
-          if img[:format] =~ /GIF/
+          if img[:format].include?('GIF')
             img.format("png")
           end
 

gems/canvas_cache/lib/canvas_cache/redis.rb

@@ -122,7 +122,7 @@ def self.log_style
     def self.redis_failure?(redis_name)
       return false unless last_redis_failure[redis_name]
       # i feel this dangling rescue is justifiable, given the try-to-be-failsafe nature of this code
-      if redis_name =~ /localhost/
+      if redis_name.include?('localhost')
         # talking to local redis should not short ciruit as long
         return (Time.zone.now - last_redis_failure[redis_name]) < (settings_store.get('redis_local_failure_time', '2').to_i rescue 2)
       end

gems/canvas_cache/spec/canvas_cache/redis_spec.rb

@@ -360,7 +360,7 @@
         end
         # we don't log the second message under spring, cause reasons; we only
         # care about the primary message anyway
-        msgs = messages.select { |m| m =~ /Query failure/ }
+        msgs = messages.select { |m| m.include?('Query failure') }
         expect(msgs.length).to eq(1)
         m = msgs.first
         expect(m).to match(/\[REDIS\] Query failure/)

gems/canvas_http/lib/canvas_http.rb

@@ -180,7 +180,7 @@ def self.validate_url(value, host: nil, scheme: nil, allowed_schemes: %w{http ht
     begin
       uri = URI.parse(value)
     rescue URI::InvalidURIError => e
-      if e.message =~ /URI must be ascii only/
+      if e.message.include?('URI must be ascii only')
         uri = URI.parse(Addressable::URI.normalized_encode(value).chomp("/"))
         value = uri.to_s
       else

gems/canvas_stringex/lib/lucky_sneaks/string_extensions.rb

@@ -35,12 +35,10 @@ def to_html(lite_mode = false)
       if defined?(RedCloth)
         if lite_mode
           RedCloth.new(self, [:lite_mode]).to_html
+        elsif self.include?('<pre>')
+          RedCloth.new(self).to_html.tr("\t", "")
         else
-          if self =~ /<pre>/
-            RedCloth.new(self).to_html.tr("\t", "")
-          else
-            RedCloth.new(self).to_html.tr("\t", "").gsub(/\n\n/, "")
-          end
+          RedCloth.new(self).to_html.tr("\t", "").gsub(/\n\n/, "")
         end
       else
         self
@@ -142,7 +140,7 @@ def convert_misc_characters
         /(\s|^)\$(\d+)\.(\d+)(\s|$)/ => '\2 dollars \3 cents',
         /(\s|^)£(\d+)\.(\d+)(\s|$)/u => '\2 pounds \3 pence',
       }.each do |found, replaced|
-        replaced = " #{replaced} " unless replaced =~ /\\1/
+        replaced = " #{replaced} " unless replaced.include?('\\1')
         dummy.gsub!(found, replaced)
       end
       # Back to normal rules
@@ -158,7 +156,7 @@ def convert_misc_characters
         /\s*%\s*/ => "percent",
         /\s*(\\|\/)\s*/ => "slash",
       }.each do |found, replaced|
-        replaced = " #{replaced} " unless replaced =~ /\\1/
+        replaced = " #{replaced} " unless replaced.include?('\\1')
         dummy.gsub!(found, replaced)
       end
       dummy = dummy.gsub(/(^|\w)'(\w|$)/, '\1\2').gsub(/[.,:;()\[\]\/?!\^'"_]/, " ")

gems/html_text_helper/lib/html_text_helper.rb

@@ -304,7 +304,7 @@ def format_message(message, opts = { :url => nil, :notification_id => nil })
   def add_notification_to_link(url, notification_id)
     parts = url.to_s.split("#", 2)
     link = parts[0]
-    link += link.match(/\?/) ? "&" : "?"
+    link += link.include?('?') ? "&" : "?"
     link += "clear_notification_id=#{notification_id}"
     link += parts[1] if parts[1]
     link

gems/i18n_extraction/lib/i18n_extraction/i18nliner_extensions.rb

@@ -190,7 +190,7 @@ def scope_for(filename)
       remove_whitespace = true
       scope = case filename
               when /app\/messages\//
-                remove_whitespace = false unless filename =~ /html/
+                remove_whitespace = false unless filename.include?('html')
                 filename.gsub(/.*app\/|\.erb/, '').gsub(/\/_?/, '.')
               when /app\/views\//
                 filename.gsub(/.*app\/views\/|\.(html\.|fbml\.)?erb\z/, '').gsub(/\/_?/, '.')

gems/i18n_tasks/lib/i18n_tasks/i18n_import.rb

@@ -144,7 +144,7 @@ def markdown_and_wrappers(str)
                 .concat(scan_and_report(dashed_str, /(!?\[)[^\]]+\]\(([^)"']+).*?\)/).map { |m| "link:#{m.last}" }) # links
 
       # only do fancy markdown checks on multi-line strings
-      if dashed_str =~ /\n/
+      if dashed_str.include?("\n")
         matches.concat(scan_and_report(dashed_str, /^(\#{1,6})\s+[^#]*#*$/).map { |m| "h#{m.first.size}" }) # headings
                .concat(scan_and_report(dashed_str, /^[^=\-\n]+\n^(=+|-+)$/).map { |m| m.first[0] == '=' ? 'h1' : 'h2' }) # moar headings
                .concat(scan_and_report(dashed_str, /^((\s*\*\s*){3,}|(\s*-\s*){3,}|(\s*_\s*){3,})$/).map { "hr" })

gems/i18n_tasks/lib/tasks/i18n.rake

@@ -414,7 +414,7 @@ namespace :i18n do
     import = I18nTasks::I18nImport.new(source_translations, new_translations)
 
     complete_translations = import.compile_complete_translations do |error_items, description|
-      if description =~ /mismatches/
+      if description.include?('mismatches')
         # Output malformed stuff and don't import them
         errors.concat error_items
         :discard

gems/rubocop-canvas/spec/rubocop/cop/specs/no_before_once_stubs_spec.rb

@@ -62,7 +62,7 @@
         end
       })
       expect(cop.offenses.size).to eq(5)
-      expect(cop.messages.all? { |msg| msg =~ /Use `before\(:once\)`/ })
+      expect(cop.messages.all? { |msg| msg.include?('Use `before(:once)`') })
       expect(cop.offenses.all? { |off| off.severity.name == :warning })
     end
   end

gems/tatl_tael/lib/tatl_tael/linters.rb

@@ -8,7 +8,7 @@ class BaseLinter
       class << self
         def inherited(subclass)
           super
-          Linters.linters << subclass unless subclass.name =~ /SimpleLinter/
+          Linters.linters << subclass unless subclass.name&.include?('SimpleLinter')
         end
       end
 

lib/api.rb

@@ -434,7 +434,7 @@ def self.build_links_from_hash(links)
   end
 
   def self.build_links_hash(base_url, opts = {})
-    base_url += (base_url =~ /\?/ ? '&' : '?')
+    base_url += (base_url.include?('?') ? '&' : '?')
     qp = opts[:query_parameters] || {}
     qp = qp.with_indifferent_access.except(*EXCLUDE_IN_PAGINATION_LINKS)
     base_url += "#{qp.to_query}&" if qp.present?
@@ -652,7 +652,7 @@ def self.api_type_to_canvas_name(api_type)
   end
 
   def accepts_jsonapi?
-    !!(/application\/vnd\.api\+json/ =~ request.headers['Accept'].to_s)
+    !!request.headers['Accept'].to_s.include?('application/vnd.api+json')
   end
 
   # Return a template url that follows the root links key for the jsonapi.org

lib/api/v1/submission.rb

@@ -227,7 +227,7 @@ def submission_attempt_json(attempt, assignment, user, session, context = nil, p
 
     # include the discussion topic entries
     if other_fields.include?('discussion_entries') &&
-       assignment.submission_types =~ /discussion_topic/ &&
+       assignment.submission_types&.include?('discussion_topic') &&
        assignment.discussion_topic
       # group assignments will have a child topic for each group.
       # it's also possible the student posted in the main topic, as well as the