Merge pull request #2 from thatch-health/olivier/callbacks-fix

Better support for callbacks

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.0.3 (2024-06-05)
+
+- Better support for callbacks
+
 ## 0.0.2 (2024-06-05)
 
 - Added support for callbacks (`before`, `after`, etc.)

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    grape_sorbet (0.0.2)
+    grape_sorbet (0.0.3)
       activesupport
       grape (~> 2.0)
       sorbet-runtime (~> 0.5.10741)

README.md

@@ -90,7 +90,29 @@ module Twitter
 end
 ```
 
-At this point Sorbet is still reporting errors in the `get :home_timeline` block, because it doesn't know that these blocks are executed in a context where the `Helpers` module methods will be available. The Tapioca compiler provided by this gem will fix that. Run `bundle exec tapioca dsl`. This should generate a `sorbet/rbi/dsl/twitter/api.rbi` file (assuming the source file is in `lib/twitter/api.rb`). After this,
+At this point Sorbet is still reporting errors in the `get :home_timeline` block, because it doesn't know that these blocks are executed in a context where the `Helpers` module methods will be available. The Tapioca compiler provided by this gem will fix that. Run `bundle exec tapioca dsl`. This should generate a `sorbet/rbi/dsl/twitter/api.rbi` file (assuming the source file is in `lib/twitter/api.rb`).
+
+After this, Sorbet should no longer report any errors.
+
+## Limitations and known issues
+
+### Subclassing from `Grape::API::Instance` instead of `Grape::API`
+
+Grape overrides `Grape::API.new` and uses the `inherited` hook so that subclasses of `Grape::API` are really subclasses of `Grape::API::Instance`.
+
+This might be fixable in a future update of grape_sorbet, but is very low priority.
+
+### Not being able to call `helpers` with block arguments
+
+Possibly fixable in a future update.
+
+### Having to use `T.bind(self, T.any(Grape::Endpoint, <HelpersModuleName>))` in helper methods
+
+Possibly fixable in a future update.
+
+### Having to use `T.bind(self, T.any(Grape::Endpoint, <HelpersModuleName>))` in `before` and `after` callback blocks
+
+The compiler already generates proper signatures for callback methods so `T.bind` should not be needed (and is in fact unneeded for the other callback methods, `before_validation`, `after_validation` and `finally`). The reason it doesn't work for `before` and `after` is because of a [bug](https://github.com/sorbet/sorbet/issues/7950) in Sorbet itself.
 
 ## Development
 

lib/grape_sorbet/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module GrapeSorbet
-  VERSION = "0.0.2"
+  VERSION = "0.0.3"
 end

lib/tapioca/dsl/compilers/grape_endpoints.rb

@@ -80,13 +80,13 @@ def gather_constants
           end
         end
 
-        HTTP_VERB_METHODS = T.let(
-          [:get, :post, :put, :patch, :delete, :head, :options].freeze,
+        CALLBACKS_METHODS = T.let(
+          [:before, :before_validation, :after_validation, :after, :finally].freeze,
           T::Array[Symbol],
         )
 
-        CALLBACKS_METHODS = T.let(
-          [:before, :before_validation, :after_validation, :after, :finally].freeze,
+        HTTP_VERB_METHODS = T.let(
+          [:get, :post, :put, :patch, :delete, :head, :options].freeze,
           T::Array[Symbol],
         )
 
@@ -100,6 +100,14 @@ def api
           )
         end
 
+        sig { returns(RBI::Scope) }
+        def callbacks_methods_module
+          @callbacks_methods_module ||= T.let(
+            api.create_module(CallbacksMethodsModuleName),
+            T.nilable(RBI::Scope),
+          )
+        end
+
         sig { returns(RBI::Scope) }
         def routing_methods_module
           @routing_methods_module ||= T.let(
@@ -110,6 +118,7 @@ def routing_methods_module
 
         sig { void }
         def create_classes_and_includes
+          api.create_extend(CallbacksMethodsModuleName)
           api.create_extend(RoutingMethodsModuleName)
           create_api_class
           create_endpoint_class
@@ -141,14 +150,27 @@ def create_endpoint_class
           end
         end
 
+        sig { void }
+        def create_callbacks_methods
+          CALLBACKS_METHODS.each do |callback|
+            callbacks_methods_module.create_method(
+              callback.to_s,
+              parameters: [
+                create_block_param("block", type: "T.proc.bind(#{EndpointClassName}).void"),
+              ],
+              return_type: "void",
+            )
+          end
+        end
+
         sig { void }
         def create_routing_methods
           HTTP_VERB_METHODS.each do |verb|
             routing_methods_module.create_method(
               verb.to_s,
               parameters: [
                 create_rest_param("args", type: "T.untyped"),
-                create_block_param("blk", type: "T.nilable(T.proc.bind(#{EndpointClassName}).void)"),
+                create_block_param("block", type: "T.nilable(T.proc.bind(#{EndpointClassName}).void)"),
               ],
               return_type: "void",
             )
@@ -158,26 +180,12 @@ def create_routing_methods
             "route_param",
             parameters: [
               create_param("param", type: "Symbol"),
-              create_opt_param("options", type: "T.nilable(T::Hash[Symbol, T.untyped])", default: "nil"),
-              create_block_param("blk", type: "T.nilable(T.proc.bind(T.class_of(#{APIInstanceClassName})).void)"),
+              create_opt_param("options", type: "T::Hash[Symbol, T.untyped]", default: "{}"),
+              create_block_param("block", type: "T.nilable(T.proc.bind(T.class_of(#{APIInstanceClassName})).void)"),
             ],
             return_type: "void",
           )
         end
-
-        sig { void }
-        def create_callbacks_methods
-          CALLBACKS_METHODS.each do |callback|
-            routing_methods_module.create_method(
-              callback.to_s,
-              parameters: [
-                create_rest_param("args", type: "T.untyped"),
-                create_block_param("blk", type: "T.nilable(T.proc.bind(#{EndpointClassName}).void)"),
-              ],
-              return_type: "void",
-            )
-          end
-        end
       end
     end
   end

lib/tapioca/dsl/helpers/grape_constants_helper.rb

@@ -7,6 +7,7 @@ module Helpers
       module GrapeConstantsHelper
         extend T::Sig
 
+        CallbacksMethodsModuleName = "GeneratedCallbacksMethods"
         RoutingMethodsModuleName = "GeneratedRoutingMethods"
 
         APIInstanceClassName = "PrivateAPIInstance"

rbi/grape.rbi

@@ -2,11 +2,6 @@
 # frozen_string_literal: true
 
 module Grape
-  module DSL::Callbacks::ClassMethods
-    sig { params(block: T.proc.bind(Grape::Endpoint).void).void }
-    def before(&block); end
-  end
-
   module DSL::Desc
     # grape evaluates config_block in the context of a dynamically created module that implements the DSL it exposes
     # at runtime. There's no good way to represent this statically, so block is just typed as T.untyped to prevent

spec/tapioca/dsl/compilers/grape_endpoints_spec.rb

@@ -63,47 +63,50 @@ def authenticate!
                 # typed: strong
 
                 class TwitterAPI
+                  extend GeneratedCallbacksMethods
                   extend GeneratedRoutingMethods
 
-                  module GeneratedRoutingMethods
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def after(*args, &blk); end
+                  module GeneratedCallbacksMethods
+                    sig { params(block: T.proc.bind(PrivateEndpoint).void).void }
+                    def after(&block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def after_validation(*args, &blk); end
+                    sig { params(block: T.proc.bind(PrivateEndpoint).void).void }
+                    def after_validation(&block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def before(*args, &blk); end
+                    sig { params(block: T.proc.bind(PrivateEndpoint).void).void }
+                    def before(&block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def before_validation(*args, &blk); end
+                    sig { params(block: T.proc.bind(PrivateEndpoint).void).void }
+                    def before_validation(&block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def delete(*args, &blk); end
+                    sig { params(block: T.proc.bind(PrivateEndpoint).void).void }
+                    def finally(&block); end
+                  end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def finally(*args, &blk); end
+                  module GeneratedRoutingMethods
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def delete(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def get(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def get(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def head(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def head(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def options(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def options(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def patch(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def patch(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def post(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def post(*args, &block); end
 
-                    sig { params(args: T.untyped, blk: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
-                    def put(*args, &blk); end
+                    sig { params(args: T.untyped, block: T.nilable(T.proc.bind(PrivateEndpoint).void)).void }
+                    def put(*args, &block); end
 
-                    sig { params(param: Symbol, options: T.nilable(T::Hash[Symbol, T.untyped]), blk: T.nilable(T.proc.bind(T.class_of(PrivateAPIInstance)).void)).void }
-                    def route_param(param, options = nil, &blk); end
+                    sig { params(param: Symbol, options: T::Hash[Symbol, T.untyped], block: T.nilable(T.proc.bind(T.class_of(PrivateAPIInstance)).void)).void }
+                    def route_param(param, options = {}, &block); end
                   end
 
                   class PrivateAPIInstance < ::Grape::API::Instance