Skip to content

Commit bbaf7f9

Browse files
apiologyclaude
andcommitted
Narrow bare, implicit-self attr_reader-style accessor calls
A nil-guard on a bare call (e.g. 'return nil if steps.nil?', where steps is an argless method like an attr_reader) previously left a later call to the same accessor (e.g. 'steps.empty?') unnarrowed -- FlowSensitiveTyping's chain-narrowing (added for explicit-receiver chains like 'pin.location') only resolved a single-word chain via find_var, which looks up tracked local/instance variables and can never match a method call. chain_pin now recognizes when a length-1 chain word actually came from a :send node (a method call, since the parser only emits :lvar for names already assigned as locals in scope) rather than an :lvar node, and synthesizes a pin rooted at the enclosing closure instead of a variable's. FlowSensitiveTyping now takes that closure as a constructor argument from each node processor's `region.closure`. process_call_chain's bare-truthy-check handling is extended from chain_words.length >= 2 to length >= 1 for the same reason, so 'return nil unless steps' narrows the same way 'return nil if steps.nil?' does. SKIP=Solargraph: this branch (castwide#1258, unmerged) already has 28 pre-existing `solargraph typecheck --level strong` problems in these files before this commit; this change adds none (verified line-by-line against the pre-existing baseline). Fixes castwide#1258 (comment) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PMbuVPLPj8CjHG8EkEQjrh
1 parent 2190a35 commit bbaf7f9

6 files changed

Lines changed: 92 additions & 15 deletions

File tree

lib/solargraph/parser/flow_sensitive_typing.rb

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ class FlowSensitiveTyping
99
# @param ivars [Array<Solargraph::Pin::InstanceVariable>]
1010
# @param enclosing_breakable_pin [Solargraph::Pin::Breakable, nil]
1111
# @param enclosing_compound_statement_pin [Solargraph::Pin::CompoundStatement, nil]
12-
def initialize locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin
12+
# @param closure [Solargraph::Pin::Closure] The pin enclosing the
13+
# code being processed (e.g. the current method), used to
14+
# resolve a bare, implicit-self call like 'steps' as a call to
15+
# a 0-arg method rather than a local variable.
16+
def initialize locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin, closure
1317
@locals = locals
1418
@ivars = ivars
1519
@enclosing_breakable_pin = enclosing_breakable_pin
1620
@enclosing_compound_statement_pin = enclosing_compound_statement_pin
21+
@closure = closure
1722
end
1823

1924
# @param and_node [Parser::AST::Node]
@@ -340,8 +345,10 @@ def find_var variable_name, position
340345
end
341346

342347
# Finds (for a single tracked local/instance variable) or builds
343-
# (for a chain of simple calls off of one, e.g. ['pin', 'location'])
344-
# the pin flow-sensitive-typing facts should be recorded against.
348+
# (for a chain of simple calls off of one, e.g. ['pin', 'location'],
349+
# or for a bare/explicit-self 0-arg method call, e.g. ['steps'] from
350+
# 'steps' or 'self.steps') the pin flow-sensitive-typing facts
351+
# should be recorded against.
345352
#
346353
# A synthesized pin's type is computed lazily, from `node` itself,
347354
# by Pin::BaseVariable#probe the same way a real local variable's
@@ -356,8 +363,19 @@ def find_var variable_name, position
356363
# @param position [Position]
357364
# @return [Solargraph::Pin::LocalVariable, Solargraph::Pin::InstanceVariable, nil]
358365
def chain_pin chain_words, node, position
359-
# @sg-ignore chain_words is never empty - callers already checked
360-
return find_var(chain_words.first, position) if chain_words.length == 1
366+
if chain_words.length == 1
367+
# A bare word is ambiguous from chain_words alone -- 'steps'
368+
# could be a real local variable (node.type == :lvar) or a
369+
# 0-arg method call to self (node.type == :send, since the
370+
# parser only emits :lvar for a name already assigned as a
371+
# local in this scope). Only the former is a tracked variable.
372+
# @sg-ignore chain_words is never empty - callers already checked
373+
return find_var(chain_words.first, position) unless node.is_a?(::Parser::AST::Node) && node.type == :send
374+
375+
return unless closure
376+
377+
return self_call_pin(node)
378+
end
361379

362380
# @sg-ignore chain_words is never empty - callers already checked
363381
root_pin = find_var(chain_words.first, position)
@@ -372,6 +390,26 @@ def chain_pin chain_words, node, position
372390
)
373391
end
374392

393+
# Builds the synthesized pin for a bare, implicit-self call to a
394+
# 0-arg method, e.g. 'steps'. Rooted at `closure` rather than at a
395+
# tracked variable's pin, since there is no variable to inherit a
396+
# closure from. Named after the bare method word itself (not
397+
# e.g. 'self.steps') so it lines up with how Chain::Call#resolve
398+
# looks up a head-position call: by the call's word, via
399+
# ApiMap#var_at_location.
400+
#
401+
# @param node [Parser::AST::Node] the call node, e.g. 'steps'
402+
# @return [Solargraph::Pin::LocalVariable]
403+
def self_call_pin node
404+
Pin::LocalVariable.new(
405+
location: Location.from_node(node),
406+
closure: closure,
407+
name: node.children[1].to_s,
408+
assignment: node,
409+
source: :flow_sensitive_typing
410+
)
411+
end
412+
375413
# @param isa_node [Parser::AST::Node]
376414
# @param true_presences [Array<Range>]
377415
# @param false_presences [Array<Range>]
@@ -501,10 +539,11 @@ def process_variable node, true_presences, false_presences
501539
end
502540

