Disallow keyword arguments on handlers with deserialized arguments (#265)

Fixes #226

Author: cretz

README.md

@@ -490,7 +490,8 @@ definition/behavior of the method:
   side effects, meaning they should never mutate state or try to wait on anything.
 
 Workflows can be inherited, but subclass workflow-level decorators override superclass ones, and the same method can't
-be decorated with different handler types/names in the hierarchy.
+be decorated with different handler types/names in the hierarchy. Workflow handlers (execute or any marked method)
+cannot accept keyword arguments.
 
 #### Running Workflows
 
@@ -955,6 +956,7 @@ Some notes about activity definition:
 * `workflow_raw_args` can be used to have activity arguments delivered to `execute` as
   `Temporalio::Converters::RawValue`s. These are wrappers for the raw payloads that have not been converted to types
   (but they have been decoded by the codec if present). They can be converted with `payload_converter` on the context.
+* Activities cannot accept keyword arguments.
 
 #### Activity Context
 

temporalio/.rubocop.yml

@@ -67,7 +67,7 @@ Metrics/ModuleLength:
 
 # The default is too small
 Metrics/PerceivedComplexity:
-  Max: 40
+  Max: 60
 
 # We want the `*args` syntax instead of `*` so we can document clearly in YARD
 Style/ArgumentsForwarding:

temporalio/lib/temporalio/activity/definition.rb

@@ -85,6 +85,11 @@ def self._activity_definition_details
         activity_name = @activity_name
         raise 'Cannot have activity name specified for dynamic activity' if activity_name && @activity_dynamic
 
+        # Disallow kwargs in execute parameters
+        if instance_method(:execute).parameters.any? { |t, _| t == :key || t == :keyreq }
+          raise 'Activity execute cannot have keyword arguments'
+        end
+
         # Default to unqualified class name if not dynamic
         activity_name ||= name.to_s.split('::').last unless @activity_dynamic
         {

temporalio/lib/temporalio/workflow/definition.rb

@@ -235,6 +235,15 @@ def self.method_added(method_name)
         # Reset details
         self.pending_handler_details = nil
 
+        # Disallow kwargs in parameters
+        begin
+          if instance_method(method_name).parameters.any? { |t, _| t == :key || t == :keyreq }
+            raise "Workflow #{handler[:type]} cannot have keyword arguments"
+          end
+        rescue NameError
+          # Ignore name error
+        end
+
         # Initialize class variables if not done already
         @workflow_signals ||= {}
         @workflow_queries ||= {}
@@ -350,6 +359,11 @@ def self._build_workflow_definition
           raise "Leftover #{pending_handler_details&.[](:type)} handler not applied to a method"
         end
 
+        # Disallow kwargs in execute parameters
+        if instance_method(:execute).parameters.any? { |t, _| t == :key || t == :keyreq }
+          raise 'Workflow execute cannot have keyword arguments'
+        end
+
         # Apply all update validators before merging with super
         updates = @workflow_updates&.dup || {}
         @workflow_update_validators&.each_value do |validator|

temporalio/test/worker_activity_test.rb

@@ -924,6 +924,17 @@ def test_reserved_name
     assert_includes err.message, "'__temporal_activity' cannot start with '__temporal_'"
   end
 
+  class KeywordArgumentActivity < Temporalio::Activity::Definition
+    def execute(foo, bar: 'baz'); end
+  end
+
+  def test_keyword_arguments
+    err = assert_raises { Temporalio::Activity::Definition::Info.from_activity(KeywordArgumentActivity) }
+    assert_includes err.message, 'Activity execute cannot have keyword arguments'
+    err = assert_raises { Temporalio::Activity::Definition::Info.from_activity(KeywordArgumentActivity.new) }
+    assert_includes err.message, 'Activity execute cannot have keyword arguments'
+  end
+
   # steep:ignore
   def execute_activity(
     activity,

temporalio/test/workflow/definition_test.rb

@@ -343,5 +343,57 @@ def some_update; end
         end
       CODE
     end
+
+    def test_kwargs
+      assert_invalid_workflow_code 'Workflow execute cannot have keyword arguments', <<~CODE
+        class TestExecuteKeywordArgs < Temporalio::Workflow::Definition
+          def execute(foo, bar:)
+          end
+        end
+      CODE
+      assert_invalid_workflow_code 'Workflow init cannot have keyword arguments', <<~CODE
+        class TestInitKeywordArgs < Temporalio::Workflow::Definition
+          workflow_init
+          def initialize(foo, bar:); end
+
+          def execute; end
+        end
+      CODE
+      assert_invalid_workflow_code 'Workflow signal cannot have keyword arguments', <<~CODE
+        class TestSignalKeywordArgs < Temporalio::Workflow::Definition
+          def execute; end
+
+          workflow_signal
+          def some_handler(foo, bar: 'baz'); end
+        end
+      CODE
+      assert_invalid_workflow_code 'Workflow query cannot have keyword arguments', <<~CODE
+        class TestQueryKeywordArgs < Temporalio::Workflow::Definition
+          def execute; end
+
+          workflow_query
+          def some_handler(foo, bar:); end
+        end
+      CODE
+      assert_invalid_workflow_code 'Workflow update cannot have keyword arguments', <<~CODE
+        class TestUpdateKeywordArgs < Temporalio::Workflow::Definition
+          def execute; end
+
+          workflow_update
+          def some_handler(foo, bar:); end
+        end
+      CODE
+      assert_invalid_workflow_code 'Workflow update_validator cannot have keyword arguments', <<~CODE
+        class TestUpdateValidatorKeywordArgs < Temporalio::Workflow::Definition
+          def execute; end
+
+          workflow_update_validator(:some_handler)
+          def validate_some_handler(foo, bar:); end
+
+          workflow_update
+          def some_handler(foo, bar:); end
+        end
+      CODE
+    end
   end
 end