Fix collection counter and iteration detection when forwarded through anonymous arguments

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*

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,26 +790,52 @@ 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
         @__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

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
+        "<p>#{@item.name} #{@counter} #{@iteration.last?}</p>".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
+        "<p>#{@item.name} #{@counter}</p>".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
+        "<p>#{@options[:consumed_options].name}</p>".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