Skip to content

Commit 98958db

Browse files
authored
Flow-sensitive typing - automatically downcast from is_a? calls (#856)
* Use 'if foo.is_a?' syntax to downcast types for initial cases * Add node processor for :if * Adjust presence of then clause * Specify more type behavior for variable reassignment * Adjust local variable presence to start after assignment, not before * Mark pin as overriding type * Fix position-related bug that broke 'unless' * Extract to new file * Extract to new file * Fix strict type-checking issues * Handle namespace names recursively * Fix merge-related duplication * Support elsif in flow_sensitive_typing.rb * flow_sensitive_typing.rb cleanup * Test simple if/then/else * Handle flow-sensitive-typing for 'break unless' * Handle nil case * Drop outdated @return annotation * Type fixes * Add support for 'break unless' * Mark methods as private * Add unless-in-.each test * Use capture block pin in clip.rb for strong typing * Add simple processing for && * Add simple processing for && * Add simple processing for && * Add more complex processing for && * Take advantage of flow-sensitive typing on internal code * Add regression test around assignment in return position * Fix assignment visibility code, which relied on bad asgn semantics * Calculate local variable visibility in Chain::Call * declaration? -> presence_certain? * Add spec for future flow-sensitive typing scope * Populate location information from RBS files (#768) * Populate location information from RBS files The 'rbs' gem maps the location of different definitions to the relevant point in the RGS files themselves - this change provides the ability to jump into the right place in those files to see the type definition via the LSP. * Prefer source location in language server * Resolve merge issue * Fix Path vs String type error * Consolidate parameter handling into Pin::Callable (#844) * Consolidate parameter handling into Pin::Closure * Clarify clobbered variable names * Fix bug in to_rbs, add spec, then fix new bug found after running spec * Catch one more Signature.new to translate from strict typechecking * Introduce Pin::Callable * Introduce Pin::Callable * Introduce Pin::Callable * Introduce Pin::Callable * Introduce Pin::Callable * Introduce Pin::Callable * Introduce Pin::Callable * Use Pin::Callable type in args_node.rb * Select String#each_line overload with mandatory vs optional arg info * Improve type annotations * Fix types * Mark test as pending upcoming PR * Add debug logging
1 parent 5c12a11 commit 98958db

30 files changed

+703
-25
lines changed

lib/solargraph/api_map.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ def get_instance_variable_pins(namespace, scope = :instance)
319319
result
320320
end
321321

322+
# @see Solargraph::Parser::FlowSensitiveTyping#visible_pins
323+
def visible_pins(*args, **kwargs, &blk)
324+
Solargraph::Parser::FlowSensitiveTyping.visible_pins(*args, **kwargs, &blk)
325+
end
326+
322327
# Get an array of class variable pins for a namespace.
323328
#
324329
# @param namespace [String] A fully qualified namespace

lib/solargraph/parser.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module Parser
44
autoload :ParserGem, 'solargraph/parser/parser_gem'
55
autoload :Region, 'solargraph/parser/region'
66
autoload :NodeProcessor, 'solargraph/parser/node_processor'
7+
autoload :FlowSensitiveTyping, 'solargraph/parser/flow_sensitive_typing'
78
autoload :Snippet, 'solargraph/parser/snippet'
89

910
class SyntaxError < StandardError
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
module Solargraph
2+
module Parser
3+
class FlowSensitiveTyping
4+
include Solargraph::Parser::NodeMethods
5+
6+
# @param locals [Array<Solargraph::Pin::LocalVariable, Solargraph::Pin::Parameter>]
7+
def initialize(locals, enclosing_breakable_pin = nil)
8+
@locals = locals
9+
@enclosing_breakable_pin = enclosing_breakable_pin
10+
end
11+
12+
# @param and_node [Parser::AST::Node]
13+
def process_and(and_node, true_ranges = [])
14+
lhs = and_node.children[0]
15+
rhs = and_node.children[1]
16+
17+
before_rhs_loc = rhs.location.expression.adjust(begin_pos: -1)
18+
before_rhs_pos = Position.new(before_rhs_loc.line, before_rhs_loc.column)
19+
20+
rhs_presence = Range.new(before_rhs_pos,
21+
get_node_end_position(rhs))
22+
process_isa(lhs, true_ranges + [rhs_presence])
23+
end
24+
25+
# @param if_node [Parser::AST::Node]
26+
def process_if(if_node)
27+
#
28+
# See if we can refine a type based on the result of 'if foo.nil?'
29+
#
30+
# [3] pry(main)> require 'parser/current'; Parser::CurrentRuby.parse("if foo.is_a? Baz; then foo; else bar; end")
31+
# => s(:if,
32+
# s(:send,
33+
# s(:send, nil, :foo), :is_a?,
34+
# s(:const, nil, :Baz)),
35+
# s(:send, nil, :foo),
36+
# s(:send, nil, :bar))
37+
# [4] pry(main)>
38+
conditional_node = if_node.children[0]
39+
then_clause = if_node.children[1]
40+
else_clause = if_node.children[2]
41+
42+
true_ranges = []
43+
if always_breaks?(else_clause)
44+
unless enclosing_breakable_pin.nil?
45+
rest_of_breakable_body = Range.new(get_node_end_position(if_node),
46+
get_node_end_position(enclosing_breakable_pin.node))
47+
true_ranges << rest_of_breakable_body
48+
end
49+
end
50+
51+
unless then_clause.nil?
52+
#
53+
# Add specialized locals for the then clause range
54+
#
55+
before_then_clause_loc = then_clause.location.expression.adjust(begin_pos: -1)
56+
before_then_clause_pos = Position.new(before_then_clause_loc.line, before_then_clause_loc.column)
57+
true_ranges << Range.new(before_then_clause_pos,
58+
get_node_end_position(then_clause))
59+
end
60+
61+
process_conditional(conditional_node, true_ranges)
62+
end
63+
64+
class << self
65+
include Logging
66+
end
67+
68+
# Find a variable pin by name and where it is used.
69+
#
70+
# Resolves our most specific view of this variable's type by
71+
# preferring pins created by flow-sensitive typing when we have
72+
# them based on the Closure and Location.
73+
#
74+
# @param pins [Array<Pin::LocalVariable>]
75+
# @param closure [Pin::Closure]
76+
# @param location [Location]
77+
def self.visible_pins(pins, name, closure, location)
78+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location})" }
79+
pins_with_name = pins.select { |p| p.name == name }
80+
if pins_with_name.empty?
81+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location}) => [] - no pins with name" }
82+
return []
83+
end
84+
pins_with_specific_visibility = pins.select { |p| p.name == name && p.presence && p.visible_at?(closure, location) }
85+
if pins_with_specific_visibility.empty?
86+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location}) => #{pins_with_name} - no pins with specific visibility" }
87+
return pins_with_name
88+
end
89+
visible_pins_specific_to_this_closure = pins_with_specific_visibility.select { |p| p.closure == closure }
90+
if visible_pins_specific_to_this_closure.empty?
91+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location}) => #{pins_with_specific_visibility} - no visible pins specific to this closure (#{closure})}" }
92+
return pins_with_specific_visibility
93+
end
94+
flow_defined_pins = pins_with_specific_visibility.select { |p| p.presence_certain? }
95+
if flow_defined_pins.empty?
96+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location}) => #{visible_pins_specific_to_this_closure} - no flow-defined pins" }
97+
return visible_pins_specific_to_this_closure
98+
end
99+
100+
logger.debug { "FlowSensitiveTyping#visible_pins(name=#{name}, closure=#{closure}, location=#{location}) => #{flow_defined_pins}" }
101+
102+
flow_defined_pins
103+
end
104+
105+
include Logging
106+
107+
private
108+
109+
# @param pin [Pin::LocalVariable]
110+
# @param if_node [Parser::AST::Node]
111+
def add_downcast_local(pin, downcast_type_name, presence)
112+
# @todo Create pin#update method
113+
new_pin = Solargraph::Pin::LocalVariable.new(
114+
location: pin.location,
115+
closure: pin.closure,
116+
name: pin.name,
117+
assignment: pin.assignment,
118+
comments: pin.comments,
119+
presence: presence,
120+
return_type: ComplexType.try_parse(downcast_type_name),
121+
presence_certain: true
122+
)
123+
locals.push(new_pin)
124+
end
125+
126+
# @param facts_by_pin [Hash{Pin::LocalVariable => Array<Hash{Symbol => String}>}]
127+
# @param presences [Array<Range>]
128+
# @return [void]
129+
def process_facts(facts_by_pin, presences)
130+
#
131+
# Add specialized locals for the rest of the block
132+
#
133+
facts_by_pin.each_pair do |pin, facts|
134+
facts.each do |fact|
135+
downcast_type_name = fact.fetch(:type)
136+
presences.each do |presence|
137+
add_downcast_local(pin, downcast_type_name, presence)
138+
end
139+
end
140+
end
141+
end
142+
143+
# @param conditional_node [Parser::AST::Node]
144+
def process_conditional(conditional_node, true_ranges)
145+
if conditional_node.type == :send
146+
process_isa(conditional_node, true_ranges)
147+
elsif conditional_node.type == :and
148+
process_and(conditional_node, true_ranges)
149+
end
150+
end
151+
152+
# @param isa_node [Parser::AST::Node]
153+
# @return [Array(String, String)]
154+
def parse_isa(isa_node)
155+
return unless isa_node.type == :send && isa_node.children[1] == :is_a?
156+
# Check if conditional node follows this pattern:
157+
# s(:send,
158+
# s(:send, nil, :foo), :is_a?,
159+
# s(:const, nil, :Baz)),
160+
isa_receiver = isa_node.children[0]
161+
isa_type_name = type_name(isa_node.children[2])
162+
return unless isa_type_name
163+
164+
# check if isa_receiver looks like this:
165+
# s(:send, nil, :foo)
166+
# and set variable_name to :foo
167+
if isa_receiver.type == :send && isa_receiver.children[0].nil? && isa_receiver.children[1].is_a?(Symbol)
168+
variable_name = isa_receiver.children[1].to_s
169+
end
170+
# or like this:
171+
# (lvar :repr)
172+
variable_name = isa_receiver.children[0].to_s if isa_receiver.type == :lvar
173+
return unless variable_name
174+
175+
[isa_type_name, variable_name]
176+
end
177+
178+
def find_local(variable_name, position)
179+
pins = locals.select { |pin| pin.name == variable_name && pin.presence.include?(position) }
180+
return unless pins.length == 1
181+
pins.first
182+
end
183+
184+
def process_isa(isa_node, true_presences)
185+
isa_type_name, variable_name = parse_isa(isa_node)
186+
return if variable_name.nil? || variable_name.empty?
187+
isa_position = Range.from_node(isa_node).start
188+
189+
pin = find_local(variable_name, isa_position)
190+
return unless pin
191+
192+
if_true = {}
193+
if_true[pin] ||= []
194+
if_true[pin] << { type: isa_type_name }
195+
process_facts(if_true, true_presences)
196+
end
197+
198+
# @param node [Parser::AST::Node]
199+
def type_name(node)
200+
# e.g.,
201+
# s(:const, nil, :Baz)
202+
return unless node.type == :const
203+
module_node = node.children[0]
204+
class_node = node.children[1]
205+
206+
return class_node.to_s if module_node.nil?
207+
208+
module_type_name = type_name(module_node)
209+
return unless module_type_name
210+
211+
"#{module_type_name}::#{class_node}"
212+
end
213+
214+
# @todo "return type could not be inferred" should not trigger here
215+
# @sg-ignore
216+
# @param clause_node [Parser::AST::Node]
217+
def always_breaks?(clause_node)
218+
clause_node&.type == :break
219+
end
220+
221+
attr_reader :locals
222+
223+
attr_reader :enclosing_breakable_pin
224+
end
225+
end
226+
end

