From 9de81a8e60c435087b333c1a741ce4ebffad534f Mon Sep 17 00:00:00 2001 From: "Ben Sheldon [he/him]" Date: Mon, 12 Oct 2026 06:44:41 -0700 Subject: [PATCH] Fix collection counter and iteration detection when forwarded through anonymous arguments --- docs/CHANGELOG.md | 4 + lib/view_component/base.rb | 50 ++++++-- test/sandbox/test/collection_test.rb | 168 +++++++++++++++++++++++++++ 3 files changed, 212 insertions(+), 10 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7e55e92d5..ebf3163de 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Fix collection counter and iteration detection, and collection and reserved parameter validation, when `initialize` arguments are forwarded through anonymous arguments (`...` or `**`), such as by prepended modules and subclasses calling `super`. + + *Ben Sheldon* + * Update development and documentation dependencies, baseline existing Audition findings, and accommodate updated lint rules. *Joel Hawksley* diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index b525c7f5a..21b0d5460 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -716,7 +716,7 @@ def __vc_compiler # @param parameter [Symbol] The parameter name used when rendering elements of a collection. def with_collection_parameter(parameter) @__vc_provided_collection_parameter = parameter - @__vc_initialize_parameters = nil + @__vc_collection_arguments = nil end # Strips trailing whitespace from templates before compiling them. @@ -746,10 +746,14 @@ def strip_trailing_whitespace? # rendering is optional. # @private def __vc_validate_collection_parameter!(validate_default: false) - parameter = validate_default ? __vc_collection_parameter : __vc_provided_collection_parameter + if validate_default + return if __vc_collection_arguments[:parameter] - return unless parameter - return if __vc_initialize_parameter_names.include?(parameter) || __vc_splatted_keyword_argument_present? + raise MissingCollectionArgumentError.new(name, __vc_collection_parameter) + end + + parameter = __vc_provided_collection_parameter + return if __vc_collection_parameter_accepted?(parameter) raise MissingCollectionArgumentError.new(name, parameter) end @@ -776,7 +780,7 @@ def __vc_collection_counter_parameter # @private def __vc_counter_argument_present? - __vc_initialize_parameter_names.include?(__vc_collection_counter_parameter) + __vc_collection_arguments[:counter] end # @private @@ -786,13 +790,31 @@ def __vc_collection_iteration_parameter # @private def __vc_iteration_argument_present? - __vc_initialize_parameter_names.include?(__vc_collection_iteration_parameter) + __vc_collection_arguments[:iteration] end private + def __vc_collection_arguments + @__vc_collection_arguments ||= { + parameter: __vc_collection_parameter_accepted?(__vc_collection_parameter), + counter: __vc_initialize_parameter_names.include?(__vc_collection_counter_parameter), + iteration: __vc_initialize_parameter_names.include?(__vc_collection_iteration_parameter) + }.freeze + end + + def __vc_collection_parameter_accepted?(parameter) + parameter.nil? || __vc_initialize_parameter_names.include?(parameter) || __vc_splatted_keyword_argument_present? + end + def __vc_splatted_keyword_argument_present? - __vc_initialize_parameters.flatten.include?(:keyrest) + prepended_modules = ancestors.grep(Class).flat_map { |klass| klass.ancestors.take_while { |mod| mod != klass } } + + __vc_initialize_signatures.any? do |owner, parameters| + parameters.any? do |type, name| + type == :keyrest && (name != :** || !prepended_modules.include?(owner)) + end + end end def __vc_initialize_parameter_names @@ -800,12 +822,20 @@ def __vc_initialize_parameter_names if respond_to?(:attribute_names) attribute_names.map(&:to_sym) else - __vc_initialize_parameters.map(&:last) + __vc_initialize_signatures.flat_map { |_owner, parameters| parameters.map(&:last) } end end - def __vc_initialize_parameters - @__vc_initialize_parameters ||= instance_method(:initialize).parameters + def __vc_initialize_signatures + signatures = [] + method = instance_method(:initialize) + while method + signatures << [method.owner, method.parameters] + break unless method.parameters.any? { |type, _| type == :keyrest } + + method = method.super_method + end + signatures end def __vc_provided_collection_parameter diff --git a/test/sandbox/test/collection_test.rb b/test/sandbox/test/collection_test.rb index 2a73edd88..9b10d9afb 100644 --- a/test/sandbox/test/collection_test.rb +++ b/test/sandbox/test/collection_test.rb @@ -56,5 +56,173 @@ def test_collection_escapes_html_unsafe_around_render_output assert_includes @rendered_content, "<script>alert(1)</script>" assert_predicate @rendered_content, :html_safe? end + + module ForwardAllArguments + def initialize(...) + super + end + end + + module ForwardAnonymousSplats + def initialize(*, **) + super + end + end + + class ForwardedCounterComponent < ViewComponent::Base + with_collection_parameter :item + + def initialize(item:, item_counter:, item_iteration:) + @item = item + @counter = item_counter + @iteration = item_iteration + end + + def call + "

