Destructure block parameter groups and keep union-typed tuple elements in one position

lib/solargraph/complex_type/unique_type.rb

@@ -421,8 +421,8 @@ def resolve_generics_from_context generics_to_resolve, context_type, resolved_ge
       # @return [Array<ComplexType>]
       def resolve_param_generics_from_context generics_to_resolve, context_type, resolved_generic_values
         types = yield self
-        types.each_with_index.flat_map do |ct, i|
-          ct.items.flat_map do |ut|
+        types.each_with_index.map do |ct, i|
+          resolved = ct.items.flat_map do |ut|
             context_params = yield context_type if context_type
             if context_params && context_params[i]
               type_arg = context_params[i]
@@ -431,10 +431,16 @@ def resolve_param_generics_from_context generics_to_resolve, context_type, resol
                                                  resolved_generic_values: resolved_generic_values
               end
             else
-              ut.resolve_generics_from_context generics_to_resolve, nil,
-                                               resolved_generic_values: resolved_generic_values
+              [ut.resolve_generics_from_context(generics_to_resolve, nil,
+                                                resolved_generic_values: resolved_generic_values)]
             end
           end
+          # A single position's resolution may be a union (e.g. a Hash key
+          # type of `String, Symbol` binding one slot of the yielded
+          # [K, V] pair). Keep the union inside one parameter position
+          # instead of splicing its members into extra positions, which
+          # inflates a tuple's arity (Array(K, V) must stay a pair).
+          ComplexType.new(resolved.flat_map { |t| t.is_a?(ComplexType) ? t.items : [t] })
         end
       end
 
@@ -552,8 +558,12 @@ def transform new_name = nil, &transform_type
           new_key_types = @key_types
           new_subtypes = @subtypes
         else
-          new_key_types = @key_types.flat_map { |ct| ct.items.map { |ut| ut.transform(&transform_type) } }
-          new_subtypes = @subtypes.flat_map { |ct| ct.items.map { |ut| ut.transform(&transform_type) } }
+          # Rebuild per parameter position: a position holding (or
+          # transformed into) a union must stay one position, not have
+          # its members spliced in as extra positions (which would
+          # inflate a tuple's arity).
+          new_key_types = @key_types.map { |ct| transform_position(ct, &transform_type) }
+          new_subtypes = @subtypes.map { |ct| transform_position(ct, &transform_type) }
         end
         new_type = recreate(new_name: new_name || name, new_key_types: new_key_types, new_subtypes: new_subtypes,
                             make_rooted: @rooted)
@@ -564,6 +574,18 @@ def expand named_types
         named_types[name] || self
       end
 
+      # Transform one parameter position, keeping however many types the
+      # transformation produces inside that single position.
+      #
+      # @param position_type [ComplexType]
+      # @yieldparam t [UniqueType]
+      # @yieldreturn [self]
+      # @return [ComplexType]
+      def transform_position position_type, &transform_type
+        results = position_type.items.map { |ut| ut.transform(&transform_type) }
+        ComplexType.new(results.flat_map { |t| t.is_a?(ComplexType) ? t.items : [t] })
+      end
+
       # Generate a ComplexType that fully qualifies this type's namespaces.
       #
       # @param api_map [ApiMap] The ApiMap that performs qualification

lib/solargraph/parser/parser_gem/node_processors/args_node.rb

@@ -12,6 +12,10 @@ def process
                 forward(callable)
               else
                 node.children.each do |u|
