Repository: domluna/JuliaFormatter.jl (julia)

Issue:
error formatting `stdlib` of Julia
```julia

julia> format("./stdlib/", MinimalStyle(); normalize_line_endings = "unix", align_assignment=true)
[ Info: Error in formatting file stdlib/InteractiveUtils/test/runtests.jl
ERROR: ArgumentError: invalid index: nothing of type Nothing
```

Pull request:
For example:

```julia
@foo @noinline Base.@constprop :none aaaaaaaaaaaaa() = 0
@foo @noinline Base.@constprop :none bbbbbbbbbbbbbbbbbbb()     = 0
@foo @foo @ccccccccccccccccccccccccccccccccccccccccccccccccc() = 0
```

Previously this would throw an error (which was uncaught) since the binary op call was not found at the top level.

With this PR we search until we reach a binary op call or there are no more nodes and then align to that binary op. Additionally we store the reference to the exact node which will be aligned in the AlignGroup. This small change saves time in finding the node again later on.

fix #644

Patch:
diff --git a/src/align.jl b/src/align.jl
index 26aa9ca2c..10788addd 100644
--- a/src/align.jl
+++ b/src/align.jl
@@ -1,8 +1,8 @@
 function align_fst!(fst::FST, opts::Options)
     is_leaf(fst) && return
-    const_idxs = Int[]
-    assignment_idxs = Int[]
-    pair_arrow_idxs = Int[]
+    const_inds = Int[]
+    assignment_inds = Int[]
+    pair_arrow_inds = Int[]
 
     for (i, n) in enumerate(fst.nodes)
         if is_leaf(n)
@@ -22,14 +22,14 @@ function align_fst!(fst::FST, opts::Options)
         if opts.align_assignment && (is_assignment(n) || n.typ === Kw)
             # Gather all assignments within the current code block
             # they will be aligned at the end
-            push!(assignment_idxs, i)
+            push!(assignment_inds, i)
         elseif opts.align_pair_arrow && n.typ === Binary && op_kind(n) === Tokens.PAIR_ARROW
-            push!(pair_arrow_idxs, i)
+            push!(pair_arrow_inds, i)
         end
     end
 
-    align_binaryopcalls!(fst, assignment_idxs)
-    align_binaryopcalls!(fst, pair_arrow_idxs)
+    align_binaryopcalls!(fst, assignment_inds)
+    align_binaryopcalls!(fst, pair_arrow_inds)
 end
 
 """
@@ -37,7 +37,8 @@ end
 
 Group of FST node indices and required metadata to potentially align them.
 
-    - `node_idxs`. Indices of FST nodes affected by alignment.
+    - `node_inds`. Indices of FST nodes affected by alignment.
+    - `nodes`. FST nodes affected by alignment.
     - `line_offsets`. Line offset of the character nodes may be aligned to
     in the source file.
     - `lens`. Length of the FST node prior to the alignment character. Used
@@ -47,15 +48,17 @@ Group of FST node indices and required metadata to potentially align them.
     manually added by the user since the formatter would only use 0 or 1 whitespaces.
 """
 struct AlignGroup
-    node_idxs::Vector{Int}
+    nodes::Vector{Ref{FST}}
+    node_inds::Vector{Int}
     line_offsets::Vector{Int}
     lens::Vector{Int}
     whitespaces::Vector{Int}
 end
-AlignGroup() = AlignGroup(Int[], Int[], Int[], Int[])
+AlignGroup() = AlignGroup(Vector{Ref{FST}}[], Int[], Int[], Int[], Int[])
 
-function Base.push!(g::AlignGroup, idx::Int, line_offset::Int, len::Int, ws::Int)
-    push!(g.node_idxs, idx)
+function Base.push!(g::AlignGroup, n::FST, ind::Int, line_offset::Int, len::Int, ws::Int)
+    push!(g.nodes, Ref(n))
+    push!(g.node_inds, ind)
     push!(g.line_offsets, line_offset)
     push!(g.lens, len)
     push!(g.whitespaces, ws)
@@ -65,14 +68,14 @@ end
 function align_to(g::AlignGroup)::Union{Nothing,Int}
     length(g.lens) > 0 || return nothing
     # determine whether alignment might be warranted
-    max_len, max_idx = findmax(g.lens)
-    max_idxs = findall(==(g.line_offsets[max_idx]), g.line_offsets)
-    length(max_idxs) > 1 || return nothing
+    max_len, max_ind = findmax(g.lens)
+    max_inds = findall(==(g.line_offsets[max_ind]), g.line_offsets)
+    length(max_inds) > 1 || return nothing
 
     # Is there custom whitespace?
     # Formatter would only add 0 or 1 whitespaces.
     # > 2 implies a manual edit in the source file.
-    for i in max_idxs
+    for i in max_inds
         g.whitespaces[i] > 1 && return max_len
     end
 
