Move declaration-coherence checks into ValidationsSpec

CHANGELOG.md

@@ -32,6 +32,7 @@
 * [#2710](https://github.com/ruby-grape/grape/pull/2710): Tidy up `Grape::DeclaredParamsHandler` - [@ericproulx](https://github.com/ericproulx).
 * [#2714](https://github.com/ruby-grape/grape/pull/2714): Drop unused `Grape::Middleware::Globals` and its `grape.request*` env constants - [@ericproulx](https://github.com/ericproulx).
 * [#2717](https://github.com/ruby-grape/grape/pull/2717): Convert `Grape::Exceptions::ErrorResponse` to a `Data` value object - [@ericproulx](https://github.com/ericproulx).
+* [#2720](https://github.com/ruby-grape/grape/pull/2720): Move declaration-coherence checks into `Grape::Validations::ValidationsSpec` - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/validations/params_scope.rb

@@ -334,8 +334,6 @@ def validates(attrs, validations)
         process_oneof!(validations) if validations.key?(:oneof)
         spec = ValidationsSpec.from(validations)
 
-        check_incompatible_option_values(spec.default, spec.values, spec.except_values)
-        validate_value_coercion(spec.coerce_type, spec.values, spec.except_values)
         document_params(attrs, spec)
 
         # Presence runs first — `required` is forwarded to every subsequent
@@ -380,16 +378,6 @@ def validate_coerce(spec, attrs)
         validate('coerce', coerce_options, attrs, spec.required?, spec.shared_opts)
       end
 
-      def check_incompatible_option_values(default, values, except_values)
-        return if default.nil? || default.is_a?(Proc)
-
-        raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values) if values && !values.is_a?(Proc) && Array(default).any? { |def_val| !values.include?(def_val) }
-
-        return unless except_values && !except_values.is_a?(Proc) && Array(default).any? { |def_val| except_values.include?(def_val) }
-
-        raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :except, except_values)
-      end
-
       # Translate a `oneof: [proc, proc, ...]` declaration into a list of
       # captured validator arrays — one array per variant. Each variant's
       # block is evaluated in its own +ParamsScope+ backed by an
@@ -418,19 +406,6 @@ def validate(type, options, attrs, required, opts)
         @api.inheritable_setting.namespace_stackable[:validations] = validator_instance
       end
 
-      def validate_value_coercion(coerce_type, *values_list)
-        return unless coerce_type
-
-        coerce_type = coerce_type.first if coerce_type.is_a?(Enumerable)
-        values_list.each do |values|
-          next if !values || values.is_a?(Proc)
-
-          value_types = values.is_a?(Range) ? [values.begin, values.end].compact : values
-          value_types = value_types.map { |type| Grape::API::Boolean.build(type) } if coerce_type == Grape::API::Boolean
-          raise Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) unless value_types.all?(coerce_type)
-        end
-      end
-
       def all_element_blank?(scoped_params)
         scoped_params.respond_to?(:all?) && scoped_params.all?(&:blank?)
       end

lib/grape/validations/validations_spec.rb

@@ -55,6 +55,8 @@ def initialize(raw)
         @shared_opts = { allow_blank: @allow_blank, fail_fast: @fail_fast }.freeze
         @validator_entries = build_validator_entries(raw)
 
+        validate!
+
         freeze
       end
 
@@ -68,6 +70,38 @@ def coerce_options
 
       private
 
+      # Cross-field consistency checks on the parsed declaration. Run at
+      # construction so an incoherent spec (e.g. a +default+ outside +values+,
+      # or +values+ whose elements don't match +type+) can never exist —
+      # callers no longer have to remember to invoke these separately.
+      def validate!
+        check_incompatible_option_values(@default, @values, @except_values)
+        validate_value_coercion(@coerce_type, @values, @except_values)
+      end
+
+      def check_incompatible_option_values(default, values, except_values)
+        return if default.nil? || default.is_a?(Proc)
+
+        raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values) if values && !values.is_a?(Proc) && Array(default).any? { |def_val| !values.include?(def_val) }
+
+        return unless except_values && !except_values.is_a?(Proc) && Array(default).any? { |def_val| except_values.include?(def_val) }
+
+        raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :except, except_values)
+      end
+
+      def validate_value_coercion(coerce_type, *values_list)
+        return unless coerce_type
+
+        coerce_type = coerce_type.first if coerce_type.is_a?(Enumerable)
+        values_list.each do |values|
+          next if !values || values.is_a?(Proc)
+
+          value_types = values.is_a?(Range) ? [values.begin, values.end].compact : values
+          value_types = value_types.map { |type| Grape::API::Boolean.build(type) } if coerce_type == Grape::API::Boolean
+          raise Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) unless value_types.all?(coerce_type)
+        end
+      end
+
       def build_validator_entries(raw)
         raw.reject do |k, _|
           SPEC_CONSUMED_KEYS.include?(k) || ParamsScope::RESERVED_DOCUMENTATION_KEYWORDS.include?(k)

spec/grape/validations/params_documentation_spec.rb

@@ -35,7 +35,7 @@ def spec_for(validations)
       validations = {
         presence: true,
         type: Integer,
-        default: 42,
+        default: 1,
         length: { min: 1, max: 10 },
         desc: 'A foo',
         documentation: { note: 'doc' },
@@ -50,7 +50,7 @@ def spec_for(validations)
         type: 'Integer',
         values: [1, 2, 3],
         except_values: [4, 5, 6],
-        default: 42,
+        default: 1,
         min_length: 1,
         max_length: 10,
         desc: 'A foo',