don't include unnecessary modules into Object, fixes SD-2013

also add a cop to prevent it in the future

Change-Id: Ic949cc4448f546736aa3aeee17fc1fc33a1f2808
Reviewed-on: https://gerrit.instructure.com/99847
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>

gems/canvas_stringex/spec/lucky_sneaks/unicode_point_suite/basic_latin_spec.rb

@@ -1,9 +1,9 @@
 require "spec_helper"
 
 require_relative "codepoint_test_helper"
-include CodepointTestHelper
 
 describe "BasicLatin" do
+  include CodepointTestHelper
   # This test suite is just regression test and debugging
   # to better transliterate the Basic Latin Unicode codepoints
   # 
@@ -121,4 +121,4 @@ describe "BasicLatin" do
       assert_equal_encoded expected, encode_mes
     end
   end
-end
+end

gems/rubocop-canvas/config/default.yml

@@ -114,3 +114,9 @@ Specs/DeterministicDescribedClasses:
   Include:
     - "**/spec/**/*.rb"
     - "**/spec_canvas/**/*.rb"
+
+Specs/ScopeIncludes:
+  Enabled: true
+  Include:
+    - "**/spec/**/*.rb"
+    - "**/spec_canvas/**/*.rb"

gems/rubocop-canvas/lib/rubocop_canvas.rb

@@ -33,6 +33,7 @@ require 'rubocop_canvas/cops/specs/no_strftime'
 require 'rubocop_canvas/cops/specs/prefer_f_over_fj'
 require 'rubocop_canvas/cops/specs/scope_helper_modules'
 require 'rubocop_canvas/cops/specs/deterministic_described_classes'
+require 'rubocop_canvas/cops/specs/scope_includes'
 
 module RuboCop
   module Canvas

gems/rubocop-canvas/lib/rubocop_canvas/cops/specs/scope_includes.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+module RuboCop
+  module Cop
+    module Specs
+      class ScopeIncludes < Cop
+        MSG = "Never `include` a module at the top-level. Otherwise its "\
+              "methods will be added to `Object` (and thus everything), "\
+              "causing all sorts of mayhem. Move this inside a `describe`, "\
+              "`shared_context`, etc."
+
+        WHITELISTED_BLOCKS = %i[
+          class_eval
+          context
+          describe
+          shared_context
+          shared_examples
+          shared_examples_for
+          new
+        ].freeze
+
+        def on_send(node)
+          receiver, method_name, *_args = *node
+          return unless receiver.nil?
+          return unless method_name == :include
+          return if whitelisted_ancestor?(node)
+
+          add_offense node, :expression, MSG, :error
+        end
+
+        private
+
+        def whitelisted_ancestor?(node)
+          node.ancestors.any? do |ancestor|
+            ancestor.module_type? ||
+            ancestor.class_type? ||
+            ancestor.def_type? ||
+            ancestor.block_type? &&
+              WHITELISTED_BLOCKS.include?(ancestor.method_name)
+          end
+        end
+      end
+    end
+  end
+end

gems/rubocop-canvas/rubocop-canvas.gemspec

@@ -14,7 +14,7 @@ Gem::Specification.new do |spec|
   spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ["lib"]
 
-  spec.add_runtime_dependency "rubocop", "~> 0.40"
+  spec.add_runtime_dependency "rubocop", "0.46"
   spec.add_development_dependency "bundler", "~> 1.7"
   spec.add_development_dependency "pry", "~> 0.10.1"
   spec.add_development_dependency "pry-nav", "~> 0.2.4"

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

