Simplify handling of validation input args

lib/dry/operation/extensions/validation.rb

@@ -13,9 +13,45 @@ module Extensions
       #
       # When this extension is included, define your contract on your operation class using
       # `params`, `schema`, or `contract`, or make a `#contract` dependency available from your
-      # operation instance.
+      # operation instance. The operation's input is validated before its method body runs, and the
+      # method receives the contract's output in place of the input it was called with.
       #
-      # @see https://dry-rb.org/gems/dry-validation/
+      # The input is the last positional argument (when positional args are present), or the keyword
+      # arguments in full. Both of these validate `{name: "Alice"}`:
+      #
+      # ```
+      # operation.call(name: "Alice")
+      # operation.call({name: "Alice"})
+      # ```
+      #
+      # When the input is given as a positional argument, any earlier positional arguments and any
+      # keyword arguments are considered the operation's own. They never pass through the validation
+      # contract, and are never set by the contract's output. Use these for arguments that control
+      # the operation's own behavior alongside its validated input.
+      #
+      # ```
+      # class UpdateUser < Dry::Operation
+      #   include Dry::Operation::Extensions::Validation
+      #
+      #   params do
+      #     required(:name).filled(:string)
+      #   end
+      #
+      #   def call(id, attrs, notify: false)
+      #     step persist(id, attrs)
+      #   end
+      # end
+      #
+      # update_user = UpdateUser.new
+      # update_user.call(123, {name: "Alice", admin: true}, notify: true)
+      # # id is 123 - not passed through the contract
+      # # attrs is {name: "Alice"} - admin filtered out by the contract
+      # # notify is true - not passed through the contract
+      # ```
+      #
+      # When there is no contract defined, all arguments are forwarded untouched.
+      #
+      # @see https://hanakai.org/learn/dry/dry-validation
       #
       # @api public
       # @since 1.2.0
@@ -168,60 +204,22 @@ def name
 
           private
 
-          # rubocop:disable Metrics/PerceivedComplexity
           def define_validation_method
-            # Cache named kwargs outside the method closure so we only search for them once.
-            named_kwargs = nil
-            find_named_kwargs = method(:find_named_kwargs)
-
-            define_method(@method_name) do |input = {}, *rest, **kwargs, &block|
-              use_kwargs = !kwargs.empty? && input.empty? && rest.empty?
-              actual_input = use_kwargs ? kwargs : input
-
-              validation_result = validate(actual_input)
-
-              case validation_result
-              when Dry::Monads::Success
-                validated_input = validation_result.value!
-
-                if use_kwargs
-                  # Ensure named kwargs from the wrapped method are still passed through even if
-                  # they are not in the validation output. This is important for kwargs that exist
-                  # to serve the method's own logic, separate to the scope of validatable input.
-                  named_kwargs ||= find_named_kwargs.call(method(__method__).super_method)
-                  passthrough_keys = actual_input
-                    .slice(*named_kwargs)
-                    .reject { |k, _| validated_input.key?(k) }
-                  validated_input = passthrough_keys.merge(validated_input)
-
-                  super(**validated_input, &block)
-                else
-                  super(validated_input, *rest, **kwargs, &block)
-                end
-              when Dry::Monads::Failure
-                throw_failure(validation_result)
+            define_method(@method_name) do |*args, **kwargs, &block|
+              # Without a contract there's nothing to validate, so the arguments the method was
+              # called with are forwarded untouched.
+              return super(*args, **kwargs, &block) unless contract
+
+              if args.empty?
+                # The keyword arguments are the input.
+                super(**step(validate(kwargs)), &block)
+              else
+                # The last positional argument is the input. Pass through all other args.
+                *rest, input = args
+                super(*rest, step(validate(input)), **kwargs, &block)
               end
             end
           end
-          # rubocop:enable Metrics/PerceivedComplexity
-
-          NAMED_KWARG_TYPES = %i[key keyreq].freeze
-
-          def find_named_kwargs(method)
-            # Walk up the method chain to find the first method with named kwargs.
-            while method
-              named_kwargs = method
-                .parameters
-                .select { |type, _| NAMED_KWARG_TYPES.include?(type) }
-                .map(&:last)
-
-              return named_kwargs if named_kwargs.any?
-
-              method = method.super_method
-            end
-
-            []
-          end
         end
       end
     end

spec/integration/extensions/validation_spec.rb

@@ -375,18 +375,18 @@ def call(input)
     end
   end
 
-  describe "selective validation with unvalidated keys preserved" do
-    it "preserves unvalidated named kwargs and coerces validated values" do
+  describe "arguments passed through alongside validated input" do
+    it "passes through keyword arguments when input given as a positional argument" do
       calculate = Class.new(Dry::Operation) do
         include Dry::Operation::Extensions::Validation
 
         params do
