Check keyword arguments passed through a ** splat

lib/solargraph/bench.rb

@@ -29,7 +29,6 @@ def initialize source_maps: [], workspace: Workspace.new, live_map: nil, externa
                                             .to_set
     end
 
-    # @sg-ignore flow sensitive typing needs better handling of ||= on lvars
     # @return [Hash{String => SourceMap}]
     def source_map_hash
       # @todo Work around #to_h bug in current Ruby head (3.5) with #map#to_h

lib/solargraph/complex_type/unique_type.rb

@@ -198,6 +198,16 @@ def literal?
         non_literal_name != name
       end
 
+      # Whether this type's tag is a literal value (`:a`, `"Index"`, `1`,
+      # `true`) rather than a class name. Unlike #literal?, this does not
+      # take part in type inference; it only reports what the tag says, so
+      # a literal written in an annotation survives qualification.
+      #
+      # @return [Boolean]
+      def literal_tag?
+        non_literal_name != name
+      end
+
       # @return [String]
       def non_literal_name
         @non_literal_name ||= determine_non_literal_name
@@ -726,23 +736,43 @@ def expand named_types
         named_types[name] || self
       end
 
+      # A literal tag has to survive qualification: widening `:a` to Symbol
+      # erases which key the type names. map (not flat_map) keeps each
+      # entry as one parameter position.
+      #
+      # @param cts [::Array<ComplexType>]
+      # @param api_map [ApiMap]
+      # @param gates [::Array<String>]
+      # @return [::Array<ComplexType>]
+      def qualify_positions cts, api_map, gates
+        cts.map do |ct|
+          ComplexType.new(ct.items.map { |ut| ut.literal_tag? ? ut : ut.qualify(api_map, *gates) })
+        end
+      end
+      private :qualify_positions
+
       # Generate a ComplexType that fully qualifies this type's namespaces.
       #
       # @param api_map [ApiMap] The ApiMap that performs qualification
       # @param gates [Array<String>] The namespaces from which to resolve names
       # @return [self, ComplexType, UniqueType] The generated ComplexType
       def qualify api_map, *gates
-        transform do |t|
-          next t if t.name == GENERIC_TAG_NAME
-          next t if t.duck_type? || t.void? || t.undefined? || t.literal?
-          open = t.rooted? ? [''] : gates
-          fqns = api_map.qualify(t.non_literal_name, *open)
-          if fqns.nil?
-            next UniqueType::BOOLEAN if t.tag == 'Boolean'
-            next UniqueType::UNDEFINED
-          end
-          t.recreate(new_name: fqns, make_rooted: true)
+        if name == GENERIC_TAG_NAME
+          new_key_types = @key_types
+          new_subtypes = @subtypes
+        else
+          new_key_types = qualify_positions(@key_types, api_map, gates)
+          new_subtypes = qualify_positions(@subtypes, api_map, gates)
+        end
+        qualified = recreate(new_key_types: new_key_types, new_subtypes: new_subtypes)
+        return qualified if name == GENERIC_TAG_NAME || duck_type? || void? || undefined? || literal?
+        open = rooted? ? [''] : gates
+        fqns = api_map.qualify(non_literal_name, *open)
+        if fqns.nil?
+          return UniqueType::BOOLEAN if tag == 'Boolean'
+          return UniqueType::UNDEFINED
         end
+        qualified.recreate(new_name: fqns, make_rooted: true)
       end
 
       # Expand this type if it names a type alias; otherwise qualify it.

lib/solargraph/complex_type/unique_type/intersection.rb

@@ -192,6 +192,14 @@ def implicit_union?(*, **, &)
           raise NotImplementedError, "Intersection cannot answer ##{__method__} - resolve each conjunct instead"
         end
 
+        # The tag is the compound `"A & B"` string, never a literal value,
+        # so #qualify descends into the conjuncts rather than keeping it whole.
+        #
+        # @return [Boolean]
+        def literal_tag?
+          false
+        end
+
         # @param other [Object]
         # @return [Boolean]
         def eql? other
@@ -368,6 +376,16 @@ def transform _new_name = nil, &transform_type
           Intersection.new(conjuncts.map { |conjunct| conjunct.transform(&transform_type) })
         end
 
+        # UniqueType#qualify walks key_types and subtypes; an
+        # intersection holds neither, so it qualifies its conjuncts.
+        #
+        # @param api_map [ApiMap]
+        # @param gates [Array<String>]
+        # @return [Intersection]
+        def qualify api_map, *gates
+          Intersection.new(conjuncts.map { |conjunct| conjunct.qualify(api_map, *gates) })
+        end
+
         # @param api_map [ApiMap]
         # @param gates [Array<String>]
         # @return [Intersection]