#{@item.name} #{@counter} #{@iteration.last?}

".html_safe + end + end + + class ForwardAllArgumentsComponent < ForwardedCounterComponent + prepend ForwardAllArguments + end + + class ForwardAnonymousSplatsComponent < ForwardedCounterComponent + prepend ForwardAnonymousSplats + end + + def test_collection_counter_through_prepended_argument_forwarding + render_inline(ForwardAllArgumentsComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock 0 false") + assert_selector("p", text: "Mints 1 true") + end + + def test_collection_counter_through_prepended_anonymous_splats + render_inline(ForwardAnonymousSplatsComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock 0 false") + assert_selector("p", text: "Mints 1 true") + end + + class ForwardingChildComponent < ForwardedCounterComponent + def initialize(**) + super + end + end + + class ForwardingGrandchildComponent < ForwardingChildComponent + def initialize(...) + super + end + end + + def test_collection_counter_through_subclasses_forwarding_to_super + render_inline(ForwardingGrandchildComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock 0 false") + assert_selector("p", text: "Mints 1 true") + end + + module ConsumeCounter + def initialize(*, item_counter:, **) + @counter = item_counter + super(*, **) + end + end + + class ConsumedCounterComponent < ViewComponent::Base + prepend ConsumeCounter + + with_collection_parameter :item + + def initialize(item:) + @item = item + end + + def call + "

#{@item.name} #{@counter}

".html_safe + end + end + + def test_collection_counter_consumed_by_prepended_module + render_inline(ConsumedCounterComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock 0") + assert_selector("p", text: "Mints 1") + end + + class NarrowedCounterComponent < ForwardedCounterComponent + def initialize(item:) + super(item:, item_counter: 99, item_iteration: ActionView::PartialIteration.new(1)) + end + end + + def test_collection_counter_not_passed_to_subclass_that_narrows_initialize + render_inline(NarrowedCounterComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock 99 true") + assert_selector("p", text: "Mints 99 true") + end + + class ForwardedMissingParameterComponent < ViewComponent::Base + prepend ForwardAllArguments + + def initialize(name:) + @name = name + end + + def call + @name + end + end + + def test_prepended_argument_forwarding_does_not_skip_collection_parameter_validation + assert_raises(ViewComponent::MissingCollectionArgumentError) do + render_inline(ForwardedMissingParameterComponent.with_collection(@products)) + end + end + + class ForwardedReservedParameterComponent < ViewComponent::Base + prepend ForwardAllArguments + + def initialize(content:) + @content = content + end + + def call + "" + end + end + + def test_prepended_argument_forwarding_does_not_skip_reserved_parameter_validation + with_new_cache do + assert_raises(ViewComponent::ReservedParameterError) do + ForwardedReservedParameterComponent.__vc_compile(raise_errors: true) + end + end + end + + module ConsumeOptions + def initialize(*, **options) + @options = options + super() + end + end + + class ConsumedOptionsComponent < ViewComponent::Base + prepend ConsumeOptions + + def call + "

#{@options[:consumed_options].name}

".html_safe + end + end + + def test_prepended_named_keyword_splat_accepts_collection_parameter + render_inline(ConsumedOptionsComponent.with_collection(@products)) + + assert_selector("p", text: "Radio clock") + assert_selector("p", text: "Mints") + end end end