Skip to content

Commit 0966b19

Browse files
vinistockrafaelfranca
authored andcommitted
Turn ActionController::Base inclusions explicit
Currently, we are using dynamic inclusions to guarantee that the list of MODULES is always up to date with what gets included into Base. However, that prevents static analysis tools from understanding the ancestors of controllers, which prevents completion and other editor features from working correctly. We can instead use a unit test to verify that both lists are synchronized, which retains the original behavior while allowing for more accurate static analysis.
1 parent 16d8b82 commit 0966b19

File tree

2 files changed

+55
-15
lines changed

2 files changed

+55
-15
lines changed

actionpack/lib/action_controller/base.rb

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,6 @@ def self.without_modules(*modules)
231231
AbstractController::Rendering,
232232
AbstractController::Translation,
233233
AbstractController::AssetPaths,
234-
235234
Helpers,
236235
UrlFor,
237236
Redirecting,
@@ -261,26 +260,55 @@ def self.without_modules(*modules)
261260
HttpAuthentication::Token::ControllerMethods,
262261
DefaultHeaders,
263262
Logging,
264-
265-
# Before callbacks should also be executed as early as possible, so also include
266-
# them at the bottom.
267263
AbstractController::Callbacks,
268-
269-
# Append rescue at the bottom to wrap as much as possible.
270264
Rescue,
271-
272-
# Add instrumentations hooks at the bottom, to ensure they instrument all the
273-
# methods properly.
274265
Instrumentation,
275-
276-
# Params wrapper should come before instrumentation so they are properly showed
277-
# in logs
278266
ParamsWrapper
279267
]
280268

281-
MODULES.each do |mod|
282-
include mod
283-
end
269+
include AbstractController::Rendering
270+
include AbstractController::Translation
271+
include AbstractController::AssetPaths
272+
include Helpers
273+
include UrlFor
274+
include Redirecting
275+
include ActionView::Layouts
276+
include Rendering
277+
include Renderers::All
278+
include ConditionalGet
279+
include EtagWithTemplateDigest
280+
include EtagWithFlash
281+
include Caching
282+
include MimeResponds
283+
include ImplicitRender
284+
include StrongParameters
285+
include ParameterEncoding
286+
include Cookies
287+
include Flash
288+
include FormBuilder
289+
include RequestForgeryProtection
290+
include ContentSecurityPolicy
291+
include PermissionsPolicy
292+
include RateLimiting
293+
include AllowBrowser
294+
include Streaming
295+
include DataStreaming
296+
include HttpAuthentication::Basic::ControllerMethods
297+
include HttpAuthentication::Digest::ControllerMethods
298+
include HttpAuthentication::Token::ControllerMethods
299+
include DefaultHeaders
300+
include Logging
301+
# Before callbacks should also be executed as early as possible, so also include
302+
# them at the bottom.
303+
include AbstractController::Callbacks
304+
# Append rescue at the bottom to wrap as much as possible.
305+
include Rescue
306+
# Add instrumentations hooks at the bottom, to ensure they instrument all the
307+
# methods properly.
308+
include Instrumentation
309+
# Params wrapper should come before instrumentation so they are properly showed
310+
# in logs
311+
include ParamsWrapper
284312
setup_renderer!
285313

286314
# Define some internal variables that should not be propagated to the view.

actionpack/test/controller/base_test.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,3 +350,15 @@ def test_named_routes_with_path_without_doing_a_request_first
350350
end
351351
end
352352
end
353+
354+
class BaseTest < ActiveSupport::TestCase
355+
def test_included_modules_are_tracked
356+
base_content = File.read("#{__dir__}/../../lib/action_controller/base.rb")
357+
included_modules = base_content.scan(/(?<=include )[A-Z].*/)
358+
359+
assert_equal(
360+
ActionController::Base::MODULES.map { |m| m.to_s.delete_prefix("ActionController::") },
361+
included_modules
362+
)
363+
end
364+
end

0 commit comments

Comments
 (0)