Remove remaining RuboCop TODOs

Closes #1202

We have fixed the bulk of the RuboCop TODOs. The main TODO remaining
(line length) involves a large number of files,
and I don't think it is worth trying to update them all at once. I did
fix a few of the worst offenders so we could bring the max down a bit.
We can gradually bring this number down as we fix more of the
violations.

.rubocop.yml

@@ -1,47 +1,26 @@
 inherit_from:
   - https://raw.githubusercontent.com/thoughtbot/guides/master/style/ruby/.rubocop.yml
+
 AllCops:
   TargetRubyVersion: 2.3
   Exclude:
     - 'gemfiles/*'
     - 'tmp/**/*'
 
-# TODO:
-# This configuration was generated by
-# `rubocop --auto-gen-config`
-# on 2018-09-18 22:44:09 -0400 using RuboCop version 0.54.0.
-# The point is for the user to remove these configuration records
-# one by one as the offenses are removed from the code base.
-# Note that changes in the inspected code, or installation of new
-# versions of RuboCop, may require this file to be generated again.
-
-# Offense count: 1
-# Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 26
+  CountComments: true
-
+  Max: 25
-# Offense count: 196
+  ExcludedMethods: []
-# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
-# URISchemes: http, https
-Metrics/LineLength:
-  Max: 189
-
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, IgnoredMethods.
-# SupportedStyles: line_count_based, semantic, braces_for_chaining
-# ProceduralMethods: benchmark, bm, bmbm, create, each_with_object, measure, new, realtime, tap, with_object
-# FunctionalMethods: let, let!, subject, watch
-# IgnoredMethods: lambda, proc, it
-Style/BlockDelimiters:
   Exclude:
-    - 'spec/**/*'
+    - "spec/**/*"
+    - '*.gemspec'
+
+Style/SymbolArray:
+  EnforcedStyle: brackets
 
-# Configuration parameters: .
-# SupportedStyles: annotated, template, unannotated
 Style/FormatStringToken:
-  EnforcedStyle: unannotated
+  Enabled: false
 
-# Offense count: 4
 Style/MethodMissing:
   Exclude:
     - 'lib/factory_bot/decorator.rb'
@@ -49,8 +28,8 @@ Style/MethodMissing:
     - 'lib/factory_bot/definition_proxy.rb'
     - 'lib/factory_bot/evaluator.rb'
 
-# Cop supports --auto-correct.
+# TODO: gradually bring this down to 80 as we fix files
-# Configuration parameters: MinSize.
+# Let's not open a big PR to fix all of these at once - 
-# SupportedStyles: percent, brackets
+# we can fix gradually if we happen to be editing a file that has a violation
-Style/SymbolArray:
+Metrics/LineLength:
-  EnforcedStyle: brackets
+  Max: 142

lib/factory_bot/attribute_assigner.rb

@@ -90,7 +90,9 @@ module FactoryBot
 
     def alias_names_to_ignore
       @attribute_list.non_ignored.flat_map do |attribute|
-        override_names.map { |override| attribute.name if attribute.alias_for?(override) && attribute.name != override && !ignored_attribute_names.include?(override) }
+        override_names.map do |override|
+          attribute.name if attribute.alias_for?(override) && attribute.name != override && !ignored_attribute_names.include?(override)
+        end
       end.compact
     end
   end

spec/acceptance/build_stubbed_spec.rb

@@ -176,7 +176,8 @@ describe "defaulting `created_at`" do
   end
 
   it "doesn't allow setting created_at on an object that doesn't define it" do
-    expect { build_stubbed(:thing_without_timestamp, created_at: Time.now) }.to raise_error(NoMethodError, /created_at=/)
+    expect { build_stubbed(:thing_without_timestamp, created_at: Time.now) }.
+      to raise_error(NoMethodError, /created_at=/)
   end
 end
 

spec/acceptance/modify_factories_spec.rb

@@ -173,10 +173,12 @@ describe "modifying factories" do
   end
 
   it "raises an exception if the factory was not defined before" do
-    expect {
+    modify_unknown_factory = -> do
       FactoryBot.modify do
         factory :unknown_factory
       end
-    }.to raise_error(ArgumentError)
+    end
+
+    expect(modify_unknown_factory).to raise_error(ArgumentError)
   end
 end

spec/acceptance/nested_attributes_spec.rb

@@ -25,6 +25,8 @@ describe "association assignment from nested attributes" do
   end
 
   it "assigns the correct amount of comments when overridden" do
-    expect(FactoryBot.create(:post, comments_attributes: [FactoryBot.attributes_for(:comment)]).comments.count).to eq 1
+    post = FactoryBot.create(:post, comments_attributes: [FactoryBot.attributes_for(:comment)])
+
+    expect(post.comments.count).to eq 1
   end
 end

spec/acceptance/traits_spec.rb

@@ -147,16 +147,16 @@ describe "an instance generated by a factory with multiple traits" do
   context "factory created with alternate syntax for specifying trait" do
     subject      { FactoryBot.create(:male_user) }
     its(:gender) { should eq "Male" }
