DEV: Add rubocop-rspec (#9288)

This adds rubocop-rspec, and enables some cops that were either already passing or are passing now, after fixing them in this commit.

Some new cops are disabled for now, with annotation: "TODO" or "To be decided". Those either need to be discussed first, or require manual changes, or the number of found and fixed offenses is too large to bundle them up in a single PR.

Includes:

* DEV: Update rubocop's `TargetRubyVersion` to 2.6
* DEV: Enable RSpec/VoidExpect
* DEV: Enable RSpec/SharedContext
* DEV: Enable RSpec/EmptyExampleGroup (Removed an obsolete empty spec file)
* DEV: Enable RSpec/ItBehavesLike
* DEV: Remove RSpec/ScatteredLet (It's too strict, as it doesn't recognize fab! as a let-like)
* DEV: Remove RSpec/MultipleExpectations

.rubocop.yml

@@ -1,8 +1,9 @@
 require:
   - rubocop-discourse
+  - rubocop-rspec
 
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 2.6
   DisabledByDefault: true
   Exclude:
     - "db/schema.rb"
@@ -143,3 +144,212 @@ Style/GlobalVars:
     - 'script/**/*'
     - 'spec/**/*.rb'
     - 'plugins/*/spec/**/*'
+
+# Specs
+
+RSpec/AnyInstance:
+  Enabled: false # To be decided
+
+RSpec/AroundBlock:
+  Enabled: true
+
+RSpec/BeforeAfterAll:
+  Enabled: false # To be decided
+
+RSpec/ContextMethod:
+  Enabled: false # TODO
+
+RSpec/ContextWording:
+  Enabled: false # To be decided
+
+RSpec/DescribeClass:
+  Enabled: false # To be decided
+
+RSpec/DescribeMethod:
+  Enabled: true
+
+RSpec/DescribeSymbol:
+  Enabled: false # To be decided
+
+RSpec/DescribedClass:
+  Enabled: false # To be decided
+
+RSpec/DescribedClassModuleWrapping:
+  Enabled: false # To be decided
+
+RSpec/EmptyExampleGroup:
+  Enabled: true
+
+RSpec/EmptyLineAfterExample:
+  Enabled: false # TODO
+
+RSpec/EmptyLineAfterExampleGroup:
+  Enabled: false # TODO
+
+RSpec/EmptyLineAfterFinalLet:
+  Enabled: false # TODO
+
+RSpec/EmptyLineAfterHook:
+  Enabled: false # TODO
+
+RSpec/EmptyLineAfterSubject:
+  Enabled: false # TODO
+
+RSpec/ExampleLength:
+  Enabled: false # To be decided
+
+RSpec/ExampleWithoutDescription:
+  Enabled: true
+
+RSpec/ExampleWording:
+  Enabled: false # TODO
+
+RSpec/ExpectActual:
+  Enabled: true
+
+RSpec/ExpectChange:
+  Enabled: false # To be decided
+
+RSpec/ExpectInHook:
+  Enabled: false # To be decided
+
+RSpec/ExpectOutput:
+  Enabled: true
+
+RSpec/FilePath:
+  Enabled: false # To be decided
+
+RSpec/Focus:
+  Enabled: true
+
+RSpec/HookArgument:
+  Enabled: false # TODO
+
+RSpec/HooksBeforeExamples:
+  Enabled: false # TODO
+
+RSpec/ImplicitBlockExpectation:
+  Enabled: true
+
+RSpec/ImplicitExpect:
+  Enabled: false # To be decided
+
+RSpec/ImplicitSubject:
+  Enabled: false # To be decided
+
+RSpec/InstanceSpy:
+  Enabled: true
+
+RSpec/InstanceVariable:
+  Enabled: false # TODO
+
+RSpec/InvalidPredicateMatcher:
+  Enabled: true
+
+RSpec/ItBehavesLike:
+  Enabled: true
+
+RSpec/IteratedExpectation:
+  Enabled: false # To be decided
+
+RSpec/LeadingSubject:
+  Enabled: false # TODO
+
+RSpec/LeakyConstantDeclaration:
+  Enabled: false # To be decided
+
+RSpec/LetBeforeExamples:
+  Enabled: false # TODO
+
+RSpec/LetSetup:
+  Enabled: false # TODO
+
+RSpec/MessageChain:
+  Enabled: true
+
+RSpec/MessageSpies:
+  Enabled: true
+
+RSpec/MissingExampleGroupArgument:
+  Enabled: true
+
+RSpec/MultipleDescribes:
+  Enabled: false # TODO
+
+RSpec/MultipleSubjects:
+  Enabled: true
+
+RSpec/NamedSubject:
+  Enabled: false # To be decided
+
+RSpec/NestedGroups:
+  Enabled: false # To be decided
+
+RSpec/OverwritingSetup:
+  Enabled: true
+
+RSpec/ReceiveCounts:
+  Enabled: true
+
+RSpec/ReceiveNever:
+  Enabled: true
+
+RSpec/RepeatedDescription:
+  Enabled: false # TODO
+
+RSpec/RepeatedExample:
+  Enabled: false # TODO
+
+RSpec/RepeatedExampleGroupBody:
+  Enabled: false # TODO
+
+RSpec/RepeatedExampleGroupDescription:
+  Enabled: false # TODO
+
+RSpec/ReturnFromStub:
+  Enabled: true
+
+RSpec/ScatteredSetup:
+  Enabled: false # TODO
+
+RSpec/SharedContext:
+  Enabled: true
+
+RSpec/SharedExamples:
+  Enabled: true
+
+RSpec/SingleArgumentMessageChain:
+  Enabled: true
+
+RSpec/SubjectStub:
+  Enabled: true
+
+RSpec/UnspecifiedException:
+  Enabled: true
+
+RSpec/VerifiedDoubles:
+  Enabled: true
+
+RSpec/VoidExpect:
+  Enabled: true
+
+RSpec/Yield:
+  Enabled: true
+
+Capybara/CurrentPathExpectation:
+  Enabled: true
+
+Capybara/FeatureMethods:
+  Enabled: true
+
+FactoryBot/AttributeDefinedStatically:
+  Enabled: true
+
+FactoryBot/CreateList:
+  Enabled: true
+
+FactoryBot/FactoryClassName:
+  Enabled: true
+
+Rails/HttpStatus:
+  Enabled: true

Gemfile

@@ -180,6 +180,7 @@ group :test, :development do
   gem 'byebug', require: ENV['RM_INFO'].nil?, platform: :mri
   gem 'rubocop', require: false
   gem "rubocop-discourse", require: false
+  gem "rubocop-rspec", require: false
   gem 'parallel_tests'
 end
 

Gemfile.lock

@@ -350,6 +350,8 @@ GEM
       unicode-display_width (>= 1.4.0, < 1.7)
     rubocop-discourse (2.0.1)
       rubocop (>= 0.69.0)
+    rubocop-rspec (1.38.1)
+      rubocop (>= 0.68.1)
     ruby-prof (1.3.1)
     ruby-progressbar (1.10.1)
     ruby-readability (0.7.0)
@@ -527,6 +529,7 @@ DEPENDENCIES
   rtlit
   rubocop
   rubocop-discourse
+  rubocop-rspec
   ruby-prof
   ruby-readability
   rubyzip

spec/lib/backup_restore/restorer_spec.rb

@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-describe BackupRestore::Restorer do
-
-end

spec/lib/seed_data/categories_spec.rb

@@ -33,7 +33,7 @@ describe SeedData::Categories do
       expect(category.user_id).to eq(Discourse::SYSTEM_USER_ID)
       expect(category.category_groups.count).to eq(1)
       expect(category.category_groups.first).to have_attributes(permissions(:staff, :full))
-      expect(Topic.exists?(category.topic_id))
+      expect(Topic.exists?(category.topic_id)).to eq(true)
       expect(description_post(category).raw).to eq(I18n.t("staff_category_description"))
       expect(SiteSetting.staff_category_id).to eq(category.id)
     end

spec/lib/topic_upload_security_manager_spec.rb

@@ -120,7 +120,7 @@ describe TopicUploadSecurityManager do
         end
 
         it "changes the upload secure status to true and changes the ACL and rebakes the post and sets the access control post" do
-          expect(Post.any_instance.expects(:rebake!).once)
+          Post.any_instance.expects(:rebake!).once
           subject.run
           expect(upload3.reload.secure?).to eq(true)
           expect(upload3.reload.access_control_post).to eq(post4)
@@ -146,7 +146,7 @@ describe TopicUploadSecurityManager do
         end
 
         it "does not change the upload secure status and does not set the access control post" do
-          expect(Post.any_instance.expects(:rebake!).never)
+          Post.any_instance.expects(:rebake!).never
           subject.run
           expect(upload3.reload.secure?).to eq(false)
           expect(upload3.reload.access_control_post).to eq(nil)
@@ -174,14 +174,14 @@ describe TopicUploadSecurityManager do
   end
 
   def expect_upload_status_not_to_change
-    expect(Post.any_instance.expects(:rebake!).never)
+    Post.any_instance.expects(:rebake!).never
     subject.run
     expect(upload.reload.secure?).to eq(true)
     expect(upload2.reload.secure?).to eq(true)
   end
 
   def expect_upload_status_to_change_and_rebake
-    expect(Post.any_instance.expects(:rebake!).twice)
+    Post.any_instance.expects(:rebake!).twice
     subject.run
     expect(upload.reload.secure?).to eq(false)
     expect(upload2.reload.secure?).to eq(false)

spec/mailers/user_notifications_spec.rb

@@ -968,7 +968,7 @@ describe UserNotifications do
   end
 
   # notification emails derived from templates are translated into the user's locale
-  shared_examples "notification derived from template" do
+  shared_context "notification derived from template" do
     let(:user) { Fabricate(:user, locale: locale) }
     let(:mail_type) { mail_type }
     let(:notification) { Fabricate(:notification, user: user) }

spec/models/site_setting_spec.rb

@@ -178,7 +178,7 @@ describe SiteSetting do
         end.to change { @fake_logger.warnings.count }.by(2)
 
         expect do
-          expect(SiteSetting.use_https(warn: false))
+          SiteSetting.use_https(warn: false)
         end.to_not change { @fake_logger.warnings }
 
         SiteSetting.use_https = false

spec/models/topic_spec.rb

@@ -1122,7 +1122,7 @@ describe Topic do
 
     context 'closed' do
       let(:status) { 'closed' }
-      it_should_behave_like 'a status that closes a topic'
+      it_behaves_like 'a status that closes a topic'
 
       it 'should archive group message' do
         group = Fabricate(:group)
@@ -1135,7 +1135,7 @@ describe Topic do
 
     context 'autoclosed' do
       let(:status) { 'autoclosed' }
-      it_should_behave_like 'a status that closes a topic'
+      it_behaves_like 'a status that closes a topic'
 
       context 'topic was set to close when it was created' do
         it 'includes the autoclose duration in the moderator post' do