503541
# Handles a bare truthy check on a call chain, e.g. 'pin.location'
504-
# in 'return nil unless pin.location'. Bare references to a single
505-
# local/instance variable are handled by #process_variable instead;
506-
# this only fires once there's an explicit receiver (chain_words
507-
# has more than one word).
542+
# in 'return nil unless pin.location', or on a bare, implicit-self
543+
# 0-arg method call, e.g. 'steps' in 'return nil unless steps'.
544+
# Bare references to a single local/instance *variable* are
545+
# handled by #process_variable instead (node.type would be :lvar
546+
# or :ivar there, not :send, so this never double-processes them).
508547
#
509548
# @param node [Parser::AST::Node]
510549
# @param true_presences [Array<Range>]
@@ -518,7 +557,7 @@ def process_call_chain node, true_presences, false_presences
518557
return if %i[nil? !].include?(node.children[1])
519558

520559
chain_words = parse_receiver_chain(node)
521-
return if chain_words.nil? || chain_words.length < 2
560+
return if chain_words.nil? || chain_words.empty?
522561

523562
# @sg-ignore Range.from_node is nil only for a node without
524563
# source location info, which doesn't happen for real parsed
@@ -576,7 +615,7 @@ def always_leaves_compound_statement? clause_node
576615
%i[return raise next redo retry].include?(clause_node&.type)
577616
end
578617

579-
attr_reader :locals, :ivars, :enclosing_breakable_pin, :enclosing_compound_statement_pin
618+
attr_reader :locals, :ivars, :enclosing_breakable_pin, :enclosing_compound_statement_pin, :closure
580619
end
581620
end
582621
end

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ def process
1313
FlowSensitiveTyping.new(locals,
1414
ivars,
1515
enclosing_breakable_pin,
16-
enclosing_compound_statement_pin).process_and(node)
16+
enclosing_compound_statement_pin,
17+
region.closure).process_and(node)
1718
end
1819
end
1920
end

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ def process
1111
FlowSensitiveTyping.new(locals,
1212
ivars,
1313
enclosing_breakable_pin,
14-
enclosing_compound_statement_pin).process_if(node)
14+
enclosing_compound_statement_pin,
15+
region.closure).process_if(node)
1516
condition_node = node.children[0]
1617
if condition_node
1718
pins.push Solargraph::Pin::CompoundStatement.new(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ def process
1313
FlowSensitiveTyping.new(locals,
1414
ivars,
1515
enclosing_breakable_pin,
16-
enclosing_compound_statement_pin).process_or(node)
16+
enclosing_compound_statement_pin,
17+
region.closure).process_or(node)
1718
end
1819
end
1920
end

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ def process
1111
FlowSensitiveTyping.new(locals,
1212
ivars,
1313
enclosing_breakable_pin,
14-
enclosing_compound_statement_pin).process_while(node)
14+
enclosing_compound_statement_pin,
15+
region.closure).process_while(node)
1516

1617
# Note - this should not be considered a block, as the
1718
# while statement doesn't create a closure - e.g.,

spec/parser/flow_sensitive_typing_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,6 +1075,40 @@ def bundled_filename(pin)
10751075
expect(clip.infer.rooted_tags).to eq('::String')
10761076
end
10771077

1078+
it 'narrows a bare, implicit-self attr_reader-style accessor after a .nil? guard' do
1079+
source = Solargraph::Source.load_string(%(
1080+
class Repro
1081+
# @return [Array<Hash>, nil]
1082+
attr_reader :steps
1083+
1084+
def identify
1085+
return nil if steps.nil?
1086+
steps.empty?
1087+
end
1088+
end
1089+
), 'test.rb')
1090+
api_map = Solargraph::ApiMap.new.map(source)
1091+
clip = api_map.clip_at('test.rb', [7, 15])
1092+
expect(clip.infer.rooted_tags).to eq('::Array<::Hash>')
1093+
end
1094+
1095+
it 'narrows a bare, implicit-self attr_reader-style accessor after a truthy guard' do
1096+
source = Solargraph::Source.load_string(%(
1097+
class Repro
1098+
# @return [Array<Hash>, nil]
1099+
attr_reader :steps
1100+
1101+
def identify
1102+
return nil unless steps
1103+
steps.empty?
1104+
end
1105+
end
1106+
), 'test.rb')
1107+
api_map = Solargraph::ApiMap.new.map(source)
1108+
clip = api_map.clip_at('test.rb', [7, 15])
1109+
expect(clip.infer.rooted_tags).to eq('::Array<::Hash>')
1110+
end
1111+
10781112
it 'narrows a repeated call to the same attr_reader-style accessor rooted in an ivar' do
10791113
source = Solargraph::Source.load_string(%(
10801114
class Location

0 commit comments

Comments
 (0)