@@ -81,35 +84,35 @@ end
 
 function align_binaryopcall!(fst::FST, diff::Int)
     # insert whitespace before and after operator
-    fidx = findfirst(x -> x.typ === WHITESPACE, fst.nodes)
-    lidx = findlast(x -> x.typ === WHITESPACE, fst.nodes)
+    find = findfirst(x -> x.typ === WHITESPACE, fst.nodes)
+    lind = findlast(x -> x.typ === WHITESPACE, fst.nodes)
 
-    if fidx === nothing
+    if find === nothing
         insert!(fst, 2, Whitespace(diff))
     else
-        fst[fidx] = Whitespace(diff)
+        fst[find] = Whitespace(diff)
     end
 
-    if lidx === nothing
+    if lind === nothing
         insert!(fst, 4, Whitespace(1))
     end
 end
 
 """
-    align_binaryopcalls!(fst::FST, op_idxs::Vector{Int})
+    align_binaryopcalls!(fst::FST, op_inds::Vector{Int})
 
 Aligns binary operator expressions.
 
 Additionally handles the case where a keyword such as `const` is used
 prior to the binary op call.
 """
-function align_binaryopcalls!(fst::FST, op_idxs::Vector{Int})
-    length(op_idxs) > 1 || return
-    prev_endline = fst[op_idxs[1]].endline
+function align_binaryopcalls!(fst::FST, op_inds::Vector{Int})
+    length(op_inds) > 1 || return
+    prev_endline = fst[op_inds[1]].endline
     groups = AlignGroup[]
     g = AlignGroup()
 
-    for i in op_idxs
+    for i in op_inds
         n = fst[i]
         if n.startline - prev_endline > 1
             push!(groups, g)
@@ -120,14 +123,34 @@ function align_binaryopcalls!(fst::FST, op_idxs::Vector{Int})
             nlen = length(n[1])
             n, nlen, (n[3].line_offset - n.line_offset) - nlen
         else
-            binop_idx = findfirst(nn -> nn.typ === Binary, n.nodes)
-            binop = n[binop_idx]
-            nlen = length(binop[1]) + sum(length.(n[1:binop_idx-1]))
-            binop, nlen, (binop[3].line_offset - n.line_offset) - nlen
+            binop::Union{FST,Nothing} = nothing
+            sn = n
+            nlen = 0
+            ws = 0
+
+            # the binary op may not be at the top level
+            # for instance, @call1 @call2 @call3 x = 10
+            while sn.nodes !== nothing
+                binop_ind = findfirst(nn -> nn.typ === Binary, sn.nodes)
+                nlen += sum(length.(sn[1:end-1]))
+                if binop_ind === nothing
+                    sn = sn.nodes[end]
+                else
+                    binop = sn.nodes[binop_ind]
+                    nlen += length(binop[1])
+                    ws = (binop[3].line_offset - n.line_offset) - nlen
+                    break
+                end
+            end
+            binop, nlen, ws
         end
 
-        push!(g, i, binop[3].line_offset, nlen, ws)
+        if binop === nothing
+            @warn "Binary operator not found in node" n.typ
+            continue
+        end
 
+        push!(g, binop, i, binop[3].line_offset, nlen, ws)
         prev_endline = n.endline
     end
     push!(groups, g)
@@ -136,17 +159,12 @@ function align_binaryopcalls!(fst::FST, op_idxs::Vector{Int})
         align_len = align_to(g)
         align_len === nothing && continue
 
-        for (i, idx) in enumerate(g.node_idxs)
+        for (i, nr) in enumerate(g.nodes)
             diff = align_len - g.lens[i] + 1
-
-            typ = fst[idx].typ
-            if typ === Binary || typ === Kw
-                align_binaryopcall!(fst[idx], diff)
-            else
-                binop_idx = findfirst(n -> n.typ === Binary, fst[idx].nodes)
-                align_binaryopcall!(fst[idx][binop_idx], diff)
-            end
-            fst[idx].nest_behavior = NeverNest
+            # reference to node
+            n = nr[]
+            align_binaryopcall!(n, diff)
+            n.nest_behavior = NeverNest
         end
     end
 
