diff --git a/lib/solargraph/parser/flow_sensitive_typing.rb b/lib/solargraph/parser/flow_sensitive_typing.rb index c6f28dfce..0e874cf1b 100644 --- a/lib/solargraph/parser/flow_sensitive_typing.rb +++ b/lib/solargraph/parser/flow_sensitive_typing.rb @@ -9,11 +9,16 @@ class FlowSensitiveTyping # @param ivars [Array] # @param enclosing_breakable_pin [Solargraph::Pin::Breakable, nil] # @param enclosing_compound_statement_pin [Solargraph::Pin::CompoundStatement, nil] - def initialize locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin + # @param closure [Solargraph::Pin::Closure] The pin enclosing the + # code being processed (e.g. the current method), used to + # resolve a bare, implicit-self call like 'steps' as a call to + # a 0-arg method rather than a local variable. + def initialize locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin, closure @locals = locals @ivars = ivars @enclosing_breakable_pin = enclosing_breakable_pin @enclosing_compound_statement_pin = enclosing_compound_statement_pin + @closure = closure end # @param and_node [Parser::AST::Node] @@ -340,8 +345,10 @@ def find_var variable_name, position end # Finds (for a single tracked local/instance variable) or builds - # (for a chain of simple calls off of one, e.g. ['pin', 'location']) - # the pin flow-sensitive-typing facts should be recorded against. + # (for a chain of simple calls off of one, e.g. ['pin', 'location'], + # or for a bare/explicit-self 0-arg method call, e.g. ['steps'] from + # 'steps' or 'self.steps') the pin flow-sensitive-typing facts + # should be recorded against. # # A synthesized pin's type is computed lazily, from `node` itself, # by Pin::BaseVariable#probe the same way a real local variable's @@ -356,8 +363,19 @@ def find_var variable_name, position # @param position [Position] # @return [Solargraph::Pin::LocalVariable, Solargraph::Pin::InstanceVariable, nil] def chain_pin chain_words, node, position - # @sg-ignore chain_words is never empty - callers already checked - return find_var(chain_words.first, position) if chain_words.length == 1 + if chain_words.length == 1 + # A bare word is ambiguous from chain_words alone -- 'steps' + # could be a real local variable (node.type == :lvar) or a + # 0-arg method call to self (node.type == :send, since the + # parser only emits :lvar for a name already assigned as a + # local in this scope). Only the former is a tracked variable. + # @sg-ignore chain_words is never empty - callers already checked + return find_var(chain_words.first, position) unless node.is_a?(::Parser::AST::Node) && node.type == :send + + return unless closure + + return self_call_pin(node) + end # @sg-ignore chain_words is never empty - callers already checked root_pin = find_var(chain_words.first, position) @@ -372,6 +390,26 @@ def chain_pin chain_words, node, position ) end + # Builds the synthesized pin for a bare, implicit-self call to a + # 0-arg method, e.g. 'steps'. Rooted at `closure` rather than at a + # tracked variable's pin, since there is no variable to inherit a + # closure from. Named after the bare method word itself (not + # e.g. 'self.steps') so it lines up with how Chain::Call#resolve + # looks up a head-position call: by the call's word, via + # ApiMap#var_at_location. + # + # @param node [Parser::AST::Node] the call node, e.g. 'steps' + # @return [Solargraph::Pin::LocalVariable] + def self_call_pin node + Pin::LocalVariable.new( + location: Location.from_node(node), + closure: closure, + name: node.children[1].to_s, + assignment: node, + source: :flow_sensitive_typing + ) + end + # @param isa_node [Parser::AST::Node] # @param true_presences [Array] # @param false_presences [Array] @@ -501,10 +539,11 @@ def process_variable node, true_presences, false_presences end # Handles a bare truthy check on a call chain, e.g. 'pin.location' - # in 'return nil unless pin.location'. Bare references to a single - # local/instance variable are handled by #process_variable instead; - # this only fires once there's an explicit receiver (chain_words - # has more than one word). + # in 'return nil unless pin.location', or on a bare, implicit-self + # 0-arg method call, e.g. 'steps' in 'return nil unless steps'. + # Bare references to a single local/instance *variable* are + # handled by #process_variable instead (node.type would be :lvar + # or :ivar there, not :send, so this never double-processes them). # # @param node [Parser::AST::Node] # @param true_presences [Array] @@ -518,7 +557,7 @@ def process_call_chain node, true_presences, false_presences return if %i[nil? !].include?(node.children[1]) chain_words = parse_receiver_chain(node) - return if chain_words.nil? || chain_words.length < 2 + return if chain_words.nil? || chain_words.empty? # @sg-ignore Range.from_node is nil only for a node without # source location info, which doesn't happen for real parsed @@ -576,7 +615,7 @@ def always_leaves_compound_statement? clause_node %i[return raise next redo retry].include?(clause_node&.type) end - attr_reader :locals, :ivars, :enclosing_breakable_pin, :enclosing_compound_statement_pin + attr_reader :locals, :ivars, :enclosing_breakable_pin, :enclosing_compound_statement_pin, :closure end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/and_node.rb b/lib/solargraph/parser/parser_gem/node_processors/and_node.rb index 83f14a415..56fd0f921 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/and_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/and_node.rb @@ -13,7 +13,8 @@ def process FlowSensitiveTyping.new(locals, ivars, enclosing_breakable_pin, - enclosing_compound_statement_pin).process_and(node) + enclosing_compound_statement_pin, + region.closure).process_and(node) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/if_node.rb b/lib/solargraph/parser/parser_gem/node_processors/if_node.rb index 0b9a75e77..bb3e3fdd0 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/if_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/if_node.rb @@ -11,7 +11,8 @@ def process FlowSensitiveTyping.new(locals, ivars, enclosing_breakable_pin, - enclosing_compound_statement_pin).process_if(node) + enclosing_compound_statement_pin, + region.closure).process_if(node) condition_node = node.children[0] if condition_node pins.push Solargraph::Pin::CompoundStatement.new( diff --git a/lib/solargraph/parser/parser_gem/node_processors/or_node.rb b/lib/solargraph/parser/parser_gem/node_processors/or_node.rb index 6c54f1c8c..3ce837d66 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/or_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/or_node.rb @@ -13,7 +13,8 @@ def process FlowSensitiveTyping.new(locals, ivars, enclosing_breakable_pin, - enclosing_compound_statement_pin).process_or(node) + enclosing_compound_statement_pin, + region.closure).process_or(node) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/while_node.rb b/lib/solargraph/parser/parser_gem/node_processors/while_node.rb index 6c4fe33d8..48c0d91f9 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/while_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/while_node.rb @@ -11,7 +11,8 @@ def process FlowSensitiveTyping.new(locals, ivars, enclosing_breakable_pin, - enclosing_compound_statement_pin).process_while(node) + enclosing_compound_statement_pin, + region.closure).process_while(node) # Note - this should not be considered a block, as the # while statement doesn't create a closure - e.g., diff --git a/spec/parser/flow_sensitive_typing_spec.rb b/spec/parser/flow_sensitive_typing_spec.rb index d69f6287e..bf397044d 100644 --- a/spec/parser/flow_sensitive_typing_spec.rb +++ b/spec/parser/flow_sensitive_typing_spec.rb @@ -1075,6 +1075,59 @@ def bundled_filename(pin) expect(clip.infer.rooted_tags).to eq('::String') end + it 'narrows a bare, implicit-self attr_reader-style accessor after a .nil? guard' do + source = Solargraph::Source.load_string(%( + class Repro + # @return [Array, nil] + attr_reader :steps + + def identify + return nil if steps.nil? + steps.empty? + end + end + ), 'test.rb') + api_map = Solargraph::ApiMap.new.map(source) + clip = api_map.clip_at('test.rb', [7, 15]) + expect(clip.infer.rooted_tags).to eq('::Array<::Hash>') + end + + it 'narrows a bare, implicit-self attr_reader-style accessor after a truthy guard' do + source = Solargraph::Source.load_string(%( + class Repro + # @return [Array, nil] + attr_reader :steps + + def identify + return nil unless steps + steps.empty? + end + end + ), 'test.rb') + api_map = Solargraph::ApiMap.new.map(source) + clip = api_map.clip_at('test.rb', [7, 15]) + expect(clip.infer.rooted_tags).to eq('::Array<::Hash>') + end + + it 'narrows a bare, implicit-self attr_reader-style accessor assigned into a fresh local variable' do + source = Solargraph::Source.load_string(%( + class Repro + # @return [Array, nil] + attr_reader :steps + + def identify + return nil if steps.nil? + + local = steps + local.empty? + end + end + ), 'test.rb') + api_map = Solargraph::ApiMap.new.map(source) + clip = api_map.clip_at('test.rb', [9, 15]) + expect(clip.infer.rooted_tags).to eq('::Array<::Hash>') + end + it 'narrows a repeated call to the same attr_reader-style accessor rooted in an ivar' do source = Solargraph::Source.load_string(%( class Location diff --git a/spec/type_checker/levels/strong_spec.rb b/spec/type_checker/levels/strong_spec.rb index 9a0bd3e8a..8b549ff96 100644 --- a/spec/type_checker/levels/strong_spec.rb +++ b/spec/type_checker/levels/strong_spec.rb @@ -997,5 +997,46 @@ def baz(bases) # an error when trying to declare sub as Subclass expect(checker.problems.map(&:message)).not_to include('Unresolved call to bar on Base') end + + it 'accepts a non-nil @type on a local assigned from a bare accessor guarded by .nil?' do + checker = type_checker(%( + class Repro + # @return [Array, nil] + attr_reader :steps + + # @return [Array, nil] + def unwrap + return nil if steps.nil? + + # @type [Array] + steps_list = steps + steps_list.each { |step| step } + steps_list + end + end + )) + + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'accepts a non-nil @type on a local assigned from a bare accessor guarded by a non-nil return' do + checker = type_checker(%( + class Repro + # @return [Array, nil] + attr_reader :substeps + + # @return [Array] + def extract + return ['', nil] if substeps.nil? + + # @type [Array] + steps_list = substeps + [steps_list] + end + end + )) + + expect(checker.problems.map(&:message)).to eq([]) + end end end