enable rubocop on some files and address the resulting complaints (#5664)

Author: jamis

lib/mongoid/atomic.rb

@@ -1,28 +1,25 @@
 # frozen_string_literal: true
-# rubocop:todo all
 
-require "mongoid/atomic/modifiers"
-require "mongoid/atomic/paths"
+require 'mongoid/atomic/modifiers'
+require 'mongoid/atomic/paths'
 
 module Mongoid
-
   # This module contains the logic for supporting atomic operations against the
   # database.
   module Atomic
     extend ActiveSupport::Concern
 
-    UPDATES = [
-      :atomic_array_pushes,
-      :atomic_array_pulls,
-      :atomic_array_add_to_sets,
-      :atomic_pulls,
-      :delayed_atomic_sets,
-      :delayed_atomic_pulls,
-      :delayed_atomic_unsets
-    ]
+    UPDATES = %i[
+      atomic_array_pushes
+      atomic_array_pulls
+      atomic_array_add_to_sets
+      atomic_pulls
+      delayed_atomic_sets
+      delayed_atomic_pulls
+      delayed_atomic_unsets
+    ].freeze
 
     included do
-
       # When MongoDB finally fully implements the positional operator, we can
       # get rid of all indexing related code in Mongoid.
       attr_accessor :_index
@@ -117,6 +114,8 @@ def atomic_array_add_to_sets
     #   person.atomic_updates(children)
     #
     # @return [ Hash ] The updates and their modifiers.
+    #
+    # rubocop:disable Style/OptionalBooleanParameter
     def atomic_updates(_use_indexes = false)
       process_flagged_destroys
       mods = Modifiers.new
@@ -127,7 +126,8 @@ def atomic_updates(_use_indexes = false)
       end
       mods
     end
-    alias :_updates :atomic_updates
+    alias _updates atomic_updates
+    # rubocop:enable Style/OptionalBooleanParameter
 
     # Get the removal modifier for the document. Will be nil on root
     # documents, $unset on embeds_one, $set on embeds_many.
@@ -179,13 +179,11 @@ def atomic_position
     #
     # @return [ Object ] The associated path.
     def atomic_paths
-      @atomic_paths ||= begin
-        if _association
-          _association.path(self)
-        else
-          Atomic::Paths::Root.new(self)
-        end
-      end
+      @atomic_paths ||= if _association
+                          _association.path(self)
+                        else
+                          Atomic::Paths::Root.new(self)
+                        end
     end
 
     # Get all the attributes that need to be pulled.
@@ -202,7 +200,7 @@ def atomic_pulls
           path ||= doc.flag_as_destroyed
           doc._id
         end
-        pulls[path] = { "_id" => { "$in" => ids }} and path = nil
+        pulls[path] = { '_id' => { '$in' => ids } } and path = nil
       end
       pulls
     end
@@ -224,7 +222,13 @@ def atomic_pushes
     #
     # @return [ Hash ] The $set operations.
     def atomic_sets
-      updateable? ? setters : settable? ? { atomic_path => as_attributes } : {}
+      if updateable?
+        setters
+      elsif settable?
+        { atomic_path => as_attributes }
+      else
+        {}
+      end
     end
 
     # Get all the attributes that need to be unset.

lib/mongoid/changeable.rb

@@ -1,8 +1,6 @@
 # frozen_string_literal: true
-# rubocop:todo all
 
 module Mongoid
-
   # Defines behavior for dirty tracking.
   module Changeable
     extend ActiveSupport::Concern
@@ -53,12 +51,10 @@ def changed_attributes
     #
     # @return [ Hash<String, Array<Object, Object> ] The changes.
     def changes
-      _changes = {}
-      changed.each do |attr|
+      changed.each_with_object({}) do |attr, changes|
         change = attribute_change(attr)
-        _changes[attr] = change if change
-      end
-      _changes.with_indifferent_access
+        changes[attr] = change if change
+      end.with_indifferent_access
     end
 
     # Call this method after save, so the changes can be properly switched.
@@ -120,15 +116,15 @@ def remove_change(name)
     def setters
       mods = {}
       changes.each_pair do |name, changes|
-        if changes
-          old, new = changes
-          field = fields[name]
-          key = atomic_attribute_name(name)
-          if field && field.resizable?
-            field.add_atomic_changes(self, name, key, mods, new, old)
-          else
-            mods[key] = new unless atomic_unsets.include?(key)
-          end
+        next unless changes
+
+        old, new = changes
+        field = fields[name]
+        key = atomic_attribute_name(name)
+        if field&.resizable?
+          field.add_atomic_changes(self, name, key, mods, new, old)
+        else
+          mods[key] = new unless atomic_unsets.include?(key)
         end
       end
       mods
@@ -164,24 +160,19 @@ def saved_change_to_attribute(attr)
     #   in an attribute during the save that triggered the callbacks to run.
     #
     # @param [ String ] attr The name of the attribute.
-    # @param **kwargs The optional keyword arguments.
-    #
-    # @option **kwargs [ Object ] :from The object the attribute was changed from.
-    # @option **kwargs [ Object ] :to The object the attribute was changed to.
+    # @param [ Object ] from The object the attribute was changed from (optional).
+    # @param [ Object ] to The object the attribute was changed to (optional).
     #
     # @return [ true | false ] Whether the attribute has changed during the last save.
-    def saved_change_to_attribute?(attr, **kwargs)
+    def saved_change_to_attribute?(attr, from: Utils::PLACEHOLDER, to: Utils::PLACEHOLDER)
       changes = saved_change_to_attribute(attr)
       return false unless changes.is_a?(Array)
-      if kwargs.key?(:from) && kwargs.key?(:to)
-        changes.first == kwargs[:from] && changes.last == kwargs[:to]
-      elsif kwargs.key?(:from)
-        changes.first == kwargs[:from]
-      elsif kwargs.key?(:to)
-        changes.last == kwargs[:to]
-      else
-        true
-      end
+
+      return true if Utils.placeholder?(from) && Utils.placeholder?(to)
+      return changes.first == from if Utils.placeholder?(to)
+      return changes.last == to if Utils.placeholder?(from)
+
+      changes.first == from && changes.last == to
     end
 
     # Returns whether this attribute change the next time we save.
@@ -227,31 +218,48 @@ def attributes_before_last_save
     # @return [ Array<Object> ] The old and new values.
     def attribute_change(attr)
       attr = database_field_name(attr)
-      [changed_attributes[attr], attributes[attr]] if attribute_changed?(attr)
+      [ changed_attributes[attr], attributes[attr] ] if attribute_changed?(attr)
+    end
+
+    # A class for representing the default value that an attribute was changed
+    # from or to.
+    #
+    # @api private
+    class Anything
+      # `Anything` objects are always equal to everything. This simplifies
+      # the logic for asking whether an attribute has changed or not. If the
+      # `from` or `to` value is a `Anything` (because it was not
+      # explicitly given), any comparison with it will suggest the value has
+      # not changed.
+      #
+      # @param [ Object ] _other The object being compared with this object.
+      #
+      # @return [ true ] Always returns true.
+      def ==(_other)
+        true
+      end
     end
 
+    # a singleton object to represent an optional `to` or `from` value
+    # that was not explicitly provided to #attribute_changed?
+    ATTRIBUTE_UNCHANGED = Anything.new
+
     # Determine if a specific attribute has changed.
     #
     # @example Has the attribute changed?
     #   model.attribute_changed?("name")
     #
     # @param [ String ] attr The name of the attribute.
-    # @param **kwargs The optional keyword arguments.
-    #
-    # @option **kwargs [ Object ] :from The object the attribute was changed from.
-    # @option **kwargs [ Object ] :to The object the attribute was changed to.
+    # @param [ Object ] from The object the attribute was changed from (optional).
+    # @param [ Object ] to The object the attribute was changed to (optional).
     #
     # @return [ true | false ] Whether the attribute has changed.
-    def attribute_changed?(attr, **kwargs)
+    def attribute_changed?(attr, from: ATTRIBUTE_UNCHANGED, to: ATTRIBUTE_UNCHANGED)
       attr = database_field_name(attr)
       return false unless changed_attributes.key?(attr)
       return false if changed_attributes[attr] == attributes[attr]
-      if kwargs.key?(:from)
-        return false if changed_attributes[attr] != kwargs[:from]
-      end
-      if kwargs.key?(:to)
-        return false if attributes[attr] != kwargs[:to]
-      end
+      return false if from != changed_attributes[attr]
+      return false if to != attributes[attr]
 
       true
     end
@@ -265,8 +273,8 @@ def attribute_changed?(attr, **kwargs)
     #
     # @return [ true | false ] If the attribute differs.
     def attribute_changed_from_default?(attr)
-      field = fields[attr]
-      return false unless field
+      return false unless (field = fields[attr])
+
       attributes[attr] != field.eval_default(self)
     end
 
@@ -309,9 +317,9 @@ def attribute_previously_was(attr)
     #
     # @return [ Object ] The old value.
     def attribute_will_change!(attr)
-      unless changed_attributes.key?(attr)
-        changed_attributes[attr] = read_raw_attribute(attr).__deep_copy__
-      end
+      return if changed_attributes.key?(attr)
+
+      changed_attributes[attr] = read_raw_attribute(attr).__deep_copy__
     end
 
     # Set the attribute back to its old value.
@@ -329,7 +337,7 @@ def reset_attribute!(attr)
 
     def reset_attribute_to_default!(attr)
       attr = database_field_name(attr)
-      if field = fields[attr]
+      if (field = fields[attr])
         __send__("#{attr}=", field.eval_default(self))
       else
         __send__("#{attr}=", nil)
@@ -340,8 +348,8 @@ def reset_attributes_before_type_cast
       @attributes_before_type_cast = @attributes.dup
     end
 
+    # Class-level methods for changeable objects.
     module ClassMethods
-
       private
 
       # Generate all the dirty methods needed for the attribute.
@@ -495,7 +503,7 @@ def create_dirty_reset_to_default(name, meth)
       def create_dirty_previously_changed?(name, meth)
         generated_methods.module_eval do
           re_define_method("#{meth}_previously_changed?") do
-            previous_changes.keys.include?(name)
+            previous_changes.key?(name)
           end
         end
       end