Insert explicit return expressions in functions/macros/do-blocks

This patch make sure that function definitions end with an explicit
`return` statement before the last expression in the body.

If `for` or `while` is the last expression a `return nothing` node is
inserted after the loop instead of `return for ...`.

`if`, `try`, `let`, and `begin` are currently are left as they are.
Perhaps in the future Runic will recurse down into branches, or insert
`return if` but for now this is left alone.

Another exception is also made whenever the last expression is a call to
`throw` or `error` because it is pretty clear that the function will not
return in this case (`return throw(...)` look silly).

Closes #43.

src/Runic.jl

@@ -61,6 +61,7 @@ function Base.show(io::IO, ::MIME"text/plain", node::Node)
     show(io, node)
     println(io)
     _show_green_node(io, node, "", 1, nothing, true)
+    return nothing
 end
 
 function Base.show(io::IO, node::Node)
@@ -435,6 +436,7 @@ function format_node!(ctx::Context, node::Node)::Union{Node, Nothing, NullNode}
     @return_something no_spaces_around_colon_etc(ctx, node)
     @return_something parens_around_op_calls_in_colon(ctx, node)
     @return_something for_loop_use_in(ctx, node)
+    @return_something explicit_return(ctx, node)
     @return_something braces_around_where_rhs(ctx, node)
     @return_something indent_multiline_strings(ctx, node)
     @return_something four_space_indent(ctx, node)

src/ToggleableAsserts.jl

@@ -30,7 +30,7 @@ assert_enabled() = true
 
 macro assert(expr)
     code = macroexpand_assert(expr)
-    :(assert_enabled() ? $(code) : nothing)
+    return :(assert_enabled() ? $(code) : nothing)
 end
 
 const toggle_lock = ReentrantLock()

src/debug.jl

@@ -25,6 +25,7 @@ function Base.showerror(io::IO, err::AssertionError)
             "please file an issue with a reproducible example at " *
             "https://github.com/fredrikekre/Runic.jl/issues/new."
     )
+    return nothing
 end
 
 function macroexpand_assert(expr)

src/main.jl

@@ -84,6 +84,7 @@ function maybe_expand_directory!(outfiles, dir)
             end
         end
     end
+    return nothing
 end
 
 function main(argv)
@@ -299,6 +300,7 @@ function main(argv)
                 cmd = setenv(ignorestatus(cmd); dir = dir)
                 cmd = pipeline(cmd, stdout = stderr, stderr = stderr)
                 run(cmd)
+                return nothing
             end
         end
 

src/runestone.jl

@@ -3346,3 +3346,160 @@ function remove_trailing_semicolon(ctx::Context, node::Node)
     seek(ctx.fmt_io, pos)
     return any_changed ? make_node(node, kids′) : nothing
 end
