Discourage use of private methods in Rails helpers
Summary
Methods in helper modules (files in app/helpers and ee/app/helpers) that are declared private are not private to the scope of the module.
Because all helpers are included into the scope of all views, these methods are accessible to each view.
We have many helper methods declared private.
The authors of these presumably thought they would only be accessible from within the scope of the current helper module.
This creates a risk of private helpers being used outside of their intended scope.
See !140345 (comment 1703525828) for an example.
Example of using a private helper method in a view
diff --git a/app/helpers/demo_helper.rb b/app/helpers/demo_helper.rb
new file mode 100644
index 000000000000..36896da8820e
--- /dev/null
+++ b/app/helpers/demo_helper.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module DemoHelper
+ private
+
+ def demo_private_helper
+ "private_helper"
+ end
+end
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 457eaf5e194a..0479abc25a26 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -1,3 +1,5 @@
+=debug demo_private_helper
+
- add_to_breadcrumbs _("Issues"), project_issues_path(@project)
- breadcrumb_title @issue.to_reference
Improvements
To reduce the risk of these private helper methods being used accidentally, we could:
- Rename existing
privatehelper methods with and_prefix to signal the intent that they are not to be used. - Create a cop to enforce
_prefixes and recommend alternative strategies for complicated helpers (extraction to extracted into classes, presenters or view components.
Risks
Renaming existing helper methods runs the risk of breaking existing code without test coverage.