Drop trailing semicolons in block contexts (#47)

This patch removes trailing semicolons in blocklike contexts (`for`,
`if`, ...). Semicolons are left alone in top level contexts since they
are sometimes used there for output suppression (e.g. Documenter
examples or scripts that are copy-pasted/included in the REPL).

Semicolons before comments are replaced with a single space instead of
removed so that if the comments are aligned before, they will be aligned
after, for example
```julia
begin
    x = 1; # This is x
    y = 2  # This is y
end
```
will become
```julia
begin
    x = 1  # This is x
    y = 2  # This is y
end
```

Closes #34.

src/Runic.jl

@@ -421,6 +421,7 @@ function format_node!(ctx::Context, node::Node)::Union{Node, Nothing, NullNode}
     @return_something no_leading_and_single_trailing_newline(ctx, node)
     @return_something max_three_consecutive_newlines(ctx, node)
     @return_something insert_delete_mark_newlines(ctx, node)
+    @return_something remove_trailing_semicolon(ctx, node)
     @return_something trim_trailing_whitespace(ctx, node)
     @return_something format_hex_literals(ctx, node)
     @return_something format_float_literals(ctx, node)

src/runestone.jl

@@ -1809,7 +1809,6 @@ function indent_let(ctx::Context, node::Node)
     return any_kid_changed ? make_node(node, kids) : nothing
 end
 
-# TODO: Reuse indent_block?
 function indent_begin(ctx::Context, node::Node, block_kind = K"begin")
     @assert kind(node) === K"block"
     pos = position(ctx.fmt_io)
@@ -3200,3 +3199,131 @@ function indent_multiline_strings(ctx::Context, node::Node)
     seek(ctx.fmt_io, pos)
     return any_changes ? make_node(node, kids′) : nothing
 end
+
+# Pattern matching for "bad" semicolons:
+#  - `\s*;\n` -> `\n`
+#  - `\s*;\s*#\n` -> `\s* \s*#\n`
+function remove_trailing_semicolon_block(ctx::Context, node::Node)
+    kind(node) === K"block" || return nothing
+    @assert !is_leaf(node)
+    pos = position(ctx.fmt_io)
+    kids = verified_kids(node)
+    kids′ = kids
+    dealias() = kids′ === kids ? copy(kids) : kids′
+    function kmatch(i, kinds)
+        if i < 1 || i + length(kinds) - 1 > length(kids′)
+            return false
+        end
+        for (j, k) in pairs(kinds)
+            if kind(kids′[i + j - 1]) !== k
+                return false
+            end
+        end
+        return true
+    end
+    semi_idx = findfirst(x -> kind(x) === K";", kids′)
+    while semi_idx !== nothing
+        search_index = semi_idx + 1
+        if kmatch(semi_idx, KSet"; NewlineWs")
+            # `\s*;\n` -> `\n`
+            kids′ = dealias()
+            space_before = kmatch(semi_idx - 1, KSet"Whitespace ;")
+            if space_before
+                span_overwrite = span(kids′[semi_idx - 1]) + span(kids′[semi_idx])
+                nodes_to_skip_over = semi_idx - 2
+                deleteat!(kids′, semi_idx)
+                deleteat!(kids′, semi_idx - 1)
+                search_index = semi_idx - 1
+            else
+                span_overwrite = span(kids′[semi_idx])
+                nodes_to_skip_over = semi_idx - 1
+                deleteat!(kids′, semi_idx)
+                search_index = semi_idx
+            end
+            let p = position(ctx.fmt_io)
+                for i in 1:nodes_to_skip_over
+                    accept_node!(ctx, kids′[i])
+                end
+                replace_bytes!(ctx, "", span_overwrite)
+                seek(ctx.fmt_io, p)
+            end
+        elseif kmatch(semi_idx, KSet"; Comment NewlineWs") ||
+                kmatch(semi_idx, KSet"; Whitespace Comment NewlineWs")
+            # `\s*;\s*#\n` -> `\s* \s*#\n`
+            # The `;` is replaced by ` ` here in case comments are aligned
+            kids′ = dealias()
+            ws_span = span(kids′[semi_idx])
+            @assert ws_span == 1
+            space_before = kmatch(semi_idx - 1, KSet"Whitespace ;")
+            if space_before
+                ws_span += span(kids′[semi_idx - 1])
+            end
+            space_after = kmatch(semi_idx, KSet"; Whitespace")
+            if space_after
+                ws_span += span(kids′[semi_idx + 1])
+            end
+            let p = position(ctx.fmt_io)
+                for i in 1:(semi_idx - 1)
+                    accept_node!(ctx, kids′[i])
+                end
+                replace_bytes!(ctx, " ", span(kids′[semi_idx]))
+                seek(ctx.fmt_io, p)
+            end
+            # Insert new node
+            @assert kind(kids′[semi_idx]) === K";"
+            ws = Node(JuliaSyntax.SyntaxHead(K"Whitespace", JuliaSyntax.TRIVIA_FLAG), ws_span)
+            kids′[semi_idx] = ws
+            # Delete the consumed whitespace nodes
+            space_after && deleteat!(kids′, semi_idx + 1)
+            space_before && deleteat!(kids′, semi_idx - 1)
+        end
+        # Compute next index
+        semi_idx = findnext(x -> kind(x) === K";", kids′, search_index)
+    end
+    # Reset the stream and return
+    seek(ctx.fmt_io, pos)
+    return kids′ === kids ? nothing : make_node(node, kids′)
+end
+
+function remove_trailing_semicolon(ctx::Context, node::Node)
+    if is_begin_block(node)
+        r = remove_trailing_semicolon_block(ctx, node)
+        return r
+    end
+    if !(!is_leaf(node) && kind(node) in KSet"if elseif quote function for let while macro try catch finally else")
+        return nothing
+    end
+    if kind(node) === K"quote" && JuliaSyntax.has_flags(node, JuliaSyntax.COLON_QUOTE)
+        # This node is `:(...)` and not `quote...end`
+        return nothing
+    end
+    pos = position(ctx.fmt_io)
+    kids = verified_kids(node)
+    kids′ = kids
+    block_idx = findfirst(x -> kind(x) === K"block", kids′)
+    if kind(node) === K"let"
+        # The first block of let is the variables
+        block_idx = findnext(x -> kind(x) === K"block", kids′, block_idx + 1)
+    end
+    any_changed = false
+    while block_idx !== nothing
+        let p = position(ctx.fmt_io)
+            for i in 1:(block_idx - 1)
+                accept_node!(ctx, kids′[i])
+            end
+            block′ = remove_trailing_semicolon_block(ctx, kids′[block_idx])
+            if block′ !== nothing
+                any_changed = true
+                if kids′ === kids
+                    kids′ = copy(kids)
+                end
+                kids′[block_idx] = block′
+            end
+            seek(ctx.fmt_io, p)
+        end
+        block_idx = findnext(x -> kind(x) === K"block", kids′, block_idx + 1)
+    end
+    # Reset the stream and return
+    seek(ctx.fmt_io, pos)
+    return any_changed ? make_node(node, kids′) : nothing
+end

test/runtests.jl

@@ -5,7 +5,7 @@ using Runic:
 using Test:
     @test, @testset, @test_broken, @inferred, @test_throws
 using JuliaSyntax:
-    JuliaSyntax
+    JuliaSyntax, @K_str, @KSet_str
 
 @testset "Node" begin
     node = Runic.Node(JuliaSyntax.parseall(JuliaSyntax.GreenNode, "a = 1 + b\n"))
@@ -53,6 +53,12 @@ end
     @test read(io, String) == "axc"
 end
 
+@testset "JuliaSyntax assumptions" begin
+    # Duplicates are kept in KSet
+    @test KSet"; ;" == (K";", K";")
+    @test KSet"Whitespace ; Whitespace" == (K"Whitespace", K";", K"Whitespace")
+end
+
 @testset "Trailing whitespace" begin
     io = IOBuffer()
     println(io, "a = 1  ") # Trailing space
@@ -1109,6 +1115,87 @@ end
     end
 end
 
+@testset "trailing semicolon" begin
+    body = """
+        # Semicolons on their own lines
+        ;
+        ;;
+        # Trailing semicolon
+        a;
+        a;;
+        # Trailing semicolon with ws after
+        b; 
+        b;; 
+        # Trailing semicolon with ws before
+        c ;
+        c ;;
+        # Trailing semicolon with ws before and after
+        d ; 
+        d ;; 
+        # Trailing semicolon before comment
+        e;# comment
+        e;;# comment
+        # Trailing semicolon before ws+comment
+        f; # comment
+        f;; # comment
+    """
+    bodyfmt = """
+        # Semicolons on their own lines
+
+
+        # Trailing semicolon
+        a
+        a
+        # Trailing semicolon with ws after
+        b
+        b
+        # Trailing semicolon with ws before
+        c
+        c
+        # Trailing semicolon with ws before and after
+        d
+        d
+        # Trailing semicolon before comment
+        e # comment
+        e  # comment
+        # Trailing semicolon before ws+comment
+        f  # comment
+        f   # comment
+    """
+    for prefix in (
+            "begin", "quote", "for i in I", "let", "let x = 1", "while cond",
+            "if cond", "macro f()", "function f()",
+        )
+        @test format_string("$(prefix)\n$(body)\nend") == "$prefix\n$(bodyfmt)\nend"
+    end
+    @test format_string(
+        "if cond1\n$(body)\nelseif cond2\n$(body)\nelseif cond3\n$(body)\nelse\n$(body)\nend",
+    ) ==
+        "if cond1\n$(bodyfmt)\nelseif cond2\n$(bodyfmt)\nelseif cond3\n$(bodyfmt)\nelse\n$(bodyfmt)\nend"
+    @test format_string("try\n$(body)\ncatch\n$(body)\nend") ==
+        "try\n$(bodyfmt)\ncatch\n$(bodyfmt)\nend"
+    @test format_string("try\n$(body)\ncatch err\n$(body)\nend") ==
+        "try\n$(bodyfmt)\ncatch err\n$(bodyfmt)\nend"
+    @test format_string("try\n$(body)\nfinally\n$(body)\nend") ==
+        "try\n$(bodyfmt)\nfinally\n$(bodyfmt)\nend"
+    @test format_string("try\n$(body)\ncatch\n$(body)\nfinally\n$(body)\nend") ==
+        format_string("try\n$(bodyfmt)\ncatch\n$(bodyfmt)\nfinally\n$(bodyfmt)\nend")
+    @test format_string("try\n$(body)\ncatch err\n$(body)\nfinally\n$(body)\nend") ==
+        format_string("try\n$(bodyfmt)\ncatch err\n$(bodyfmt)\nfinally\n$(bodyfmt)\nend")
+    @test format_string("try\n$(body)\ncatch err\n$(body)\nelse\n$(body)\nend") ==
+        format_string("try\n$(bodyfmt)\ncatch err\n$(bodyfmt)\nelse\n$(bodyfmt)\nend")
+    # Top-level semicolons are kept (useful if you want to supress output in various
+    # contexts)
+    let str = """
+        f(x) = 1;
+        module A
+            g(x) = 2;
+        end;
+        """
+        @test format_string(str) == str
+    end
+end
+
 @testset "# runic: (on|off)" begin
     for exc in ("", "!"), word in ("runic", "format")
         on = "#$(exc) $(word): on"