Fix revise#718 (#609)

Fixes timholy/Revise.jl#718

Also adds nested docstring test

Author: timholy

src/construct.jl

@@ -338,11 +338,14 @@ end
 """
     ExprSplitter(mod::Module, ex::Expr; lnn=nothing)
 
-Create an iterable that returns individual expressions together with their module of evaluation.
+Given a module `mod` and a top-level expression `ex` in `mod`, create an iterable that returns
+individual expressions together with their module of evaluation.
 Optionally supply an initial `LineNumberNode` `lnn`.
 
 # Example
 
+In a fresh session,
+
 ```
 julia> expr = quote
            public(x::Integer) = true
@@ -375,13 +378,13 @@ mod = Main
 ex = :($(Expr(:toplevel, :(#= REPL[7]:6 =#), :(const threshold = 0.1))))
 ```
 
-Note that `Main.Private` was created for you so that its internal expressions could be evaluated.
+`ExprSplitter` created `Main.Private` was created for you so that its internal expressions could be evaluated.
 `ExprSplitter` will check to see whether the module already exists and if so return it rather than
 try to create a new module with the same name.
 
 In general each returned expression is a block with two parts: a `LineNumberNode` followed by a single expression.
 In some cases the returned expression may be `:toplevel`, as shown in the `const` declaration,
-but otherwise it will be a `:block`.
+but otherwise it will preserve its parent's `head` (e.g., `expr.head`).
 
 # World age, frame creation, and evaluation
 
@@ -441,7 +444,7 @@ function push_modex!(iter::ExprSplitter, mod::Module, ex::Expr)
         modifies_scope = false
         if ex.head === :block
             for a in ex.args
-                if isa(a, Expr) && a.head ∈ (:local, :global)
+                if isa(a, Expr) && a.head === :local
                     modifies_scope = true
                     break
                 end

test/toplevel.jl

@@ -29,13 +29,26 @@ end
         end
         end
     end
-    modexs = collect(ExprSplitter(Main, ex))
-    m, ex = first(modexs)
+    modexs = collect(ExprSplitter(JIVisible, ex))
+    m, ex = first(modexs)        # FIXME don't use index in tests
     @test JuliaInterpreter.is_doc_expr(ex.args[2])
     Core.eval(m, ex)
     io = IOBuffer()
-    show(io, @doc(Main.DocStringTest))
+    show(io, @doc(JIVisible.DocStringTest))
     @test occursin("Special", String(take!(io)))
+
+    ex = Base.parse_input_line("""
+        "docstring"
+        module OuterModDocstring
+            "docstring for InnerModDocstring"
+            module InnerModDocstring
+            end
+        end
+        """)
+    modexs = collect(ExprSplitter(JIVisible, ex))
+    @test isdefined(JIVisible, :OuterModDocstring)
+    @test isdefined(JIVisible.OuterModDocstring, :InnerModDocstring)
+
     # issue #538
     @test !JuliaInterpreter.is_doc_expr(:(Core.@doc "string"))
     ex = quote
@@ -44,11 +57,11 @@ end
         sum
     end
     modexs = collect(ExprSplitter(Main, ex))
-    m, ex = first(modexs)
+    m, ex = first(modexs)       # FIXME don't use index in tests
     @test !JuliaInterpreter.is_doc_expr(ex.args[2])
 
     @test !isdefined(Main, :JIInvisible)
-    collect(ExprSplitter(JIVisible, :(module JIInvisible f() = 1 end)))
+    collect(ExprSplitter(JIVisible, :(module JIInvisible f() = 1 end)))  # this looks up JIInvisible rather than create it
     @test !isdefined(Main, :JIInvisible)
     @test  isdefined(JIVisible, :JIInvisible)
     mktempdir() do path
@@ -71,14 +84,32 @@ end
         # Every package is technically parented in Main but the name may not be visible in Main
         @test isdefined(@__MODULE__, :TmpPkg1)
         @test !isdefined(@__MODULE__, :TmpPkg2)
-        collect(ExprSplitter(Main, quote
-                                                     module TmpPkg2
-                                                     f() = 2
-                                                     end
-                                                 end))
+        collect(ExprSplitter(@__MODULE__, quote
+                module TmpPkg2
+                f() = 2
+                end
+            end))
         @test isdefined(@__MODULE__, :TmpPkg1)
         @test !isdefined(@__MODULE__, :TmpPkg2)
     end
+
+    # Revise issue #718
+    ex = Base.parse_input_line("""
+        module TestPkg718
+
+        module TestModule718
+            export _VARIABLE_UNASSIGNED
+            global _VARIABLE_UNASSIGNED = -84.0
+        end
+
+        using .TestModule718
+
+        end
+        """)
+    for (mod, ex) in ExprSplitter(JIVisible, ex)
+        @test JuliaInterpreter.finish!(Frame(mod, ex), true) === nothing
+    end
+    @test JIVisible.TestPkg718._VARIABLE_UNASSIGNED == -84.0
 end
 
 module Toplevel end
@@ -495,6 +526,7 @@ end
 
 @testset "Recursive type definitions" begin
     # See https://github.com/timholy/Revise.jl/issues/417
+    # See also the `Node` test above
     ex = :(struct RecursiveType x::Vector{RecursiveType} end)
     frame = Frame(Toplevel, ex)
     JuliaInterpreter.finish!(frame, true)
@@ -523,9 +555,9 @@ end
         sin(foo)
     end)
     for (mod, ex) in ExprSplitter(@__MODULE__, ex)
-        @test JuliaInterpreter.finish!(Frame(mod, ex), true) === nothing
+        @test JuliaInterpreter.finish_and_return!(Frame(mod, ex), true) == sin(10)
     end
-    @test length(collect(ExprSplitter(@__MODULE__, ex))) == 1
+
     ex = :(begin
         3 + 7
         module Local
@@ -534,7 +566,7 @@ end
         end
     end)
     modexs = collect(ExprSplitter(@__MODULE__, ex))
-    @test length(modexs) == 2
+    @test length(modexs) == 2   # FIXME don't use index in tests
     @test modexs[2][1] == getfield(@__MODULE__, :Local)
     for (mod, ex) in modexs
         @test JuliaInterpreter.finish!(Frame(mod, ex), true) === nothing
@@ -560,7 +592,7 @@ end
     for (mod, ex) in modexs
         @test JuliaInterpreter.finish!(Frame(mod, ex), true) === nothing
     end
-    @test length(modexs) == 2
+    @test length(modexs) == 2    # FIXME don't use index in tests
 
     ex = Base.parse_input_line("""
     local foo = 10
@@ -570,5 +602,5 @@ end
     for (mod, ex) in modexs
         @test JuliaInterpreter.finish!(Frame(mod, ex), true) === nothing
     end
-    @test length(modexs) == 2
+    @test length(modexs) == 2     # FIXME don't use index in tests
 end