@@ -159,11 +177,11 @@ end
 Aligns struct fields.
 """
 function align_struct!(fst::FST)
-    idx = findfirst(n -> n.typ === Block, fst.nodes)
-    idx === nothing && return
-    length(fst[idx]) == 0 && return
+    ind = findfirst(n -> n.typ === Block, fst.nodes)
+    ind === nothing && return
+    length(fst[ind]) == 0 && return
 
-    block_fst = fst[idx]
+    block_fst = fst[ind]
     prev_endline = block_fst[1].endline
     groups = AlignGroup[]
     g = AlignGroup()
@@ -176,10 +194,10 @@ function align_struct!(fst::FST)
             end
 
             nlen = length(n[1])
-            idx = findfirst(x -> x.typ === OPERATOR, n.nodes)
-            ws = n[idx].line_offset - (n.line_offset + nlen)
+            ind = findfirst(x -> x.typ === OPERATOR, n.nodes)
+            ws = n[ind].line_offset - (n.line_offset + nlen)
 
-            push!(g, i, n[idx].line_offset, nlen, ws)
+            push!(g, n, i, n[ind].line_offset, nlen, ws)
             prev_endline = n.endline
         end
     end
@@ -188,10 +206,11 @@ function align_struct!(fst::FST)
     for g in groups
         align_len = align_to(g)
         align_len === nothing && continue
-        for (i, idx) in enumerate(g.node_idxs)
+        for (i, nr) in enumerate(g.nodes)
+            n = nr[]
             diff = align_len - g.lens[i] + 1
-            align_binaryopcall!(block_fst[idx], diff)
-            block_fst[idx].nest_behavior = NeverNest
+            align_binaryopcall!(n, diff)
+            n.nest_behavior = NeverNest
         end
     end
 end
@@ -215,14 +234,14 @@ function align_conditional!(fst::FST)
             if cond_prev_endline != n.endline
                 nlen = length(nodes[i-2])
                 ws = n.line_offset - (nodes[i-2].line_offset + nlen)
-                push!(cond_group, i, n.line_offset, nlen, ws)
+                push!(cond_group, n, i, n.line_offset, nlen, ws)
             end
             cond_prev_endline = n.endline
         elseif n.typ === OPERATOR && n.val == ":"
             if colon_prev_endline != n.endline
                 nlen = length(nodes[i-2])
                 ws = n.line_offset - (nodes[i-2].line_offset + nlen)
-                push!(colon_group, i, n.line_offset, nlen, ws)
+                push!(colon_group, n, i, n.line_offset, nlen, ws)
             end
             colon_prev_endline = n.endline
         end
@@ -235,22 +254,22 @@ function align_conditional!(fst::FST)
     cond_len === nothing && colon_len === nothing && return
 
     if cond_len !== nothing
-        for (i, idx) in enumerate(cond_group.node_idxs)
+        for (i, ind) in enumerate(cond_group.node_inds)
             diff = cond_len - cond_group.lens[i] + 1
-            nodes[idx-1] = Whitespace(diff)
+            nodes[ind-1] = Whitespace(diff)
         end
     end
 
-    for (i, idx) in enumerate(colon_group.node_idxs)
+    for (i, ind) in enumerate(colon_group.node_inds)
         # the placeholder would be i+1 if not for a possible inline comment
-        nidx = findnext(n -> n.typ === PLACEHOLDER, nodes, idx + 1)
-        if nodes[nidx+1].startline != nodes[nidx].startline
-            nodes[nidx] = Newline(nest_behavior = AlwaysNest)
+        nind = findnext(n -> n.typ === PLACEHOLDER, nodes, ind + 1)
+        if nodes[nind+1].startline != nodes[nind].startline
+            nodes[nind] = Newline(nest_behavior = AlwaysNest)
         end
 
         if colon_len !== nothing
             diff = colon_len - colon_group.lens[i] + 1
-            nodes[idx-1] = Whitespace(diff)
+            nodes[ind-1] = Whitespace(diff)
         end
     end
 


Test patch:
diff --git a/test/issues.jl b/test/issues.jl
index 0718643f0..a5bb38298 100644
--- a/test/issues.jl
+++ b/test/issues.jl
@@ -1464,4 +1464,26 @@
         """
         @test fmt(s, 4, 92, pipe_to_function_call = true) == s_
     end
+
+    @testset "644" begin
+        s = """
+        @foo @noinline Base.@constprop :none aaaaaaaaaaaaa() = 0
+        @foo @noinline Base.@constprop :none bbbbbbbbbbbbbbbbbbb()     = 0
+        @foo @foo @ccccccccccccccccccccccccccccccccccccccccccccccccc() = 0
+        """
+        s_ = """
+        @foo @noinline Base.@constprop :none aaaaaaaaaaaaa()           = 0
+        @foo @noinline Base.@constprop :none bbbbbbbbbbbbbbbbbbb()     = 0
+        @foo @foo @ccccccccccccccccccccccccccccccccccccccccccccccccc() = 0
+        """
+        @test fmt(s, 4, 92, align_assignment = true) == s_
+
+        # no manual edit
+        s = """
+        @foo @noinline Base.@constprop :none aaaaaaaaaaaaa() = 0
+        @foo @noinline Base.@constprop :none bbbbbbbbbbbbbbbbbbbbbbb() = 0
+        @foo @foo @ccccccccccccccccccccccccccccccccccccccccccccccccc() = 0
+        """
+        @test fmt(s, 4, 92, align_assignment = true) == s
+    end
 end