@@ -0,0 +1,34 @@
+describe RuboCop::Cop::Specs::ScopeIncludes do
+  subject(:cop) { described_class.new }
+
+  context "within describe" do
+    it 'allows includes' do
+      inspect_source(cop, %{
+        describe JumpStick do
+          include Foo
+        end
+      })
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  context "within module" do
+    it 'allows includes' do
+      inspect_source(cop, %{
+        module JumpStick
+          include Foo
+        end
+      })
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  it "disallows defs on Object" do
+    inspect_source(cop, %{
+      include Foo
+    })
+    expect(cop.offenses.size).to eq(1)
+    expect(cop.messages.first).to match(/Never `include`/)
+    expect(cop.offenses.first.severity.name).to eq(:error)
+  end
+end

spec/apis/v1/collaboration_json_spec.rb

@@ -18,9 +18,9 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper')
 
-include Api::V1::Collaboration
-
 describe Api::V1::Collaboration do
+  include Api::V1::Collaboration
+
   before(:once) do
     @current_user  = user_with_pseudonym(:active_all => true)
     course = course_with_teacher(user: @current_user).course

spec/apis/v1/conferences_api_spec.rb

@@ -18,11 +18,11 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper')
 
-include Api::V1::Conferences
-include Api::V1::Json
-include Api
-
 describe "Conferences API", type: :request do
+  include Api::V1::Conferences
+  include Api::V1::Json
+  include Api
+
   before :once do
     # these specs need an enabled web conference plugin
     @plugin = PluginSetting.create!(name: 'wimba')

spec/selenium/announcements/announcement_permission_spec.rb

@@ -20,11 +20,12 @@ require_relative '../common'
 require_relative '../announcements/announcement_helpers'
 require_relative '../discussions/discussion_helpers'
 require_relative '../helpers/shared_examples_common'
-include DiscussionHelpers
-include AnnouncementHelpers
-include SharedExamplesCommon
 
 describe 'announcement permissions' do
+  include DiscussionHelpers
+  include AnnouncementHelpers
+  include SharedExamplesCommon
+
   include_context "in-process server selenium tests"
   include_context "announcements_page_shared_context"
   include_context "discussions_page_shared_context"

spec/selenium/calendar/calendar2_new_scheduler_teacher_spec.rb

@@ -1,8 +1,8 @@
 require_relative '../common'
 require_relative '../helpers/calendar2_common'
-include Calendar2Common
 
 describe "scheduler" do
+  include Calendar2Common
   include_context "in-process server selenium tests"
   include Calendar2Common
 

spec/selenium/conversations/conversations_submission_comments_spec.rb

@@ -1,9 +1,9 @@
 require File.expand_path(File.dirname(__FILE__) + '/../helpers/conversations_common')
 require_relative '../helpers/shared_examples_common'
-include SharedExamplesCommon
 
 describe "conversations new" do
   include_context "in-process server selenium tests"
+  include SharedExamplesCommon
   include ConversationsCommon
 
   before do

spec/selenium/dashboard_spec.rb

@@ -1,9 +1,8 @@
 require_relative 'common'
 require_relative 'helpers/notifications_common'
-include NotificationsCommon
-
 
 describe "dashboard" do
+  include NotificationsCommon
   include_context "in-process server selenium tests"
 
   shared_examples_for 'load events list' do
@@ -156,7 +155,7 @@ describe "dashboard" do
     end
 
     it "shows an assignment stream item under Recent Activity in dashboard", priority: "1", test_id: 108725 do
-      NotificationsCommon.setup_notification(@student, name: 'Assignment Created')
+      setup_notification(@student, name: 'Assignment Created')
       assignment_model({:submission_types => ['online_text_entry'], :course => @course})
       get "/"
       f('#dashboardToggleButton').click

spec/selenium/dashboard_teacher_spec.rb

@@ -1,9 +1,8 @@
 require_relative 'common'
 require_relative 'helpers/notifications_common'
-include NotificationsCommon
-
 
 describe "dashboard" do
+  include NotificationsCommon
   include_context "in-process server selenium tests"
 
   context "as a teacher" do
@@ -76,7 +75,7 @@ describe "dashboard" do
 
     context 'stream items' do
       before :once do
