Check restarg argument types against the receiver's element type

TypeChecker#signature_argument_problems_for used to bail out on any
signature with a restarg parameter, skipping type checking entirely
for the rest of the call. That's why `y = [1]; y.push 'two'` (Fred's
second example on #1223) went unflagged even though `push` expects an
Integer.

Restarg params are now checked argument-by-argument against the
restarg's declared type, resolved against the receiver's actual
generic parameters (e.g. `Integer` for an `Array<Integer>` receiver).
Trailing positional parameters and an implicit kwargs hash appended
to the call's arguments are excluded from the restarg's own checks.

This surfaced a real bug in RbsTranslator#to_parameter_pin: restarg
and kwrestarg parameters had their per-element type discarded and
hardcoded to bare `Array` / `Hash{Symbol => Object}`, so there was
never any element type to check against in the first place. Fixed to
preserve the real per-element type, falling back to the old bare
Array/Hash only when the element type is genuinely untyped (e.g. an
inline `#: (*bar) -> bool` annotation with no declared element type).
Two specs in spec/pin/method_spec.rb asserted the old erased-to-bare
behavior and are updated to reflect the now-tracked type.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LCTZUQv2ijSU5Cqgw4CU1f

Author: apiology

lib/solargraph/rbs_translator.rb

@@ -25,16 +25,45 @@ def self.to_complex_type(type)
     # @param closure [Pin::Closure]
     # @return [Pin::Parameter]
     def self.to_parameter_pin(param_type, name, decl, closure)
-      return_type = if decl == :restarg
-        ComplexType.parse('Array')
-      elsif decl == :kwrestarg
-        ComplexType.parse('Hash{Symbol => Object}')
-      else
-        RbsTranslator.to_complex_type(param_type.type)
-      end
+      return_type = case decl
+                    when :restarg
+                      # @sg-ignore RBS type understanding issue - see to_complex_type
+                      RbsTranslator.to_restarg_return_type(param_type.type)
+                    when :kwrestarg
+                      # @sg-ignore RBS type understanding issue - see to_complex_type
+                      RbsTranslator.to_kwrestarg_return_type(param_type.type)
+                    else
+                      # @sg-ignore RBS type understanding issue - to_complex_type's own param type is too narrow
+                      RbsTranslator.to_complex_type(param_type.type)
+                    end
       Solargraph::Pin::Parameter.new(decl: decl, name: name, closure: closure, return_type: return_type, source: :rbs, type_location: to_sg_location(param_type.location) || closure.type_location)
     end
 
+    # The type of the local variable a restarg is captured into
+    # inside the method body - a wrapped Array of its per-element
+    # type, e.g. `Array<Integer>` for `*args: Integer`. When the
+    # element type isn't known (e.g. an untyped inline `#:`
+    # annotation), falls back to a bare, unparameterized Array.
+    #
+    # @param elem_rbs_type [RBS::Types::Bases::Base]
+    # @return [ComplexType]
+    def self.to_restarg_return_type elem_rbs_type
+      elem_type = RbsTranslator.to_complex_type(elem_rbs_type)
+      return ComplexType.parse('Array') if elem_type.undefined?
+      ComplexType.new([ComplexType::UniqueType.new('Array', [], [elem_type], rooted: true, parameters_type: :list)])
+    end
+
+    # Likewise, the type of the local variable a kwrestarg is
+    # captured into - a wrapped Hash of Symbol to its per-value type.
+    #
+    # @param elem_rbs_type [RBS::Types::Bases::Base]
+    # @return [ComplexType]
+    def self.to_kwrestarg_return_type elem_rbs_type
+      elem_type = RbsTranslator.to_complex_type(elem_rbs_type)
+      return ComplexType.parse('Hash{Symbol => Object}') if elem_type.undefined?
+      ComplexType.new([ComplexType::UniqueType.new('Hash', [ComplexType.try_parse('Symbol')], [elem_type], rooted: true, parameters_type: :hash)])
+    end
+
     # @param method_type [RBS::MethodType]
     # @param closure [Pin::Closure]
     # @param parameter_names [Array<String>]

lib/solargraph/type_checker.rb

@@ -407,10 +407,12 @@ def argument_problems_for chain, api_map, closure_pin, locals, location
         return [] if !rules.validate_calls? || base.links.first.is_a?(Solargraph::Source::Chain::ZSuper)
 
         all_errors = []
+        receiver_type = base.base.infer(api_map, closure_pin, locals)
         pin.signatures.sort_by { |sig| sig.parameters.length }.each do |sig|
           params = param_details_from_stack(sig, pins)
 