lib/solargraph/parser/parser_gem/node_chainer.rb

@@ -177,14 +177,19 @@ def send_node
           @send_node ||= @tree&.find { |n| n.type == :send }
         end
 
+        # Whether the hash passes along keys that are not in the node -
+        # `foo(**args)` or `foo(a: 1, **args)`, but not `foo(**{ a: 1 })`,
+        # whose keys are right there.
+        #
         # @param node [Parser::AST::Node]
         def hash_is_splatted? node
           return false unless Parser.is_ast_node?(node) && node.type == :hash
-          return false unless Parser.is_ast_node?(node.children.last) && node.children.last.type == :kwsplat
-          if Parser.is_ast_node?(node.children.last.children[0]) && node.children.last.children[0].type == :hash
-            return false
+          node.children.any? do |child|
+            # @sg-ignore Translate to something flow sensitive typing understands
+            next false unless Parser.is_ast_node?(child) && child.type == :kwsplat
+            # @sg-ignore Translate to something flow sensitive typing understands
+            !(Parser.is_ast_node?(child.children[0]) && child.children[0].type == :hash)
           end
-          true
         end
 
         # Chains each key/value pair so Chain::Hash can infer

lib/solargraph/parser/parser_gem/node_methods.rb

@@ -144,22 +144,29 @@ def simple_convert_hash node
           result
         end
 
+        # The keyword arguments a hash node passes by name. A `**` splat
+        # of a literal hash contributes that hash's own keys; a `**` splat
+        # of anything else contributes none, because the keys are not in
+        # the node - see NodeChainer.hash_is_splatted? for the predicate
+        # that tells the two apart.
+        #
         # @param node [Parser::AST::Node, nil]
         # @return [Hash{Symbol => Chain}]
         def convert_hash node
           return {} unless Parser.is_ast_node?(node)
           # @sg-ignore Translate to something flow sensitive typing understands
           return convert_hash(node.children[0]) if node.type == :kwsplat
           # @sg-ignore Translate to something flow sensitive typing understands
-          if Parser.is_ast_node?(node.children[0]) && node.children[0].type == :kwsplat
-            # @sg-ignore Translate to something flow sensitive typing understands
-            return convert_hash(node.children[0])
-          end
-          # @sg-ignore Translate to something flow sensitive typing understands
           return {} unless node.type == :hash
           result = {}
           # @sg-ignore Translate to something flow sensitive typing understands
           node.children.each do |pair|
+            next unless Parser.is_ast_node?(pair)
+            if pair.type == :kwsplat
+              result.merge!(convert_hash(pair))
+              next
+            end
+            next unless Parser.is_ast_node?(pair.children[0])
             result[pair.children[0].children[0]] = Solargraph::Parser.chain(pair.children[1])
           end
           result

lib/solargraph/pin/keyword.rb

@@ -4,7 +4,6 @@ module Solargraph
   module Pin
     class Keyword < Base
       def initialize(name, **kwargs)
-        # @sg-ignore "Unrecognized keyword argument kwargs to Solargraph::Pin::Base#initialize"
         super(name: name, **kwargs)
       end
 

lib/solargraph/pin/symbol.rb

@@ -7,7 +7,6 @@ class Symbol < Base
       # @param name [String]
       # @param [Hash{Symbol => Object}] kwargs
       def initialize(location, name, **kwargs)
-        # @sg-ignore "Unrecognized keyword argument kwargs to Solargraph::Pin::Base#initialize"
         super(location: location, name: name, **kwargs)
         # @name = name
         # @location = location

lib/solargraph/type_checker.rb

@@ -491,6 +491,9 @@ def argument_problems_for chain, api_map, closure_pin, locals, location
     # @return [Array<Problem>]
     def signature_argument_problems_for location, locals, closure_pin, params, arguments, sig, pin
       errors = []
+      splat_keywords, unverifiable_splat = keyword_splat_types(arguments.last, closure_pin, locals)
+      # @sg-ignore multiple assignment from a tuple gives every variable the first element type
+      errors.concat keyword_splat_problems_for(sig, pin, location, arguments, splat_keywords, unverifiable_splat)
       # @todo add logic mapping up restarg parameters with
       #   arguments (including restarg arguments).  Use tuples
       #   when possible, and when not, ensure provably
@@ -545,10 +548,12 @@ def signature_argument_problems_for location, locals, closure_pin, params, argum
               end
             end
           else