-          # Validate and coerce x and y, but not operation
+          # Validate and coerce x and y. `operation` is not input, so it isn't here.
           required(:x).value(:integer)
           required(:y).value(:integer)
         end
 
-        def call(operation:, **values)
+        def call(values, operation:)
           # operation is passed through (not validated)
           # x and y are coerced from strings to integers
           result = case operation
@@ -398,14 +398,14 @@ def call(operation:, **values)
         end
       end
 
-      # x and y are coerced to integers, operation is preserved as symbol
-      result = calculate.new.call(operation: :add, x: "10", y: "20")
+      # x and y are coerced to integers, operation is preserved as a symbol
+      result = calculate.new.call({x: "10", y: "20"}, operation: :add)
 
       expect(result).to be_success
       expect(result.value!).to eq(operation: :add, x: 10, y: 20, result: 30)
     end
 
-    it "filters out invalid keys in splat args while preserving named kwargs" do
+    it "passes through leading positional arguments when input given as a final positional argument" do
       update_user = Class.new(Dry::Operation) do
         include Dry::Operation::Extensions::Validation
 
@@ -415,20 +415,149 @@ def call(operation:, **values)
           # Deliberately doesn't allow :admin - should be filtered
         end
 
-        def call(id:, **attrs)
-          # id should be preserved (named kwarg)
-          # admin should be filtered (not in contract, and not a named kwarg)
+        def call(id, attrs)
+          # id is a parameter, so it's untouched by the contract
+          # admin should be filtered (not allowed by the contract)
           attrs.merge(id: id)
         end
       end
 
-      # User tries to sneak in admin: true
-      result = update_user.new.call(id: 123, name: "Alice", admin: true)
+      # User tries to sneak in `id: 456` and `admin: true`
+      result = update_user.new.call(123, {name: "Alice", id: 456, admin: true})
 
       expect(result).to be_success
-      # admin should NOT be present - contract filtered it
+      # id and admin from the input are filtered out by the contract
       expect(result.value!).to eq(id: 123, name: "Alice")
       expect(result.value!).not_to have_key(:admin)
     end
+
+    it "passes through both leading positional arguments as well keyword arguments" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        params do
+          required(:name).filled(:string)
+        end
+
+        def call(suffix, attrs, upcase: false)
+          name = "#{attrs[:name]}#{suffix}"
+
+          upcase ? name.upcase : name
+        end
+      end
+
+      result = operation.new.call("!", {name: "Alice"}, upcase: true)
+
+      expect(result).to eq(Success("ALICE!"))
+    end
+  end
+
+  describe "operations taking no input" do
+    it "calls a method taking no arguments when no contract is defined" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        def call
+          step build_thing
+        end
+
+        private
+
+        def build_thing
+          Success(:thing)
+        end
+      end
+
+      expect(operation.new.call).to eq(Success(:thing))
+    end
+
+    it "forwards arguments untouched when no contract is defined" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        def call(input, extra: nil)
+          [input, extra]
+        end
+      end
+
+      expect(operation.new.call(:input, extra: :extra)).to eq(Success([:input, :extra]))
+    end
+
+    it "calls a method taking no arguments when the contract validates an empty input" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        params do
+          optional(:name).filled(:string)
+        end
+
+        def call
+          step build_thing
+        end
+
+        private
+
+        def build_thing
+          Success(:thing)
+        end
+      end
+
+      expect(operation.new.call).to eq(Success(:thing))
+    end
+
+    it "fails validation when a method taking no arguments has a contract requiring input" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        params do
+          required(:name).filled(:string)
+        end
+
+        def call
+          Success(:thing)
+        end
+      end
+
+      result = operation.new.call
+
+      expect(result).to be_failure
+      failure_type, validation_result = result.failure
+      expect(failure_type).to eq(:invalid)
+      expect(validation_result.errors.to_h).to eq(name: ["is missing"])
+    end
+
+    it "raises when input is given to a method that takes no arguments" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        params do
+          optional(:name).filled(:string)
+        end
+
+        def call
+          Success(:thing)
+        end
+      end
+
+      expect { operation.new.call(name: "Alice") }
+        .to raise_error(ArgumentError, /wrong number of arguments/)
+    end
+
+    it "validates an empty input for a method taking only keyword arguments" do
+      operation = Class.new(Dry::Operation) do
+        include Dry::Operation::Extensions::Validation
+
+        params do
+          optional(:name).filled(:string)
+        end
+
+        def call(name: "anonymous")
+          name
+        end
+      end
+
+      expect(operation.new.call).to eq(Success("anonymous"))
+      expect(operation.new.call(name: "Alice")).to eq(Success("Alice"))
+    end
   end
 end