-          signature_errors = signature_argument_problems_for location, locals, closure_pin, params, arguments, sig, pin
+          signature_errors = signature_argument_problems_for(location, locals, closure_pin, params, arguments, sig,
+                                                             pin, receiver_type)
 
           if signature_errors.empty?
             # we found a signature that works - meaning errors from
@@ -431,16 +433,28 @@ def argument_problems_for chain, api_map, closure_pin, locals, location
     # @param arguments [Array<Source::Chain>]
     # @param sig [Pin::Signature]
     # @param pin [Pin::Method]
+    # @param receiver_type [ComplexType] the type of the object the
+    #   method is being called on, used to resolve the restarg's
+    #   declared type (e.g. `Elem` for `Array#push`) against the
+    #   receiver's actual generic parameters (e.g. `Integer` for an
+    #   `Array<Integer>` receiver)
     #
     # @return [Array<Problem>]
-    def signature_argument_problems_for location, locals, closure_pin, params, arguments, sig, pin
+    def signature_argument_problems_for location, locals, closure_pin, params, arguments, sig, pin, receiver_type
       errors = []
       # @todo add logic mapping up restarg parameters with
       #   arguments (including restarg arguments).  Use tuples
       #   when possible, and when not, ensure provably
       #   incorrect situations are detected.
       sig.parameters.each_with_index do |par, idx|
-        return errors if par.decl == :restarg # bail out and assume the rest is valid pending better arg processing
+        if par.decl == :restarg
+          # A restarg absorbs every remaining positional argument at
+          # the call site - check each of them against the restarg's
+          # own declared/resolved type instead of bailing out on the
+          # whole signature. This is what catches e.g. `y.push('two')`
+          # on a `y: Array<Integer>`.
+          return restarg_problems_for(location, locals, closure_pin, arguments, sig, pin, receiver_type, par, idx)
+        end
         argchain = arguments[idx]
         if argchain.nil?
           final_arg = arguments.last
@@ -500,6 +514,91 @@ def signature_argument_problems_for location, locals, closure_pin, params, argum
       errors
     end
 
+    # Checks each call-site argument absorbed by a restarg parameter
+    # against the restarg's own declared/resolved type, instead of
+    # bailing out on the whole signature. This is what catches e.g.
+    # `y.push('two')` on a `y: Array<Integer>`.
+    #
+    # @param location [Location]
+    # @param locals [Array<Pin::LocalVariable>]
+    # @param closure_pin [Pin::Closure]
+    # @param arguments [Array<Source::Chain>]
+    # @param sig [Pin::Signature]
+    # @param pin [Pin::Method]
+    # @param receiver_type [ComplexType] the type of the object the
+    #   method is being called on, used to resolve the restarg's
+    #   declared type (e.g. `Elem` for `Array#push`) against the
+    #   receiver's actual generic parameters (e.g. `Integer` for an
+    #   `Array<Integer>` receiver)
+    # @param par [Pin::Parameter] the restarg parameter
+    # @param idx [Integer] the restarg's index within sig.parameters
+    #
+    # @return [Array<Problem>]
+    def restarg_problems_for location, locals, closure_pin, arguments, sig, pin, receiver_type, par, idx
+      errors = []
+
+      # par.return_type is the type of the local variable the
+      # restarg is captured into inside the method body (e.g.
+      # `Array<Integer>`, not the unwrapped per-element `Integer`) -
+      # resolve any remaining generics against the receiver, then
+      # unwrap one level to get the type each individual argument
+      # must conform to.
+      # @sg-ignore pin.closure is a Pin::Namespace for a top-level method pin
+      wrapped_ptype = par.return_type.resolve_generics(pin.closure, receiver_type)
+      ptype = ComplexType.new(wrapped_ptype.items.flat_map(&:subtypes).flat_map(&:items))
+      # @sg-ignore pin.closure is a Pin::Namespace for a top-level method pin
+      ptype = ptype.qualify(api_map, *pin.closure.gates).self_to_type(par.context)
+      return errors if ptype.nil? || ptype.undefined?
+
+      restarg_arguments(sig, arguments, idx).each do |restargchain|
+        # A spread argument's own element types aren't statically
+        # known here - skip it rather than guess.
+        next if restargchain.nil? || restargchain.node.type == :splat
+
+        restargtype = restargchain.infer(api_map, closure_pin, locals).self_to_type(closure_pin.context)
+        next unless restargtype.defined?
+        next if arg_conforms_to?(restargtype, ptype)
+
+        errors.push Problem.new(location,
+                                "Wrong argument type for #{pin.path}: #{par.name} expected #{ptype}, received #{restargtype}")
+      end
+      errors
+    end
+
+    # @param sig [Pin::Signature]
+    # @param arguments [Array<Source::Chain>]
+    # @param idx [Integer] the restarg's index within sig.parameters
+    # @return [Array<Source::Chain>] the call-site arguments absorbed
+    #   by the restarg at idx
+    # @sg-ignore flow sensitive typing incorrectly includes an
+    #   intermediate local variable's type in the inferred return type
+    def restarg_arguments sig, arguments, idx
+      # A restarg can be followed by trailing positional parameters
+      # (`def foo(*path, baz)`) - those consume the last N call-site
+      # arguments, so they don't belong to this restarg's own
+      # arguments.
+      # @type [Array<Pin::Parameter>]
+      trailing_positional_params = sig.parameters[(idx + 1)..] || []
+      trailing_positional_count = trailing_positional_params.count { |p| p.decl == :arg }
+      # @type [Array<Source::Chain>]
+      # @sg-ignore flow sensitive typing issue with the ternary above
+      restargs = trailing_positional_count.zero? ? arguments[idx..] || [] : arguments[idx...-trailing_positional_count] || []
+
+      # A trailing bare hash argument (`foo(*args, key: val)`) is
+      # parsed as an implicit kwargs hash appended to the call's
+      # arguments - it belongs to the signature's keyword parameters,
+      # not the restarg.
+      # @type [Source::Chain, nil]
+      last_arg = restargs.last
+      has_trailing_hash = last_arg && last_arg.links.last.is_a?(Solargraph::Source::Chain::Hash)
+      has_keyword_params = sig.parameters.any? { |p| %i[kwarg kwoptarg kwrestarg].include?(p.decl) }
+      if has_trailing_hash && has_keyword_params
+        restargs[0...-1]
+      else
+        restargs
+      end
+    end
+
     # @param sig [Pin::Signature]
     # @param argchain [Solargraph::Source::Chain]
     # @param api_map [ApiMap]

