Merge pull request #197 from Krastanov/juliasyntax1.10alt

misc fixes to `flatten`

Project.toml

@@ -1,6 +1,6 @@
 name = "MacroTools"
 uuid = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
-version = "0.5.10"
+version = "0.5.11"
 
 [deps]
 Markdown = "d6f4376e-aef5-505a-96c1-9c027394607a"

src/utils.jl

@@ -435,7 +435,7 @@ See also: [`combinearg`](@ref)
 function splitarg(arg_expr)
     if @capture(arg_expr, arg_expr2_ = default_)
       # This assert will only be triggered if a `nothing` literal was somehow spliced into the Expr.
-      # A regular `nothing` default value is a `Symbol` when it gets here. See #178 
+      # A regular `nothing` default value is a `Symbol` when it gets here. See #178
       @assert default !== nothing "splitarg cannot handle `nothing` as a default. Use a quoted `nothing` if possible. (MacroTools#35)"
     else
        arg_expr2 = arg_expr
@@ -458,16 +458,38 @@ function flatten1(ex)
   for x in ex.args
     isexpr(x, :block) ? append!(ex′.args, x.args) : push!(ex′.args, x)
   end
-  # Don't use `unblock` to preserve line nos
+  # Don't use `unblock` to preserve line nos.
   return length(ex′.args) == 1 ? ex′.args[1] : ex′
 end
 
+# Helpers for flattening try blocks
+bflatten(x) = x
+function bflatten(x::Expr) # flatten down to a block (i.e. a `begin symbol end` is turned into `begin symbol end`, unlike `flatten` which would just return the symbol)
+    fx = flatten(x)
+    return isexpr(fx, :block) || !isexpr(x, :block) ? fx : Expr(:block,fx)
+end
+
 """
     flatten(ex)
 
 Flatten any redundant blocks into a single block, over the whole expression.
 """
-flatten(ex) = postwalk(flatten1, ex)
+function flatten end
+
+flatten(x) = x
+function flatten(x::Expr)
+  if isexpr(x, :try) # begin _ end can be turned to _ everywhere in julia _except_ in try blocks. See #196 for details
+    3 <= length(x.args) <= 5 || error("Misformed `try` block.")
+    isa(x.args[2], Symbol) || x.args[2] == false || error("Misformed `try` block.")
+    return Expr(x.head, map(bflatten, x.args)...)
+    # args[2] can be a symbol or false
+    # args[3] can be a block (or false if there is no catch, which requires an else or finally)
+    # args[4] can be a block (or false if there is no finally but there is an else) if it exists
+    # args[5] can only be a block if it exists
+  else
+    return flatten1(Expr(x.head, map(flatten, x.args)...))
+  end
+end
 
 function makeif(clauses, els = nothing)
   foldr((c, ex)->:($(c[1]) ? $(c[2]) : $ex), clauses; init=els)

test/flatten_try.jl

@@ -0,0 +1,59 @@
+using MacroTools: flatten, striplines
+
+
+@testset "flatten try" begin # see julia#50710 and MacroTools#194
+    # These tests were prompted due to the following two issues:
+    # 1. on all Julia versions `flatten(striplines(:(try catch; false finally false end)))`
+    # was completely breaking the try block due to turning `begin false end` into `false`
+    # which have drastically different interpretations as 3rd or 4th argument of a try block.
+    # 2. only on Julia 1.10, due to the new parser generating slightly more annotations (line number nodes)
+    # `flatten(:(try f() catch end))` was turning into `Expr(:try, call, false, linenumbernode)`
+    # instead of `Expr(:try, call, false, empty_block)`. Downstream consumers of the AST
+    # were not expecting the line number node and were breaking, which is how this issue was
+    # discovered.
+    # The two issues have the same underlying cause: `begin expr end` was being turned into `expr`
+    # which is valid everywhere in julia except in try blocks.
+    # Notice that issue 1 is triggered only if one uses `striplines`. As such it was not really
+    # triggered in the wild. However, issue 2 was seen with the slight modification to
+    # parser annotations in Julia 1.10 which led to the discovery of issue 1.
+    exs = [
+        quote try; f(); catch; end; end,
+        quote try; f(); catch; else; finally; end; end,
+        quote try; f(); catch E; else; finally; end; end,
+        quote try; f(); catch; finally; end; end,
+        quote try; f(); catch E; finally; end; end,
+        quote try; f(); catch E; 3+3; finally; 4+4; end; end,
+        quote try; f(); catch E; 3+3; else; 2+2; finally; 4+4; end; end,
+        quote try; f(); finally; end; end,
+        quote try; f(); catch; false; finally; end; end,
+        quote try; f(); catch; else; finally; false; end; end,
+        quote try; f(); catch; else; end; end,
+        quote try; f(); catch; 3+3; else; 2+2; end; end,
+        quote try; f(); catch E; else; end; end,
+        quote try; f(); catch E; 3+3; else; 2+2; end; end
+    ]
+    for ex in exs
+        #@show ex
+        @test flatten(ex) |> striplines == ex |> striplines
+        @test flatten(striplines(ex)) == striplines(ex).args[1]
+    end
+    @test 123 == eval(flatten(striplines(:(try error() catch; 123 finally end))))
+    @test 123 == eval(flatten(:(try error() catch; 123 finally end)))
+    @test 234 == eval(flatten(striplines(:(try 1+1 catch; false; else 234; finally end))))
+    @test 234 == eval(flatten(:(try 1+1 catch; false; else 234; finally end)))
+    for (exa, exb) in [
+        (quote try; begin f(); g(); end; catch; end; end,                               quote try; f(); g(); catch; end; end),
+        (quote try; catch; begin f(); g(); end;  end; end,                              quote try; catch; f(); g(); end; end),
+        (quote try; begin f(); g(); end; catch; finally; begin m(); n(); end; end; end, quote try; f(); g(); catch; finally; m(); n(); end; end)
+    ]
+        @test exa |> flatten |> striplines == exb |> striplines
+        @test exa |> striplines |> flatten == (exb |> striplines).args[1]
+    end
+    # unnatural expressions that can not be generated by the Julia parser, but still get accepted and we do not want to break
+    for ex in [
+        Expr(:try, 1, false, 2)
+        Expr(:try, 1, false, false, false)
+    ]
+        @test flatten(ex)==ex
+    end
+end

