Merge pull request rails#30919 from seanlinsley/17622-before_save_strict_arguments

Add strict argument checking to ActiveRecord callbacks

Author: kamipo

activemodel/lib/active_model/callbacks.rb

@@ -127,26 +127,28 @@ def define_model_callbacks(*callbacks)
     private
 
       def _define_before_model_callback(klass, callback)
-        klass.define_singleton_method("before_#{callback}") do |*args, &block|
-          set_callback(:"#{callback}", :before, *args, &block)
+        klass.define_singleton_method("before_#{callback}") do |*args, **options, &block|
+          options.assert_valid_keys(:if, :unless, :prepend)
+          set_callback(:"#{callback}", :before, *args, options, &block)
         end
       end
 
       def _define_around_model_callback(klass, callback)
-        klass.define_singleton_method("around_#{callback}") do |*args, &block|
-          set_callback(:"#{callback}", :around, *args, &block)
+        klass.define_singleton_method("around_#{callback}") do |*args, **options, &block|
+          options.assert_valid_keys(:if, :unless, :prepend)
+          set_callback(:"#{callback}", :around, *args, options, &block)
         end
       end
 
       def _define_after_model_callback(klass, callback)
-        klass.define_singleton_method("after_#{callback}") do |*args, &block|
-          options = args.extract_options!
+        klass.define_singleton_method("after_#{callback}") do |*args, **options, &block|
+          options.assert_valid_keys(:if, :unless, :prepend)
           options[:prepend] = true
           conditional = ActiveSupport::Callbacks::Conditionals::Value.new { |v|
             v != false
           }
           options[:if] = Array(options[:if]) << conditional
-          set_callback(:"#{callback}", :after, *(args << options), &block)
+          set_callback(:"#{callback}", :after, *args, options, &block)
         end
       end
   end

activerecord/test/cases/callbacks_test.rb

@@ -476,4 +476,31 @@ def test_inheritance_of_callbacks
     child.save
     assert child.after_save_called
   end
+
+  def test_before_save_doesnt_allow_on_option
+    exception = assert_raises ArgumentError do
+      Class.new(ActiveRecord::Base) do
+        before_save(on: :create) {}
+      end
+    end
+    assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message
+  end
+
+  def test_around_save_doesnt_allow_on_option
+    exception = assert_raises ArgumentError do
+      Class.new(ActiveRecord::Base) do
+        around_save(on: :create) {}
+      end
+    end
+    assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message
+  end
+
+  def test_after_save_doesnt_allow_on_option
+    exception = assert_raises ArgumentError do
+      Class.new(ActiveRecord::Base) do
+        after_save(on: :create) {}
+      end
+    end
+    assert_equal "Unknown key: :on. Valid keys are: :if, :unless, :prepend", exception.message
+  end
 end