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 633474a29..40acf6354 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/and_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/and_node.rb @@ -12,7 +12,15 @@ def process # any assignment there isn't guaranteed to have executed lhs, rhs = node.children NodeProcessor.process(lhs, region, pins, locals, ivars) - NodeProcessor.process(rhs, region.update(conditional_boundary: Range.from_node(rhs)), pins, locals, ivars) + # not pushed onto `pins` - see resbody_node.rb for why + rhs_cs = Solargraph::Pin::CompoundStatement.new( + location: get_node_location(rhs), + closure: region.closure, + compound_statement: region.compound_statement, + node: rhs, + source: :parser + ) + NodeProcessor.process(rhs, region.update(compound_statement: rhs_cs, conditional: true), pins, locals, ivars) FlowSensitiveTyping.new(locals, ivars, diff --git a/lib/solargraph/parser/parser_gem/node_processors/block_node.rb b/lib/solargraph/parser/parser_gem/node_processors/block_node.rb index 0e87cf935..cf210cb5d 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/block_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/block_node.rb @@ -20,6 +20,7 @@ def process block_pin = Solargraph::Pin::Block.new( location: location, closure: region.closure, + compound_statement: region.compound_statement, node: node, context: context, receiver: node.children[0], @@ -31,7 +32,7 @@ def process # a block's body may execute zero or multiple times (e.g. # Enumerable#each), so an assignment inside it is never # guaranteed to have executed - process_children region.update(closure: block_pin, conditional_boundary: Range.from_node(node)) + process_children region.update(closure: block_pin, compound_statement: block_pin, conditional: true) end private diff --git a/lib/solargraph/parser/parser_gem/node_processors/def_node.rb b/lib/solargraph/parser/parser_gem/node_processors/def_node.rb index f45f5544d..c93f0f80f 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/def_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/def_node.rb @@ -15,6 +15,7 @@ def process methpin = Solargraph::Pin::Method.new( location: get_node_location(node), closure: region.closure, + compound_statement: region.compound_statement, name: name, context: method_context, comments: comments_for(node), @@ -51,7 +52,7 @@ def process else pins.push methpin end - process_children region.update(closure: methpin, scope: methpin.scope) + process_children region.update(closure: methpin, scope: methpin.scope, compound_statement: methpin, conditional: false) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/defs_node.rb b/lib/solargraph/parser/parser_gem/node_processors/defs_node.rb index 09679c7f7..70f058334 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/defs_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/defs_node.rb @@ -22,6 +22,7 @@ def process pins.push Solargraph::Pin::Method.new( location: loc, closure: closure, + compound_statement: region.compound_statement, name: node.children[1].to_s, comments: comments_for(node), scope: :class, @@ -29,7 +30,7 @@ def process node: node, source: :parser ) - process_children region.update(closure: pins.last, scope: :class) + process_children region.update(closure: pins.last, scope: :class, compound_statement: pins.last, conditional: false) 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 0f3a4800c..6120a6ed6 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/if_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/if_node.rb @@ -17,6 +17,7 @@ def process pins.push Solargraph::Pin::CompoundStatement.new( location: get_node_location(condition_node), closure: region.closure, + compound_statement: region.compound_statement, node: condition_node, source: :parser ) @@ -24,26 +25,32 @@ def process end then_node = node.children[1] if then_node - pins.push Solargraph::Pin::CompoundStatement.new( + # @sg-ignore Need to add nil check here + then_cs = Solargraph::Pin::CompoundStatement.new( location: get_node_location(then_node), closure: region.closure, + compound_statement: region.compound_statement, node: then_node, source: :parser ) + pins.push then_cs # @sg-ignore Need to add nil check here - NodeProcessor.process(then_node, region.update(conditional_boundary: Range.from_node(then_node)), pins, locals, ivars) + NodeProcessor.process(then_node, region.update(compound_statement: then_cs, conditional: true), pins, locals, ivars) end else_node = node.children[2] if else_node - pins.push Solargraph::Pin::CompoundStatement.new( + # @sg-ignore Need to add nil check here + else_cs = Solargraph::Pin::CompoundStatement.new( location: get_node_location(else_node), closure: region.closure, + compound_statement: region.compound_statement, node: else_node, source: :parser ) + pins.push else_cs # @sg-ignore Need to add nil check here - NodeProcessor.process(else_node, region.update(conditional_boundary: Range.from_node(else_node)), pins, locals, ivars) + NodeProcessor.process(else_node, region.update(compound_statement: else_cs, conditional: true), pins, locals, ivars) end true diff --git a/lib/solargraph/parser/parser_gem/node_processors/lvasgn_node.rb b/lib/solargraph/parser/parser_gem/node_processors/lvasgn_node.rb index 6d0f97f7f..c3eb6dfac 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/lvasgn_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/lvasgn_node.rb @@ -19,8 +19,8 @@ def process assignment: node.children[1], comments: comments_for(node), presence: presence, - definite: region.conditional_boundary.nil?, - conditional_override_boundary: region.conditional_boundary, + definite: !region.conditional, + compound_statement: region.compound_statement, source: :parser ) process_children diff --git a/lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb b/lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb index 0acbf7ee0..a38762a57 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb @@ -20,6 +20,7 @@ def process type: node.type, location: loc, closure: region.closure, + compound_statement: region.compound_statement, name: name, comments: comments, visibility: :public, @@ -36,7 +37,7 @@ def process source: :parser ) end - process_children region.update(closure: nspin, visibility: :public) + process_children region.update(closure: nspin, visibility: :public, compound_statement: nspin, conditional: false) end private 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 de85f87b4..e64270045 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/or_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/or_node.rb @@ -12,7 +12,15 @@ def process # any assignment there isn't guaranteed to have executed lhs, rhs = node.children NodeProcessor.process(lhs, region, pins, locals, ivars) - NodeProcessor.process(rhs, region.update(conditional_boundary: Range.from_node(rhs)), pins, locals, ivars) + # not pushed onto `pins` - see resbody_node.rb for why + rhs_cs = Solargraph::Pin::CompoundStatement.new( + location: get_node_location(rhs), + closure: region.closure, + compound_statement: region.compound_statement, + node: rhs, + source: :parser + ) + NodeProcessor.process(rhs, region.update(compound_statement: rhs_cs, conditional: true), pins, locals, ivars) FlowSensitiveTyping.new(locals, ivars, diff --git a/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb b/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb index 85f161bf4..271286645 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb @@ -10,7 +10,16 @@ def process new_node = node.updated(node.children[0].type, node.children[0].children + [node.children[1]]) # `x ||= y` only assigns when x is falsy/undefined, so # it's never a guaranteed override of x's prior type - NodeProcessor.process(new_node, region.update(conditional_boundary: Range.from_node(node)), pins, locals, ivars) + # + # not pushed onto `pins` - see resbody_node.rb for why + asgn_cs = Solargraph::Pin::CompoundStatement.new( + location: get_node_location(node), + closure: region.closure, + compound_statement: region.compound_statement, + node: node, + source: :parser + ) + NodeProcessor.process(new_node, region.update(compound_statement: asgn_cs, conditional: true), pins, locals, ivars) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb b/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb index f88b2c7e4..c5f699e09 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb @@ -30,8 +30,20 @@ def process source: :parser ) end + # not pushed onto `pins` - and/or/orasgn/resbody bodies are + # too common to warrant a pin per occurrence, so only the + # pointer is needed for the compound_statement chain # @sg-ignore Need to add nil check here - NodeProcessor.process(node.children[2], region.update(conditional_boundary: Range.from_node(node.children[2])), pins, locals, ivars) + rescue_body_cs = Solargraph::Pin::CompoundStatement.new( + # @sg-ignore Need to add nil check here + location: node.children[2] ? get_node_location(node.children[2]) : nil, + closure: region.closure, + compound_statement: region.compound_statement, + node: node.children[2], + source: :parser + ) + # @sg-ignore Need to add nil check here + NodeProcessor.process(node.children[2], region.update(compound_statement: rescue_body_cs, conditional: true), pins, locals, ivars) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/until_node.rb b/lib/solargraph/parser/parser_gem/node_processors/until_node.rb index 9a9d276bf..47a2d3570 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/until_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/until_node.rb @@ -13,14 +13,16 @@ def process # until statement doesn't create a closure - e.g., # variables created inside can be seen from outside as # well - pins.push Solargraph::Pin::Until.new( + until_pin = Solargraph::Pin::Until.new( location: location, closure: region.closure, + compound_statement: region.compound_statement, node: node, comments: comments_for(node), source: :parser ) - process_children region.update(conditional_boundary: Range.from_node(node)) + pins.push until_pin + process_children region.update(compound_statement: until_pin, conditional: true) end end end diff --git a/lib/solargraph/parser/parser_gem/node_processors/when_node.rb b/lib/solargraph/parser/parser_gem/node_processors/when_node.rb index 60ddb5f18..74a887791 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/when_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/when_node.rb @@ -8,13 +8,15 @@ class WhenNode < Parser::NodeProcessor::Base include ParserGem::NodeMethods def process - pins.push Solargraph::Pin::CompoundStatement.new( + cs = Solargraph::Pin::CompoundStatement.new( location: get_node_location(node), closure: region.closure, + compound_statement: region.compound_statement, node: node, source: :parser ) - process_children region.update(conditional_boundary: Range.from_node(node)) + pins.push cs + process_children region.update(compound_statement: cs, conditional: true) 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 df7841332..866ad0303 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/while_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/while_node.rb @@ -17,14 +17,16 @@ def process # while statement doesn't create a closure - e.g., # variables created inside can be seen from outside as # well - pins.push Solargraph::Pin::While.new( + while_pin = Solargraph::Pin::While.new( location: location, closure: region.closure, + compound_statement: region.compound_statement, node: node, comments: comments_for(node), source: :parser ) - process_children region.update(conditional_boundary: Range.from_node(node)) + pins.push while_pin + process_children region.update(compound_statement: while_pin, conditional: true) end end end diff --git a/lib/solargraph/parser/region.rb b/lib/solargraph/parser/region.rb index 0dd3d9334..fbf34a069 100644 --- a/lib/solargraph/parser/region.rb +++ b/lib/solargraph/parser/region.rb @@ -21,31 +21,47 @@ class Region # @return [Array] attr_reader :lvars - # The source range of the nearest enclosing construct that may - # be skipped at runtime (e.g., an if/while/until body), meaning - # an assignment made at the current position isn't guaranteed - # to have executed at a later position - except at a position - # that itself falls within this same range, where the - # assignment is still guaranteed to dominate. nil if the - # current position isn't inside any such construct. + # The nearest enclosing CompoundStatement pin (an if/when/while/ + # rescue/&&/||/||= body, a method/block body, or a namespace + # body) - a series of statements/expressions where a later one + # executing implies the earlier ones in the same series + # executed too. Every Closure is also a CompoundStatement, so + # this is a superset of the `closure` chain: it additionally + # includes branch bodies that aren't scopes. # - # @return [Range, nil] - attr_reader :conditional_boundary + # @return [Pin::CompoundStatement] + attr_reader :compound_statement + + # True if the current position may be skipped, or run zero or + # multiple times, at runtime - e.g. inside an if/while/until/ + # rescue/&&/||/||= body, or inside a block body (which, despite + # its Block pin being a Closure like Method/Namespace, may run + # zero or many times depending on the method it's passed to, + # unlike a method/namespace body which always runs exactly once + # when reached). Not derivable from `compound_statement.is_a? + # (Closure)` alone for that reason - Block is the case where + # "is a Closure" and "unconditionally executes" diverge. + # + # @return [Boolean] + attr_reader :conditional # @param source [Source] # @param closure [Pin::Closure, nil] # @param scope [Symbol, nil] # @param visibility [Symbol] # @param lvars [Array] - # @param conditional_boundary [Range, nil] + # @param compound_statement [Pin::CompoundStatement, nil] + # @param conditional [Boolean] def initialize source: Solargraph::Source.load_string(''), closure: nil, - scope: nil, visibility: :public, lvars: [], conditional_boundary: nil + scope: nil, visibility: :public, lvars: [], + compound_statement: nil, conditional: false @source = source @closure = closure || Pin::Namespace.new(name: '', location: source.location, source: :parser) + @compound_statement = compound_statement || @closure @scope = scope @visibility = visibility @lvars = lvars - @conditional_boundary = conditional_boundary + @conditional = conditional end # @return [String, nil] @@ -67,16 +83,19 @@ def namespace_pin # @param scope [Symbol, nil] # @param visibility [Symbol, nil] # @param lvars [Array, nil] - # @param conditional_boundary [Range, nil] + # @param compound_statement [Pin::CompoundStatement, nil] + # @param conditional [Boolean, nil] # @return [Region] - def update closure: nil, scope: nil, visibility: nil, lvars: nil, conditional_boundary: nil + def update closure: nil, scope: nil, visibility: nil, lvars: nil, + compound_statement: nil, conditional: nil Region.new( source: source, closure: closure || self.closure, scope: scope || self.scope, visibility: visibility || self.visibility, lvars: lvars || self.lvars, - conditional_boundary: conditional_boundary || self.conditional_boundary + compound_statement: compound_statement || self.compound_statement, + conditional: conditional.nil? ? self.conditional : conditional ) end diff --git a/lib/solargraph/pin/base.rb b/lib/solargraph/pin/base.rb index f7ae58d38..b4a611cf8 100644 --- a/lib/solargraph/pin/base.rb +++ b/lib/solargraph/pin/base.rb @@ -75,6 +75,7 @@ def assert_location_provided # @return [Pin::Closure, nil] def closure + @closure ||= derive_closure_from_compound_statement unless @closure Solargraph.assert_or_log(:closure, "Closure not set on #{self.class} #{name.inspect} from #{source.inspect}") @@ -731,6 +732,25 @@ def equality_fields private + # Fallback for pins with no directly-assigned @closure: walk the + # CompoundStatement parent chain (present only on + # CompoundStatement-family pins - Closure, While, Until, etc.) + # until an ancestor is_a?(Closure). Every pin built through + # Region-threaded node processors already gets an explicit + # closure:, so this only matters for a pin constructed purely + # from a compound_statement chain with no closure: override. + # + # @return [Pin::Closure, nil] + def derive_closure_from_compound_statement + return nil unless is_a?(CompoundStatement) + + # @sg-ignore flow sensitive typing doesn't narrow self past an is_a? guard + cs = compound_statement + # @sg-ignore flow sensitive typing doesn't narrow self past an is_a? guard + cs = cs.compound_statement while cs && !cs.is_a?(Closure) + cs + end + # @return [void] def parse_comments # HACK: Avoid a NoMethodError on nil with empty overload tags diff --git a/lib/solargraph/pin/base_variable.rb b/lib/solargraph/pin/base_variable.rb index 7ef5aa42c..38d409b34 100644 --- a/lib/solargraph/pin/base_variable.rb +++ b/lib/solargraph/pin/base_variable.rb @@ -17,8 +17,13 @@ class BaseVariable < Base # @return [Boolean] attr_reader :definite - # @return [Range, nil] - attr_reader :conditional_override_boundary + # The CompoundStatement pin this variable's (re)assignment was + # made within - i.e. Region#compound_statement at the point of + # assignment. Used by #definite_reaches? to decide whether a + # non-definite assignment still dominates a given reference. + # + # @return [Pin::CompoundStatement, nil] + attr_reader :compound_statement # @param return_type [ComplexType, nil] # @param assignment [Parser::AST::Node, nil] First assignment @@ -58,20 +63,18 @@ class BaseVariable < Base # reassignment's type may safely override a variable's # previously declared/inferred type instead of merely being # unioned with it. - # @param conditional_override_boundary [Range, nil] When - # `definite` is false because this assignment is inside a - # conditional branch or loop, the source range of that - # construct's body - i.e., the extent within which this - # assignment, though not globally guaranteed, is still - # guaranteed to dominate any reference. A reference at a - # position inside this range may still treat the assignment - # as an override rather than merely unioning it with earlier - # possible types. + # @param compound_statement [Pin::CompoundStatement, nil] The + # CompoundStatement this variable's (re)assignment was made + # within. When `definite` is false, a reference whose location + # falls within this pin's own range may still treat the + # assignment as an override rather than merely unioning it + # with earlier possible types - see #definite_reaches?. # @param [Hash{Symbol => Object}] splat def initialize assignment: nil, assignments: [], mass_assignment: nil, presence: nil, return_type: nil, intersection_return_type: nil, exclude_return_type: nil, - definite: true, conditional_override_boundary: nil, + definite: true, + compound_statement: nil, **splat super(**splat) @assignments = (assignment.nil? ? [] : [assignment]) + assignments @@ -82,7 +85,7 @@ def initialize assignment: nil, assignments: [], mass_assignment: nil, @exclude_return_type = exclude_return_type @presence = presence @definite = definite - @conditional_override_boundary = conditional_override_boundary + @compound_statement = compound_statement end # @param presence [Range] @@ -112,7 +115,7 @@ def reset_generated! # @param location [Location, nil] The position being resolved, # if known - used to decide whether a not-globally-definite # `other` should still override us because the position falls - # within `other`'s conditional_override_boundary. + # within `other`'s compound_statement. def combine_with other, attrs = {}, location: nil new_assignments = combine_assignments(other, location) new_attrs = attrs.merge({ @@ -387,7 +390,7 @@ def within_own_assignment? other_loc # @param other [self] # @param location [Location, nil] The position being resolved, # if known - lets a conditional `other` still override us when - # `location` falls inside `other`'s conditional_override_boundary. + # `location` falls within `other`'s compound_statement. # @return [Boolean] def override_assignments? other, location = nil (other.definite || other.definite_reaches?(location)) && other.closure == closure && @@ -398,18 +401,24 @@ def override_assignments? other, location = nil # True if this pin's assignment, though not globally definite, # is still guaranteed to dominate `location` - i.e., `location` - # falls inside the conditional construct's body that this - # assignment was made in, so no earlier branch exit could have - # skipped it by the time `location` is reached. + # falls within the CompoundStatement body (an if/while/until/ + # rescue/&&/||/||= branch) this assignment was made in, so no + # earlier branch exit could have skipped it by the time + # `location` is reached. A nested CompoundStatement's location + # is always a subrange of its parent's, so this single + # containment check already accounts for arbitrarily nested + # branches without walking the compound_statement chain further. # # @param location [Location, nil] # @return [Boolean] def definite_reaches? location - boundary = conditional_override_boundary - return false unless location && boundary + cs = compound_statement + return false unless location && cs&.location&.range - location.filename == self.location&.filename && - boundary.contain?(location.range.start) + # @sg-ignore flow sensitive typing needs to handle attrs + cs.location.filename == location.filename && + # @sg-ignore flow sensitive typing needs to handle attrs + cs.location.range.contain?(location.range.start) end private diff --git a/lib/solargraph/pin/compound_statement.rb b/lib/solargraph/pin/compound_statement.rb index 39d9cf2d5..dec7ab994 100644 --- a/lib/solargraph/pin/compound_statement.rb +++ b/lib/solargraph/pin/compound_statement.rb @@ -44,11 +44,54 @@ module Pin class CompoundStatement < Pin::Base attr_reader :node + # The immediately enclosing CompoundStatement, if any - nil only + # for the synthetic root Namespace Region creates for top-level + # code. Since Closure < CompoundStatement, walking this chain + # until an ancestor is_a?(Closure) is how Base#closure is + # derived when a pin has no directly-assigned @closure. + # + # @return [Pin::CompoundStatement, nil] + attr_reader :compound_statement + # @param node [Parser::AST::Node, nil] + # @param compound_statement [Pin::CompoundStatement, nil] # @param [Hash{Symbol => Object}] splat - def initialize node: nil, **splat + def initialize node: nil, compound_statement: nil, **splat super(**splat) @node = node + @compound_statement = compound_statement + end + + # @param other [self] + # @param attrs [Hash{Symbol => Object}] + # @return [self] + def combine_with other, attrs = {} + new_attrs = { compound_statement: combine_compound_statement(other) }.merge(attrs) + super(other, new_attrs) + end + + # Bare CompoundStatement pins (if/when/rescue/&&/||/||= bodies) + # all share name == '', so the same-name-assertion in + # Base#choose_pin_attr_with_same_name (used by #combine_closure) + # would be meaningless noise here - pick by location instead, + # mirroring BaseVariable#combine_closure. + # + # @param other [self] + # @return [Pin::CompoundStatement, nil] + def combine_compound_statement other + return compound_statement if compound_statement == other.compound_statement + return compound_statement || other.compound_statement if compound_statement.nil? || other.compound_statement.nil? + + # @sg-ignore flow sensitive typing needs to handle attrs + if compound_statement.location.nil? || other.compound_statement.location.nil? + # @sg-ignore flow sensitive typing needs to handle attrs + return compound_statement.location.nil? ? other.compound_statement : compound_statement + end + + # @sg-ignore flow sensitive typing needs to handle attrs + return compound_statement if compound_statement.location <= other.compound_statement.location + + other.compound_statement end end end diff --git a/spec/pin/compound_statement_spec.rb b/spec/pin/compound_statement_spec.rb new file mode 100644 index 000000000..5a6a1e0d9 --- /dev/null +++ b/spec/pin/compound_statement_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +describe Solargraph::Pin::CompoundStatement do + # Every pin built through Region-threaded node processors still gets + # an explicit `closure:`, so `Pin::Base#closure` returns the stored + # value, not the derived one - the derivation only kicks in as a + # fallback. These specs check the two would agree anyway, so a + # future node processor that updates one threading (closure: or + # compound_statement:) without the other gets caught here instead + # of silently drifting. + def derive_closure pin + cs = pin.compound_statement + cs = cs.compound_statement while cs && !cs.is_a?(Solargraph::Pin::Closure) + cs + end + + it 'agrees with the stored closure for compound statements nested in a method, if, and while' do + source_map = Solargraph::SourceMap.load_string(%( + class Foo + def bar(flag) + if flag + while flag + local = 1 + end + end + end + end + )) + + compound_statement_pins = source_map.pins.select { |pin| pin.is_a?(described_class) } + expect(compound_statement_pins).not_to be_empty + + compound_statement_pins.each do |pin| + expect(derive_closure(pin)).to eq(pin.closure), "mismatch for #{pin.inspect}" + end + end + + it 'agrees with the stored closure for compound statements nested in a block' do + source_map = Solargraph::SourceMap.load_string(%( + class Foo + def bar + [1].each do |i| + if i + local = i + end + end + end + end + )) + + compound_statement_pins = source_map.pins.select { |pin| pin.is_a?(described_class) } + expect(compound_statement_pins).not_to be_empty + + compound_statement_pins.each do |pin| + expect(derive_closure(pin)).to eq(pin.closure), "mismatch for #{pin.inspect}" + end + end + + it 'derives the enclosing method as closure for a bare CompoundStatement built only with compound_statement:' do + source_map = Solargraph::SourceMap.load_string(%( + class Foo + def bar + 1 + end + end + )) + method_pin = source_map.pins.find { |pin| pin.is_a?(Solargraph::Pin::Method) && pin.name == 'bar' } + + bare_pin = described_class.new( + location: method_pin.location, + compound_statement: method_pin, + source: :parser + ) + + expect(bare_pin.closure).to eq(method_pin) + end + + describe '#combine_with' do + let(:earlier_location) { Solargraph::Location.new('test.rb', Solargraph::Range.from_to(1, 0, 3, 0)) } + let(:later_location) { Solargraph::Location.new('test.rb', Solargraph::Range.from_to(5, 0, 7, 0)) } + + it 'prefers the compound_statement with the earlier location' do + earlier_cs = described_class.new(location: earlier_location, source: :parser) + later_cs = described_class.new(location: later_location, source: :parser) + pin1 = described_class.new(location: earlier_location, compound_statement: earlier_cs, source: :parser) + pin2 = described_class.new(location: later_location, compound_statement: later_cs, source: :parser) + + expect(pin1.combine_with(pin2).compound_statement).to eq(earlier_cs) + expect(pin2.combine_with(pin1).compound_statement).to eq(earlier_cs) + end + + it 'prefers a non-nil compound_statement over a nil one' do + cs = described_class.new(location: earlier_location, source: :parser) + pin1 = described_class.new(location: earlier_location, compound_statement: nil, source: :parser) + pin2 = described_class.new(location: earlier_location, compound_statement: cs, source: :parser) + + expect(pin1.combine_with(pin2).compound_statement).to eq(cs) + expect(pin2.combine_with(pin1).compound_statement).to eq(cs) + end + end +end diff --git a/spec/type_checker/levels/strong_spec.rb b/spec/type_checker/levels/strong_spec.rb index 728b52d7c..195e862c1 100644 --- a/spec/type_checker/levels/strong_spec.rb +++ b/spec/type_checker/levels/strong_spec.rb @@ -956,6 +956,23 @@ def conditional_reassign(str, num, flag) expect(checker.problems.map(&:message)).to eq([]) end + it 'does not let a loop-body reassignment override a reference textually before it' do + checker = type_checker(%( + # @param str [String] + # @param num [Integer] + # @param flag [Boolean] + # @return [void] + def loop_reassign(str, num, flag) + local = num + while flag + local.abs + local = str + end + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + it 'updates a local variable type after reassignment to a different literal type' do checker = type_checker(%( # @return [void]