+
+function return_node(ctx::Context, ret::Node)
+    ws = Node(JuliaSyntax.SyntaxHead(K"Whitespace", JuliaSyntax.TRIVIA_FLAG), 1)
+    kids = [
+        Node(JuliaSyntax.SyntaxHead(K"return", 0), 6),
+    ]
+    if is_leaf(ret)
+        replace_bytes!(ctx, "return ", 0)
+        push!(kids, ws)
+        push!(kids, ret)
+    else
+        @assert kind(first_leaf(ret)) !== K"NewlineWs"
+        has_ws = kind(first_leaf(ret)) === K"Whitespace"
+        if has_ws
+            replace_bytes!(ctx, "return", 0)
+            push!(kids, ret)
+        else
+            replace_bytes!(ctx, "return ", 0)
+            push!(kids, ws)
+            push!(kids, ret)
+        end
+    end
+    return Node(JuliaSyntax.SyntaxHead(K"return", 0), kids)
+end
+
+function explicit_return_block(ctx, node)
+    @assert kind(node) === K"block"
+    kids = verified_kids(node)
+    rexpr_idx = findlast(!JuliaSyntax.is_whitespace, kids)
+    rexpr_idx === nothing && return nothing
+    rexpr = kids[rexpr_idx]
+    kind(rexpr) === K"return" && return nothing
+    if is_leaf(rexpr) ||
+            kind(rexpr) in KSet"call dotcall tuple vect ref hcat typed_hcat vcat typed_vcat \
+                ? && || :: juxtapose <: >: comparison string . -> comprehension do macro \
+                typed_comprehension where parens curly function quote global local =" ||
+            is_string_macro(rexpr) ||
+            is_assignment(rexpr) ||
+            kind(rexpr) === K"quote" && JuliaSyntax.has_flags(rexpr, JuliaSyntax.COLON_QUOTE)
+        # The cases caught in this branch are simple, just wrap the last expression in a
+        # return node. Also make sure the previous node is a K"NewlineWs".
+        for i in 1:(rexpr_idx - 1)
+            accept_node!(ctx, kids[i])
+        end
+        # If this is a call node, and the call the function name is `throw` or `error` we
+        # bail because `return throw(...)` looks kinda stupid.
+        if kind(rexpr) === K"call"
+            call_kids = verified_kids(rexpr)
+            fname_idx = findfirst(!JuliaSyntax.is_whitespace, call_kids)::Int
+            local fname
+            let p = position(ctx.fmt_io)
+                for i in 1:(fname_idx - 1)
+                    accept_node!(ctx, call_kids[i])
+                end
+                fname = String(read_bytes(ctx, call_kids[fname_idx]))
+                seek(ctx.fmt_io, p)
+            end
+            if fname === "throw" || fname === "error"
+                return nothing
+            end
+        end
+        # We will make changes so copy
+        kids′ = copy(kids)
+        # Make sure the previous node is a K"NewlineWs"
+        if !kmatch(kids′, KSet"NewlineWs", rexpr_idx - 1)
+            spn = 0
+            @assert kind(first_leaf(rexpr)) !== K"NewlineWs"
+            # Can it happen that there are whitespace hidden in the previous node?
+            # Let's see if this assert ever fire.
+            if rexpr_idx > 1
+                prev = kids′[rexpr_idx - 1]
+                if !is_leaf(prev)
+                    @assert !(kind(last_leaf(prev)) in KSet"Whitespace NewlineWs")
+                end
+            end
+            # Check whether there are whitespace we need to overwrite
+            if kmatch(kids′, KSet"Whitespace", rexpr_idx - 1)
+                # The previous node is whitespace
+                spn = span(popat!(kids′, rexpr_idx - 1))
+                seek(ctx.fmt_io, position(ctx.fmt_io) - spn)
+                rexpr_idx -= 1
+                @assert kind(first_leaf(rexpr)) !== K"Whitespace"
+            elseif kind(first_leaf(rexpr)) === K"Whitespace"
+                # The first child in the return node is whitespace
+                spn = span(first_leaf(rexpr))
+                rexpr = replace_first_leaf(rexpr, nullnode)
+            end
+            nl = "\n" * " "^(4 * ctx.indent_level)
+            nlnode = Node(JuliaSyntax.SyntaxHead(K"NewlineWs", JuliaSyntax.TRIVIA_FLAG), sizeof(nl))
+            insert!(kids′, rexpr_idx, nlnode)
+            rexpr_idx += 1
+            replace_bytes!(ctx, nl, spn)
+            accept_node!(ctx, nlnode)
+        end
+        ret = return_node(ctx, rexpr)
+        kids′[rexpr_idx] = ret
+        return make_node(node, kids′)
+    elseif kind(rexpr) in KSet"for while"
+        # For `for` and `while` loops we add `return nothing` after the block.
+        @assert kind(kids[end]) === K"NewlineWs"
+        @assert kind(last_leaf(rexpr)) === K"end"
+        insert_idx = lastindex(kids)
+        kids′ = copy(kids)
+        for i in 1:(insert_idx - 1)
+            accept_node!(ctx, kids′[i])
+        end
+        # Insert newline
+        nl = "\n" * " "^(4 * ctx.indent_level)
+        nlnode = Node(JuliaSyntax.SyntaxHead(K"NewlineWs", JuliaSyntax.TRIVIA_FLAG), sizeof(nl))
+        insert!(kids′, insert_idx, nlnode)
+        replace_bytes!(ctx, nl, 0)
+        accept_node!(ctx, nlnode)
+        # Insert `return nothing`
+        replace_bytes!(ctx, "return nothing", 0)
+        retnode = Node(
+            JuliaSyntax.SyntaxHead(K"return", 0), [
+                Node(JuliaSyntax.SyntaxHead(K"return", 0), 6),
+                Node(JuliaSyntax.SyntaxHead(K"Whitespace", JuliaSyntax.TRIVIA_FLAG), 1),
+                Node(JuliaSyntax.SyntaxHead(K"Identifier", 0), 7),
+            ]
+        )
+        insert!(kids′, insert_idx + 1, retnode)
+        return make_node(node, kids′)
+    elseif kind(rexpr) in KSet"if try let block macrocall $"
+        # These have been seen in the wild.
+        # TODO:
+        #  - `if` and `try` could be recursed into, but is that really so nice? `return if`?
+        #    Perhaps if no branches have `return`?
+        #    `if` without `else` could have `return nothing` at the end.
+        #  - `macrocall` could be inspected and have return if the argument is simple, e.g.
+        #    `@inbounds x[i]` -> `return @inbounds x[i]`.
+        #  - block (begin-end) and let could be recursed into?
+        #  - `$` could be anything but only used in eval so not super common...
+    else
+        # error("Unhandled node in explicit_return_block: $(kind(rexpr))")
+    end
+    return nothing
+end
+
+function explicit_return(ctx::Context, node::Node)
+    if !(!is_leaf(node) && kind(node) in KSet"function macro do")
+        return nothing
+    end
+    kids = verified_kids(node)
+    pos = position(ctx.fmt_io)
+    block_idx = findlast(x -> kind(x) === K"block", verified_kids(node))
+    block_idx === nothing && return nothing
+    for i in 1:(block_idx - 1)
+        accept_node!(ctx, kids[i])
+    end
+    block′ = explicit_return_block(ctx, kids[block_idx])
+    seek(ctx.fmt_io, pos)
+    block′ === nothing && return nothing
+    kids′ = copy(kids)
+    kids′[block_idx] = block′
+    return make_node(node, kids′)
+end