From 5d9634ec025957045e625575d222c0ae2012aa03 Mon Sep 17 00:00:00 2001 From: Fredrik Ekre Date: Sun, 25 Aug 2024 10:16:19 +0200 Subject: [PATCH] Insert explicit return expressions in functions/macros/do-blocks This patch make sure that function and macro definitions, as well as do-blocks, end with an explicit `return` statement before the last expression in the body. The following exceptions are made: - If the last expression is a `for` or `while` loop (which both always evaluate to `nothing`) `return` is added *after* the loop. - If the last expression is a `if` or `try` block the `return` is only added in case there is no `return` inside any of the branches. - If the last expression is a `let` or `begin` block the `return` is only added in case there is no `return` inside the block. - If the last expression is a function call, and the function name is `throw`, `rethrow`, or `error`, no `return` is added. This is because it is pretty obvious that these calls terminate the function without the explicit `return`. Since adding `return` changes the expression that a macro will see, this rule is disabled for function definitions inside of macros with the exception of some known ones from Base (e.g. `@inline`, `@generated`, ...). Closes #43. --- README.md | 57 ++++++++++++ src/JuliaSyntax.jl | 1 + src/Runic.jl | 12 ++- src/ToggleableAsserts.jl | 4 +- src/chisels.jl | 51 +++++++++++ src/debug.jl | 1 + src/main.jl | 2 + src/runestone.jl | 186 +++++++++++++++++++++++++++++++++++++++ test/runtests.jl | 125 +++++++++++++++++++++----- 9 files changed, 416 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 8f1943c..40d50ad 100644 --- a/README.md +++ b/README.md @@ -244,6 +244,7 @@ This is a list of things that Runic currently is doing: - [Line width limit](#line-width-limit) - [Newlines in blocks](#newlines-in-blocks) - [Indentation](#indentation) + - [Explicit `return`](#explicit-return) - [Spaces around operators, assignment, etc](#spaces-around-operators-assignment-etc) - [Spaces around keywords](#spaces-around-keywords) - [Multiline listlike expressions](#multiline-listlike-expressions) @@ -399,6 +400,62 @@ x = a + b * d ``` +### Explicit `return` + +Explicit `return` statements are ensured in function/macro definitions as well as in +`do`-blocks by adding `return` in front of the last expression, with some exceptions listed +below. + + - If the last expression is a `for` or `while` loop (which both always evaluate to + `nothing`) `return nothing` is added *after* the loop. + - If the last expression is a `if` or `try` block the `return` is only added in case + there is no `return` inside any of the branches. + - If the last expression is a `let` or `begin` block the `return` is only added in case + there is no `return` inside the block. + - If the last expression is a function call, and the function name is `throw`, `rethrow`, + or `error`, no `return` is added. This is because it is already obvious that these calls + terminate the function and don't return any value. + +Note that adding `return` changes the expression in a way that is visible to macros. +Therefore it is, in general, not valid to add `return` to a function defined inside a macro +since it isn't possible to know what the macro will expand to. For this reason this +formatting rule is disabled for functions defined inside macros with the exception of some +known and safe ones from Base (e.g. `@inline`, `@generated`, ...). + +For the same reason mentioned above, if the last expression in a function is a macro call it +isn't valid to step in and add `return` inside. Instead the `return` will be added in front +of the macro call like any other expression. + +Examples: +```diff + function f(n) +- sum(rand(n)) ++ return sum(rand(n)) + end + + macro m(args...) +- :(generate_expr(args...)) ++ return :(generate_expr(args...)) + end + + function g() +- open("/dev/random", "r") do f +- read(f, 8) ++ return open("/dev/random", "r") do f ++ return read(f, 8) + end + end +``` + +#### Potential changes + - If the last expression is a `if` or `try` block it might be better to + recurse into the branches and add `return` there. Looking at real code, if a + function ends with an `if` block, it seems about 50/50 whether adding return + *after* the block or adding return inside the branches is the best choice. + Quite often `return if` is not the best but at least Runic's current + formatting will force to think about the return value. + See issue [#52](https://github.com/fredrikekre/Runic.jl/issues/52). + ### Spaces around operators, assignment, etc Runic formats spaces around infix operators, assignments, comparison chains, and type diff --git a/src/JuliaSyntax.jl b/src/JuliaSyntax.jl index d488ec9..c6c5896 100644 --- a/src/JuliaSyntax.jl +++ b/src/JuliaSyntax.jl @@ -48,4 +48,5 @@ function _show_green_node(io, node, indent, pos, str, show_trivia) p += x.span end end + return end diff --git a/src/Runic.jl b/src/Runic.jl index 0b32a3e..7fe1b48 100644 --- a/src/Runic.jl +++ b/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) @@ -137,6 +138,7 @@ mutable struct Context next_sibling::Union{Node, Nothing} # parent::Union{Node, Nothing} lineage_kinds::Vector{JuliaSyntax.Kind} + lineage_macros::Vector{String} end function Context( @@ -167,11 +169,12 @@ function Context( call_depth = 0 prev_sibling = next_sibling = nothing lineage_kinds = JuliaSyntax.Kind[] + lineage_macros = String[] format_on = true return Context( src_str, src_tree, src_io, fmt_io, fmt_tree, quiet, verbose, assert, debug, check, diff, filemode, indent_level, call_depth, format_on, prev_sibling, next_sibling, - lineage_kinds + lineage_kinds, lineage_macros ) end @@ -306,6 +309,9 @@ function format_node_with_kids!(ctx::Context, node::Node) ctx.prev_sibling = nothing ctx.next_sibling = nothing push!(ctx.lineage_kinds, kind(node)) + if kind(node) === K"macrocall" + push!(ctx.lineage_macros, macrocall_name(ctx, node)) + end # The new node parts. `kids′` aliases `kids` and only copied below if any of the # nodes change ("copy-on-write"). @@ -381,6 +387,9 @@ function format_node_with_kids!(ctx::Context, node::Node) ctx.prev_sibling = prev_sibling ctx.next_sibling = next_sibling pop!(ctx.lineage_kinds) + if kind(node) === K"macrocall" + pop!(ctx.lineage_macros) + end ctx.call_depth -= 1 # Return a new node if any of the kids changed if any_kid_changed @@ -435,6 +444,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) diff --git a/src/ToggleableAsserts.jl b/src/ToggleableAsserts.jl index c9f0718..12bdd0a 100644 --- a/src/ToggleableAsserts.jl +++ b/src/ToggleableAsserts.jl @@ -30,13 +30,13 @@ assert_enabled() = true macro assert(expr) code = macroexpand_assert(expr) - :(assert_enabled() ? $(code) : nothing) + return :(assert_enabled() ? $(code) : nothing) end const toggle_lock = ReentrantLock() function enable_assert(enable::Bool) - @lock toggle_lock begin + return @lock toggle_lock begin if assert_enabled() != enable @eval Runic assert_enabled() = $enable end diff --git a/src/chisels.jl b/src/chisels.jl index 6dad995..d22efde 100644 --- a/src/chisels.jl +++ b/src/chisels.jl @@ -746,6 +746,57 @@ function kmatch(kids, kinds, i = firstindex(kids)) return true end +# Extract the macro name as written in the source. +function macrocall_name(ctx, node) + @assert kind(node) === K"macrocall" + kids = verified_kids(node) + pred = x -> kind(x) in KSet"MacroName StringMacroName CmdMacroName core_@cmd" + mkind = kind(first_leaf_predicate(node, pred)::Node) + if kmatch(kids, KSet"@ MacroName") + p = position(ctx.fmt_io) + bytes = read(ctx.fmt_io, span(kids[1]) + span(kids[2])) + seek(ctx.fmt_io, p) + return String(bytes) + elseif kmatch(kids, KSet".") || kmatch(kids, KSet"CmdMacroName") || + kmatch(kids, KSet"StringMacroName") + bytes = read_bytes(ctx, kids[1]) + if mkind === K"CmdMacroName" + append!(bytes, "_cmd") + elseif mkind === K"StringMacroName" + append!(bytes, "_str") + end + return String(bytes) + elseif kmatch(kids, KSet"core_@cmd") + bytes = read_bytes(ctx, kids[1]) + @assert length(bytes) == 0 + return "core_@cmd" + else + # Don't bother looking in more complex expressions, just return unknown + return "" + end +end + +# Inserting `return` modifies the AST in a way that is visible to macros.. In general it is +# never safe to change the AST inside a macro, but we make an exception for some common +# "known" macros in order to be able to format functions that e.g. have an `@inline` +# annotation in front. +const MACROS_SAFE_TO_INSERT_RETURN = let set = Set{String}() + for m in ( + "inline", "noinline", "propagate_inbounds", "generated", "eval", + "assume_effects", "doc", + ) + push!(set, "@$m", "Base.@$m") + end + push!(set, "Core.@doc") + set +end +function safe_to_insert_return(ctx, node) + for m in ctx.lineage_macros + m in MACROS_SAFE_TO_INSERT_RETURN || return false + end + return true +end + ########################## # Utilities for IOBuffer # ########################## diff --git a/src/debug.jl b/src/debug.jl index 37130ae..5cb9585 100644 --- a/src/debug.jl +++ b/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) diff --git a/src/main.jl b/src/main.jl index f3a1df3..9308f8d 100644 --- a/src/main.jl +++ b/src/main.jl @@ -90,6 +90,7 @@ function maybe_expand_directory!(outfiles, dir) end end end + return nothing end function main(argv) @@ -305,6 +306,7 @@ function main(argv) cmd = setenv(ignorestatus(cmd); dir = dir) cmd = pipeline(cmd, stdout = stderr, stderr = stderr) run(cmd) + return nothing end end diff --git a/src/runestone.jl b/src/runestone.jl index c4a2320..8d31390 100644 --- a/src/runestone.jl +++ b/src/runestone.jl @@ -3346,3 +3346,189 @@ 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 has_return(node::Node) + kids = verified_kids(node) + if kind(node) in KSet"let catch else finally" + idx = findfirst(x -> kind(x) === K"block", kids)::Int + if kind(node) === K"let" + idx = findnext(x -> kind(x) === K"block", kids, idx + 1)::Int + end + return has_return(kids[idx]) + elseif kind(node) in KSet"try if elseif" + # Look for the initial try/if block and then for + # catch/else/finally (for try) or elseif/else (for if). + pred = function(x) + return !is_leaf(x) && kind(x) in KSet"catch else finally elseif block" + end + idx = findfirst(pred, kids) + while idx !== nothing + has_return(kids[idx]) && return true + idx = findnext(pred, kids, idx + 1) + end + return false + elseif kind(node) === K"block" + # Don't care whether this is the last expression, + # that is the job of a linter or something I guess. + return findfirst(x -> kind(x) === K"return", kids) !== nothing + else + error("unreachable") + end +end + +function explicit_return_block(ctx, node) + @assert kind(node) === K"block" + if has_return(node) + # If the block already has a return node (anywhere) we accept it and move on. + return nothing + end + kids = verified_kids(node) + kids′ = kids + rexpr_idx = findlast(!JuliaSyntax.is_whitespace, kids′) + if rexpr_idx === nothing + # Empty block. TODO: Perhaps add `return nothing`? + return nothing + end + rexpr = kids′[rexpr_idx] + @assert kind(rexpr) !== K"return" # Should have been caught by has_return + 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 = macrocall" || + is_string_macro(rexpr) || is_assignment(rexpr) || + (kind(rexpr) in KSet"let if try" && !has_return(rexpr)) || + (is_begin_block(rexpr) && !has_return(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 === "rethrow" || fname === "error" + return nothing + end + end + # We will make changes so copy + kids′ = kids′ === kids ? copy(kids) : 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′ = kids′ === kids ? copy(kids) : 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` + replace_bytes!(ctx, "return", 0) + retnode = Node( + JuliaSyntax.SyntaxHead(K"return", 0), [ + Node(JuliaSyntax.SyntaxHead(K"return", 0), 6), + ] + ) + insert!(kids′, insert_idx + 1, retnode) + return make_node(node, kids′) + 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 + if !safe_to_insert_return(ctx, node) + 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 diff --git a/test/runtests.jl b/test/runtests.jl index a5563ce..e6aec73 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -462,9 +462,9 @@ end @test format_string("try\nerror()\ncatch$(sp)e\nend") == "try\n error()\ncatch e\nend" @test format_string("A$(sp)where$(sp){T}") == "A where {T}" @test format_string("A$(sp)where$(sp){T}$(sp)where$(sp){S}") == "A where {T} where {S}" - @test format_string("f()$(sp)do$(sp)x\ny\nend") == "f() do x\n y\nend" - @test format_string("f()$(sp)do\ny\nend") == "f() do\n y\nend" - @test format_string("f()$(sp)do; y end") == "f() do;\n y\nend" + @test format_string("f()$(sp)do$(sp)x\ny\nend") == "f() do x\n return y\nend" + @test format_string("f()$(sp)do\ny\nend") == "f() do\n return y\nend" + @test format_string("f()$(sp)do; y end") == "f() do;\n return y\nend" # After `where` (anywhere else?) a newline can be used instead of a space @test format_string("A$(sp)where$(sp)\n{A}") == "A where\n{A}" end @@ -527,14 +527,14 @@ end for sp in ("", " ", " ", " ") # function-end @test format_string("function f()\n$(sp)x\n$(sp)end") == - "function f()\n x\nend" + "function f()\n return x\nend" @test format_string("function f end") == "function f end" @test_broken format_string("function f\nend") == "function f\nend" # TODO @test format_string("function ∉ end") == "function ∉ end" # macro-end @test format_string("macro f()\n$(sp)x\n$(sp)end") == - "macro f()\n x\nend" - @test format_string("macro f() x end") == "macro f()\n x\nend" + "macro f()\n return x\nend" + @test format_string("macro f() x end") == "macro f()\n return x\nend" # let-end @test format_string("let a = 1\n$(sp)x\n$(sp)end") == "let a = 1\n x\nend" @test format_string("let\n$(sp)x\n$(sp)end") == "let\n x\nend" @@ -624,8 +624,8 @@ end ) == "try\n x\ncatch err\n y\nelse\n z\nfinally\n z\nend" end # do-end - @test format_string("open() do\n$(sp)a\n$(sp)end") == "open() do\n a\nend" - @test format_string("open() do io\n$(sp)a\n$(sp)end") == "open() do io\n a\nend" + @test format_string("open() do\n$(sp)a\n$(sp)end") == "open() do\n return a\nend" + @test format_string("open() do io\n$(sp)a\n$(sp)end") == "open() do io\n return a\nend" # module-end, baremodule-end for b in ("", "bare") # Just a module @@ -722,7 +722,7 @@ end @test format_string("begin\n x end") == "begin\n x\nend" # Functors @test format_string("function$(sp)(a::A)(b)\nx\nend") == - "function (a::A)(b)\n x\nend" + "function (a::A)(b)\n return x\nend" # TODO: Spaces after function keyword isn't removed. @test format_string("function$(sp)(a * b)\nreturn\nend") == "function$(sp)(a * b)\n return\nend" @@ -762,7 +762,7 @@ end # Blocklike RHS for thing in ( "if c\n x\nend", "try\n x\ncatch\n y\nend", - "let c = 1\n c\nend", "function()\n x\nend", + "let c = 1\n c\nend", "function()\n return x\nend", "\"\"\"\nfoo\n\"\"\"", "r\"\"\"\nfoo\n\"\"\"", "```\nfoo\n```", "r```\nfoo\n```", "```\nfoo\n```x", ) @@ -845,8 +845,8 @@ end @test format_string(f * nl^n * g) == f * nl^m * g @test format_string("module A" * nl^n * "end") == "module A" * nl^m * "end" @test format_string("function f()" * nl^n * "end") == "function f()" * nl^m * "end" - @test format_string("function f()" * nl^2 * "x = 1" * nl^n * "end") == - "function f()" * nl^2 * " x = 1" * nl^m * "end" + @test format_string("function f()" * nl^2 * "return x" * nl^n * "end") == + "function f()" * nl^2 * " return x" * nl^m * "end" end end @@ -1052,22 +1052,22 @@ end "try\n x\ncatch err\n y\nelse\n z\nfinally\n z\nend" end # do-end - @test format_string("open() do\na$(d)end") == "open() do\n a\nend" + @test format_string("open() do\na$(d)end") == "open() do\n return a\nend" @test format_string("open() do\nend") == "open() do\nend" @test_broken format_string("open() do;a$(d)end") == "open() do\n a\nend" @test_broken format_string("open() do ;a$(d)end") == "open() do\n a\nend" - @test format_string("open() do io$(d)a end") == "open() do io\n a\nend" + @test format_string("open() do io$(d)a end") == "open() do io\n return a\nend" # let-end @test format_string("let a = 1\nx$(d)end") == "let a = 1\n x\nend" @test format_string("let\nx$(d)end") == "let\n x\nend" @test format_string("let a = 1 # a\nx$(d)end") == "let a = 1 # a\n x\nend" # function-end - @test format_string("function f()$(d)x$(d)end") == "function f()\n x\nend" - @test format_string("function()$(d)x$(d)end") == "function()\n x\nend" - @test format_string("function ()$(d)x$(d)end") == "function ()\n x\nend" + @test format_string("function f()$(d)x$(d)end") == "function f()\n return x\nend" + @test format_string("function()$(d)x$(d)end") == "function()\n return x\nend" + @test format_string("function ()$(d)x$(d)end") == "function ()\n return x\nend" @test format_string("function f end") == "function f end" # macro-end - @test format_string("macro f()$(d)x$(d)end") == "macro f()\n x\nend" + @test format_string("macro f()$(d)x$(d)end") == "macro f()\n return x\nend" # quote-end @test format_string("quote$(d)x$(d)end") == "quote\n x\nend" # begin-end @@ -1186,7 +1186,8 @@ end "begin", "quote", "for i in I", "let", "let x = 1", "while cond", "if cond", "macro f()", "function f()", "f() do", "f() do x", ) - @test format_string("$(prefix)\n$(body)\nend") == "$prefix\n$(bodyfmt)\nend" + rx = prefix in ("function f()", "macro f()", "f() do", "f() do x") ? " return x\n" : "" + @test format_string("$(prefix)\n$(body)$(rx)\nend") == "$prefix\n$(bodyfmt)$(rx)\nend" end @test format_string( "if cond1\n$(body)\nelseif cond2\n$(body)\nelseif cond3\n$(body)\nelse\n$(body)\nend" @@ -1223,6 +1224,84 @@ end end end +@testset "explicit return" begin + for f in ("function f()", "function ()", "f() do", "macro m()") + # Simple cases just prepend `return` + for r in ( + "x", "*", "x, y", "(x, y)", "f()", "[1, 2]", "Int[1, 2]", "[1 2]", "Int[1 2]", + "[1 2; 3 4]", "Int[1 2; 3 4]", "x ? y : z", "x && y", "x || y", ":x", ":(x)", + ":(x; y)", "1 + 2", "f.(x)", "x .+ y", "x::Int", "2x", "T <: Integer", + "T >: Int", "Int <: T <: Integer", "x < y > z", "\"foo\"", "\"\"\"foo\"\"\"", + "a.b", "a.b.c", "x -> x^2", "[x for x in X]", "Int[x for x in X]", + "A{T} where {T}", "(@m a, b)", "A{T}", + "r\"foo\"", "r\"foo\"m", "`foo`", "```foo```", "r`foo`", + "f() do\n return x\n end", "f() do x\n return x\n end", + "function f()\n return x\n end", + "function ()\n return x\n end", + "quote\n x\n end", "begin\n x\n end", + "let\n x\n end", "let x = 42\n x\n end", + "x = 1", "x += 1", "x -= 1", "global x = 1", "local x = 1", + "@inbounds x[i]", "@inline f(x)", + "if c\n x\n end", + "if c\n x\n else\n y\n end", + "if c\n x\n elseif d\n z\n else\n y\n end", + "try\n x\n catch\n y\n end", + "try\n x\n catch e\n y\n end", + "try\n x\n catch\n y\n finally\n z\n end", + "try\n x\n catch\n y\n else\n z\n finally\n z\n end", + ) + @test format_string("$f\n $r\nend") == "$f\n return $r\nend" + @test format_string("$f\n x;$r\nend") == "$f\n x\n return $r\nend" + @test format_string("$f\n x; $r\nend") == "$f\n x\n return $r\nend" + # Nesting + @test format_string("$f\n $f\n $r\n end\nend") == + format_string("$f\n return $f\n return $r\n end\nend") + end + # If the last expression is a call to throw or error there should be no return + for r in ("throw(ArgumentError())", "error(\"foo\")", "rethrow()") + @test format_string("$f\n $r\nend") == "$f\n $r\nend" + end + # Safe/known macros + @test format_string("@inline $f\n x\nend") == + "@inline $f\n return x\nend" + @test format_string("Base.@noinline $f\n x\nend") == + "Base.@noinline $f\n return x\nend" + # Unsafe/unknown macros + @test format_string("@kernel $f\n x\nend") == "@kernel $f\n x\nend" + # `for` and `while` append `return` to the end + for r in ("for i in I\n end", "while i in I\n end") + @test format_string("$f\n $r\nend") == "$f\n $r\n return\nend" + @test format_string("$f\n $r\n # comment\nend") == + "$f\n $r\n # comment\n return\nend" + end + # If there already is a `return` anywhere (not necessarily the last expression) + # there will be no additional `return` added on the last expression. + # `for` and `while` append `return` to the end + let str = "$f\n return 42\n 1337\nend" + @test format_string(str) == str + end + # if/let/begin/try with a `return` inside should be left alone + for r in ( + "if c\n return x\n end", + "if c\n return x\n else\n y\n end", + "if c\n x\n else\n return y\n end", + "if c\n return x\n elseif d\n y\n else\n y\n end", + "if c\n x\n elseif d\n return y\n else\n z\n end", + "if c\n x\n elseif d\n y\n else\n return z\n end", + "let\n return x\n end", + "let x = 1\n return x\n end", + "begin\n return x\n end", + "try\n return x\n catch\n y\n end", + "try\n x\n catch e\n return y\n end", + "try\n x\n catch\n y\n finally\n return z\n end", + "try\n x\n catch\n y\n else\n return z\n finally\n z\n end", + ) + str = "$f\n $r\nend" + @test format_string(str) == str + end + end +end + @testset "# runic: (on|off)" begin for exc in ("", "!"), word in ("runic", "format") on = "#$(exc) $(word): on" @@ -1241,6 +1320,7 @@ end 1+1 $on 1+1 + return end """ ) == """ @@ -1249,6 +1329,7 @@ end 1+1 $on 1 + 1 + return end """ @test format_string( @@ -1258,6 +1339,7 @@ end 1+1 $bon 1+1 + return end """ ) == """ @@ -1266,6 +1348,7 @@ end 1 + 1 $bon 1 + 1 + return end """ @test format_string( @@ -1274,6 +1357,7 @@ end $off 1+1 1+1 + return end """ ) == """ @@ -1281,6 +1365,7 @@ end $off 1 + 1 1 + 1 + return end """ end @@ -1302,7 +1387,7 @@ if Sys.isunix() && isdir(share_julia) @warn "JuliaSyntax.ParseError for $path" err @test_broken false else - @error "Error when formatting file $path" + @error "Error when formatting file $path" err @test false end end