-        NotificationsCommon.setup_notification(@teacher, name: 'Assignment Created')
+        setup_notification(@teacher, name: 'Assignment Created')
       end
 
       it 'shows an assignment stream item under Recent Activity in dashboard', priority: "1", test_id: 108723 do

spec/selenium/discussions/discussion_permission_spec.rb

@@ -19,11 +19,10 @@
 require_relative '../common'
 require_relative '../discussions/discussion_helpers'
 require_relative '../helpers/shared_examples_common'
-include SharedExamplesCommon
-include DiscussionHelpers
-
 
 describe "discussion permissions" do
+  include SharedExamplesCommon
+  include DiscussionHelpers
   include_context "in-process server selenium tests"
   include_context "discussions_page_shared_context"
   extend DiscussionHelpers::SetupContext

spec/selenium/grades/student_grades_page/gradebook_student_common.rb

@@ -1,8 +1,9 @@
 require_relative '../../common'
 require_relative '../../helpers/shared_examples_common'
-include SharedExamplesCommon
 
 shared_examples 'Arrange By dropdown' do |context|
+  include SharedExamplesCommon
+
   before :each do
     enroll_context(context)
     get "/courses/#{@course.id}/grades/#{@student.id}"

spec/selenium/helpers/basic/settings_specs.rb

@@ -1,7 +1,7 @@
 require_relative '../shared_examples_common'
-include SharedExamplesCommon
 
 shared_examples_for "settings basic tests" do |account_type|
+  include SharedExamplesCommon
   include_context "in-process server selenium tests"
 
   before(:each) do

spec/selenium/helpers/groups_common.rb

@@ -1,14 +1,19 @@
 module GroupsCommon
+  def self.included(mod)
+    mod.singleton_class.include(ClassMethods)
+  end
 
-  def setup_group_page_urls
-    let(:url) {"/groups/#{@testgroup.first.id}"}
-    let(:announcements_page) {url + '/announcements'}
-    let(:people_page) {url + '/users'}
-    let(:discussions_page) {url + '/discussion_topics'}
-    let(:pages_page) {url + '/pages'}
-    let(:files_page) {url + '/files'}
-    let(:conferences_page) {url + '/conferences'}
-    let(:collaborations_page) {url + '/collaborations'}
+  module ClassMethods
+    def setup_group_page_urls
+      let(:url) {"/groups/#{@testgroup.first.id}"}
+      let(:announcements_page) {url + '/announcements'}
+      let(:people_page) {url + '/users'}
+      let(:discussions_page) {url + '/discussion_topics'}
+      let(:pages_page) {url + '/pages'}
+      let(:files_page) {url + '/files'}
+      let(:conferences_page) {url + '/conferences'}
+      let(:collaborations_page) {url + '/collaborations'}
+    end
   end
 
   def seed_students(count, base_name = 'Test Student')

spec/selenium/helpers/groups_shared_examples.rb

@@ -1,13 +1,14 @@
 require_relative '../common'
 require_relative 'groups_common'
 require_relative 'shared_examples_common'
