Update a parameter's flow-sensitive type after reassignment to a non-literal type

lib/solargraph/api_map.rb

@@ -416,7 +416,7 @@ def var_at_location candidates, name, closure, location
         !pin.visible_at?(closure, location) && !pin.starts_at?(location)
       end
 
-      vars_at_location.inject(&:combine_with)
+      vars_at_location.inject { |acc, pin| acc.combine_with(pin, location: location) }
     end
 
     # Get an array of class variable pins for a namespace.
@@ -706,14 +706,14 @@ def super_and_sub? sup, sub
       # @todo If two literals are different values of the same type, it would
       #   make more sense for super_and_sub? to return true, but there are a
       #   few callers that currently expect this to be false.
-      # @sg-ignore flow-sensitive typing should be able to handle redefinition
       return false if sup.literal? && sub.literal? && sup.to_s != sub.to_s
       # @sg-ignore flow sensitive typing should be able to handle redefinition
       sup = sup.simplify_literals.to_s
       # @sg-ignore flow sensitive typing should be able to handle redefinition
       sub = sub.simplify_literals.to_s
       return true if sup == sub
       sc_fqns = sub
+      # @sg-ignore flow sensitive typing unions rather than overrides types across multiple sequential reassignments
       while (sc = store.get_superclass(sc_fqns))
         # @sg-ignore flow sensitive typing needs to handle "if foo = bar"
         sc_new = store.constants.dereference(sc)

lib/solargraph/complex_type.rb

@@ -224,6 +224,7 @@ def conforms_to? api_map, expected,
                      situation,
                      rules = [],
                      variance: erased_variance(situation)
+      # @sg-ignore flow sensitive typing needs to handle a self-referential reassignment (x = x.foo)
       expected = expected.downcast_to_literal_if_possible
       inferred = downcast_to_literal_if_possible
 

lib/solargraph/language_server/message/text_document/formatting.rb

@@ -48,6 +48,7 @@ def log_corrections corrections
             return if corrections&.empty?
 
             Solargraph.logger.info('Formatting result:')
+            # @sg-ignore flow sensitive typing should be able to handle redefinition
             corrections.each_line do |line|
               next if line.strip.empty?
               Solargraph.logger.info(line.strip)

lib/solargraph/parser/flow_sensitive_typing.rb

@@ -9,11 +9,31 @@ class FlowSensitiveTyping
       # @param ivars [Array<Solargraph::Pin::InstanceVariable>]
       # @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 only_downcast_these_names [Array<String>, nil] If given,
+      #   only apply a downcast to variables with these names, ignoring
+      #   any other variable the analyzed condition happens to mention.
+      def initialize locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin,
+                     only_downcast_these_names: nil
         @locals = locals
         @ivars = ivars
         @enclosing_breakable_pin = enclosing_breakable_pin
         @enclosing_compound_statement_pin = enclosing_compound_statement_pin
+        @only_downcast_these_names = only_downcast_these_names
+      end
+
+      # Assert what a true/false expression implies over the given ranges.
+      # Public for instances configured with only_downcast_these_names.
+      #
+      # @param expression_node [Parser::AST::Node]
+      # @param true_ranges [Array<Range>]
+      # @param false_ranges [Array<Range>]
+      #
+      # @return [void]
+      def process_expression expression_node, true_ranges, false_ranges
+        process_calls(expression_node, true_ranges, false_ranges)
+        process_and(expression_node, true_ranges, false_ranges)
+        process_or(expression_node, true_ranges, false_ranges)
+        process_variable(expression_node, true_ranges, false_ranges)
       end
 
       # @param and_node [Parser::AST::Node]
@@ -153,6 +173,9 @@ def process_if if_node, true_ranges = [], false_ranges = []
         end
 
         process_expression(conditional_node, true_ranges, false_ranges)
+
+        # @sg-ignore RBS Array[self] indexing infers Array instead of self
+        process_guarded_reassignment(if_node, conditional_node, then_clause, else_clause)
       end
 
       # @param while_node [Parser::AST::Node]
@@ -198,13 +221,78 @@ class << self
 
       private
 
+      # For `tasks = ['a'] if tasks.nil?`, code after the conditional also
+      # gets the else-branch facts; the firing path is already handled by
+      # unioning in the assignment pin. Restricted to names the clause
+      # definitely reassigns, or `x.nil? || y.nil?` would narrow `y` too.
+      #
+      # @param if_node [Parser::AST::Node]
+      # @param conditional_node [Parser::AST::Node]
+      # @param then_clause [Parser::AST::Node, nil]
+      # @param else_clause [Parser::AST::Node, nil]
+      #
+      # @return [void]
+      def process_guarded_reassignment if_node, conditional_node, then_clause, else_clause
+        compound_statement_node = enclosing_compound_statement_pin&.node
+        return if compound_statement_node.nil?
+
+        rest_of_compound_statement = Range.new(get_node_end_position(if_node),
+                                               get_node_end_position(compound_statement_node))
+
+        # the then clause ran only when the condition was true, so the
+        # path that preserved the original value is the false one -
+        # and vice versa for the else clause
+        assert_after_guard(conditional_node, definitely_assigned_names(then_clause),
+                           [], [rest_of_compound_statement])
+        assert_after_guard(conditional_node, definitely_assigned_names(else_clause),
+                           [rest_of_compound_statement], [])
+      end
+
+      # Applies, not checks: `names` is already assigned unconditionally
+      # over these ranges, so the narrowed types take effect as fact.
+      #
+      # @param conditional_node [Parser::AST::Node]
+      # @param names [Array<String>]
+      # @param true_ranges [Array<Range>]
+      # @param false_ranges [Array<Range>]
+      #
+      # @return [void]
+      def assert_after_guard conditional_node, names, true_ranges, false_ranges
+        return if names.empty?
+
+        FlowSensitiveTyping.new(locals, ivars, enclosing_breakable_pin, enclosing_compound_statement_pin,
+                                only_downcast_these_names: names)
+                           .process_expression(conditional_node, true_ranges, false_ranges)
+      end
+
+      # Names this clause assigns on every path. Only unconditional plain
+      # assignments count; `||=`/`+=` keep the previous value in play.
+      #
+      # @param clause_node [Parser::AST::Node, nil]
+      #
+      # @return [Array<String>]
+      def definitely_assigned_names clause_node
+        return [] if clause_node.nil?
+
+        case clause_node.type
+        when :lvasgn, :ivasgn
+          [clause_node.children[0].to_s]
+        when :begin, :kwbegin
+          clause_node.children.flat_map { |child| definitely_assigned_names(child) }
+        else
+          []
+        end
+      end
+
       # @param pin [Pin::BaseVariable]
       # @param presence [Range]
       # @param downcast_type [ComplexType, nil]
       # @param downcast_not_type [ComplexType, nil]
       #
       # @return [void]
       def add_downcast_var pin, presence:, downcast_type:, downcast_not_type:
+        return if only_downcast_these_names && !only_downcast_these_names.include?(pin.name)
+
         new_pin = pin.downcast(exclude_return_type: downcast_not_type,
                                intersection_return_type: downcast_type,
                                source: :flow_sensitive_typing,
@@ -240,18 +328,6 @@ def process_facts facts_by_pin, presences
         end
       end
 
-      # @param expression_node [Parser::AST::Node]
-      # @param true_ranges [Array<Range>]
-      # @param false_ranges [Array<Range>]
-      #
-      # @return [void]
-      def process_expression expression_node, true_ranges, false_ranges
-        process_calls(expression_node, true_ranges, false_ranges)
-        process_and(expression_node, true_ranges, false_ranges)
-        process_or(expression_node, true_ranges, false_ranges)
-        process_variable(expression_node, true_ranges, false_ranges)
-      end
-
       # @param call_node [Parser::AST::Node]
       # @param method_name [Symbol]
       # @return [Array(String, String), nil] Tuple of rgument to
@@ -298,13 +374,19 @@ def parse_isa isa_node
       #   return type could not be inferred
       # @return [Solargraph::Pin::LocalVariable, Solargraph::Pin::InstanceVariable, nil]
       def find_var variable_name, position
-        if variable_name.start_with?('@')
+        pins = variable_name.start_with?('@') ? ivars : locals
+        # Latest-starting presence wins: an original declaration and a
+        # later reassignment can both cover this position. Skip pins
+        # still evaluating their own RHS (`baz ||= begin ... end`).
+        matches = pins.select do |pin|
+          next false unless pin.name == variable_name
           # @sg-ignore flow sensitive typing needs to handle attrs
-          ivars.find { |ivar| ivar.name == variable_name && (!ivar.presence || ivar.presence.include?(position)) }
-        else
-          # @sg-ignore flow sensitive typing needs to handle attrs
-          locals.find { |pin| pin.name == variable_name && (!pin.presence || pin.presence.include?(position)) }
+          next false unless !pin.presence || pin.presence.include?(position)
+
+          other_loc = Location.new(pin.location&.filename, Range.new(position, position))
+          !pin.within_own_assignment?(other_loc)
         end
+        matches.max_by { |pin| pin.presence&.start || Position.new(0, 0) }
       end
 
       # @param isa_node [Parser::AST::Node]
@@ -465,7 +547,8 @@ 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,
+                  :only_downcast_these_names
     end
   end
 end

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

@@ -8,7 +8,20 @@ class AndNode < Parser::NodeProcessor::Base
           include ParserGem::NodeMethods
 
           def process
-            process_children
+            # the rhs of `a && b` only executes if `a` is truthy, so
+            # any assignment there isn't guaranteed to have executed
+            lhs, rhs = node.children
+            NodeProcessor.process(lhs, region, 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,
+              conditional: true,
+              node: rhs,
+              source: :parser
+            )
+            NodeProcessor.process(rhs, region.update(compound_statement: rhs_cs), pins, locals, ivars)
 
             FlowSensitiveTyping.new(locals,
                                     ivars,

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

@@ -23,6 +23,9 @@ def process
                     # @sg-ignore Need to add nil check here
                     presence: callable.location.range,
                     decl: get_decl(u),
+                    # a default value is assigned only when the caller
+                    # omits the arg, so it cannot override @param
+                    definite: false,
                     source: :parser
                   )
                   callable.parameters.push locals.last

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

@@ -20,6 +20,11 @@ def process
             block_pin = Solargraph::Pin::Block.new(
               location: location,
               closure: region.closure,
+              compound_statement: region.compound_statement,
+              # 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
+              conditional: true,
               node: node,
               context: context,
               receiver: node.children[0],
@@ -28,7 +33,7 @@ def process
               source: :parser
             )
             pins.push block_pin
-            process_children region.update(closure: block_pin)
+            process_children region.update(closure: block_pin, compound_statement: block_pin)
           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)
           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)
           end
         end
       end

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

@@ -17,31 +17,42 @@ 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 RBS Array[self] indexing infers Array instead of self
+              then_cs = Solargraph::Pin::CompoundStatement.new(
                 location: get_node_location(then_node),
                 closure: region.closure,
+                compound_statement: region.compound_statement,
+                conditional: true,
                 node: then_node,
                 source: :parser
               )
-              NodeProcessor.process(then_node, region, pins, locals, ivars)
+              pins.push then_cs
+              # @sg-ignore RBS Array[self] indexing infers Array instead of self
+              NodeProcessor.process(then_node, region.update(compound_statement: then_cs), pins, locals, ivars)
             end
 
             else_node = node.children[2]
             if else_node
-              pins.push Solargraph::Pin::CompoundStatement.new(
+              # @sg-ignore RBS Array[self] indexing infers Array instead of self
+              else_cs = Solargraph::Pin::CompoundStatement.new(
                 location: get_node_location(else_node),
                 closure: region.closure,
+                compound_statement: region.compound_statement,
+                conditional: true,
                 node: else_node,
                 source: :parser
               )
-              NodeProcessor.process(else_node, region, pins, locals, ivars)
+              pins.push else_cs
+              # @sg-ignore RBS Array[self] indexing infers Array instead of self
+              NodeProcessor.process(else_node, region.update(compound_statement: else_cs), pins, locals, ivars)
             end
 
             true

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

@@ -19,6 +19,8 @@ def process
               assignment: node.children[1],
               comments: comments_for(node),
               presence: presence,
+              definite: !region.compound_statement.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,
@@ -35,7 +36,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)
           end
 
           private

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

@@ -8,7 +8,20 @@ class OrNode < Parser::NodeProcessor::Base
           include ParserGem::NodeMethods
 
           def process
-            process_children
+            # the rhs of `a || b` only executes if `a` is falsy, so
+            # any assignment there isn't guaranteed to have executed
+            lhs, rhs = node.children
+            NodeProcessor.process(lhs, region, 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,
+              conditional: true,
+              node: rhs,
+              source: :parser
+            )
+            NodeProcessor.process(rhs, region.update(compound_statement: rhs_cs), pins, locals, ivars)
 
             FlowSensitiveTyping.new(locals,
                                     ivars,