lib/solargraph/parser/node_methods.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,20 @@ def process node
7878
def convert_hash node
7979
raise NotImplementedError
8080
end
81+
82+
# @abstract
83+
# @param node [Parser::AST::Node]
84+
# @return [Position]
85+
def get_node_start_position(node)
86+
raise NotImplementedError
87+
end
88+
89+
# @abstract
90+
# @param node [Parser::AST::Node]
91+
# @return [Position]
92+
def get_node_end_position(node)
93+
raise NotImplementedError
94+
end
8195
end
8296
end
8397
end

lib/solargraph/parser/parser_gem/node_chainer.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,22 @@ def generate_links n
5757
elsif n.type == :send
5858
if n.children[0].is_a?(::Parser::AST::Node)
5959
result.concat generate_links(n.children[0])
60-
result.push Chain::Call.new(n.children[1].to_s, node_args(n), passed_block(n))
60+
result.push Chain::Call.new(n.children[1].to_s, Location.from_node(n), node_args(n), passed_block(n))
6161
elsif n.children[0].nil?
6262
args = []
6363
n.children[2..-1].each do |c|
6464
args.push NodeChainer.chain(c, @filename, n)
6565
end
66-
result.push Chain::Call.new(n.children[1].to_s, node_args(n), passed_block(n))
66+
result.push Chain::Call.new(n.children[1].to_s, Location.from_node(n), node_args(n), passed_block(n))
6767
else
6868
raise "No idea what to do with #{n}"
6969
end
7070
elsif n.type == :csend
7171
if n.children[0].is_a?(::Parser::AST::Node)
7272
result.concat generate_links(n.children[0])
73-
result.push Chain::QCall.new(n.children[1].to_s, node_args(n))
73+
result.push Chain::QCall.new(n.children[1].to_s, Location.from_node(n), node_args(n))
7474
elsif n.children[0].nil?
75-
result.push Chain::QCall.new(n.children[1].to_s, node_args(n))
75+
result.push Chain::QCall.new(n.children[1].to_s, Location.from_node(n), node_args(n))
7676
else
7777
raise "No idea what to do with #{n}"
7878
end
@@ -82,15 +82,15 @@ def generate_links n
8282
result.push Chain::ZSuper.new('super')
8383
elsif n.type == :super
8484
args = n.children.map { |c| NodeChainer.chain(c, @filename, n) }
85-
result.push Chain::Call.new('super', args)
85+
result.push Chain::Call.new('super', Location.from_node(n), args)
8686
elsif n.type == :yield
8787
args = n.children.map { |c| NodeChainer.chain(c, @filename, n) }
88-
result.push Chain::Call.new('yield', args)
88+
result.push Chain::Call.new('yield', Location.from_node(n), args)
8989
elsif n.type == :const
9090
const = unpack_name(n)
9191
result.push Chain::Constant.new(const)
9292
elsif [:lvar, :lvasgn].include?(n.type)
93-
result.push Chain::Call.new(n.children[0].to_s)
93+
result.push Chain::Call.new(n.children[0].to_s, Location.from_node(n))
9494
elsif [:ivar, :ivasgn].include?(n.type)
9595
result.push Chain::InstanceVariable.new(n.children[0].to_s)
9696
elsif [:cvar, :cvasgn].include?(n.type)

