Refactor Statements to use explicit keyword arguments

CHANGELOG.md

@@ -9,6 +9,9 @@ changelog, see the [commits] for each version via the version links.
 
 [Unreleased]: https://github.com/teoljungberg/fx/compare/v0.10.0..HEAD
 
+- Refactor Statements module to use explicit keyword arguments instead of `**options` hash
+- Update CommandRecorder to use `ruby2_keywords` for proper argument forwarding, following Rails' migration pattern
+
 ## [0.10.0]
 
 [0.10.0]: https://github.com/teoljungberg/fx/compare/v0.9.0...v0.10.0

lib/fx/command_recorder.rb

@@ -1,17 +1,20 @@
 module Fx
   # @api private
   module CommandRecorder
-    def create_function(*args)
-      record(:create_function, args)
+    def create_function(*args, &block)
+      record(:create_function, args, &block)
     end
+    ruby2_keywords :create_function if respond_to?(:ruby2_keywords, true)
 
-    def drop_function(*args)
-      record(:drop_function, args)
+    def drop_function(*args, &block)
+      record(:drop_function, args, &block)
     end
+    ruby2_keywords :drop_function if respond_to?(:ruby2_keywords, true)
 
-    def update_function(*args)
-      record(:update_function, args)
+    def update_function(*args, &block)
+      record(:update_function, args, &block)
     end
+    ruby2_keywords :update_function if respond_to?(:ruby2_keywords, true)
 
     def invert_create_function(args)
       [:drop_function, args]
@@ -25,17 +28,20 @@ def invert_update_function(args)
       perform_inversion(:update_function, args)
     end
 
-    def create_trigger(*args)
-      record(:create_trigger, args)
+    def create_trigger(*args, &block)
+      record(:create_trigger, args, &block)
     end
+    ruby2_keywords :create_trigger if respond_to?(:ruby2_keywords, true)
 
-    def drop_trigger(*args)
-      record(:drop_trigger, args)
+    def drop_trigger(*args, &block)
+      record(:drop_trigger, args, &block)
     end
+    ruby2_keywords :drop_trigger if respond_to?(:ruby2_keywords, true)
 
-    def update_trigger(*args)
-      record(:update_trigger, args)
+    def update_trigger(*args, &block)
+      record(:update_trigger, args, &block)
     end
+    ruby2_keywords :update_trigger if respond_to?(:ruby2_keywords, true)
 
     def invert_create_trigger(args)
       [:drop_trigger, args]
@@ -51,12 +57,13 @@ def invert_update_trigger(args)
 
     private
 
+    MESSAGE_IRREVERSIBLE = "`%s` is reversible only if given a `revert_to_version`".freeze
+
     def perform_inversion(method, args)
       arguments = Arguments.new(args)
 
       if arguments.revert_to_version.nil?
-        message = "`#{method}` is reversible only if given a `revert_to_version`"
-        raise ActiveRecord::IrreversibleMigration, message
+        raise ActiveRecord::IrreversibleMigration, format(MESSAGE_IRREVERSIBLE, method)
       end
 
       [method, arguments.invert_version.to_a]
@@ -68,19 +75,19 @@ def initialize(args)
       end
 
       def function
-        args[0]
+        args.fetch(0)
       end
 
       def version
-        options[:version]
+        options.fetch(:version)
       end
 
       def revert_to_version
-        options[:revert_to_version]
+        options.fetch(:revert_to_version, nil)
       end
 
       def invert_version
-        Arguments.new([function, options_for_revert])
+        self.class.new([function, options_for_revert])
       end
 
       def to_a
@@ -92,14 +99,24 @@ def to_a
       attr_reader :args
 
       def options
-        @options ||= args[1] || {}
+        @options ||= args.fetch(1, {}).dup
       end
 
       def options_for_revert
-        options.clone.tap do |revert_options|
+        opts = options.clone.tap do |revert_options|
           revert_options[:version] = revert_to_version
           revert_options.delete(:revert_to_version)
         end
+
+        keyword_hash(opts)
+      end
+
+      def keyword_hash(hash)
+        if Hash.respond_to?(:ruby2_keywords_hash)
+          Hash.ruby2_keywords_hash(hash)
+        else
+          hash
+        end
       end
     end
     private_constant :Arguments

lib/fx/statements.rb

@@ -26,10 +26,7 @@ module Statements
     #     $$ LANGUAGE plpgsql;
     #   SQL
     #
-    def create_function(name, options = {})
-      version = options.fetch(:version, 1)
-      sql_definition = options[:sql_definition]
-
+    def create_function(name, version: 1, sql_definition: nil, revert_to_version: nil)
       validate_version_or_sql_definition_present!(version, sql_definition)
       sql_definition = resolve_sql_definition(sql_definition, name, version, :function)
 
@@ -47,7 +44,7 @@ def create_function(name, options = {})
     # @example Drop a function, rolling back to version 2 on rollback
     #   drop_function(:uppercase_users_name, revert_to_version: 2)
     #
-    def drop_function(name, options = {})
+    def drop_function(name, revert_to_version: nil)
       Fx.database.drop_function(name)
     end
 
@@ -80,10 +77,7 @@ def drop_function(name, options = {})
     #     $$ LANGUAGE plpgsql;
     #   SQL
     #
-    def update_function(name, options = {})
-      version = options[:version]
-      sql_definition = options[:sql_definition]
-
+    def update_function(name, version: nil, sql_definition: nil, revert_to_version: nil)
       validate_version_or_sql_definition_present!(version, sql_definition)
 
       sql_definition = resolve_sql_definition(sql_definition, name, version, :function)
@@ -113,10 +107,7 @@ def update_function(name, options = {})
     #         EXECUTE FUNCTION uppercase_users_name();
     #    SQL
     #
-    def create_trigger(name, options = {})
-      version = options[:version]
-      sql_definition = options[:sql_definition]
-
+    def create_trigger(name, version: nil, sql_definition: nil, on: nil, revert_to_version: nil)
       validate_version_and_sql_definition_exclusive!(version, sql_definition)
 
       version ||= 1
@@ -139,8 +130,7 @@ def create_trigger(name, options = {})
     # @example Drop a trigger, rolling back to version 3 on rollback
     #   drop_trigger(:log_inserts, on: :users, revert_to_version: 3)
     #
-    def drop_trigger(name, options = {})
-      on = options.fetch(:on)
+    def drop_trigger(name, on:, revert_to_version: nil)
       Fx.database.drop_trigger(name, on: on)
     end
 
@@ -176,18 +166,10 @@ def drop_trigger(name, options = {})
     #         EXECUTE FUNCTION uppercase_users_name();
     #    SQL
     #
-    def update_trigger(name, options = {})
-      version = options[:version]
-      on = options[:on]
-      sql_definition = options[:sql_definition]
-
+    def update_trigger(name, on:, version: nil, sql_definition: nil, revert_to_version: nil)
       validate_version_or_sql_definition_present!(version, sql_definition)
       validate_version_and_sql_definition_exclusive!(version, sql_definition)
 
-      if on.nil?
-        raise ArgumentError, "on is required"
-      end
-
       sql_definition = resolve_sql_definition(sql_definition, name, version, :trigger)
 
       Fx.database.update_trigger(

spec/fx/statements_spec.rb

@@ -176,6 +176,7 @@
       expect do
         connection.update_trigger(
           :whatever,
+          on: :users,
           version: nil,
           sql_definition: nil
         )
@@ -191,6 +192,7 @@
       expect do
         connection.update_trigger(
           :whatever,
+          on: :users,
           version: 1,
           sql_definition: "a definition"
         )