Add a CompoundStatement parent chain and use it for reassignment override eligibility

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,

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

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

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

@@ -22,14 +22,15 @@ 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,
               visibility: s_visi,
               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

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

@@ -17,33 +17,40 @@ 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
               )
               NodeProcessor.process(condition_node, region, pins, locals, ivars)
             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

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

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

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,

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

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

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

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

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

lib/solargraph/parser/region.rb

@@ -21,31 +21,47 @@ class Region
       # @return [Array<Symbol>]
       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<Symbol>]
-      # @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<Symbol>, 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
 

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