Fix YAS reindentation issues (#1221)

Closes #1219 
Closes #1220 
Closes #1103

Author: penelopeysm

HISTORY_v2.md

@@ -1,3 +1,9 @@
+# v2.11.1
+
+Fixed a bug where formatting a do-block with no argument after the `do` would cause YASStyle to crash. (#1219, #1221)
+
+Fixed a number of YASStyle issues with indentation of the right-hand side of binary operators. (#1103, #1220, #1221)
+
 # v2.11.0
 
 Added a new formatting option, `enforce_triplequoted_docstrings`, which controls whether single-quoted docstrings are expanded to triple-quoted docstrings.

Project.toml

@@ -1,6 +1,6 @@
 name = "JuliaFormatter"
 uuid = "98e50ef6-434e-11e9-1051-2b60c6c9e899"
-version = "2.11.0"
+version = "2.11.1"
 authors = ["Dominique Luna <dluna132@gmail.com> and contributors"]
 
 [workspace]

src/nest_utils.jl

@@ -103,7 +103,7 @@ function nl_to_ws!(fst::FST, s::State)
     return
 end
 
-function dedent!(::AbstractStyle, fst::FST, s::State)
+function dedent!(fst::FST, s::State)
     if is_closer(fst) || fst.typ === NOTCODE
         fst.indent -= s.opts.indent
     elseif is_leaf(fst) || fst.typ === StringN
@@ -113,30 +113,13 @@ function dedent!(::AbstractStyle, fst::FST, s::State)
     end
 end
 
-function dedent!(::YASStyle, fst::FST, s::State)
-    if is_closer(fst) || fst.typ === NOTCODE
-        fst.indent -= s.opts.indent
-    elseif is_leaf(fst) || fst.typ === StringN
-        return
-    elseif is_unnamed_iterable(fst)
-        fst.indent = s.line_offset
-        if is_opener(fst[1])
-            fst.indent += 1
-        end
-    elseif is_named_iterable(fst)
-        fst.indent = s.line_offset + length(fst[1]) + length(fst[2])
-    else
-        fst.indent = s.line_offset
-    end
-end
-
 function unnest!(style::AbstractStyle, fst::FST, s::State; dedent::Bool)
     if is_leaf(fst)
         s.line_offset += length(fst)
     end
 
     if dedent
-        dedent!(style, fst, s)
+        dedent!(fst, s)
     end
 
     if is_leaf(fst) || fst.typ === StringN || !can_nest(fst)

src/styles/default/nest.jl

@@ -897,13 +897,11 @@ function n_binaryopcall!(
     # If there are no top-level placeholders in the FST, the binary call itself is not
     # nestable (although its children, i.e. the operands, might be).
     nodes = fst.nodes::Vector
-    idxs = findall(n -> n.typ === PLACEHOLDER, nodes)
+    placeholder_idxs = findall(n -> n.typ === PLACEHOLDER, nodes)
 
     rhs = fst[end]
     if rhs.typ === Block
-        (rhs = rhs[1])
-    else
-        false
+        rhs = rhs[1]
     end
 
     # is the LHS on a different line than the RHS ?
@@ -912,10 +910,10 @@ function n_binaryopcall!(
 
     nested = false
 
-    if length(idxs) == 2 &&
+    if length(placeholder_idxs) == 2 &&
        (line_margin > s.opts.margin || must_nest(fst) || must_nest(rhs) || src_diff_line)
-        i1 = idxs[1]
-        i2 = idxs[2]
+        i1 = placeholder_idxs[1]
+        i2 = placeholder_idxs[2]
         fst[i1] = Newline(; length = fst[i1].len)
         nested = true
 
@@ -952,14 +950,17 @@ function n_binaryopcall!(
         # "lhs op" rhs
         s.line_offset = line_offset
 
-        # extra margin for " op"
+        # Nest the LHS, but first update the extra margin corresponding to " op"
         fst[1].extra_margin = length(fst[2]) + length(fst[3])
-        nest!(style, fst[1], s, lineage) # lhs
+        nest!(style, fst[1], s, lineage)
+        # Nest everything up to the operator
         for n in fst[2:i1]
             nest!(style, n, s, lineage)
         end
 
-        # Undo nest if possible
+        # It's possible that nesting the LHS means that it's shorter, and thus we might not
+        # need to nest the entire binaryop (i.e., we might not need newlines around the
+        # operator). Attempt to undo the nesting here.
         if can_nest(fst) && !no_unnest(rhs) && !src_diff_line
             line_margin = s.line_offset
 
@@ -981,6 +982,7 @@ function n_binaryopcall!(
                     line_margin += sum(length.(rhs[1:(idx-1)]))
                 end
             else
+                # Calculate how much of the RHS we need to fit on the current line
                 rw, _ = length_to(fst, (NEWLINE,); start = i2 + 1)
                 line_margin += rw
             end
@@ -989,7 +991,24 @@ function n_binaryopcall!(
                 fst[i1] = Whitespace(1)
                 if indent_nest || style isa YASStyle
                     fst[i2] = Whitespace(0)
+                    line_offset = s.line_offset
                     walk(unnest!(style; dedent = true), rhs, s)
+                    # Iterables in YASStyle need to be aligned with the
+                    # open bracket
+                    if style isa YASStyle
+                        if is_unnamed_iterable(rhs)
+                            extra_indent = if !isempty(rhs.nodes) && is_opener(rhs[1])
+                                line_offset - rhs.indent + 1
+                            else
+                                line_offset - rhs.indent
+                            end
+                            add_indent!(rhs, s, extra_indent)
+                        elseif is_named_iterable(rhs)
+                            extra_indent =
+                                line_offset - rhs.indent + length(rhs[1]) + length(rhs[2])
+                            add_indent!(rhs, s, extra_indent)
+                        end
+                    end
                 end
             end
         end
@@ -1024,6 +1043,20 @@ function n_binaryopcall!(
             nested |= nest!(style, n, s, lineage)
         elseif i == length(nodes)
             # rhs
+            if style isa YASStyle
+                if is_unnamed_iterable(rhs)
+                    extra_indent = if !isempty(rhs.nodes) && is_opener(rhs[1])
+                        s.line_offset - fst.indent + 1
+                    else
+                        s.line_offset - fst.indent
+                    end
+                    add_indent!(rhs, s, extra_indent)
+                elseif is_named_iterable(rhs)
+                    extra_indent =
+                        s.line_offset - fst.indent + length(rhs[1]) + length(rhs[2])
+                    add_indent!(rhs, s, extra_indent)
+                end
+            end
             n.extra_margin = fst.extra_margin
             nested |= nest!(style, n, s, lineage)
         else

src/styles/yas/nest.jl

@@ -455,7 +455,8 @@ function n_binaryopcall!(
 )
     style = getstyle(ys)
     nodes = fst.nodes::Vector
-    if findfirst(n -> n.typ === PLACEHOLDER, nodes) !== nothing
+
+    if any(n -> n.typ === PLACEHOLDER, nodes)
         return n_binaryopcall!(DefaultStyle(style), fst, s, lineage; indent = indent)
     end
 

test/format_repo.jl

@@ -4,23 +4,28 @@ using JuliaFormatter: format
 using Test
 
 @testset "Format repo" begin
-    try
-        sandbox_dir = joinpath(tempdir(), join(rand('a':'z', 40)))
-        @info "formatting in directory" sandbox_dir
-        mkdir(sandbox_dir)
-        cp(@__DIR__, sandbox_dir; force = true)
-
-        # initial format
-        format(sandbox_dir)
-
-        # follow up formats should be the same
-        @test format(sandbox_dir; join_lines_based_on_source = true) == true
-        @test format(sandbox_dir; join_lines_based_on_source = true, margin = 10_000) ==
-              true
-    finally
+    @testset "$name" for (name, kwargs) in (
+        ("empty", (;)),
+        ("jlbos", (; join_lines_based_on_source=true)),
+        ("jlbos+margin", (; join_lines_based_on_source=true, margin=10_000))
+    )
         try
-            rm(sandbox_dir; recursive = true)
-        catch
+            sandbox_dir = joinpath(tempdir(), join(rand('a':'z', 40)))
+            if isdir(sandbox_dir)
+                rm(sandbox_dir; recursive=true)
+            end
+            mkdir(sandbox_dir)
+            cp(@__DIR__, sandbox_dir; force=true)
+            format(sandbox_dir; kwargs...)
+            # directly doing `@test format(...)` is broken on 1.13, see
+            # https://github.com/JuliaLang/julia/issues/62296
+            res = format(sandbox_dir; kwargs...)
+            @test res
+        finally
+            try
+                rm(sandbox_dir; recursive=true)
+            catch
+            end
         end
     end
 end

test/issues.jl

@@ -2810,6 +2810,28 @@ end
         end
     end
 
+    @testset "1103 YAS indentation after =>" begin
+        for s in (
+        """
+        long_name => [
+                      #
+                      1,
+                      2,
+                      3
+                      #
+                      ]""",
+
+        """
+        ccc => function ()
+            return f() do x
+                return aaaaaaaaaaaaa
+            end
+        end""",
+        )
+            test_format(s, s, YASStyle())
+        end
+    end
+
     @testset "1105 typed comprehension idempotence" begin
         s = """
         function f()
@@ -3549,6 +3571,16 @@ end
         )"""
         test_format(s, expected; separate_kwargs_with_semicolon=true)
     end
+
+    @testset "1219 dedenting empty do-block argument" begin
+        s = """
+        ccc => function ()
+            return f() do
+                return aaaaaaaaaaaaa
+            end
+        end"""
+        test_format(s, s, YASStyle(); margin=30)
+    end
 end
 
 end

test/yas_style.jl

@@ -774,6 +774,33 @@ using JuliaFormatter: format_text
         test_format(str, formatted_str1, YASStyle(); variable_call_indent = ["test"])
         test_format(str, formatted_str2, YASStyle(); variable_call_indent = ["SVector", "test"])
     end
+
+    @testset "indentation of binary ops" begin
+        s1 = """
+        begin
+            @test aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == ["theta[1]",
+                                                           "theta[2]",
+                                                           "theta[3]",
+                                                           "theta[4]",
+                                                           "tau"]
+        end"""
+        # different margins have different code paths so test both
+        for margin in (80, 2000)
+            test_format(s1, s1, YASStyle(); margin=margin)
+        end
+
+        s2 = """
+        begin
+            @test ccc => function ()
+                return f() do x
+                    return aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+                end
+            end
+        end"""
+        for margin in (80, 2000)
+            test_format(s2, s2, YASStyle(); margin=margin)
+        end
+    end
 end
 
 end