Add Grape.config.warn_on_helper_overrides (opt-in dev-time signal)

[ericproulx]

Adds an opt-in Grape.config.warn_on_helper_overrides flag. When enabled, defining a helper that masks a Grape::Endpoint instance method emits a warning to $stderr.

Follow-up to #2704 — that PR introduced Grape::Endpoint#logger, which is silently shadowed by any pre-existing helpers do; def logger; …; end. The helper-wins behaviour is correct (singleton_class.include(@helpers) makes it so), but it's invisible. This PR gives users a way to find out.

Usage

# config/environments/development.rb
Grape.config.warn_on_helper_overrides = true

class MyAPI < Grape::API
  helpers do
    def params      # ← warns: overrides Grape::Endpoint#params
      'overridden'
    end

    def my_unique_helper  # ← does not warn
      :ok
    end
  end
end

Grape: helper method `params` overrides Grape::Endpoint#params. The helper takes precedence. To use the framework implementation, remove the helper. Silence this warning by setting Grape.config.warn_on_helper_overrides = false.

Behaviour

• Default off. Zero runtime cost in production. Apps that intentionally override (e.g. the def logger; MyAPI.logger; end pattern) won't suddenly start warning when they upgrade.
• One warning per masking method per helpers definition. Boot-time only — helpers runs at class definition, not per request.
• Catches both forms — helpers do; def foo; end; end and helpers SomeModule where SomeModule defines an instance method colliding with Endpoint.
• Opts to noisy-but-actionable: includes the conflicting method name, the framework method it masks, and the silence-switch.

Files

File	What
lib/grape.rb	New setting :warn_on_helper_overrides, default: false.
lib/grape/dsl/helpers.rb	inject_api_helpers_to_mod now calls a new private warn_on_endpoint_overrides(mod) when the flag is on.
spec/grape/dsl/helpers_spec.rb	5 new examples — single mask, multiple masks (both reported, non-mask not reported), external-module form, no-mask non-warning, flag-off non-warning.
CHANGELOG.md	Features entry.

Test plan

• bundle exec rspec spec/grape/dsl/helpers_spec.rb — 11 examples, 0 failures (5 new + 6 existing)
• bundle exec rspec — full suite green (2,259 examples, up from 2,254)
• bundle exec rubocop lib/ spec/ — clean

Why opt-in

Plenty of existing apps deliberately override Endpoint methods via helpers. An always-on warning would force-rename every one of them on upgrade. Off-by-default keeps the framework friendly, while the flag gives anyone who wants the signal a one-line opt-in. Suggested home is config/environments/development.rb so prod stays quiet.

🤖 Generated with Claude Code

[github-actions]

Danger Report

No issues found.

View run

[dblock]

Thanks for this — @dblock prompted me (Copilot) to suggest an alternative worth considering.

Rather than a global flag, what if the warning were always on but silenced at the point of definition using the familiar Ruby method-modifier pattern?

helpers do
  override def params   # explicit intent — no warning
    "overridden"
  end

  def logger            # warns: shadows Grape::Endpoint#logger
    MyApp.logger
  end
end

override would be a method on BaseHelper that registers the name and returns it (exactly like private def foo):

module BaseHelper
  def override(method_name)
    @intentional_overrides ||= Set.new
    @intentional_overrides << method_name
    method_name
  end
end

The check skips anything in @intentional_overrides and warns for the rest. No Grape.config key, no spec_helper toggle — the warning is always active at class-load time and the suppression is local and self-documenting. The warning message can even teach the fix: "If intentional, use \override def #{name}`."`

[ericproulx]

@dblock thanks (and tell Copilot thanks too) — the override def shape is genuinely nicer than a flag for intent at the call site. Two things make me think the cleanest path is to ship #2705 as-is and revisit this proposal separately, though.

1. Backwards compatibility for the documented pattern

The pattern this PR's UPGRADING note was originally responding to —

class MyAPI < Grape::API
  logger Logger.new($stdout)

  helpers do
    def logger
      MyAPI.logger
    end
  end
end

— is documented and was used in benchmark/remounting.rb itself until #2704 made it unnecessary. If we make the warning always-on without a flag, every existing app with this pattern starts warning on every boot of every test the moment they upgrade from 3.3 to 3.4. That's a lot of noise for a non-bug, and the migration ("wrap each in override def") has to happen everywhere before the warnings go away.

The flag inverts this: zero noise for the existing world, the audit is opt-in.

2. helpers SomeModule doesn't fit override def

The override def foo shape works inside an inline helpers do … end block, but most Rails-shaped apps mix the inline form with helpers SomeExternalModule:

# app/api/concerns/authentication.rb — plain Ruby module, authored without Grape in mind
module Authentication
  def logger      # accidental mask of Grape::Endpoint#logger
  def status      # accidental mask of Grape::Endpoint#status
end

class MyAPI < Grape::API
  helpers Authentication  # ← where does `override` live?
end

Two ways to acknowledge a mask in this case:

• helpers Authentication, override: %i[logger status] — call-site list
• helpers do; override :logger, :status; helpers Authentication; end — inclusion-block form

Either works on paper, but it means the "self-documenting at the call site" pitch is only true for the inline-block form. External modules pull intent back to the inclusion site, and there are now two acknowledgment shapes for the same concept (override def … vs override :foo, :bar).

Suggested path

• Land Add Grape.config.warn_on_helper_overrides (opt-in dev-time signal) #2705 as-is (off by default) — gives auditors a one-line opt-in without noise for existing apps.
• Open a follow-up PR for override def — design the inclusion-site acknowledgment for external modules at the same time, so the mental model is unified.
• Revisit "always-on" as a v4 default when the API surface can be redesigned holistically and a noisy upgrade is acceptable.

Happy to leave this PR open while the follow-up is sketched if you'd prefer to land both together.

[dblock]

Your plan is good.