Simplify endpoint block execution and deprecate return call in it

Author: ericproulx

lib/grape/endpoint.rb

@@ -37,33 +37,6 @@ def run_before_each(endpoint)
         superclass.run_before_each(endpoint) unless self == Endpoint
         before_each.each { |blk| blk.try(:call, endpoint) }
       end
-
-      # @api private
-      #
-      # Create an UnboundMethod that is appropriate for executing an endpoint
-      # route.
-      #
-      # The unbound method allows explicit calls to +return+ without raising a
-      # +LocalJumpError+. The method will be removed, but a +Proc+ reference to
-      # it will be returned. The returned +Proc+ expects a single argument: the
-      # instance of +Endpoint+ to bind to the method during the call.
-      #
-      # @param [String, Symbol] method_name
-      # @return [Proc]
-      # @raise [NameError] an instance method with the same name already exists
-      def generate_api_method(method_name, &block)
-        raise NameError.new("method #{method_name.inspect} already exists and cannot be used as an unbound method name") if method_defined?(method_name)
-
-        define_method(method_name, &block)
-        method = instance_method(method_name)
-        remove_method(method_name)
-
-        proc do |endpoint_instance|
-          ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do
-            method.bind_call(endpoint_instance)
-          end
-        end
-      end
     end
 
     # Create a new endpoint.
@@ -108,12 +81,18 @@ def initialize(new_settings, options = {}, &block)
       @status = nil
       @stream = nil
       @body = nil
-      @proc = nil
 
       return unless block
 
       @source = block
-      @block = self.class.generate_api_method(method_name, &block)
+      @block = @block = lambda do |endpoint_instance|
+        ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do
+          endpoint_instance.instance_exec(&block)
+        rescue LocalJumpError => e
+          Grape.deprecator.warn 'Using `return` in an endpoint has been deprecated.'
+          return e.exit_value
+        end
+      end
     end
 
     # Update our settings from a given set of stackable parameters. Used when
@@ -131,14 +110,6 @@ def require_option(options, key)
       raise Grape::Exceptions::MissingOption.new(key) unless options.key?(key)
     end
 
-    def method_name
-      [options[:method],
-       Namespace.joined_space(namespace_stackable(:namespace)),
-       (namespace_stackable(:mount_path) || []).join('/'),
-       options[:path].join('/')]
-        .join(' ')
-    end
-
     def routes
       @routes ||= endpoints&.collect(&:routes)&.flatten || to_routes
     end

spec/grape/endpoint_spec.rb

@@ -785,29 +785,13 @@ def memoized
       return 'Hello'
     end
 
+    expect(Grape.deprecator).to receive(:warn).with('Using `return` in an endpoint has been deprecated.')
+
     get '/home'
     expect(last_response.status).to eq(200)
     expect(last_response.body).to eq('Hello')
   end
 
-  describe '.generate_api_method' do
-    it 'raises NameError if the method name is already in use' do
-      expect do
-        described_class.generate_api_method('version', &proc {})
-      end.to raise_error(NameError)
-    end
-
-    it 'raises ArgumentError if a block is not given' do
-      expect do
-        described_class.generate_api_method('GET without a block method')
-      end.to raise_error(ArgumentError)
-    end
-
-    it 'returns a Proc' do
-      expect(described_class.generate_api_method('GET test for a proc', &proc {})).to be_a Proc
-    end
-  end
-
   context 'filters' do
     describe 'before filters' do
       it 'runs the before filter if set' do