Skip to content

Commit

Permalink
Fix "Not enough arguments to" false positives on kwarg type checking (#…
Browse files Browse the repository at this point in the history
…725)

* Fix "Not enough arguments to" false positives on kwarg type checking

* Add tests, including issue failing on main branch

* Acccount for older Ruby kwarg handling

* Convert kwarg_problems_for to focus on one parameter

* Verify newer Ruby versions as well - kwargs are a moving target

* Drop Ruby 3.3 from matrix due to #726

---------

Co-authored-by: Fred Snyder <[email protected]>
  • Loading branch information
apiology and castwide authored Jan 15, 2025
1 parent d9a0a89 commit 1249f0e
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 19 deletions.
47 changes: 28 additions & 19 deletions lib/solargraph/type_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ def call_problems
result
end

# @param chain [Solargraph::Source::Chain]
# @param api_map [Solargraph::ApiMap]
# @param block_pin [Solargraph::Pin::Base]
# @param locals [Array<Solargraph::Pin::Base>]
# @param location [Solargraph::Location]
def argument_problems_for chain, api_map, block_pin, locals, location
result = []
base = chain
Expand All @@ -279,9 +284,14 @@ def argument_problems_for chain, api_map, block_pin, locals, location
errors = []
sig.parameters.each_with_index do |par, idx|
argchain = base.links.last.arguments[idx]
if argchain.nil? && par.decl == :arg
errors.push Problem.new(location, "Not enough arguments to #{pin.path}")
next
if argchain.nil?
if par.decl == :arg
errors.push Problem.new(location, "Not enough arguments to #{pin.path}")
next
else
last = base.links.last.arguments.last
argchain = last if last && [:kwsplat, :HASH].include?(last.node.type)
end
end
if argchain
if par.decl != :arg
Expand Down Expand Up @@ -317,30 +327,29 @@ def argument_problems_for chain, api_map, block_pin, locals, location
result
end

def kwarg_problems_for argchain, api_map, block_pin, locals, location, pin, params, first
def kwarg_problems_for argchain, api_map, block_pin, locals, location, pin, params, idx
result = []
kwargs = convert_hash(argchain.node)
pin.signatures.first.parameters[first..-1].each_with_index do |par, cur|
idx = first + cur
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, block_pin, locals, location, pin, params, kwargs)
else
if argchain
data = params[par.name]
if data.nil?
# @todo Some level (strong, I guess) should require the param here
else
ptype = data[:qualified]
next if ptype.undefined?
par = pin.signatures.first.parameters[idx]
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, block_pin, locals, location, pin, params, kwargs)
else
if argchain
data = params[par.name]
if data.nil?
# @todo Some level (strong, I guess) should require the param here
else
ptype = data[:qualified]
unless ptype.undefined?
argtype = argchain.infer(api_map, block_pin, locals)
if argtype.defined? && ptype && !any_types_match?(api_map, ptype, argtype)
result.push Problem.new(location, "Wrong argument type for #{pin.path}: #{par.name} expected #{ptype}, received #{argtype}")
end
end
elsif par.decl == :kwarg
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}")
end
end
result
Expand Down
47 changes: 47 additions & 0 deletions spec/type_checker/levels/strict_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,53 @@ def bar(baz)
expect(checker.problems.first.message).to include('Not enough arguments')
end

it 'reports solo missing kwarg' do
checker = type_checker(%(
class Foo
def bar(baz:)
end
end
Foo.new.bar
))
expect(checker.problems).to be_one
expect(checker.problems.first.message).to include('Missing keyword arguments')
end

it 'reports not enough kwargs' do
checker = type_checker(%(
class Foo
def bar(foo:, baz:)
end
end
Foo.new.bar(foo: 100)
))
expect(checker.problems).to be_one
expect(checker.problems.first.message).to include('Missing keyword argument')
expect(checker.problems.first.message).to include('baz')
end

it 'accepts passed kwargs' do
checker = type_checker(%(
class Foo
def bar(baz:)
end
end
Foo.new.bar(baz: 123)
))
expect(checker.problems).to be_empty
end

it 'accepts multiple passed kwargs' do
checker = type_checker(%(
class Foo
def bar(baz:, bing:)
end
end
Foo.new.bar(baz: 123, bing: 456)
))
expect(checker.problems).to be_empty
end

it 'requires strict return tags' do
checker = type_checker(%(
class Foo
Expand Down

0 comments on commit 1249f0e

Please sign in to comment.