Format spaces in between let variables (#118)

CHANGELOG.md

@@ -18,13 +18,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
    ```
  - Fix a bug that caused "single space after keyword" to not apply after `let` ([#117]).
    This bug is classified as a [spec-bug] and the fix will result in diffs like the
-   following when `let` is followed by multiple spaces in the source:
+   following when `let` is followed by multiple spaces (which should be rare) in the source:
    ```diff
    -let  a = 1
    +let a = 1
         a
     end
    ```
+ - Fix formatting of whitespace in between `let`-variables ([#118]). This bug is classified
+   as a [spec-bug] and the fix will result in diffs like the following in rare cases where
+   e.g. multiple spaces, or spaces *before* comma, is used in the variable list:
+   ```diff
+   -let  a = 1,  b = 2
+   +let  a = 1, b = 2
+        a + b
+    end
+   ```
  - Fix a bug that caused multiline variable blocks in `let` to not indent correctly ([#97],
    [#116]). This bug is classified as a [spec-bug] and the fix will result in diffs like the
    following whenever multiline variable blocks exist in the source:

src/Runic.jl

@@ -444,6 +444,7 @@ function format_node!(ctx::Context, node::Node)::Union{Node, Nothing, NullNode}
     @return_something spaces_around_keywords(ctx, node)
     @return_something spaces_in_import_using(ctx, node)
     @return_something spaces_in_export_public(ctx, node)
+    @return_something spaces_in_let(ctx, node)
     @return_something spaces_around_comments(ctx, node)
     @return_something no_spaces_around_colon_etc(ctx, node)
     @return_something parens_around_op_calls_in_colon(ctx, node)

src/runestone.jl

@@ -882,6 +882,120 @@ function spaces_in_export_public(ctx::Context, node::Node)
     return any_changes ? make_node(node, kids′) : nothing
 end
 
+function spaces_in_let(ctx::Context, node::Node)
+    p = position(ctx.fmt_io)
+    if kind(node) !== K"let" || is_leaf(node)
+        return nothing
+    end
+    let_kids = verified_kids(node)
+    let_leaf = let_kids[1]
+    @assert kind(let_leaf) === K"let" && is_leaf(let_leaf)
+    vars_idx = 2
+    vars_node = let_kids[vars_idx]
+    @assert !is_leaf(vars_node) && kind(vars_node) === K"block"
+    kids = verified_kids(vars_node)
+    if length(kids) == 0
+        @assert span(vars_node) == 0
+        return nothing
+    end
+    accept_node!(ctx, let_leaf)
+    # First node *must* be a space (?)
+    idx = 1
+    kid = kids[idx]
+    @assert kind(kid) === K"Whitespace"
+    # Second node must be a variable or assignment (at least non-whitespace)
+    idx = findnext(x -> !JuliaSyntax.is_whitespace(x), kids, idx + 1)
+    for i in 1:idx
+        accept_node!(ctx, kids[i])
+    end
+    # Now we expect comma -> space -> variable -> comma
+    state = :expect_comma
+    changed = false
+    kids′ = kids
+    idx += 1
+    space = Node(JuliaSyntax.SyntaxHead(K"Whitespace", JuliaSyntax.TRIVIA_FLAG), 1)
+    while idx <= length(kids)
+        kid′ = kids[idx]
+        if state === :expect_comma
+            state = :expect_space
+            if kind(kid′) === K","
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+            elseif kind(kid′) === K"Comment" || kmatch(kids, KSet"Whitespace Comment", idx)
+                state = :expect_comma
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+            elseif kind(kid′) === K"Whitespace"
+                @assert !kmatch(kids, KSet"Comment", idx + 1)
+                # Delete this space and keep looking for comma
+                state = :expect_comma
+                changed = true
+                if kids′ === kids
+                    kids′ = kids[1:(idx - 1)]
+                end
+                replace_bytes!(ctx, "", span(kid′))
+            else
+                unreachable()
+            end
+        elseif state === :expect_space
+            state = :expect_var
+            if kind(kid′) === K"NewlineWs" ||
+                    (kind(kid′) === K"Whitespace" && span(kid′) == 1) ||
+                    kmatch(kids, KSet"Whitespace Comment", idx)
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+            elseif kind(kid′) === K"Comment"
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+                state = :expect_space
+            elseif kind(kid′) === K"Whitespace"
+                if kids′ === kids
+                    kids′ = kids[1:(idx - 1)]
+                end
+                push!(kids′, space)
+                changed = true
+                replace_bytes!(ctx, " ", span(kid′))
+                accept_node!(ctx, space)
+            else
+                @assert !JuliaSyntax.is_whitespace(kid′)
+                if kids′ === kids
+                    kids′ = kids[1:(idx - 1)]
+                end
+                push!(kids′, space)
+                changed = true
+                replace_bytes!(ctx, " ", 0)
+                accept_node!(ctx, space)
+                continue # Skip the idx increment
+                # push!(kids′, kid′)
+                # accept_node!(ctx, kid′)
+            end
+        elseif state === :expect_var
+            state = :expect_comma
+            if kind(kid′) in KSet"Comment NewlineWs"
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+                state = kind(kid′) === K"Comment" ? (:expect_space) : (:expect_var)
+            else
+                @assert !JuliaSyntax.is_whitespace(kid′)
+                accept_node!(ctx, kid′)
+                changed && push!(kids′, kid′)
+            end
+        else
+            unreachable()
+        end
+        idx += 1
+    end
+    seek(ctx.fmt_io, p)
+    if changed
+        vars_node′ = make_node(vars_node, kids′)
+        let_kids′ = copy(let_kids)
+        let_kids′[vars_idx] = vars_node′
+        return make_node(node, let_kids′)
+    else
+        return nothing
+    end
+end
+
 # Used in `spaces_in_import_using` and `format_as`
 function format_importpath(ctx::Context, node::Node)
     @assert kind(node) === K"importpath"

test/runtests.jl

@@ -444,6 +444,22 @@ end
     @test format_string("[ # a\n]") == "[ # a\n]"
 end
 
+@testset "whitespace in let" begin
+    for sp in ("", " ", "  ")
+        msp = sp == "" ? " " : sp
+        @test format_string("let$(sp)\nend") == "let\nend"
+        @test format_string("let @inline a() = 1\nend") == "let @inline a() = 1\nend"
+        for a in ("a", "a = 1", "a() = 1", "\$a"), b in ("b", "b = 2")
+            @test format_string("let $(sp)$(a)$(sp),$(sp)$(b)\nend") == "let $(a), $(b)\nend"
+            @test format_string("let $(sp)$(a),$(sp)$(b)\nend") == "let $(a), $(b)\nend"
+            @test format_string("let $(sp)$(a)$(sp),\n$(sp)$(b)\nend") == "let $(a),\n        $(b)\nend"
+            # Comments
+            @test format_string("let $(sp)$(a)$(sp)#=c=#$(sp),$(sp)$(b)\nend") == "let $(a)$(msp)#=c=#, $(b)\nend"
+            @test format_string("let $(sp)$(a)$(sp),$(sp)#=c=#$(sp)$(b)\nend") == "let $(a),$(msp)#=c=# $(b)\nend"
+        end
+    end
+end
+
 @testset "whitespace around ->" begin
     for sp in ("", " ", "  ")
         @test format_string("a$(sp)->$(sp)b") == "a -> b"