lib/solargraph/parser/parser_gem/node_processors.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module NodeProcessors
1313
autoload :NamespaceNode, 'solargraph/parser/parser_gem/node_processors/namespace_node'
1414
autoload :SclassNode, 'solargraph/parser/parser_gem/node_processors/sclass_node'
1515
autoload :IvasgnNode, 'solargraph/parser/parser_gem/node_processors/ivasgn_node'
16+
autoload :IfNode, 'solargraph/parser/parser_gem/node_processors/if_node'
1617
autoload :CvasgnNode, 'solargraph/parser/parser_gem/node_processors/cvasgn_node'
1718
autoload :LvasgnNode, 'solargraph/parser/parser_gem/node_processors/lvasgn_node'
1819
autoload :GvasgnNode, 'solargraph/parser/parser_gem/node_processors/gvasgn_node'
@@ -24,6 +25,9 @@ module NodeProcessors
2425
autoload :OrasgnNode, 'solargraph/parser/parser_gem/node_processors/orasgn_node'
2526
autoload :SymNode, 'solargraph/parser/parser_gem/node_processors/sym_node'
2627
autoload :ResbodyNode, 'solargraph/parser/parser_gem/node_processors/resbody_node'
28+
autoload :UntilNode, 'solargraph/parser/parser_gem/node_processors/until_node'
29+
autoload :WhileNode, 'solargraph/parser/parser_gem/node_processors/while_node'
30+
autoload :AndNode, 'solargraph/parser/parser_gem/node_processors/and_node'
2731
end
2832
end
2933