-            errors.concat kwarg_problems_for sig, argchain, api_map, closure_pin, locals, location, pin, params, idx
+            # @sg-ignore multiple assignment from a tuple gives every variable the first element type
+            errors.concat kwarg_problems_for(sig, argchain, api_map, closure_pin, locals, location, pin, params,
+                                             idx, splat_keywords, unverifiable_splat)
             next
           end
-        elsif par.decl == :kwarg
+        elsif par.decl == :kwarg && unverifiable_splat.nil?
           errors.push Problem.new(location, "Call to #{pin.path} is missing keyword argument #{par.name}")
           next
         end
@@ -565,40 +570,164 @@ def signature_argument_problems_for location, locals, closure_pin, params, argum
     # @param pin [Pin::Method]
     # @param params [Hash{String => Hash{Symbol => undefined}}]
     # @param idx [Integer]
+    # @param splat_keywords [Hash{Symbol => ComplexType}] keywords a `**` splat records in its type
+    # @param unverifiable_splat [ComplexType, nil] the type of a `**` splat that records no keys
     #
     # @return [Array<Problem>]
-    def kwarg_problems_for sig, argchain, api_map, closure_pin, locals, location, pin, params, idx
+    def kwarg_problems_for sig, argchain, api_map, closure_pin, locals, location, pin, params, idx, splat_keywords,
+                           unverifiable_splat
       result = []
       kwargs = convert_hash(argchain.node)
+      # idx comes from an each_with_index over these same parameters.
+      #
+      # @sg-ignore Array#[] is nilable even with an in-range index
+      # @type [Pin::Parameter]
       par = sig.parameters[idx]
-      # @type [Solargraph::Source::Chain]
-      argchain = kwargs[par.name.to_sym]
       if par.decl == :kwrestarg || (par.decl == :optarg && idx == pin.parameters.length - 1 && par.asgn_code == '{}')
-        result.concat kwrestarg_problems_for(api_map, closure_pin, locals, location, pin, params, kwargs)
-      elsif argchain
-        data = params[par.name]
-        if data.nil?
-          # @todo Some level (strong, I guess) should require the param here
-        else
-          # @type [ComplexType, ComplexType::UniqueType]
-          ptype = data[:qualified]
-          ptype = ptype.self_to_type(pin.context)
-          unless ptype.undefined?
-            # @type [ComplexType]
-            argtype = argchain.infer(api_map, closure_pin, locals).self_to_type(closure_pin.context)
-            # @todo Unresolved call to defined?
-            if argtype.defined? && ptype && !arg_conforms_to?(argtype, ptype)
-              result.push Problem.new(location,
-                                      "Wrong argument type for #{pin.path}: #{par.name} expected #{ptype}, received #{argtype}")
-            end
-          end
+        return kwrestarg_problems_for(api_map, closure_pin, locals, location, pin, params, kwargs)
+      end
+      # @type [Solargraph::Source::Chain, nil]
+      valuechain = kwargs[par.name.to_sym]
+      argtype = if valuechain
+                  valuechain.infer(api_map, closure_pin, locals).self_to_type(closure_pin.context)
+                else
+                  splat_keywords[par.name.to_sym]
+                end
+      if argtype.nil?
+        if par.decl == :kwarg && unverifiable_splat.nil?
+          result.push Problem.new(location, "Call to #{pin.path} is missing keyword argument #{par.name}")
         end
-      elsif par.decl == :kwarg
-        result.push Problem.new(location, "Call to #{pin.path} is missing keyword argument #{par.name}")
+        return result
+      end
+      data = params[par.name]
+      # @todo Some level (strong, I guess) should require the param here
+      return result if data.nil?
+
+      # @type [ComplexType, ComplexType::UniqueType]
+      ptype = data[:qualified]
+      ptype = ptype.self_to_type(pin.context)
+      return result if ptype.undefined?
+
+      # @sg-ignore defined? is unresolved and the nil case above is not narrowed
+      if argtype.defined? && !arg_conforms_to?(argtype, ptype)
+        result.push Problem.new(location,
+                                "Wrong argument type for #{pin.path}: #{par.name} expected #{ptype}, received #{argtype}")
       end
       result
     end
 
