Stop reassigning method parameters across lib

[ericproulx]

A method that reassigns one of its parameters gives the same name two meanings: what the signature documents, and what the lines below the reassignment actually hold. Nothing escapes to the caller — Ruby parameters are local bindings — but the reader has to carry the reassignment for the rest of the method, and a later edit that moves a line above it changes behaviour silently.

This is a sweep of every such site in lib. Each derived value gets its own local; the parameter keeps what the caller passed.

The two that mattered

Most of the diff is naming. Two sites were reassignment wrapped around a mutation of the argument itself, and those did escape.

Middleware::Formatter#ensure_content_type wrote into the response headers Hash and returned the same object, which the caller then reassigned over its own parameter:

headers = ensure_content_type(headers)   # caller

def ensure_content_type(headers)
  return headers if headers[Rack::CONTENT_TYPE]

  headers[Rack::CONTENT_TYPE] = content_type_for(env[Grape::Env::API_FORMAT])
  headers
end

The Hash belongs to the response the app returned; negotiating a content type for it is not a reason to reach back into it. It merges now. Grape::Util::Header#merge preserves the class and its case-insensitive lookup on both Rack 2 (Rack::Utils::HeaderHash) and Rack 3 (Rack::Headers).

ParamsScope#process_oneof! wrote the collected variants back into the options Hash the requires / optional call site built:

validations[:oneof] = variants.map { |block| OneofCollector.collect(block) }

#validates only copied that Hash on the required path (validations.merge(presence: …)), so on the optional path the write landed on the caller's Hash. It is contained today only because **opts manufactures a fresh Hash per call — an accidental defensive copy, one signature change away from being load-bearing. #collected_oneof returns the variants now and #validates merges them.

The rest

File	Method	Parameter → local
dsl/desc.rb	desc	options → resolved_options
dsl/inside_route.rb	error!	status → resolved_status
dsl/parameters.rb	params	params → scoped
dsl/parameters.rb	declare	as → declared_as
dsl/routing.rb	mount	opts → mount_opts
dsl/routing.rb	route_param	requirements → param_requirements
error_formatter/base.rb	present	message → payload
exceptions/validation.rb	initialize	message → translated
middleware/error.rb	Options#initialize	rescue_options, default_error_formatter
middleware/error.rb	rack_response	message → body
middleware/error.rb	run_rescue_handler	handler → callable
middleware/formatter.rb	build_formatted_response	headers → typed_headers
middleware/formatter.rb	read_rack_input	body → parsed
middleware/stack.rb	insert, insert_after	index → at
util/path_normalizer.rb	call	path → normalized
validations/contract_scope.rb	initialize	contract → declared / schema
validations/params_scope.rb	validates	validations → declared
validations/types.rb	create_coercer_instance	type → mapped
types/multiple_type_coercer.rb	call	val → candidate
types/variant_collection_coercer.rb	call	value → coerced
validations/validations_spec.rb	validate_value_coercion	coerce_type → element_type

Three are worth a second look:

• run_rescue_handler is the strongest case, because the type changed rather than the value. handler arrives as a Symbol, a Method or a Proc — rescue_from with: :some_endpoint_method produces the first, find_handler's method(:error_response) the second, a rescue_from block the third — and arity / & are valid only after resolution. The method re-enters itself from five places, so tracing the recursion meant re-deriving the shape on each pass. callable states the invariant.
• path_normalizer.rb and error_formatter/base.rb reassign in order to protect the caller: path = "/#{path}" before a chain of squeeze! / delete_suffix! / gsub!, and message = message.dup before delete(:with). The naming was what made that hard to see — it reads as mutating the argument. normalized and payload say the mutated object is a fresh one, and path_normalizer gets a comment recording why the bangs are safe.
• middleware/error.rb's Options#initialize needed super(...) with explicit keywords instead of zsuper, since bare super forwards whatever the parameters currently hold — which is exactly the pressure that produces x ||= default in a Data initializer.

Not in scope

Grape::Endpoint::Options has the same shape and is fixed separately in #2907, where the reassignment is hiding a real bug rather than only obscuring one.

Verification

• bundle exec rspec — 2703 examples, 0 failures.
• bundle exec rubocop — 343 files, no offenses.
• gemfiles/grape_entity.gemfile (exercises error_formatter/base.rb#present) — 2726 examples, 0 failures.
• gemfiles/dry_validation.gemfile (exercises contract_scope.rb) — 13 examples, 0 failures.
• gemfiles/multi_xml.gemfile has 3 pre-existing failures in api_spec.rb's XML error-format examples; identical on master at the same seed, so unrelated to this change.

No new specs: nothing new is asserted, and the behavioural half is covered by the existing formatter and oneof suites.

🤖 Generated with Claude Code

[github-actions]

Danger Report

No issues found.

View run