-  end
 
-  context "factory created with alternate syntax where trait name and attribute are the same" do
+    context "where trait name and attribute are the same" do
-    subject     { FactoryBot.create(:great_user) }
+      subject     { FactoryBot.create(:great_user) }
-    its(:great) { should eq "GREAT!!!" }
+      its(:great) { should eq "GREAT!!!" }
-  end
+    end
 
-  context "factory created with alternate syntax where trait name and attribute are the same and attribute is overridden" do
+    context "where trait name and attribute are the same and attribute is overridden" do
-    subject     { FactoryBot.create(:great_user, great: "SORT OF!!!") }
+      subject     { FactoryBot.create(:great_user, great: "SORT OF!!!") }
-    its(:great) { should eq "SORT OF!!!" }
+      its(:great) { should eq "SORT OF!!!" }
+    end
   end
 
   context "child factory created where trait attributes are inherited" do
@@ -390,7 +390,9 @@ describe "inline traits overriding existing attributes" do
   end
 
   it "prefers overridden attributes over attributes from traits, inline traits, or attributes on factories" do
-    expect(FactoryBot.build(:extended_declined_user, :accepted, status: "completely overridden").status).to eq "completely overridden"
+    user = FactoryBot.build(:extended_declined_user, :accepted, status: "completely overridden")
+
+    expect(user.status).to eq "completely overridden"
   end
 end
 
@@ -481,8 +483,10 @@ describe "traits with to_create" do
       end
     end
 
-    expect(FactoryBot.create(:sub_user_with_trait_and_override, :overridden).name).to eq "completely overridden"
+    sub_user = FactoryBot.create(:sub_user_with_trait_and_override, :overridden)
-    expect(FactoryBot.create(:child_user_with_trait_and_override, :overridden).name).to eq "completely overridden"
+    child_user = FactoryBot.create(:child_user_with_trait_and_override, :overridden)
+    expect(sub_user.name).to eq "completely overridden"
+    expect(child_user.name).to eq "completely overridden"
   end
 end
 
@@ -555,8 +559,10 @@ describe "traits with initialize_with" do
       end
     end
 
-    expect(FactoryBot.build(:sub_user_with_trait_and_override, :overridden).name).to eq "completely overridden"
+    sub_user = FactoryBot.build(:sub_user_with_trait_and_override, :overridden)
-    expect(FactoryBot.build(:child_user_with_trait_and_override, :overridden).name).to eq "completely overridden"
+    child_user = FactoryBot.build(:child_user_with_trait_and_override, :overridden)
+    expect(sub_user.name).to eq "completely overridden"
+    expect(child_user.name).to eq "completely overridden"
   end
 end
 

spec/factory_bot/attribute_list_spec.rb

@@ -19,9 +19,9 @@ describe FactoryBot::AttributeList, "#define_attribute" do
   it "raises if an attribute has already been defined" do
     attribute = double(:attribute, name: :attribute_name)
 
-    expect {
+    expect do
       2.times { subject.define_attribute(attribute) }
-    }.to raise_error(
+    end.to raise_error(
       FactoryBot::AttributeDefinitionError,
       "Attribute already defined: attribute_name",
     )
@@ -35,7 +35,12 @@ describe FactoryBot::AttributeList, "#define_attribute with a named attribute li
   let(:association_with_different_name) { FactoryBot::Attribute::Association.new(:author, :post, {}) }
 
   it "raises when the attribute is a self-referencing association" do
-    expect { subject.define_attribute(association_with_same_name) }.to raise_error(FactoryBot::AssociationDefinitionError, "Self-referencing association 'author' in 'author'")
+    expect do
+      subject.define_attribute(association_with_same_name)
+    end.to raise_error(
+      FactoryBot::AssociationDefinitionError,
+      "Self-referencing association 'author' in 'author'",
+    )
   end
 
   it "does not raise when the attribute is not a self-referencing association" do
@@ -62,7 +67,9 @@ describe FactoryBot::AttributeList, "#apply_attributes" do
 end
 
 describe FactoryBot::AttributeList, "#associations" do
-  let(:email_attribute)     { FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" }) }
+  let(:email_attribute) do
+    FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" })
+  end
   let(:author_attribute)    { FactoryBot::Attribute::Association.new(:author, :user, {}) }
   let(:profile_attribute)   { FactoryBot::Attribute::Association.new(:profile, :profile, {}) }
 

spec/factory_bot/registry_spec.rb

@@ -17,7 +17,8 @@ describe FactoryBot::Registry do
   end
 
   it "raises when an object cannot be found" do
-    expect { subject.find(:object_name) }.to raise_error(ArgumentError, "Great thing not registered: object_name")
+    expect { subject.find(:object_name) }.
+      to raise_error(ArgumentError, "Great thing not registered: object_name")
   end
 
   it "adds and returns the object registered" do