From f8fa9708b5559231e81c31f49a388fdc175eae95 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 16 Aug 2023 16:50:43 -0700 Subject: [PATCH 1/3] Fix gathering of abstract Controllers Helpers methods are generated for abstract controllers, and then are inherted by children, so we should be gathering them. In the added test the generated RBI for UserController references ::ApplicationController::HelperMethods. --- .../compilers/action_controller_helpers.rb | 2 +- .../action_controller_helpers_spec.rb | 96 ++++++++++++++++++- 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/lib/tapioca/dsl/compilers/action_controller_helpers.rb b/lib/tapioca/dsl/compilers/action_controller_helpers.rb index b9d9c3885..b4b052afc 100644 --- a/lib/tapioca/dsl/compilers/action_controller_helpers.rb +++ b/lib/tapioca/dsl/compilers/action_controller_helpers.rb @@ -126,7 +126,7 @@ class << self sig { override.returns(T::Enumerable[Module]) } def gather_constants - descendants_of(::ActionController::Base).reject(&:abstract?).select(&:name) + descendants_of(::ActionController::Base).select(&:name) end end diff --git a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb index cf4853d50..026a74ba2 100644 --- a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb +++ b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb @@ -50,7 +50,7 @@ class HandController < UserController assert_equal(["HandController", "UserController"], gathered_constants) end - it "ignores abstract subclasses of ActionController" do + it "gathers abstract subclasses of ActionController" do add_ruby_file("content.rb", <<~RUBY) class UserController < ActionController::Base end @@ -60,7 +60,7 @@ class HomeController < ActionController::Base end RUBY - assert_equal(["UserController"], gathered_constants) + assert_equal(["HomeController", "UserController"], gathered_constants) end it "ignores anonymous subclasses of ActionController" do @@ -349,6 +349,98 @@ class HelperProxy < ::ActionView::Base assert_equal(expected, rbi_for(:UserController)) end + + it "works correctly with abstract ApplicationController" do + add_ruby_file("greet_helper.rb", <<~RUBY) + module GreetHelper + def greet(user) + # ... + end + end + RUBY + + add_ruby_file("application_controller.rb", <<~RUBY) + # typed: false + + class ApplicationController < ActionController::Base + abstract! + + helper_method :foo + def foo; end + end + RUBY + add_ruby_file("user_controller.rb", <<~RUBY) + class UserController < ApplicationController + helper GreetHelper + end + RUBY + add_ruby_file("posts_controller.rb", <<~RUBY) + class PostsController < ApplicationController + end + RUBY + + expected = <<~RBI + # typed: strong + + class ApplicationController + sig { returns(HelperProxy) } + def helpers; end + + module HelperMethods + include ::ActionController::Base::HelperMethods + + sig { returns(T.untyped) } + def foo; end + end + + class HelperProxy < ::ActionView::Base + include HelperMethods + end + end + RBI + assert_equal expected, rbi_for(:ApplicationController) + + expected = <<~RBI + # typed: strong + + class UserController + sig { returns(HelperProxy) } + def helpers; end + + module HelperMethods + include ::ActionController::Base::HelperMethods + include ::ApplicationController::HelperMethods + include ::GreetHelper + end + + class HelperProxy < ::ActionView::Base + include HelperMethods + end + end + RBI + assert_equal expected, rbi_for(:UserController) + + expected = <<~RBI + # typed: strong + + class PostsController + sig { returns(HelperProxy) } + def helpers; end + + module HelperMethods + include ::ActionController::Base::HelperMethods + + sig { returns(T.untyped) } + def foo; end + end + + class HelperProxy < ::ActionView::Base + include HelperMethods + end + end + RBI + assert_equal expected, rbi_for(:PostsController) + end end end end From 854a947122ba2d73705699bb509d7cd47ca51983 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 16 Aug 2023 17:07:34 -0700 Subject: [PATCH 2/3] Only generate RBI for controllers with helpers Since Rails 6.1, when a controller has identical helpers to its superclass Rails will not generate a new HelperMethods modules (which saves significant memory). We should do the same to avoid declaring modules which don't actually exist in reality, this also avoids a lot of unnecesary generated RBI files which use helper/helper_methods appropriately sparingly. --- .../compilers/action_controller_helpers.rb | 4 +- .../action_controller_helpers_spec.rb | 93 +++---------------- 2 files changed, 18 insertions(+), 79 deletions(-) diff --git a/lib/tapioca/dsl/compilers/action_controller_helpers.rb b/lib/tapioca/dsl/compilers/action_controller_helpers.rb index b4b052afc..952b3d985 100644 --- a/lib/tapioca/dsl/compilers/action_controller_helpers.rb +++ b/lib/tapioca/dsl/compilers/action_controller_helpers.rb @@ -126,7 +126,9 @@ class << self sig { override.returns(T::Enumerable[Module]) } def gather_constants - descendants_of(::ActionController::Base).select(&:name) + descendants_of(::ActionController::Base).select(&:name).select do |klass| + klass.const_defined?(:HelperMethods, false) + end end end diff --git a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb index 026a74ba2..44cc4d5a7 100644 --- a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb +++ b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb @@ -13,9 +13,13 @@ class ActionControllerHelpersSpec < ::DslSpec assert_empty(gathered_constants) end - it "gathers only ActionController subclasses" do + it "gathers only ActionController subclasses with helpers" do add_ruby_file("content.rb", <<~RUBY) class UserController < ActionController::Base + helper_method :foo + end + + class AnotherController end class User @@ -32,18 +36,21 @@ module UserHelper class UserController < ActionController::Base include UserHelper + helper_method :foo end RUBY assert_equal(["UserController"], gathered_constants) end - it "gathers subclasses of ActionController subclasses" do + it "gathers subclasses of ActionController subclasses with helpers" do add_ruby_file("content.rb", <<~RUBY) class UserController < ActionController::Base + helper_method :foo end class HandController < UserController + helper_method :bar end RUBY @@ -53,10 +60,12 @@ class HandController < UserController it "gathers abstract subclasses of ActionController" do add_ruby_file("content.rb", <<~RUBY) class UserController < ActionController::Base + helper_method :foo end class HomeController < ActionController::Base abstract! + helper_method :foo end RUBY @@ -65,7 +74,9 @@ class HomeController < ActionController::Base it "ignores anonymous subclasses of ActionController" do add_ruby_file("content.rb", <<~RUBY) - Class.new(ActionController::Base) + Class.new(ActionController::Base) do + helper_method :foo + end RUBY assert_equal([], gathered_constants) @@ -73,35 +84,6 @@ class HomeController < ActionController::Base end describe "decorate" do - it "generates empty helper module when there are no helper methods specified" do - add_ruby_file("controller.rb", <<~RUBY) - class UserController < ActionController::Base - def current_user_name - # ... - end - end - RUBY - - expected = <<~RBI - # typed: strong - - class UserController - sig { returns(HelperProxy) } - def helpers; end - - module HelperMethods - include ::ActionController::Base::HelperMethods - end - - class HelperProxy < ::ActionView::Base - include HelperMethods - end - end - RBI - - assert_equal(expected, rbi_for(:UserController)) - end - it "generates helper module and helper proxy class if helper_method target does not exist" do add_ruby_file("controller.rb", <<~RUBY) class BaseController < ActionController::Base @@ -151,31 +133,6 @@ class HelperProxy < ::ActionView::Base RBI assert_equal(expected, rbi_for(:BaseController)) - - expected = <<~RBI - # typed: strong - - class UserController - sig { returns(HelperProxy) } - def helpers; end - - module HelperMethods - include ::ActionController::Base::HelperMethods - - sig { returns(T.untyped) } - def current_user_name; end - - sig { params(user_id: ::Integer).void } - def notify_user(user_id); end - end - - class HelperProxy < ::ActionView::Base - include HelperMethods - end - end - RBI - - assert_equal(expected, rbi_for(:UserController)) end it "generates helper module and helper proxy class when defining helper using helper_method" do @@ -378,6 +335,7 @@ class UserController < ApplicationController class PostsController < ApplicationController end RUBY + assert_equal(["ApplicationController", "UserController"], gathered_constants) expected = <<~RBI # typed: strong @@ -419,27 +377,6 @@ class HelperProxy < ::ActionView::Base end RBI assert_equal expected, rbi_for(:UserController) - - expected = <<~RBI - # typed: strong - - class PostsController - sig { returns(HelperProxy) } - def helpers; end - - module HelperMethods - include ::ActionController::Base::HelperMethods - - sig { returns(T.untyped) } - def foo; end - end - - class HelperProxy < ::ActionView::Base - include HelperMethods - end - end - RBI - assert_equal expected, rbi_for(:PostsController) end end end From 2a836ea31f17b22e52e8ce8d649e381b4b77ee8e Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 17 Aug 2023 10:36:38 -0700 Subject: [PATCH 3/3] Update spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb Co-authored-by: Ufuk Kayserilioglu --- spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb index 44cc4d5a7..a6bea7b93 100644 --- a/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb +++ b/spec/tapioca/dsl/compilers/action_controller_helpers_spec.rb @@ -19,7 +19,7 @@ class UserController < ActionController::Base helper_method :foo end - class AnotherController + class AnotherController < ActionController::Base end class User