Skip to content

Tighten six guard conditions via De Morgan / blank? / present? / include?#2707

Open
ericproulx wants to merge 1 commit intomasterfrom
refactor/de-morgan-conditions
Open

Tighten six guard conditions via De Morgan / blank? / present? / include?#2707
ericproulx wants to merge 1 commit intomasterfrom
refactor/de-morgan-conditions

Conversation

@ericproulx
Copy link
Copy Markdown
Contributor

@ericproulx ericproulx commented May 9, 2026

Six small condition rewrites in lib/ where an if/unless clause used multiple negations joined by &&/||, or &.any? on a Hash. Each one expresses the positive shape of the predicate, keeps if over unless where natural, and changes nothing about behaviour or allocation profile (one rewrite even saves a redundant Hash#except call).

The six

1. lib/grape/api.rbGrape::API::Boolean.build

# before
return nil if val != true && val != false

# after
VALUES = [true, false].freeze
private_constant :VALUES

return nil unless VALUES.include?(val)

"Neither true nor false" → "not one of [true, false]". Frozen constant avoids per-call array allocation; private_constant keeps the new symbol off Boolean's public surface.

2. lib/grape/request.rbRequest#make_params

# before
return params unless routing_args&.any? { |k, _| k != :version && k != :route_info }

params.deep_merge!(routing_args.except(:version, :route_info))

# after
filtered = routing_args&.except(:version, :route_info)
return params if filtered.blank?

params.deep_merge!(filtered)

The block predicate k != :version && k != :route_info is "not in the excluded set", which Hash#except says directly. Bonus: except was being called twice (once via the any? block's negative form and once on the next line) — the filtered local computes it once and reuses it. if filtered.blank? reads positively (nil or empty hash → bail).

3. lib/grape/validations/params_scope.rbcheck_incompatible_option_values (early guard)

# before
return unless default && !default.is_a?(Proc)

# after
return if default.nil? || default.is_a?(Proc)

unless A && !Bif !A || B. Switching to if + named conditions reads as "skip if there's nothing to check or if it's a Proc."

4. lib/grape/validations/params_scope.rbcheck_incompatible_option_values (raise condition)

# before
... if values && !values.is_a?(Proc) && !Array(default).all? { |def_val| values.include?(def_val) }

# after
... if values && !values.is_a?(Proc) && Array(default).any? { |def_val| !values.include?(def_val) }

!array.all? { p(x) }array.any? { !p(x) }. Rewritten form reads as "raise if any default value isn't in the allowed set" — which is the actual contract.

5. lib/grape/validations/validators/base.rbBase#scrub

# before
return value unless value.respond_to?(:valid_encoding?) && !value.valid_encoding?

# after
return value if !value.respond_to?(:valid_encoding?) || value.valid_encoding?

unless A && !Bif !A || B. Removes the unless ... && ! triple-negative.

6. lib/grape/dsl/routing.rbRouting#route

# before
all_route_options.deep_merge!(route_options) if route_options&.any?

# after
all_route_options.deep_merge!(route_options) if route_options.present?

For nil-or-Hash inputs &.any? and .present? are equivalent. present? reads as "if there's something there", drops the safe-nav noise, and aligns with the blank?/present? style already used elsewhere in lib/grape/ (router.rb, endpoint.rb, validations.rb).

Why these six

I scanned lib/ for if/unless clauses with multiple negations connected by &&/||, plus &.any? patterns on collection-or-nil values. Six passed the bar of "rewrite makes the positive intent clearer without growing the line." A handful of others (length_validator.rb triple !@x.nil? && clauses, e.g.) were either already fine or wanted a bigger refactor than a De Morgan flip.

Test plan

  • bundle exec rspec — full suite green (2,260 examples, 0 failures)
  • bundle exec rubocop lib/ — clean

Behavior-preserving by construction (each change is a boolean-algebra-equivalent rewrite); the existing test suite covers the call sites.

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

Danger Report

No issues found.

View run

@ericproulx ericproulx force-pushed the refactor/de-morgan-conditions branch from 28a97e1 to 464ccd6 Compare May 9, 2026 20:17
…ude?

Six spots where an `if`/`unless` clause used multiple negations
joined by `&&` or `||`, or `&.any?` on a Hash. Each rewrite expresses
the positive shape of the predicate and keeps `if` over `unless`
where natural. Same behaviour, same allocation profile (one form
even reuses a previously-recomputed `Hash#except` result).

* `Grape::API::Boolean.build` —
  `val != true && val != false` → frozen `VALUES` constant +
  `unless VALUES.include?(val)`.

* `Grape::Request#make_params` —
  `routing_args&.any? { |k, _| k != :version && k != :route_info }` +
  separate `routing_args.except(:version, :route_info)` two lines
  later → compute `except` once, gate on `filtered.blank?`.

* `Grape::Validations::ParamsScope#check_incompatible_option_values`
  (early guard) —
  `return unless default && !default.is_a?(Proc)` →
  `return if default.nil? || default.is_a?(Proc)`.

* `Grape::Validations::ParamsScope#check_incompatible_option_values`
  (raise condition) —
  `!Array(default).all? { |v| values.include?(v) }` →
  `Array(default).any? { |v| !values.include?(v) }`.

* `Grape::Validations::Validators::Base#scrub` —
  `unless value.respond_to?(:valid_encoding?) && !value.valid_encoding?`
  → `if !value.respond_to?(:valid_encoding?) || value.valid_encoding?`.

* `Grape::DSL::Routing#route` —
  `route_options&.any?` → `route_options.present?`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ericproulx ericproulx force-pushed the refactor/de-morgan-conditions branch from 464ccd6 to abc0d71 Compare May 9, 2026 20:20
@ericproulx ericproulx changed the title Tighten five guard conditions via De Morgan / blank? / include? Tighten six guard conditions via De Morgan / blank? / present? / include? May 9, 2026
@ericproulx ericproulx requested a review from dblock May 9, 2026 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant