diff --git a/lib/solargraph/api_map.rb b/lib/solargraph/api_map.rb index 26b42ddb47..724fc443e3 100755 --- a/lib/solargraph/api_map.rb +++ b/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,7 +706,6 @@ 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 @@ -714,6 +713,7 @@ def super_and_sub? sup, sub 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) diff --git a/lib/solargraph/complex_type.rb b/lib/solargraph/complex_type.rb index 5b0e984b25..411a9cb5d5 100644 --- a/lib/solargraph/complex_type.rb +++ b/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 diff --git a/lib/solargraph/language_server/message/text_document/formatting.rb b/lib/solargraph/language_server/message/text_document/formatting.rb index c6cc3353a5..7212d677dc 100644 --- a/lib/solargraph/language_server/message/text_document/formatting.rb +++ b/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) diff --git a/lib/solargraph/parser/flow_sensitive_typing.rb b/lib/solargraph/parser/flow_sensitive_typing.rb index 1606a32e06..9161b443dc 100644 --- a/lib/solargraph/parser/flow_sensitive_typing.rb +++ b/lib/solargraph/parser/flow_sensitive_typing.rb @@ -9,11 +9,31 @@ 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 only_downcast_these_names [Array, 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] + # @param false_ranges [Array] + # + # @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,6 +221,69 @@ 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] + # @param true_ranges [Array] + # @param false_ranges [Array] + # + # @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] + 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] @@ -205,6 +291,8 @@ class << self # # @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] - # @param false_ranges [Array] - # - # @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 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 83f14a4157..ae1ab31d71 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/and_node.rb +++ b/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, diff --git a/lib/solargraph/parser/parser_gem/node_processors/args_node.rb b/lib/solargraph/parser/parser_gem/node_processors/args_node.rb index 9a22b8edd0..dcf1b5a04d 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/args_node.rb +++ b/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 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 750bb99294..08add69d9c 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,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 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 f45f5544df..b6e6137d68 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) 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 09679c7f76..9690fcf879 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) 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 0b9a75e774..6047564f04 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,24 +25,34 @@ def process 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 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 63e2c55dcd..33c429a931 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/lvasgn_node.rb +++ b/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 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 7cecd22c09..5ffedbc88f 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, @@ -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 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 6c54f1c8c1..b27c28806a 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/or_node.rb +++ b/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, 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 17480adfb5..84c45081c7 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/orasgn_node.rb @@ -8,7 +8,17 @@ class OrasgnNode < Parser::NodeProcessor::Base # @return [void] def process new_node = node.updated(node.children[0].type, node.children[0].children + [node.children[1]]) - NodeProcessor.process(new_node, region, pins, locals, ivars) + # `x ||= y` assigns only when x is falsy, so it never overrides + # x's prior type. Not pushed onto `pins` - see resbody_node.rb. + asgn_cs = Solargraph::Pin::CompoundStatement.new( + location: get_node_location(node), + closure: region.closure, + compound_statement: region.compound_statement, + conditional: true, + node: node, + source: :parser + ) + NodeProcessor.process(new_node, region.update(compound_statement: asgn_cs), 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 24846748fb..c093fa5dba 100644 --- a/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb +++ b/lib/solargraph/parser/parser_gem/node_processors/resbody_node.rb @@ -30,7 +30,22 @@ def process source: :parser ) end - NodeProcessor.process(node.children[2], region, pins, locals, ivars) + # Not pushed onto `pins`: an and/or/orasgn/resbody body has no + # identity worth indexing the way a def/class does. It exists only + # so the pins below it carry it as `compound_statement`, and is + # discarded once processing returns. + # @sg-ignore RBS Array[self] indexing infers Array instead of self + rescue_body_cs = Solargraph::Pin::CompoundStatement.new( + # @sg-ignore RBS Array[self] indexing infers Array instead of self + location: node.children[2] ? get_node_location(node.children[2]) : nil, + closure: region.closure, + compound_statement: region.compound_statement, + conditional: true, + node: node.children[2], + source: :parser + ) + # @sg-ignore RBS Array[self] indexing infers Array instead of self + NodeProcessor.process(node.children[2], region.update(compound_statement: rescue_body_cs), 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 2e091f41d1..f345e00953 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,17 @@ 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, + conditional: true, node: node, comments: comments_for(node), source: :parser ) - process_children region + pins.push until_pin + process_children region.update(compound_statement: until_pin) 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 915eb57e65..144220d48c 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,16 @@ 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, + conditional: true, node: node, source: :parser ) - process_children + pins.push cs + process_children region.update(compound_statement: cs) 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 6c4fe33d86..44b30f84eb 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,17 @@ 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, + conditional: true, node: node, comments: comments_for(node), source: :parser ) - process_children region + pins.push while_pin + process_children region.update(compound_statement: while_pin) end end end diff --git a/lib/solargraph/parser/region.rb b/lib/solargraph/parser/region.rb index 8c4caf6acb..3edc5e369b 100644 --- a/lib/solargraph/parser/region.rb +++ b/lib/solargraph/parser/region.rb @@ -6,6 +6,9 @@ module Parser # source. # class Region + # Nearest enclosing Closure (method, block, or namespace). Kept as + # its own field: it changes far less often than `compound_statement`. + # # @return [Pin::Closure] attr_reader :closure @@ -21,15 +24,25 @@ class Region # @return [Array] attr_reader :lvars + # Nearest enclosing CompoundStatement - a statement series where a + # later one running implies the earlier ones did. Superset of + # `closure`, adding branch bodies that are not scopes. + # + # @return [Pin::CompoundStatement] + attr_reader :compound_statement + # @param source [Source] # @param closure [Pin::Closure, nil] # @param scope [Symbol, nil] # @param visibility [Symbol] # @param lvars [Array] + # @param compound_statement [Pin::CompoundStatement, nil] def initialize source: Solargraph::Source.load_string(''), closure: nil, - scope: nil, visibility: :public, lvars: [] + scope: nil, visibility: :public, lvars: [], + compound_statement: nil @source = source @closure = closure || Pin::Namespace.new(name: '', location: source.location, source: :parser) + @compound_statement = compound_statement || @closure @scope = scope @visibility = visibility @lvars = lvars @@ -54,14 +67,17 @@ def namespace_pin # @param scope [Symbol, nil] # @param visibility [Symbol, nil] # @param lvars [Array, nil] + # @param compound_statement [Pin::CompoundStatement, nil] # @return [Region] - def update closure: nil, scope: nil, visibility: nil, lvars: nil + def update closure: nil, scope: nil, visibility: nil, lvars: nil, + compound_statement: nil Region.new( source: source, closure: closure || self.closure, scope: scope || self.scope, visibility: visibility || self.visibility, - lvars: lvars || self.lvars + lvars: lvars || self.lvars, + compound_statement: compound_statement || self.compound_statement ) end diff --git a/lib/solargraph/pin/base.rb b/lib/solargraph/pin/base.rb index f7ae58d388..363908c988 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,20 @@ def equality_fields private + # Fallback for a pin with no @closure of its own: walk the + # compound_statement chain up to the first Closure ancestor. + # + # @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 c7945e5998..c2a4ba12db 100644 --- a/lib/solargraph/pin/base_variable.rb +++ b/lib/solargraph/pin/base_variable.rb @@ -14,6 +14,15 @@ class BaseVariable < Base # @return [Range, nil] attr_reader :presence + # @return [Boolean] + attr_reader :definite + + # The CompoundStatement this assignment was made within. Used by + # #definite_reaches? to decide whether it dominates a reference. + # + # @return [Pin::CompoundStatement, nil] + attr_reader :compound_statement + # @param return_type [ComplexType, nil] # @param assignment [Parser::AST::Node, nil] First assignment # that was made to this variable @@ -45,10 +54,18 @@ class BaseVariable < Base # @see https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#union-types # @see https://en.wikipedia.org/wiki/Intersection_type#TypeScript_example # @param presence [Range, nil] + # @param definite [Boolean] True if the assignment is guaranteed to + # have run by its presence start, so its type may override rather + # than union with earlier ones. + # @param compound_statement [Pin::CompoundStatement, nil] Where the + # assignment was made; lets a non-definite one still override + # within its own range - 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, + compound_statement: nil, **splat super(**splat) @assignments = (assignment.nil? ? [] : [assignment]) + assignments @@ -58,6 +75,8 @@ def initialize assignment: nil, assignments: [], mass_assignment: nil, @intersection_return_type = intersection_return_type @exclude_return_type = exclude_return_type @presence = presence + @definite = definite + @compound_statement = compound_statement end # @param presence [Range] @@ -82,20 +101,30 @@ def reset_generated! super end - def combine_with other, attrs = {} - new_assignments = combine_assignments(other) + # @param other [self] + # @param attrs [Hash] + # @param location [Location, nil] Position being resolved, if known - + # lets a non-definite `other` still override within its own range. + def combine_with other, attrs = {}, location: nil + new_assignments = combine_assignments(other, location) new_attrs = attrs.merge({ # default values don't exist in RBS parameters; it just # tells you if the arg is optional or not. Prefer a # provided value if we have one here since we can't rely on # it from RBS so we can infer from it and typecheck on it. - assignment: choose(other, :assignment), + # + # Skipped when #combine_assignments supersedes: the + # constructor would re-add the dropped node. + assignment: override_assignments?(other, location) ? nil : choose(other, :assignment), assignments: new_assignments, mass_assignment: combine_mass_assignment(other), return_type: combine_return_type(other), intersection_return_type: combine_types(other, :intersection_return_type), exclude_return_type: combine_types(other, :exclude_return_type), - presence: combine_presence(other) + presence: combine_presence(other), + # a guaranteed assignment on either side may + # override, not just union with, the rest + definite: definite || other.definite }) super(other, new_attrs) end @@ -115,9 +144,11 @@ def assignment end # @param other [self] - # + # @param location [Location, nil] # @return [::Array] - def combine_assignments other + def combine_assignments other, location = nil + return other.assignments.dup if override_assignments?(other, location) + (other.assignments + assignments).uniq end @@ -283,6 +314,7 @@ def visible_at? other_closure, other_loc location.filename == other_loc.filename && # @sg-ignore flow sensitive typing needs to handle attrs (!presence || presence.include?(other_loc.range.start)) && + !within_own_assignment?(other_loc) && visible_in_closure?(other_closure) end @@ -293,8 +325,77 @@ def visible_at? other_closure, other_loc # @return [Range] attr_writer :presence + public + + # True if `other_loc` sits inside one of our own assignment value + # nodes - the receiver `x` in `x = x.length`. Such a reference must + # resolve against our other assignments, not the value being derived. + # + # @param other_loc [Location] + # @return [Boolean] + def within_own_assignment? other_loc + return false unless location&.filename == other_loc.filename + + assignments.any? do |assignment_node| + next false unless assignment_node.respond_to?(:loc) + + rng = Range.from_node(assignment_node) + next false if rng.nil? + + # The new value is visible from the node end onward, so exclude + # only positions strictly inside it. + rng.contain?(other_loc.range.start) && other_loc.range.start != rng.ending + end + end + private + # True if `other` supersedes us rather than unioning: it reassigns + # the same variable in the same scope, guaranteed to have run. + # Excludes self-referential reassignments (`x = x.foo`), whose RHS + # needs our assignments as the base case. + # + # @param other [self] + # @param location [Location, nil] Position being resolved, if known - + # lets a conditional `other` still override within its own range. + # @return [Boolean] + def override_assignments? other, location = nil + (other.definite || other.definite_reaches?(location)) && other.closure == closure && + other.assignments.none? { |node| references_name?(node) } + end + + public + + # True if this assignment, though not globally definite, still + # dominates `location`: `location` falls inside the branch body it + # was made in. Nesting needs no walk - inner ranges are subranges. + # + # @param location [Location, nil] + # @return [Boolean] + def definite_reaches? location + cs = compound_statement + return false unless location && cs&.location&.range + + # @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 + + # @param node [Parser::AST::Node, nil] + # @return [Boolean] + def references_name? node + return false unless node.is_a?(::AST::Node) + + # @sg-ignore flow sensitive typing doesn't narrow `node` past the guard above + return true if %i[lvar ivar].include?(node.type) && node.children[0].to_s == name + + # @sg-ignore flow sensitive typing doesn't narrow `node` past the guard above + node.children.any? { |child| references_name?(child) } + end + # @param api_map [ApiMap] # @param raw_return_type [ComplexType, ComplexType::UniqueType] # diff --git a/lib/solargraph/pin/compound_statement.rb b/lib/solargraph/pin/compound_statement.rb index 39d9cf2d5c..4f79c46616 100644 --- a/lib/solargraph/pin/compound_statement.rb +++ b/lib/solargraph/pin/compound_statement.rb @@ -44,11 +44,60 @@ module Pin class CompoundStatement < Pin::Base attr_reader :node + # The immediately enclosing CompoundStatement - nil only for the + # synthetic root Namespace Region creates for top-level code. + # + # @return [Pin::CompoundStatement, nil] + attr_reader :compound_statement + + # True if this body may be skipped or run more than once - an + # if/while/until/rescue/&&/||/||= body, or a block, whose call + # count depends on the method it is passed to. Defaults false. + # + # @return [Boolean] + attr_reader :conditional + # @param node [Parser::AST::Node, nil] + # @param compound_statement [Pin::CompoundStatement, nil] + # @param conditional [Boolean] # @param [Hash{Symbol => Object}] splat - def initialize node: nil, **splat + def initialize node: nil, compound_statement: nil, conditional: false, **splat super(**splat) @node = node + @compound_statement = compound_statement + @conditional = conditional + end + + # @param other [self] + # @param attrs [Hash{Symbol => Object}] + # @return [self] + def combine_with other, attrs = {} + new_attrs = { + compound_statement: combine_compound_statement(other), + conditional: choose(other, :conditional) + }.merge(attrs) + super(other, new_attrs) + end + + # Bare CompoundStatement pins all share name == '', so pick by + # location instead, as BaseVariable#combine_closure does. + # + # @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/lib/solargraph/pin/local_variable.rb b/lib/solargraph/pin/local_variable.rb index 077da21be3..e73ba2588c 100644 --- a/lib/solargraph/pin/local_variable.rb +++ b/lib/solargraph/pin/local_variable.rb @@ -16,9 +16,9 @@ def probe api_map super end - def combine_with other, attrs = {} + def combine_with other, attrs = {}, location: nil # keep this as a parameter - return other.combine_with(self, attrs) if other.is_a?(Parameter) && !is_a?(Parameter) + return other.combine_with(self, attrs, location: location) if other.is_a?(Parameter) && !is_a?(Parameter) super end diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index 81cc94d284..2705c8ba27 100644 --- a/lib/solargraph/pin/method.rb +++ b/lib/solargraph/pin/method.rb @@ -668,14 +668,13 @@ def infer_from_return_nodes api_map end rng = Range.from_node(n) next unless rng - clip = api_map.clip_at( - # @sg-ignore Need to add nil check here - location.filename, - rng.ending - ) + # Pass the full local set: chain resolution re-checks each + # presence per sub-node, and a downcast can end early. + # @sg-ignore Need to add nil check here + all_locals = api_map.source_map(location.filename).locals # @sg-ignore Need to add nil check here chain = Solargraph::Parser.chain(n, location.filename) - type = chain.infer(api_map, self, clip.locals) + type = chain.infer(api_map, self, all_locals) result.push type unless type.undefined? end result.push ComplexType::NIL if has_nil diff --git a/lib/solargraph/pin/parameter.rb b/lib/solargraph/pin/parameter.rb index ba20976ec6..449e2dc49e 100644 --- a/lib/solargraph/pin/parameter.rb +++ b/lib/solargraph/pin/parameter.rb @@ -30,7 +30,7 @@ def location super || closure&.type_location end - def combine_with other, attrs = {} + def combine_with other, attrs = {}, location: nil # Parameters can only be combined with local variables in the same closure return self unless other.closure == closure @@ -45,7 +45,7 @@ def combine_with other, attrs = {} asgn_code: asgn_code } end - super(other, new_attrs.merge(attrs)) + super(other, new_attrs.merge(attrs), location: location) end def combine_return_type other @@ -208,6 +208,13 @@ def index # @param api_map [ApiMap] def typify api_map + if definite + # reassigned by an assignment guaranteed to have run, so the + # reassigned type wins over the declared @param type + reassigned_type = probe(api_map) + return reassigned_type if reassigned_type.defined? + end + new_type = super return new_type if new_type.defined? diff --git a/lib/solargraph/range.rb b/lib/solargraph/range.rb index e1ed895921..e7f49e0344 100644 --- a/lib/solargraph/range.rb +++ b/lib/solargraph/range.rb @@ -46,11 +46,8 @@ def to_hash # @return [Boolean] def contain? position position = Position.normalize(position) - # @sg-ignore flow sensitive typing should be able to handle redefinition return false if position.line < start.line || position.line > ending.line - # @sg-ignore flow sensitive typing should be able to handle redefinition return false if position.line == start.line && position.character < start.character - # @sg-ignore flow sensitive typing should be able to handle redefinition return false if position.line == ending.line && position.character > ending.character true end @@ -58,11 +55,9 @@ def contain? position # True if the range contains the specified position and the position does not precede it. # # @param position [Position, Array(Integer, Integer)] - # @sg-ignore flow sensitive typing should be able to handle redefinition # @return [Boolean] def include? position position = Position.normalize(position) - # @sg-ignore flow sensitive typing should be able to handle redefinition contain?(position) && !(position.line == start.line && position.character == start.character) end diff --git a/lib/solargraph/source/cursor.rb b/lib/solargraph/source/cursor.rb index 077364910a..147b03b6cb 100644 --- a/lib/solargraph/source/cursor.rb +++ b/lib/solargraph/source/cursor.rb @@ -54,7 +54,6 @@ def start_of_word # `foo.bar`, the end_of_word at position (0,6) is `r`. # # @return [String] - # @sg-ignore Need to add nil check here def end_of_word @end_of_word ||= begin match = source.code[offset..].to_s.match(end_word_pattern) diff --git a/lib/solargraph/source/source_chainer.rb b/lib/solargraph/source/source_chainer.rb index f96fa3319e..f8a778a57f 100644 --- a/lib/solargraph/source/source_chainer.rb +++ b/lib/solargraph/source/source_chainer.rb @@ -118,7 +118,6 @@ def fixed_position end # @return [String] - # @sg-ignore Need to add nil check here def end_of_phrase @end_of_phrase ||= begin match = phrase.match(/\s*(\.{1}|::)\s*$/) diff --git a/lib/solargraph/type_checker.rb b/lib/solargraph/type_checker.rb index 2bd5d530ed..ed43ce5653 100644 --- a/lib/solargraph/type_checker.rb +++ b/lib/solargraph/type_checker.rb @@ -652,6 +652,7 @@ def add_to_param_details param_details, param_names, new_param_details # @return [Hash{String => Hash{Symbol => String, ComplexType}}] def param_details_from_stack signature, method_pin_stack signature_type = signature.typify(api_map) + # @sg-ignore flow sensitive typing should be able to handle redefinition signature = signature.proxy signature_type param_details = signature_param_details(signature) param_names = signature.parameter_names diff --git a/lib/solargraph/type_checker/rules.rb b/lib/solargraph/type_checker/rules.rb index 6ce414a93c..53b85e9cc6 100644 --- a/lib/solargraph/type_checker/rules.rb +++ b/lib/solargraph/type_checker/rules.rb @@ -99,6 +99,7 @@ def require_inferred_type_params? # @todo 2: Need to handle duck-typed method calls on union types # @todo 2: Need better handling of #compact # @todo 2: flow sensitive typing should allow shadowing of Kernel#caller + # @todo 8: RBS Array[self] indexing infers Array instead of self # @todo 1: flow sensitive typing not smart enough to handle this case # @todo 1: flow sensitive typing needs to handle if foo = bar # @todo 1: flow sensitive typing needs to handle "if foo.nil?" diff --git a/spec/parser/flow_sensitive_typing_spec.rb b/spec/parser/flow_sensitive_typing_spec.rb index 4c9034873b..147dd30e4c 100644 --- a/spec/parser/flow_sensitive_typing_spec.rb +++ b/spec/parser/flow_sensitive_typing_spec.rb @@ -314,6 +314,22 @@ def baz; end expect(clip.infer.to_s).to eq('Foo') end + it 'keeps a definite reassignment visible inside a subsequent if-guard' do + source = Solargraph::Source.load_string(%( + def m + x = nil + x = 1 + if x + y = x * 2 + end + end + ), 'test.rb') + + api_map = Solargraph::ApiMap.new.map(source) + clip = api_map.clip_at('test.rb', [5, 14]) + expect(clip.infer.rooted_tags).to eq('::Integer') + end + it 'skips is_a? without a receiver' do source = Solargraph::Source.load_string(%( if is_a? Object diff --git a/spec/pin/base_variable_spec.rb b/spec/pin/base_variable_spec.rb index 0b1fff84b3..88ba50cf74 100644 --- a/spec/pin/base_variable_spec.rb +++ b/spec/pin/base_variable_spec.rb @@ -47,6 +47,39 @@ def bar expect(type.simplify_literals.to_rbs).to eq('(::Integer | nil)') end + it 'infers an undefined splat target when the source is not itself a tuple or container' do + source = Solargraph::Source.load_string(%( + class Repro + # @param mutator [Integer] + # @return [void] + def call(mutator) + command, *args = mutator + end + end + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + locals = api_map.source_map('test.rb').locals + args_pin = locals.find { |l| l.name == 'args' } + expect(args_pin.probe(api_map).tag).to eq('undefined') + end + + it 'treats downcasts with the same presence but different intersection_return_type as unequal' do + smap = Solargraph::SourceMap.load_string('foo = "foo"') + pin = smap.locals.first + narrowed_int = pin.downcast(presence: pin.presence, intersection_return_type: Solargraph::ComplexType.parse('Integer')) + narrowed_str = pin.downcast(presence: pin.presence, intersection_return_type: Solargraph::ComplexType.parse('String')) + expect(narrowed_int.eql?(narrowed_str)).to be(false) + end + + it 'treats downcasts with the same presence but different exclude_return_type as unequal' do + smap = Solargraph::SourceMap.load_string('foo = "foo"') + pin = smap.locals.first + excludes_int = pin.downcast(presence: pin.presence, exclude_return_type: Solargraph::ComplexType.parse('Integer')) + excludes_str = pin.downcast(presence: pin.presence, exclude_return_type: Solargraph::ComplexType.parse('String')) + expect(excludes_int.eql?(excludes_str)).to be(false) + end + it "understands proc kwarg parameters aren't affected by @type" do code = %( # @return [Proc] diff --git a/spec/pin/compound_statement_spec.rb b/spec/pin/compound_statement_spec.rb new file mode 100644 index 0000000000..84349fdaca --- /dev/null +++ b/spec/pin/compound_statement_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +describe Solargraph::Pin::CompoundStatement do + # The fallback derivation Pin::Base#closure uses when a pin has no + # closure: of its own - these specs check the two always agree. + 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 the compound_statement with a location when the other has none' do + cs_no_location = described_class.new(location: nil, source: :parser) + cs_with_location = described_class.new(location: later_location, source: :parser) + pin1 = described_class.new(location: earlier_location, compound_statement: cs_no_location, source: :parser) + pin2 = described_class.new(location: earlier_location, compound_statement: cs_with_location, source: :parser) + + expect(pin1.combine_with(pin2).compound_statement).to eq(cs_with_location) + expect(pin2.combine_with(pin1).compound_statement).to eq(cs_with_location) + 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/source/chain_spec.rb b/spec/source/chain_spec.rb index a6b29686e9..3b50d4942c 100644 --- a/spec/source/chain_spec.rb +++ b/spec/source/chain_spec.rb @@ -363,8 +363,6 @@ class Bar; end end it 'infers instance variables from sequential assignments' do - pending('sequential assignment support') - source = Solargraph::Source.load_string(%( def foo @foo = nil diff --git a/spec/source_map/clip_spec.rb b/spec/source_map/clip_spec.rb index 468612ab1c..1952d3bf8c 100644 --- a/spec/source_map/clip_spec.rb +++ b/spec/source_map/clip_spec.rb @@ -2441,8 +2441,6 @@ def bar; end end it 'replaces nil with reassignments' do - pending 'sequential assignment support' - source = Solargraph::Source.load_string(%( bar = nil bar @@ -2458,8 +2456,6 @@ def bar; end end it 'replaces type with reassignments' do - pending 'sequential assignment support' - source = Solargraph::Source.load_string(%( bar = 'a' bar diff --git a/spec/type_checker/levels/strong_spec.rb b/spec/type_checker/levels/strong_spec.rb index 9a0bd3e8a4..c2a4079307 100644 --- a/spec/type_checker/levels/strong_spec.rb +++ b/spec/type_checker/levels/strong_spec.rb @@ -954,6 +954,246 @@ def maybe_bar? expect(checker.problems.map(&:message)).to eq([]) end + it 'updates a parameter type after reassignment to a different non-literal type' do + checker = type_checker(%( + class Position + # @return [Integer] + def line + 1 + end + end + + module PositionNormalizer + # @param position [Position, Array(Integer, Integer)] + # @return [Position] + def self.normalize(position) + Position.new + end + end + + # @param position [Position, Array(Integer, Integer)] + # @return [Integer] + def describe(position) + position = PositionNormalizer.normalize(position) + position.line + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'does not treat a parameter reassignment inside a block as guaranteed to have run' do + checker = type_checker(%( + class Position + # @return [Integer] + def line + 1 + end + end + + module PositionNormalizer + # @param position [Position, Array(Integer, Integer)] + # @return [Position] + def self.normalize(position) + Position.new + end + end + + # @param position [Position, Array(Integer, Integer)] + # @return [Integer] + def describe(position) + [1].each { position = PositionNormalizer.normalize(position) } + position.line + end + )) + expect(checker.problems.map(&:message)).to eq([ + '#describe return type could not be inferred', + 'Unresolved call to line on Position, Array(Integer, Integer)' + ]) + end + + it 'still treats a conditional reassignment as guaranteed to have run for a use site inside the same branch' do + checker = type_checker(%( + # @param str [String] + # @param num [Integer] + # @param flag [Boolean] + # @return [void] + def conditional_reassign(str, num, flag) + local = num + if flag + local = str + local.upcase + end + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'narrows a nil-guarded default after the modifier if' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @return [void] + def guarded_default(tasks) + tasks = ['a'] if tasks.nil? + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'narrows a nil-guarded default after a non-modifier if' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @return [void] + def guarded_default(tasks) + if tasks.nil? + tasks = ['a'] + end + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'narrows a guarded default assigned in an unless modifier' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @return [void] + def guarded_default(tasks) + tasks = ['a'] unless tasks + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'narrows a guarded default assigned in an else clause' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @return [void] + def guarded_default(tasks) + if !tasks.nil? + puts 'have tasks' + else + tasks = ['a'] + end + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'narrows only the reassigned variable when an or-condition guards it' do + checker = type_checker(%( + # @param xs [Array, nil] + # @param ys [Array, nil] + # @return [void] + def or_guard(xs, ys) + xs = ['a'] if xs.nil? || ys.nil? + xs.each { |t| puts t } + ys.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq(['Unresolved call to each on Array, nil']) + end + + it 'keeps nil in the type when the guard tests something other than the variable' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @param flag [Boolean] + # @return [void] + def unrelated_guard(tasks, flag) + tasks = ['a'] if flag + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq(['Unresolved call to each on Array, nil']) + end + + it 'keeps nil in the type when the nil guard does not reassign the variable' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @return [void] + def no_reassignment(tasks) + puts 'hi' if tasks.nil? + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq(['Unresolved call to each on Array, nil']) + end + + it 'keeps nil in the type when the guarded assignment is itself conditional' do + checker = type_checker(%( + # @param tasks [Array, nil] + # @param flag [Boolean] + # @return [void] + def nested_conditional_assign(tasks, flag) + if tasks.nil? + tasks = ['a'] if flag + end + tasks.each { |t| puts t } + end + )) + expect(checker.problems.map(&:message)).to eq(['Unresolved call to each on Array, nil']) + 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] + def run + local = 5 + local = 'hello' + local.upcase + nil + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'updates an instance variable type after reassignment in the same method' do + checker = type_checker(%( + class Foo + # @return [void] + def run + @ivar = 5 + @ivar = 'hello' + @ivar.upcase + nil + end + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + + it 'resolves a self-referential reassignment against the pre-assignment type' do + checker = type_checker(%( + class Repro + # @param x [String] + # @return [Integer] + def foo(x) + x = x.length + end + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + it 'supports !@x.nil && @x.y' do checker = type_checker(%( class Bar @@ -970,6 +1210,17 @@ def foo? expect(checker.problems.map(&:message)).to eq([]) end + it 'infers a Boolean return from !!(x.nil? || x < n) on a nilable param' do + checker = type_checker(%( + # @param val [Integer, nil] + # @return [Boolean] + def check?(val) + !!(val.nil? || val < 5) + end + )) + expect(checker.problems.map(&:message)).to eq([]) + end + it 'uses cast type instead of defined type' do checker = type_checker(%( # frozen_string_literal: true