-include GroupsCommon
-include SharedExamplesCommon
 
 # ======================================================================================================================
 # Shared Examples
 # ======================================================================================================================
 shared_examples 'home_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should display a coming up section with relevant events", priority: pick_priority(context, student: "1", teacher: "2"), test_id: pick_test_id(context, student: 273602, teacher: 319909) do
     # Create an event to have something in the Coming up Section
     event = @testgroup[0].calendar_events.create!(title: "ohai",
@@ -54,6 +55,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'announcements_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should center the add announcement button if no announcements are present", priority: pick_priority(context, student: "1", teacher: "2"), test_id: pick_test_id(context, student: 273606, teacher: 324936) do
     get announcements_page
     expect(f('#content div')).to have_attribute(:style, 'text-align: center;')
@@ -105,6 +109,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'pages_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should load pages index and display all pages", priority: pick_priority(context, student: "1", teacher: "2"), test_id: pick_test_id(context, student: 273610, teacher: 324927) do
     @testgroup.first.wiki.wiki_pages.create!(title: "Page 1", user: @teacher)
     @testgroup.first.wiki.wiki_pages.create!(title: "Page 2", user: @teacher)
@@ -139,6 +146,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'people_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should allow group users to see group registered services page", priority: pick_priority(context, student: "1", teacher: "2"),test_id: pick_test_id(context, student: 323329, teacher: 324926) do
     get people_page
     expect_new_page_load do
@@ -152,6 +162,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'discussions_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should only list in-group discussions in the content right pane", priority: pick_priority(context, student: "1", teacher: "2"), test_id: pick_test_id(context, student: 273622, teacher: 324930) do
     # create group and course announcements
     group_dt = DiscussionTopic.create!(context: @testgroup.first, user: @teacher,
@@ -191,6 +204,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'files_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should allow group users to rename a file", priority: "2", test_id: pick_test_id(context, student: 312869, teacher: 315577) do
     add_test_files
     get files_page
@@ -213,6 +229,9 @@ end
 
 #-----------------------------------------------------------------------------------------------------------------------
 shared_examples 'conferences_page' do |context|
+  include GroupsCommon
+  include SharedExamplesCommon
+
   it "should allow group users to create a conference", priority: pick_priority(context, student: "1", teacher: "2"),test_id: pick_test_id(context, student: 307624, teacher: 308534) do
     skip_if_chrome('issue with invite_all_but_one_user method')
     title = 'test conference'

spec/selenium/helpers/shared_examples_common.rb

@@ -1,36 +1,41 @@
 module SharedExamplesCommon
-
-  # For use with choosing TestRail test id in shared specs based on context.
-  #
-  # usage:
-  #   pick_test_id(context, option1: <id1>, option2: <id2> [, option3: <id3])
-  #   
-  # example:
-  #   it "should manually change a course quota", priority: "1", 
-  #     test_id: pick_test_id(account_type, sub_account: 250004, root_account: 251034) do 
-  #   
-  def pick_test_id(context, opts = {})
-    if opts.empty? || !opts.key?(context)
-      raise("Error: Invalid context for test id")
-    end
-    opts[context]
+  def self.included(mod)
+    mod.singleton_class.include(ClassMethods)
   end
 
-  # For use with choosing example priority in shared specs based on context.
-  #
-  # usage:
-  #   pick_priority(context, option1: <id1>, option2: <id2> [, option3: <id3])
-  #   
-  # example:
-  #   it 'should persist', 
-  #     test_id: pick_test_id(context, student: "591860", teacher: "592108", 
-  #                                    admin: "592119", ta: "592130"), 
-  #     priority: pick_priority(context, student: "1", teacher: "1", admin: "2", ta: "2") do
-  #
-  def pick_priority(context, opts ={})
-    if opts.empty? || !opts.key?(context)
-      raise("Error: Invalid context for test id")
+  module ClassMethods
+    # For use with choosing TestRail test id in shared specs based on context.
+    #
+    # usage:
+    #   pick_test_id(context, option1: <id1>, option2: <id2> [, option3: <id3])
+    #
+    # example:
+    #   it "should manually change a course quota", priority: "1",
+    #     test_id: pick_test_id(account_type, sub_account: 250004, root_account: 251034) do
+    #
+    def pick_test_id(context, opts = {})
+      if opts.empty? || !opts.key?(context)
+        raise("Error: Invalid context for test id")
+      end
+      opts[context]
+    end
+
+    # For use with choosing example priority in shared specs based on context.
+    #
+    # usage:
+    #   pick_priority(context, option1: <id1>, option2: <id2> [, option3: <id3])
+    #
+    # example:
+    #   it 'should persist',
+    #     test_id: pick_test_id(context, student: "591860", teacher: "592108",
+    #                                    admin: "592119", ta: "592130"),
+    #     priority: pick_priority(context, student: "1", teacher: "1", admin: "2", ta: "2") do
+    #
+    def pick_priority(context, opts ={})
+      if opts.empty? || !opts.key?(context)
+        raise("Error: Invalid context for test id")
+      end
+      opts[context]
     end
-    opts[context]
   end
 end

spec/selenium/notifications_spec.rb

@@ -1,16 +1,16 @@
 require File.expand_path(File.dirname(__FILE__) + '/common')
 require File.expand_path(File.dirname(__FILE__) + '/helpers/notifications_common')
-include NotificationsCommon
 require File.expand_path(File.dirname(__FILE__) + '/helpers/calendar2_common')
 
 describe "Notifications" do
+  include NotificationsCommon
   include_context "in-process server selenium tests"
   include Calendar2Common
 
   context "admin" do
     before :once do
       course_with_student(active_all: true)
-      NotificationsCommon.setup_comm_channel(@student, 'student@example.com')
+      setup_comm_channel(@student, 'student@example.com')
       @teacher = user_with_pseudonym(username: 'teacher@example.com', active_all: 1)
       enrollment = teacher_in_course(course: @course, user: @teacher)
       enrollment.accept!

spec/selenium/offline_contents_common.rb

@@ -1,12 +1,12 @@
 require_relative 'common'
 require_relative 'helpers/shared_examples_common'
-include SharedExamplesCommon
 
 # ======================================================================================================================
 # Shared Examples
 # ======================================================================================================================
 
 shared_examples 'show courses for ePub generation' do |context|
+  include SharedExamplesCommon
   
   it "should show the courses the user is enrolled in and feature enabled in ePub exports page",
                             priority: "1", test_id: pick_test_id(context, teacher: "417579", student: "498316") do
@@ -39,6 +39,8 @@ shared_examples 'show courses for ePub generation' do |context|
 end
 
 shared_examples 'generate and download ePub' do |context|
+  include SharedExamplesCommon
+
   it "should show progress", priority: "1", test_id: pick_test_id(context, teacher: "417580", student: "498317") do
     get '/epub_exports'
     f('.ig-admin .Button').click

spec/selenium/profile_common.rb

@@ -1,12 +1,13 @@
 require_relative 'common'
 require_relative 'helpers/shared_examples_common'
-include SharedExamplesCommon
 
 # ======================================================================================================================
 # Shared Examples
 # ======================================================================================================================
 
 shared_examples 'profile_settings_page' do |context|
+  include SharedExamplesCommon
+
   it 'should give option to change profile pic', priority: "2", test_id: pick_test_id(context, student: 68936, teacher: 352617, admin: 352618) do
     enable_avatars
     get "/profile/settings"
@@ -23,6 +24,8 @@ end
 
 
 shared_examples 'profile_user_about_page' do |context|
+  include SharedExamplesCommon
+
   it 'should give option to change profile pic', priority: "2", test_id: pick_test_id(context, student: 358573, teacher: 358574, admin: 358575) do
     enable_avatars
     get "/about/#{@user.id}"
@@ -37,6 +40,8 @@ shared_examples 'profile_user_about_page' do |context|
 end
 
 shared_examples 'user settings page change pic window' do |context|
+  include SharedExamplesCommon
+
   it 'should allow user to click to change profile pic', priority: "1", test_id: pick_test_id(context, student: 68938, teacher: 368784, admin: 368785) do
     enable_avatars
     get '/profile/settings'
@@ -66,6 +71,8 @@ shared_examples 'user settings page change pic window' do |context|
 end
 
 shared_examples 'user settings change pic cancel' do |context|
+  include SharedExamplesCommon
+
   it 'closes window when cancel button is pressed', priority: "1", test_id: pick_test_id(context, student: 68939, teacher: 372132, admin: 372133) do
     enable_avatars
     get '/profile/settings'