+    # Problems that belong to the call as a whole rather than to one
+    # keyword parameter: a `**` splat whose type records no keys, and
+    # keys a record type supplies that the method does not accept.
+    #
+    # @param sig [Pin::Signature]
+    # @param pin [Pin::Method]
+    # @param location [Location]
+    # @param arguments [Array<Source::Chain>]
+    # @param splat_keywords [Hash{Symbol => ComplexType}]
+    # @param unverifiable_splat [ComplexType, nil]
+    # @return [Array<Problem>]
+    def keyword_splat_problems_for sig, pin, location, arguments, splat_keywords, unverifiable_splat
+      keyword_params = sig.parameters.select(&:keyword?)
+      return [] if keyword_params.empty?
+
+      if unverifiable_splat
+        unverified = keyword_params.select { |par| par.decl == :kwarg }.map(&:name) -
+                     convert_hash(arguments.last&.node).keys.map(&:to_s)
+        return [] if unverified.empty?
+
+        return [Problem.new(location, unverifiable_splat_message(pin, unverifiable_splat, unverified))]
+      end
+      return [] if splat_keywords.empty? || sig.parameters.any?(&:kwrestarg?)
+
+      unrecognized = splat_keywords.keys.reject { |name| keyword_params.any? { |par| par.name.to_sym == name } }
+      return [] if unrecognized.empty?
+
+      [Problem.new(location, "Unrecognized keyword argument #{unrecognized.first} to #{pin.path}")]
+    end
+
+    # @param pin [Pin::Method]
+    # @param type [ComplexType]
+    # @param unverified [Array<String>] required keywords the splat may or may not supply
+    # @return [String]
+    def unverifiable_splat_message pin, type, unverified
+      "Cannot verify keyword arguments to #{pin.path}: the ** splat is #{type}, which does not record its keys, " \
+        "so required keyword #{unverified.length == 1 ? 'argument' : 'arguments'} #{unverified.join(', ')} " \
+        'cannot be checked - give the splatted value a record type ' \
+        "(e.g. Hash{:#{unverified.first} => Object}) to check it"
+    end
+
+    # The keywords a call's `**` splats record in their types.
+    #
+    # A splat of a literal hash is not included: its keys are in the node
+    # itself, and #convert_hash already has them.
+    #
+    # @param argchain [Source::Chain, nil] the call's final argument
+    # @param closure_pin [Pin::Closure]
+    # @param locals [Array<Pin::LocalVariable>]
+    # @return [Array(Hash{Symbol => ComplexType}, ComplexType), Array(Hash{Symbol => ComplexType}, nil)]
+    #   the keywords found, and the type of the first splat that records no keys
+    def keyword_splat_types argchain, closure_pin, locals
+      node = argchain&.node
+      # @sg-ignore Translate to something flow sensitive typing understands
+      return [{}, nil] unless Parser.is_ast_node?(node) && node.type == :hash
+
+      keywords = {}
+      # @sg-ignore Translate to something flow sensitive typing understands
+      node.children.each do |child|
+        next unless Parser.is_ast_node?(child) && child.type == :kwsplat
+        inner = child.children[0]
+        next if Parser.is_ast_node?(inner) && inner.type == :hash
+
+        type = Solargraph::Parser.chain(inner).infer(api_map, closure_pin, locals)
+        recorded = recorded_keyword_types(type)
+        return [{}, type] if recorded.nil?
+
+        keywords.merge!(recorded)
+      end
+      [keywords, nil]
+    end
+
+    # The keyword names and value types a record type records - e.g.
+    # `Hash{:a => Integer} & Hash{:b => String}` records a as Integer and
+    # b as String - or nil if the type does not record its keys.
+    #
+    # @param type [ComplexType]
+    # @return [Hash{Symbol => ComplexType}, nil]
+    def recorded_keyword_types type
+      return nil if type.undefined? || type.items.empty?
+
+      keywords = {}
+      type.items.each do |item|
+        conjuncts = item.is_a?(ComplexType::UniqueType::Intersection) ? item.conjuncts : [ComplexType.new([item])]
+        conjuncts.each do |conjunct|
+          conjunct.items.each do |unique_type|
+            recorded = symbol_keyed_entries(unique_type)
+            return nil if recorded.nil?
+
+            keywords.merge!(recorded)
+          end
+        end
+      end
+      keywords
+    end
+
+    # @param unique_type [ComplexType::UniqueType]
+    # @return [Hash{Symbol => ComplexType}, nil]
+    def symbol_keyed_entries unique_type
+      return nil unless unique_type.name == 'Hash'
+      return nil if unique_type.key_types.empty? || unique_type.key_types.length != unique_type.subtypes.length
+
+      entries = {}
+      unique_type.key_types.each_with_index do |key_type, i|
+        tags = key_type.items.map(&:tag)
+        return nil unless tags.length == 1 && tags.first.start_with?(':')
+
+        entries[tags.first[1..].to_sym] = unique_type.subtypes[i]
+      end
+      entries
+    end
+
     # @param api_map [ApiMap]
     # @param closure_pin [Pin::Closure]
     # @param locals [Array<Pin::LocalVariable>]