Adjust local variable presence to start after assignment, not before (#864)

* Adjust local variable presence to start after assignment, not before

* Add regression test around assignment in return position

* Fix assignment visibility code, which relied on bad asgn semantics

Author: apiology

lib/solargraph/parser/parser_gem/node_chainer.rb

@@ -89,15 +89,21 @@ def generate_links n
           elsif n.type == :const
             const = unpack_name(n)
             result.push Chain::Constant.new(const)
-          elsif [:lvar, :lvasgn].include?(n.type)
+          elsif [:lvasgn, :ivasgn, :gvasgn, :cvasgn].include?(n.type)
+            result.concat generate_links(n.children[1])
+          elsif n.type == :lvar
             result.push Chain::Call.new(n.children[0].to_s)
-          elsif [:ivar, :ivasgn].include?(n.type)
-            result.push Chain::InstanceVariable.new(n.children[0].to_s)
-          elsif [:cvar, :cvasgn].include?(n.type)
-            result.push Chain::ClassVariable.new(n.children[0].to_s)
-          elsif [:gvar, :gvasgn].include?(n.type)
-            result.push Chain::GlobalVariable.new(n.children[0].to_s)
+          elsif n.type == :ivar
+             result.push Chain::InstanceVariable.new(n.children[0].to_s)
+          elsif n.type == :cvar
+             result.push Chain::ClassVariable.new(n.children[0].to_s)
+          elsif n.type == :gvar
+             result.push Chain::GlobalVariable.new(n.children[0].to_s)
           elsif n.type == :or_asgn
+            # @todo: Need a new Link class here that evaluates the
+            #   existing variable type with the RHS, and generates a
+            #   union type of the LHS alone if never nil, or minus nil +
+            #   RHS if it is nilable.
             result.concat generate_links n.children[1]
           elsif [:class, :module, :def, :defs].include?(n.type)
             # @todo Undefined or what?

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

@@ -8,8 +8,8 @@ class LvasgnNode < Parser::NodeProcessor::Base
           include ParserGem::NodeMethods
 
           def process
-            here = get_node_start_position(node)
-            presence = Range.new(here, region.closure.location.range.ending)
+            # variable not visible until next statement
+            presence = Range.new(get_node_end_position(node), region.closure.location.range.ending)
             loc = get_node_location(node)
             locals.push Solargraph::Pin::LocalVariable.new(
               location: loc,

spec/parser/node_chainer_spec.rb

@@ -111,10 +111,9 @@ class Foo
       foo = [1, 2]
     ))
     chain = Solargraph::Parser.chain(source.node)
-    expect(chain.links.map(&:word)).to eq(['foo'])
+    expect(chain.links.map(&:word)).to eq(['<::Array>'])
     foo_link = chain.links.first
-    expect(foo_link.class).to eq(Solargraph::Source::Chain::Call)
-    expect(foo_link.arguments).to eq([])
+    expect(foo_link.class).to eq(Solargraph::Source::Chain::Array)
   end
 
   it 'tracks complex lhs' do

spec/source_map/clip_spec.rb

@@ -361,10 +361,11 @@ def bar
       # @return [String, Array]
       def foo; end
       var = foo
+      var
     ), 'test.rb')
     map = Solargraph::ApiMap.new
     map.map source
-    clip = map.clip_at('test.rb', Solargraph::Position.new(3, 7))
+    clip = map.clip_at('test.rb', Solargraph::Position.new(4, 6))
     type = clip.infer
     expect(type.to_s).to eq('String, Array')
   end
@@ -731,10 +732,11 @@ def self.new
         end
       end
       value = Value.new
+      value
     ), 'test.rb')
     api_map = Solargraph::ApiMap.new
     api_map.map source
-    clip = api_map.clip_at('test.rb', [6, 11])
+    clip = api_map.clip_at('test.rb', [7, 11])
     expect(clip.infer.tag).to eq('Class')
   end
 
@@ -1252,6 +1254,7 @@ class Foo
       class Mod
         def meth
           arr = []
+          arr
           1.times do
             arr
           end
@@ -1261,11 +1264,11 @@ def meth
     ), 'test.rb')
     api_map = Solargraph::ApiMap.new
     api_map.map source
-    clip = api_map.clip_at('test.rb', [3, 11])
+    clip = api_map.clip_at('test.rb', [4, 11])
     expect(clip.infer.tag).to eq('Array')
-    clip = api_map.clip_at('test.rb', [5, 12])
+    clip = api_map.clip_at('test.rb', [6, 12])
     expect(clip.infer.tag).to eq('Array')
-    clip = api_map.clip_at('test.rb', [7, 10])
+    clip = api_map.clip_at('test.rb', [8, 10])
     expect(clip.infer.tag).to eq('Array')
   end
 
@@ -2267,6 +2270,55 @@ def blah
     expect(clip.infer.to_s).to eq('nil')
   end
 
+  it 'can infer assignments-in-return-position from complex expressions' do
+    source = Solargraph::Source.load_string(%(
+      class A
+        def foo
+          blah = ['foo'].map { 456 }
+        end
+
+        def bar
+          nah ||= ['foo'].map { 456 }
+        end
+
+        def foo1
+          @blah = ['foo'].map { 456 }
+        end
+
+        def bar1
+          @nah2 ||= ['foo'].map { 456 }
+        end
+
+        def baz
+          a = foo
+          a
+          b = bar
+          b
+          a1 = foo1
+          a1
+          b2 = bar1
+          b2
+        end
+      end
+  ), 'test.rb')
+
+    api_map = Solargraph::ApiMap.new.map(source)
+
+    clip = api_map.clip_at('test.rb', [20, 10])
+    expect(clip.infer.to_s).to eq('Array<Integer>')
+
+    # @todo pending https://github.com/castwide/solargraph/pull/888
+    # clip = api_map.clip_at('test.rb', [22, 10])
+    # expect(clip.infer.to_s).to eq('Array<Integer>')
+
+    clip = api_map.clip_at('test.rb', [24, 10])
+    expect(clip.infer.to_s).to eq('Array<Integer>')
+
+    # @todo pending https://github.com/castwide/solargraph/pull/888
+    # clip = api_map.clip_at('test.rb', [26, 10])
+    # expect(clip.infer.to_s).to eq('Array<Integer>')
+  end
+
   xit 'resolves overloads based on kwarg existence' do
     source = Solargraph::Source.load_string(%(
       class Blah