From 94a030e457ab293a06a38208e1a0bca8b7426155 Mon Sep 17 00:00:00 2001 From: Fredrik Ekre Date: Fri, 23 Aug 2024 14:44:19 +0200 Subject: [PATCH] Make trailing comma in multiline function/macro calls/definitions optional A trailing comma in these contexts is sometimes useful when developing but it also suggests the list is not final which function calls usually are. --- README.md | 12 +++++---- src/chisels.jl | 2 ++ src/runestone.jl | 26 +++++++++++++++----- test/runtests.jl | 64 +++++++++++++++++++++++++++++++----------------- 4 files changed, 71 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 86383d4..8f1943c 100644 --- a/README.md +++ b/README.md @@ -486,7 +486,8 @@ Consistently use single space around keywords. Examples: Listlike expressions (tuples, function calls/definitions, array literals, etc.) that *already* span multiple lines are formatted to consistently have a leading and a trailing -newline, as well as a trailing comma where applicable. Examples: +newline. Trailing commas are enforced for array/tuple literals (where adding another item is +common) but optional for function/macro calls/definitions. ```diff -(a, - b) @@ -499,7 +500,7 @@ newline, as well as a trailing comma where applicable. Examples: - b) +foo( + a, -+ b, ++ b +) -[1 2 @@ -518,9 +519,10 @@ this rule. ### Spacing in listlike expressions Listlike expressions (tuples, function calls/definitions, array literals, etc.) use a -consistent rule of no space before `,` and a single space after `,`. In single line -expressions there is no trailing `,`. In multiline expressions there is a trailing `,`. -Leading/trailing spaces are removed. Examples: +consistent rule of no space before `,` and a single space after `,`. Trailing commas are +enforced for array/tuple literals (where adding another item is common) but optional for +function/macro calls/definitions. Leading/trailing spaces are removed. Examples: + ```diff -f(a,b) -(a,b) diff --git a/src/chisels.jl b/src/chisels.jl index 1d3b586..6dad995 100644 --- a/src/chisels.jl +++ b/src/chisels.jl @@ -230,6 +230,8 @@ const TAG_PRE_DEDENT = TagType(1) << 2 const TAG_LINE_CONT = UInt32(1) << 31 # Parameters that should have a trailing comma after last item const TAG_TRAILING_COMMA = TagType(1) << 4 +# Parameters that should optinally have a trailing comma after last item +const TAG_TRAILING_COMMA_OPT = TagType(1) << 5 function add_tag(node::Node, tag::TagType) return Node(head(node), span(node), node.kids, node.tags | tag) diff --git a/src/runestone.jl b/src/runestone.jl index f61cd7d..a1d70ff 100644 --- a/src/runestone.jl +++ b/src/runestone.jl @@ -354,16 +354,15 @@ function spaces_in_listlike(ctx::Context, node::Node) # - the closing token is not on the same line as the last item (Runic-requirement) require_trailing_comma = false allow_trailing_semi = false - if implicit_tuple + allow_trailing_comma = multiline + if kind(node) in KSet"call dotcall macrocall" + require_trailing_comma = false + elseif implicit_tuple require_trailing_comma = false elseif kind(node) === K"tuple" && n_items == 1 && ctx.lineage_kinds[end] !== K"function" && kind(kids[first_item_idx::Int]) !== K"parameters" # TODO: May also have to check for K"where" and K"::" in the lineage above require_trailing_comma = true - elseif kind(node) in KSet"call" && n_items == 1 && kind(kids[first_item_idx::Int]) === K"generator" - # https://github.com/fredrikekre/Runic.jl/issues/16 - # TODO: There is probably a more generic pattern for the check above. - require_trailing_comma = false elseif kind(node) in KSet"bracescat parens" require_trailing_comma = false # Leads to parser error elseif kind(node) in KSet"block" @@ -372,6 +371,7 @@ function spaces_in_listlike(ctx::Context, node::Node) elseif kind(node) === K"parameters" # For parameters the trailing comma is configured from the parent require_trailing_comma = has_tag(node, TAG_TRAILING_COMMA) + allow_trailing_comma = has_tag(node, TAG_TRAILING_COMMA_OPT) elseif n_items > 0 && kind(kids[last_item_idx]) === K"macrocall" && !JuliaSyntax.has_flags(kids[last_item_idx], JuliaSyntax.PARENS_FLAG) && !is_string_macro(kids[last_item_idx]) @@ -475,6 +475,12 @@ function spaces_in_listlike(ctx::Context, node::Node) kid′ = add_tag(kid′, TAG_TRAILING_COMMA) this_kid_changed = true end + if kind(kid′) === K"parameters" && allow_trailing_comma && + i == last_item_idx && !has_tag(kid′, TAG_TRAILING_COMMA_OPT) + # Tag the node to optionally have a trailing comma + kid′ = add_tag(kid′, TAG_TRAILING_COMMA_OPT) + this_kid_changed = true + end if kind(kid′) === K"parameters" && !require_trailing_comma && !is_named_tuple && count( x -> !(JuliaSyntax.is_whitespace(x) || kind(x) in KSet", ;"), @@ -583,6 +589,11 @@ function spaces_in_listlike(ctx::Context, node::Node) # kids′ = kids[1:i - 1] # end end + if allow_trailing_comma && !has_tag(kid′, TAG_TRAILING_COMMA_OPT) + # Tag the parameters node to optionally allow a trailing comma + kid′ = add_tag(kid′, TAG_TRAILING_COMMA_OPT) + this_kid_changed = true + end if !require_trailing_comma && count( x -> !(JuliaSyntax.is_whitespace(x) || kind(x) in KSet", ;"), @@ -706,7 +717,8 @@ function spaces_in_listlike(ctx::Context, node::Node) end else @assert state === :expect_closing - if kind(kid′) === K"," || (kind(kid′) === K";" && !allow_trailing_semi) || + if (kind(kid′) === K"," && !allow_trailing_comma) || + (kind(kid′) === K";" && !allow_trailing_semi) || (kind(kid′) === K"Whitespace" && peek(i) !== K"Comment") # Trailing comma (when not wanted) and space not followed by a comment are # removed @@ -716,8 +728,10 @@ function spaces_in_listlike(ctx::Context, node::Node) end replace_bytes!(ctx, "", span(kid′)) elseif kind(node) === K"block" && kind(kid′) === K";" && allow_trailing_semi || + (kind(kid′) === K"," && allow_trailing_comma) || (kind(kid′) === K"Whitespace" && peek(i) !== K"Comment") allow_trailing_semi = n_items == 0 # Only one semicolon allowed + allow_trailing_comma = false # Just one please accept_node!(ctx, kid′) any_kid_changed && push!(kids′, kid′) elseif kind(kid′) === K"NewlineWs" || diff --git a/test/runtests.jl b/test/runtests.jl index 5838aba..c347bad 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -191,6 +191,7 @@ end for sp in ("", " ", " "), a in ("a", "a + a", "a(x)"), b in ("b", "b + b", "b(y)") # tuple, call, dotcall, vect, ref for (o, c) in (("(", ")"), ("f(", ")"), ("@f(", ")"), ("f.(", ")"), ("[", "]"), ("T[", "]")) + tr = o in ("f(", "@f(", "f.(") ? "" : "," # single line @test format_string("$(o)$(sp)$(c)") == "$(o)$(c)" @test format_string("$(o)$(sp)$(a)$(sp),$(sp)$(b)$(sp)$(c)") == @@ -205,35 +206,42 @@ end "$(o)$(a) #==# = 1$(c)" # line break in between items @test format_string("$(o)$(sp)$(a)$(sp),\n$(sp)$(b)$(sp)$(c)") == - format_string("$(o)$(sp)$(a)$(sp),\n$(sp)$(b)$(sp),$(sp)$(c)") == + "$(o)\n $(a),\n $(b)$(tr)\n$(c)" + @test format_string("$(o)$(sp)$(a)$(sp),\n$(sp)$(b)$(sp),$(sp)$(c)") == "$(o)\n $(a),\n $(b),\n$(c)" # line break after opening token @test format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b)$(sp)$(c)") == - format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b)$(sp),$(c)") == + "$(o)\n $(a), $(b)$(tr)\n$(c)" + @test format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b)$(sp),$(c)") == "$(o)\n $(a), $(b),\n$(c)" # line break before closing token @test format_string("$(o)$(sp)$(a)$(sp),$(sp)$(b)\n$(c)") == - format_string("$(o)$(sp)$(a)$(sp),$(sp)$(b),\n$(c)") == + "$(o)\n $(a), $(b)$(tr)\n$(c)" + @test format_string("$(o)$(sp)$(a)$(sp),$(sp)$(b),\n$(c)") == "$(o)\n $(a), $(b),\n$(c)" # line break after opening and before closing token @test format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b)\n$(c)") == - format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b),\n$(c)") == + "$(o)\n $(a), $(b)$(tr)\n$(c)" + @test format_string("$(o)\n$(sp)$(a)$(sp),$(sp)$(b),\n$(c)") == "$(o)\n $(a), $(b),\n$(c)" # line break after opening and before closing token and between items @test format_string("$(o)\n$(sp)$(a)$(sp),\n$(sp)$(b)\n$(c)") == - format_string("$(o)\n$(sp)$(a)$(sp),\n$(sp)$(b),\n$(c)") == + "$(o)\n $(a),\n $(b)$(tr)\n$(c)" + @test format_string("$(o)\n$(sp)$(a)$(sp),\n$(sp)$(b),\n$(c)") == "$(o)\n $(a),\n $(b),\n$(c)" # trailing comments @test format_string("$(o)$(sp)# x\n$(sp)$(a)$(sp),$(sp)# a\n$(sp)$(b)$(sp)# b\n$(c)") == - format_string("$(o)$(sp)# x\n$(sp)$(a)$(sp),$(sp)# a\n$(sp)$(b),$(sp)# b\n$(c)") == + "$(o)\n # x\n $(a),$(sp)# a\n $(b)$(tr)$(sp)# b\n$(c)" + @test format_string("$(o)$(sp)# x\n$(sp)$(a)$(sp),$(sp)# a\n$(sp)$(b),$(sp)# b\n$(c)") == "$(o)\n # x\n $(a),$(sp)# a\n $(b),$(sp)# b\n$(c)" # comments on separate lines between items @test format_string("$(o)\n# a\n$(a)$(sp),\n# b\n$(b)\n$(c)") == - format_string("$(o)\n# a\n$(a)$(sp),\n# b\n$(b)$(sp),\n$(c)") == + "$(o)\n # a\n $(a),\n # b\n $(b)$(tr)\n$(c)" + @test format_string("$(o)\n# a\n$(a)$(sp),\n# b\n$(b)$(sp),\n$(c)") == "$(o)\n # a\n $(a),\n # b\n $(b),\n$(c)" # comma on next line (TODO: move them up?) @test format_string("$(o)\n$(a)$(sp)\n,$(sp)$(b)\n$(c)") == - "$(o)\n $(a)\n , $(b),\n$(c)" + "$(o)\n $(a)\n , $(b)$(tr)\n$(c)" end # parens (but not block) @test format_string("($(sp)$(a)$(sp))") == "($(a))" @@ -260,18 +268,25 @@ end format_string("f($(sp)$(a)$(sp);$(sp)$(b)$(sp),$(sp))") == "f($(a); $(b))" @test format_string("f(\n$(sp)$(a)$(sp);\n$(sp)$(b)$(sp)\n)") == - format_string("f(\n$(sp)$(a)$(sp);\n$(sp)$(b)$(sp),$(sp)\n)") == + "f(\n $(a);\n $(b)\n)" + @test format_string("f(\n$(sp)$(a)$(sp);\n$(sp)$(b)$(sp),$(sp)\n)") == "f(\n $(a);\n $(b),\n)" @test format_string("f($(sp)$(a)$(sp);$(sp)b$(sp)=$(sp)$(b)$(sp))") == format_string("f($(sp)$(a)$(sp);$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp))") == "f($(a); b = $(b))" @test format_string("f(\n$(sp)$(a)$(sp);\n$(sp)b$(sp)=$(sp)$(b)$(sp)\n)") == - format_string("f(\n$(sp)$(a)$(sp);\n$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == + "f(\n $(a);\n b = $(b)\n)" + @test format_string("f(\n$(sp)$(a)$(sp);\n$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == "f(\n $(a);\n b = $(b),\n)" + @test format_string("f(\n$(sp)$(a)$(sp);$(sp)b$(sp)=$(sp)$(b)$(sp)\n)") == + "f(\n $(a); b = $(b)\n)" + @test format_string("f(\n$(sp)$(a)$(sp);$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == + "f(\n $(a); b = $(b),\n)" # Keyword arguments only with semi-colon on the same line as opening paren @test format_string("f(;\n$(sp)b$(sp)=$(sp)$(b)$(sp)\n)") == - format_string("f(;\n$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == format_string("f(;$(sp)b$(sp)=$(sp)$(b)$(sp)\n)") == + "f(;\n b = $(b)\n)" + @test format_string("f(;\n$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == format_string("f(;$(sp)b$(sp)=$(sp)$(b)$(sp),$(sp)\n)") == "f(;\n b = $(b),\n)" # vect/ref with parameter (not valid Julia syntax, but parses) @@ -338,10 +353,12 @@ end end # https://github.com/fredrikekre/Runic.jl/issues/32 @test format_string("f(@m begin\nend)") == "f(\n @m begin\n end\n)" - @test format_string("f(@m(begin\nend))") == "f(\n @m(\n begin\n end,\n ),\n)" - @test format_string("f(r\"\"\"\nf\n\"\"\")") == "f(\n r\"\"\"\n f\n \"\"\",\n)" - @test format_string("f(```\nf\n```)") == "f(\n ```\n f\n ```,\n)" - @test format_string("f(x```\nf\n```)") == "f(\n x```\n f\n ```,\n)" + @test format_string("f(@m(begin\nend))") == "f(\n @m(\n begin\n end\n )\n)" + @test format_string("f(r\"\"\"\nf\n\"\"\")") == "f(\n r\"\"\"\n f\n \"\"\"\n)" + @test format_string("f(```\nf\n```)") == "f(\n ```\n f\n ```\n)" + @test format_string("f(x```\nf\n```)") == "f(\n x```\n f\n ```\n)" + # Weird cornercase where a trailing comma messes some cases up (don't recall...) + @test format_string("{\n@f\n}") == "{\n @f\n}" end @testset "whitespace around ->" begin @@ -654,11 +671,11 @@ end @test format_string("(\n$(sp)a,\n$(sp)b,\n$(sp))") == "(\n a,\n b,\n)" # call, dotcall for sep in (",", ";"), d in ("", ".") - @test format_string("f$(d)(a$(sep)\n$(sp)b)") == "f$(d)(\n a$(sep)\n b,\n)" + @test format_string("f$(d)(a$(sep)\n$(sp)b)") == "f$(d)(\n a$(sep)\n b\n)" @test format_string("f$(d)(a$(sep)\n$(sp)b\n$(sp))") == - format_string("f$(d)(a$(sep)\n$(sp)b,\n$(sp))") == - "f$(d)(\n a$(sep)\n b,\n)" - @test format_string("f$(d)(\n$(sp)a$(sep)\n$(sp)b,\n$(sp))") == + "f$(d)(\n a$(sep)\n b\n)" + @test format_string("f$(d)(a$(sep)\n$(sp)b,\n$(sp))") == + format_string("f$(d)(\n$(sp)a$(sep)\n$(sp)b,\n$(sp))") == "f$(d)(\n a$(sep)\n b,\n)" end # paren-quote @@ -712,7 +729,7 @@ end # Multiline strings inside lists for trip in ("\"\"\"", "```") @test format_string("println(io, $(trip)\n$(sp)a\n$(sp)\n$(sp)b\n$(sp)$(trip))") == - "println(\n io, $(trip)\n a\n\n b\n $(trip),\n)" + "println(\n io, $(trip)\n a\n\n b\n $(trip)\n)" # Triple string on same line for b in ("", "\$b", "\$(b)", "\$(b)c") @test format_string("println(io, $(trip)a$b$(trip))") == @@ -776,6 +793,8 @@ end "(\n a ? b : c,\n)" @test format_string("f(\n$(sp)a ? b : c,\n)") == "f(\n a ? b : c,\n)" + @test format_string("f(\n$(sp)a ? b : c\n)") == + "f(\n a ? b : c\n)" # comparison @test format_string("a == b ==\n$(sp)c") == "a == b ==\n c" @test format_string("a <= b >=\n$(sp)c") == "a <= b >=\n c" @@ -806,14 +825,15 @@ end @testset "leading and trailing newline in multiline listlike" begin for (o, c) in (("f(", ")"), ("@f(", ")"), ("(", ")"), ("{", "}")) + tr = o in ("f(", "@f(") ? "" : "," @test format_string("$(o)a,\nb$(c)") == format_string("$(o)\na,\nb$(c)") == format_string("$(o)\na,\nb\n$(c)") == - "$(o)\n a,\n b,\n$(c)" + "$(o)\n a,\n b$(tr)\n$(c)" @test format_string("$(o)a, # a\nb$(c)") == format_string("$(o)\na, # a\nb$(c)") == format_string("$(o)\na, # a\nb\n$(c)") == - "$(o)\n a, # a\n b,\n$(c)" + "$(o)\n a, # a\n b$(tr)\n$(c)" end end