test/runtests.jl

@@ -8,4 +8,8 @@ include("split.jl")
 include("destruct.jl")
 include("utils.jl")
 
+if VERSION>=v"1.8" # because of new try/else syntax
+    include("flatten_try.jl")
+end
+
 end

test/split.jl

@@ -36,7 +36,7 @@ let
     args = splitdef(:(f(a::Int = 1) = 1))[:args]
     @test map(splitarg, args) == [(:a, :Int, false, 1)]
     args = splitdef(:(f(a::Int ... = 1) = 1))[:args]
-    @test map(splitarg, args) == [(:a, :Int, true, 1)]    # issue 165 
+    @test map(splitarg, args) == [(:a, :Int, true, 1)]    # issue 165
 
     @splitcombine foo(x) = x+2
     @test foo(10) == 12
@@ -88,7 +88,7 @@ end
 @testset "combinestructdef, splitstructdef" begin
     ex = :(struct S end)
     @test ex |> splitstructdef |> combinestructdef |> Base.remove_linenums! ==
-        :(struct S <: Any end)
+        :(struct S <: Any end) |> MacroTools.striplines
 
     @test splitstructdef(ex) == Dict(
         :constructors => Any[],
@@ -101,7 +101,7 @@ end
     ex = :(mutable struct T end)
     @test splitstructdef(ex)[:mutable] === true
     @test ex |> splitstructdef |> combinestructdef |> Base.remove_linenums! ==
-        :(mutable struct T <: Any end)
+        :(mutable struct T <: Any end) |> MacroTools.striplines
 
     ex = :(struct S{A,B} <: AbstractS{B}
                                a::A
@@ -125,7 +125,7 @@ end
     constructors = splitstructdef(ex)[:constructors]
     @test length(constructors) == 1
     @test first(constructors) ==
-        :((S(a::A) where A) = new{A}()) |> MacroTools.flatten
+        :((S(a::A) where A) = new{A}()) |> MacroTools.striplines |> MacroTools.flatten
 
     @test_throws ArgumentError splitstructdef(:(call_ex(arg)))
 end

test/utils.jl

@@ -1,4 +1,4 @@
-using MacroTools: isdef
+using MacroTools: isdef, flatten, striplines
 
 @testset "utils" begin
     ex1 = :(function foo(a) return a; end)
@@ -23,3 +23,19 @@ using MacroTools: isdef
     ex10 = :(f(a::S, b::T)::Union{S,T} where {S,T} = rand() < 0.5 ? a : b)
     @test isdef(ex10)
 end
+
+@testset "flatten" begin
+    @test flatten(quote begin; begin; f(); g(); end; begin; h(); end; f(); end; end) |> striplines == quote f(); g(); h(); f() end |> striplines
+end
+
+@testset "flatten try" begin # see julia#50710 and MacroTools#194 # only tests that do not include `else` -- for the full set of tests see flatten_try.jl
+    exs = [
+        quote try; f(); catch; end; end,
+        quote try; f(); catch; finally; end; end,
+        quote try; f(); catch E; finally; end; end,
+        quote try; f(); catch E; 3+3; finally; 4+4; end; end,
+    ]
+    for ex in exs
+        @test flatten(ex) |> striplines == ex |> striplines
+    end
+end