+                  if u.type == :mlhs
+                    process_mlhs_param(callable, u)
+                    next
+                  end
                   loc = get_node_location(u)
                   locals.push Solargraph::Pin::Parameter.new(
                     location: loc,
@@ -54,6 +58,57 @@ def forward callable
           def get_decl node
             node.type
           end
+
+          # A destructured parameter group (`|(a, b), c|`). The group
+          # itself occupies one position in the block signature; the
+          # variables inside it are locals whose types are projected from
+          # the group's tuple type by element position (see
+          # Pin::Parameter#mlhs_path).
+          #
+          # @param callable [Pin::Callable]
+          # @param mlhs_node [AST::Node]
+          # @return [void]
+          def process_mlhs_param callable, mlhs_node
+            loc = get_node_location(mlhs_node)
+            locals.push Solargraph::Pin::Parameter.new(
+              location: loc,
+              closure: callable,
+              comments: comments_for(node),
+              name: region.code_for(mlhs_node) || '()',
+              # @sg-ignore Need to add nil check here
+              presence: callable.location.range,
+              decl: :mlhs,
+              source: :parser
+            )
+            # @sg-ignore flow sensitive typing doesn't track that .last is the Parameter just pushed
+            callable.parameters.push locals.last
+            add_mlhs_locals callable, mlhs_node, [callable.parameters.length - 1]
+          end
+
+          # @param callable [Pin::Callable]
+          # @param mlhs_node [AST::Node]
+          # @param path [::Array<Integer>]
+          # @return [void]
+          def add_mlhs_locals callable, mlhs_node, path
+            mlhs_node.children.each_with_index do |child, i|
+              if child.type == :mlhs
+                add_mlhs_locals callable, child, path + [i]
+              else
+                loc = get_node_location(child)
+                locals.push Solargraph::Pin::Parameter.new(
+                  location: loc,
+                  closure: callable,
+                  comments: comments_for(node),
+                  name: child.children[0].to_s,
+                  # @sg-ignore Need to add nil check here
+                  presence: callable.location.range,
+                  decl: :arg,
+                  mlhs_path: path + [i],
+                  source: :parser
+                )
+              end
+            end
+          end
         end
       end
     end

lib/solargraph/pin/block.rb

@@ -54,6 +54,26 @@ def destructure_yield_types yield_types, parameters
         parameters.map.with_index { |_, idx| yield_types[idx] || ComplexType::UNDEFINED }
       end
 
+      # Whether the yielded types line up with the block's parameters one
+      # for one - either a tuple destructured across them, or one yielded
+      # type per parameter. When they do not (Ruby auto-splats a single
+      # yielded value whose arity we cannot match), the fallback in
+      # destructure_yield_types hands position 0 the whole value, which is
+      # worse than leaving the parameters undefined.
+      #
+      # @param yield_types [::Array<ComplexType>]
+      # @param parameters [::Array<Parameter>]
+      # @return [Boolean]
+      def per_position_yield_types? yield_types, parameters
+        return true if yield_types.length == parameters.length
+        return false unless yield_types.length == 1
+
+        only_type = yield_types.first
+        return false if only_type.nil?
+
+        only_type.tuple? && only_type.all_params.length == parameters.length
+      end
+
       # @param api_map [ApiMap]
       # @return [::Array<ComplexType>]
       def typify_parameters api_map
@@ -63,6 +83,8 @@ def typify_parameters api_map
         locals = clip.locals - [self]
         # @sg-ignore Need to add nil check here
         meths = chain.define(api_map, closure, locals)
+        # @type [::Array<ComplexType>, nil]
+        partial = nil
         # @todo Convert logic to use signatures
         # @param meth [Pin::Method]
         meths.each do |meth|
@@ -87,8 +109,16 @@ def typify_parameters api_map
             end
           end
           return param_types if param_types.all?(&:defined?)
+
+          # remember the best partial result so a single unresolvable
+          # yield type (e.g. an unbound generic) doesn't discard the
+          # positions that did resolve - but only when the positions
+          # actually correspond, never for the auto-splat fallback
+          if per_position_yield_types?(yield_types, parameters) && param_types.any? { |t| t&.defined? }
+            partial ||= param_types
+          end
         end
-        parameters.map { ComplexType::UNDEFINED }
+        partial&.map { |t| t || ComplexType::UNDEFINED } || parameters.map { ComplexType::UNDEFINED }
       end
 
       private

lib/solargraph/pin/parameter.rb

@@ -6,22 +6,30 @@ class Parameter < LocalVariable
       # @return [::Symbol]
       attr_reader :decl
 
-      # @return [String]
+      # @return [String, nil]
       attr_reader :asgn_code
 
       # allow this to be set to the method after the method itself has
       # been created
       attr_writer :closure
 
-      # @param decl [::Symbol] :arg, :optarg, :kwarg, :kwoptarg, :restarg, :kwrestarg, :block, :blockarg
+      # @param decl [::Symbol] :arg, :optarg, :kwarg, :kwoptarg, :restarg, :kwrestarg, :block, :blockarg, :mlhs
       # @param asgn_code [String, nil]
+      # @param mlhs_path [::Array<Integer>, nil] for a variable inside a
+      #   destructured block parameter group (`|(a, b), c|`): the group's
+      #   position in the block signature followed by the element index at
+      #   each nesting level (`a` -> [0, 0], `b` -> [0, 1])
       # @param [Hash{Symbol => Object}] splat
-      def initialize decl: :arg, asgn_code: nil, **splat
+      def initialize decl: :arg, asgn_code: nil, mlhs_path: nil, **splat
         super(**splat)
         @asgn_code = asgn_code
         @decl = decl
+        @mlhs_path = mlhs_path
       end
 
+      # @return [::Array<Integer>, nil]
+      attr_reader :mlhs_path
+
       def type_location
         super || closure&.type_location
       end
@@ -211,6 +219,11 @@ def typify api_map
         new_type = super
         return new_type if new_type.defined?
 
+        if mlhs_path && closure.is_a?(Pin::Block)
+          projected = typify_mlhs_element(api_map)
+          return adjust_type api_map, projected.self_to_type(full_context) if projected.defined?
+        end
+
         # sniff based on param tags
         new_type = closure.is_a?(Pin::Block) ? typify_block_param(api_map) : typify_method_param(api_map)
 
@@ -283,11 +296,35 @@ def param_tag
         params[index] if index && params[index] && (params[index].name.nil? || params[index].name.empty?)
       end
 
+      # Project this variable's type out of its destructured parameter
+      # group's tuple type, one element index per nesting level.
+      #
+      # @param api_map [ApiMap]
+      # @return [ComplexType]
+      def typify_mlhs_element api_map
+        block_pin = closure
+        path = mlhs_path
+        return ComplexType::UNDEFINED unless path && block_pin.is_a?(Pin::Block) && block_pin.receiver
+
+        type = block_pin.typify_parameters(api_map)[path.first]
+        path.drop(1).each do |idx|
+          # @sg-ignore flow sensitive typing unions rather than overrides types across multiple sequential reassignments
+          return ComplexType::UNDEFINED if type.nil? || !type.tuple?
+
+          # @sg-ignore flow sensitive typing unions rather than overrides types across multiple sequential reassignments
+          type = type.all_params[idx]
+        end
+        type || ComplexType::UNDEFINED
+      end
+
       # @param api_map [ApiMap]
       # @return [ComplexType]
       def typify_block_param api_map
         block_pin = closure
-        return block_pin.typify_parameters(api_map)[index] if block_pin.is_a?(Pin::Block) && block_pin.receiver && index
+        if block_pin.is_a?(Pin::Block) && block_pin.receiver && index
+          typed = block_pin.typify_parameters(api_map)[index]
+          return typed unless typed.nil?
+        end
         ComplexType::UNDEFINED
       end
 
@@ -305,8 +342,9 @@ def typify_method_param api_map
             found = p
             break
           end
-          if found.nil? && !index.nil? && params[index] && (params[index].name.nil? || params[index].name.empty?)
-            found = params[index]
+          if found.nil? && !index.nil?
+            positional = params[index]
+            found = positional if positional && (positional.name.nil? || positional.name.empty?)
           end
           unless found.nil? || found.types.nil?
             return ComplexType.try_parse(*found.types).qualify(api_map,
@@ -343,7 +381,7 @@ def resolve_reference ref, api_map, skip
         return nil if skip.include?(ref)
         skip.push ref
         parts = ref.split(/[.#]/)
-        if parts.first.empty?
+        if parts.first.to_s.empty?
           path = "#{namespace}#{ref}"
         else
           fqns = api_map.qualify(parts.first, namespace)