@@ -35,6 +39,7 @@ module NodeProcessor
3539
register :resbody, ParserGem::NodeProcessors::ResbodyNode
3640
register :def, ParserGem::NodeProcessors::DefNode
3741
register :defs, ParserGem::NodeProcessors::DefsNode
42+
register :if, ParserGem::NodeProcessors::IfNode
3843
register :send, ParserGem::NodeProcessors::SendNode
3944
register :class, ParserGem::NodeProcessors::NamespaceNode
4045
register :module, ParserGem::NodeProcessors::NamespaceNode
@@ -51,6 +56,9 @@ module NodeProcessor
5156
register :block, ParserGem::NodeProcessors::BlockNode
5257
register :or_asgn, ParserGem::NodeProcessors::OrasgnNode
5358
register :sym, ParserGem::NodeProcessors::SymNode
59+
register :until, ParserGem::NodeProcessors::UntilNode
60+
register :while, ParserGem::NodeProcessors::WhileNode
61+
register :and, ParserGem::NodeProcessors::AndNode
5462
end
5563
end
5664
end
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# frozen_string_literal: true
2+
3+
module Solargraph
4+
module Parser
5+
module ParserGem
6+
module NodeProcessors
7+
class AndNode < Parser::NodeProcessor::Base
8+
include ParserGem::NodeMethods
9+
10+
def process
11+
process_children
12+
13+
position = get_node_start_position(node)
14+
enclosing_breakable_pin = pins.select{|pin| pin.is_a?(Pin::Breakable) && pin.location.range.contain?(position)}.last
15+
FlowSensitiveTyping.new(locals, enclosing_breakable_pin).process_and(node)
16+
end
17+
end
18+
end
19+
end
20+
end
21+
end
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# frozen_string_literal: true
2+
3+
module Solargraph
4+
module Parser
5+
module ParserGem
6+
module NodeProcessors
7+
class IfNode < Parser::NodeProcessor::Base
8+
include ParserGem::NodeMethods
9+
10+
def process
11+
process_children
12+
13+
position = get_node_start_position(node)
14+
enclosing_breakable_pin = pins.select{|pin| pin.is_a?(Pin::Breakable) && pin.location.range.contain?(position)}.last
15+
FlowSensitiveTyping.new(locals, enclosing_breakable_pin).process_if(node)
16+
end
17+
end
18+
end
19+
end
20+
end
21+
end

0 commit comments

Comments
 (0)