spec/pin/method_spec.rb

@@ -709,7 +709,10 @@ def foo(*bar); end
       expect(pin.signatures.first.parameters).to be_one
       expect(pin.signatures.first.parameters.first.name).to eq('bar')
       expect(pin.signatures.first.parameters.first.decl).to eq(:restarg)
-      expect(pin.signatures.first.parameters.first.return_type.to_s).to eq('Array')
+      # `bar` here is the restarg's declared per-element type (RBS's
+      # inline shorthand identifies it by position, not name), now
+      # tracked instead of being erased to a bare `Array`
+      expect(pin.signatures.first.parameters.first.return_type.to_s).to eq('Array<bar>')
     end
 
     it 'sets required keyword parameters' do
@@ -754,7 +757,9 @@ def foo(**bar); end
       expect(pin.signatures.first.parameters).to be_one
       expect(pin.signatures.first.parameters.first.name).to eq('bar')
       expect(pin.signatures.first.parameters.first.decl).to eq(:kwrestarg)
-      expect(pin.signatures.first.parameters.first.return_type.to_s).to eq('Hash{Symbol => Object}')
+      # `bar` here is the kwrestarg's declared per-value type, now
+      # tracked instead of being erased to `Hash{Symbol => Object}`
+      expect(pin.signatures.first.parameters.first.return_type.to_s).to eq('Hash{Symbol => bar}')
     end
 
     it 'sets block parameters' do

spec/type_checker/levels/strict_spec.rb

@@ -49,6 +49,27 @@ def foo str; end
         .to eq(['Wrong argument type for #foo: str expected String, received Class<File>'])
     end
 
+    it 'catches a bad #push argument against an inferred Array element type (#1223)' do
+      # Reported by @castwide on PR #1223: https://github.com/castwide/solargraph/pull/1223#issuecomment-3138551901
+      #
+      #   y = [1]
+      #   y.push 'two'
+      #   y # => inferred as Array<Integer>, silently missing the pushed String
+      #
+      # Inference still can't track the mutation (see the "does not
+      # track a plain array through a mutating call" spec in
+      # clip_spec.rb), but type checking can now catch the bad
+      # argument at the call site itself, since Array#push's restarg
+      # is checked against the receiver's element type instead of
+      # being skipped entirely.
+      checker = type_checker(%(
+        y = [1]
+        y.push 'two'
+      ))
+      expect(checker.problems.map(&:message))
+        .to eq(['Wrong argument type for Array#push: objects expected Integer, received String'])
+    end
+
     it 'handles compatible interfaces with self types on call' do
       checker = type_checker(%(
         # @param a [Enumerable<String>]