Merge pull request #29 from JuliaLang/cjf/core-parsing-fixes

c42f · web-flow · commit 219d375a2f41 · 2022-06-23T06:14:51.000+10:00
Emit `LineNumberNode`s in `Expr` conversion
diff --git a/README.md b/README.md
@@ -127,7 +127,7 @@ tree", but that term has also been used for the parse tree of the full formal
 grammar for a language including any grammar hacks required to solve
 ambiguities, etc. So we avoid this term.)
 
-`JuliaSyntax` uses use a mostly recursive descent parser which closely
+`JuliaSyntax` uses a mostly recursive descent parser which closely
 follows the high level structure of the flisp reference parser. This makes the
 code familiar and reduces porting bugs. It also gives a lot of flexibility for
 designing the diagnostics, tree data structures, compatibility with different
diff --git a/src/expr.jl b/src/expr.jl
@@ -6,7 +6,7 @@ function is_eventually_call(ex)
                                       is_eventually_call(ex.args[1]))
 end
 
-function _to_expr(node::SyntaxNode, iteration_spec=false)
+function _to_expr(node::SyntaxNode, iteration_spec=false, need_linenodes=true)
     if !haschildren(node)
         if node.val isa Union{Int128,UInt128,BigInt}
             # Ignore the values of large integers and convert them back to
@@ -27,12 +27,32 @@ function _to_expr(node::SyntaxNode, iteration_spec=false)
     headsym = !isnothing(headstr) ? Symbol(headstr) :
         error("Can't untokenize head of kind $(kind(node))")
     node_args = children(node)
-    args = Vector{Any}(undef, length(node_args))
+    insert_linenums = (headsym == :block || headsym == :toplevel) && need_linenodes
+    args = Vector{Any}(undef, length(node_args)*(insert_linenums ? 2 : 1))
     if headsym == :for && length(node_args) == 2
-        args[1] = _to_expr(node_args[1], true)
-        args[2] = _to_expr(node_args[2], false)
+        # No line numbers in for loop iteration spec
+        args[1] = _to_expr(node_args[1], true, false)
+        args[2] = _to_expr(node_args[2])
+    elseif headsym == :let && length(node_args) == 2
+        # No line numbers in let statement binding list
+        args[1] = _to_expr(node_args[1], false, false)
+        args[2] = _to_expr(node_args[2])
     else
-        map!(_to_expr, args, node_args)
+        if insert_linenums
+            if isempty(node_args)
+                push!(args, source_location(LineNumberNode, node.source, node.position))
+            else
+                for i in 1:length(node_args)
+                    n = node_args[i]
+                    args[2*i-1] = source_location(LineNumberNode, n.source, n.position)
+                    args[2*i] = _to_expr(n)
+                end
+            end
+        else
+            for i in 1:length(node_args)
+                args[i] = _to_expr(node_args[i])
+            end
+        end
     end
     # Julia's standard `Expr` ASTs have children stored in a canonical
     # order which is often not always source order. We permute the children
@@ -136,31 +156,41 @@ function _to_expr(node::SyntaxNode, iteration_spec=false)
     #   Strip string from interpolations in 1.5 and lower to preserve
     #   "hi$("ho")" ==>  (string "hi" "ho")
     elseif headsym == :(=)
-        if is_eventually_call(args[1]) && !iteration_spec
-            if Meta.isexpr(args[2], :block)
-                pushfirst!(args[2].args, loc)
-            else
-                # Add block for short form function locations
-                args[2] = Expr(:block, loc, args[2])
-            end
+        if is_eventually_call(args[1]) && !iteration_spec && !Meta.isexpr(args[2], :block)
+            # Add block for short form function locations
+            args[2] = Expr(:block, loc, args[2])
         end
+    elseif headsym == :elseif
+        # Block for conditional's source location
+        args[1] = Expr(:block, loc, args[1])
     elseif headsym == :(->)
         if Meta.isexpr(args[2], :block)
-            pushfirst!(args[2].args, loc)
+            if node.parent isa SyntaxNode && kind(node.parent) != K"do"
+                pushfirst!(args[2].args, loc)
+            end
         else
             # Add block for source locations
             args[2] = Expr(:block, loc, args[2])
         end
     elseif headsym == :function
-        if length(args) > 1 && Meta.isexpr(args[1], :tuple)
-            # Convert to weird Expr forms for long-form anonymous functions.
-            #
-            # (function (tuple (... xs)) body) ==> (function (... xs) body)
-            if length(args[1].args) == 1 && Meta.isexpr(args[1].args[1], :...)
-                # function (xs...) \n body end
-                args[1] = args[1].args[1]
+        if length(args) > 1
+            if Meta.isexpr(args[1], :tuple)
+                # Convert to weird Expr forms for long-form anonymous functions.
+                #
+                # (function (tuple (... xs)) body) ==> (function (... xs) body)
+                if length(args[1].args) == 1 && Meta.isexpr(args[1].args[1], :...)
+                    # function (xs...) \n body end
+                    args[1] = args[1].args[1]
+                end
             end
+            pushfirst!(args[2].args, loc)
+        end
+    elseif headsym == :macro
+        if length(args) > 1
+            pushfirst!(args[2].args, loc)
         end
+    elseif headsym == :module
+        pushfirst!(args[3].args, loc)
     end
     if headsym == :inert || (headsym == :quote && length(args) == 1 &&
                  !(a1 = only(args); a1 isa Expr || a1 isa QuoteNode ||
diff --git a/src/hooks.jl b/src/hooks.jl
@@ -35,10 +35,10 @@ function core_parser_hook(code, filename, lineno, offset, options)
         end
 
         if any_error(stream)
-            e = Expr(:error, ParseError(SourceFile(code), stream.diagnostics))
+            e = Expr(:error, ParseError(SourceFile(code, filename=filename), stream.diagnostics))
             ex = options === :all ? Expr(:toplevel, e) : e
         else
-            ex = build_tree(Expr, stream, wrap_toplevel_as_kind=K"None")
+            ex = build_tree(Expr, stream, filename=filename, wrap_toplevel_as_kind=K"None")
             if Meta.isexpr(ex, :None)
                 # The None wrapping is only to give somewhere for trivia to be
                 # attached; unwrap!
diff --git a/src/parser.jl b/src/parser.jl
@@ -1852,7 +1852,7 @@ end
 
 # Parse if-elseif-else-end expressions
 #
-# if a xx elseif b yy else zz end   ==>  (if a (block xx) (elseif (block b) (block yy) (block zz)))
+# if a xx elseif b yy else zz end   ==>  (if a (block xx) (elseif b (block yy) (block zz)))
 function parse_if_elseif(ps, is_elseif=false, is_elseif_whitespace_err=false)
     mark = position(ps)
     word = peek(ps)
@@ -1872,23 +1872,19 @@ function parse_if_elseif(ps, is_elseif=false, is_elseif_whitespace_err=false)
         # if a xx end   ==>  (if a (block xx))
         parse_cond(ps)
     end
-    if is_elseif
-        # Wart: `elseif` condition is in a block but not `if` condition
-        emit(ps, cond_mark, K"block")
-    end
     # if a \n\n xx \n\n end   ==>  (if a (block xx))
     parse_block(ps)
     bump_trivia(ps)
     k = peek(ps)
     if k == K"elseif"
-        # if a xx elseif b yy end   ==>  (if a (block xx) (elseif (block b) (block yy)))
+        # if a xx elseif b yy end   ==>  (if a (block xx) (elseif b (block yy)))
         parse_if_elseif(ps, true)
     elseif k == K"else"
         emark = position(ps)
         bump(ps, TRIVIA_FLAG)
         if peek(ps) == K"if"
             # Recovery: User wrote `else if` by mistake ?
-            # if a xx else if b yy end  ==>  (if a (block xx) (error-t) (elseif (block b) (block yy)))
+            # if a xx else if b yy end  ==>  (if a (block xx) (error-t) (elseif b (block yy)))
             bump(ps, TRIVIA_FLAG)
             emit(ps, emark, K"error", TRIVIA_FLAG,
                  error="use `elseif` instead of `else if`")
diff --git a/src/source_files.jl b/src/source_files.jl
@@ -1,5 +1,5 @@
 """
-    SourceFile(code [, filename])
+    SourceFile(code [; filename=nothing])
 
 A UTF-8 source code string with associated file name and indexing structures.
 """
diff --git a/test/expr.jl b/test/expr.jl
@@ -0,0 +1,147 @@
+
+@testset "Expr conversion" begin
+    @testset "Quote nodes" begin
+        @test parseall(Expr, ":(a)", rule=:atom) == QuoteNode(:a)
+        @test parseall(Expr, ":(:a)", rule=:atom) == Expr(:quote, QuoteNode(:a))
+        @test parseall(Expr, ":(1+2)", rule=:atom) == Expr(:quote, Expr(:call, :+, 1, 2))
+        # Compatibility hack for VERSION >= v"1.4"
+        # https://github.com/JuliaLang/julia/pull/34077
+        @test parseall(Expr, ":true", rule=:atom) == Expr(:quote, true)
+    end
+
+    @testset "Line numbers" begin
+        @testset "Blocks" begin
+            @test parseall(Expr, "begin a\nb\n\nc\nend", rule=:statement) ==
+                Expr(:block,
+                     LineNumberNode(1),
+                     :a,
+                     LineNumberNode(2),
+                     :b,
+                     LineNumberNode(4),
+                     :c,
+                )
+            @test parseall(Expr, "begin end", rule=:statement) ==
+                Expr(:block,
+                     LineNumberNode(1)
+                )
+
+            @test parseall(Expr, "a\n\nb") ==
+                Expr(:toplevel,
+                     LineNumberNode(1),
+                     :a,
+                     LineNumberNode(3),
+                     :b,
+                )
+
+            @test parseall(Expr, "module A\n\nbody\nend", rule=:statement) ==
+                Expr(:module,
+                     true,
+                     :A,
+                     Expr(:block,
+                          LineNumberNode(1),
+                          LineNumberNode(3),
+                          :body,
+                     ),
+                )
+        end
+
+        @testset "Function definition lines" begin
+            @test parseall(Expr, "function f()\na\n\nb\nend", rule=:statement) ==
+                Expr(:function,
+                     Expr(:call, :f),
+                     Expr(:block,
+                         LineNumberNode(1),
+                         LineNumberNode(2),
+                         :a,
+                         LineNumberNode(4),
+                         :b,
+                     )
+                )
+            @test parseall(Expr, "f() = 1", rule=:statement) ==
+                Expr(:(=),
+                     Expr(:call, :f),
+                     Expr(:block,
+                          LineNumberNode(1),
+                          1
+                     )
+                )
+
+            # function/macro without methods
+            @test parseall(Expr, "function f end", rule=:statement) ==
+                Expr(:function, :f)
+            @test parseall(Expr, "macro f end", rule=:statement) ==
+                Expr(:macro, :f)
+        end
+
+        @testset "elseif" begin
+            @test parseall(Expr, "if a\nb\nelseif c\n d\nend", rule=:statement) ==
+                Expr(:if,
+                     :a,
+                     Expr(:block,
+                          LineNumberNode(2),
+                          :b),
+                     Expr(:elseif,
+                          Expr(:block,
+                               LineNumberNode(3),  # Line number for elseif condition
+                               :c),
+                          Expr(:block,
+                               LineNumberNode(4),
+                               :d),
+                     )
+                )
+        end
+
+        @testset "No line numbers in for/let bindings" begin
+            @test parseall(Expr, "for i=is, j=js\nbody\nend", rule=:statement) ==
+                Expr(:for,
+                     Expr(:block,
+                         Expr(:(=), :i, :is),
+                         Expr(:(=), :j, :js),
+                     ),
+                     Expr(:block,
+                         LineNumberNode(2),
+                         :body
+                     )
+                )
+            @test parseall(Expr, "let i=is, j=js\nbody\nend", rule=:statement) ==
+                Expr(:let,
+                     Expr(:block,
+                         Expr(:(=), :i, :is),
+                         Expr(:(=), :j, :js),
+                     ),
+                     Expr(:block,
+                         LineNumberNode(2),
+                         :body
+                     )
+                )
+        end
+    end
+
+    @testset "Short form function line numbers" begin
+        # A block is added to hold the line number node
+        @test parseall(Expr, "f() = xs", rule=:statement) ==
+            Expr(:(=),
+                 Expr(:call, :f),
+                 Expr(:block,
+                      LineNumberNode(1),
+                      :xs))
+        # flisp parser quirk: In a for loop the block is not added, despite
+        # this defining a short-form function.
+        @test parseall(Expr, "for f() = xs\nend", rule=:statement) ==
+            Expr(:for,
+                 Expr(:(=), Expr(:call, :f), :xs),
+                 Expr(:block,
+                      LineNumberNode(1)
+                     ))
+    end
+
+    @testset "Long form anonymous functions" begin
+        @test parseall(Expr, "function (xs...)\nbody end", rule=:statement) ==
+            Expr(:function,
+                 Expr(:..., :xs),
+                 Expr(:block,
+                      LineNumberNode(1),
+                      LineNumberNode(2),
+                      :body))
+    end
+end
diff --git a/test/hooks.jl b/test/hooks.jl
@@ -1,18 +1,26 @@
 @testset "Hooks for Core integration" begin
-    JuliaSyntax.enable_in_core!()
+    @testset "filename is used" begin
+        ex = JuliaSyntax.core_parser_hook("@a", "somefile", 0, :statement)[1]
+        @test Meta.isexpr(ex, :macrocall)
+        @test ex.args[2] == LineNumberNode(1, "somefile")
+    end
 
-    @test Meta.parse("x + 1") == :(x + 1)
-    @test Meta.parse("x + 1", 1) == (:(x + 1), 6)
+    @testset "enable_in_core!" begin
+        JuliaSyntax.enable_in_core!()
 
-    # Test that parsing statements incrementally works and stops after
-    # whitespace / comment trivia
-    @test Meta.parse("x + 1\n(y)\n", 1) == (:(x + 1), 7)
-    @test Meta.parse("x + 1\n(y)\n", 7) == (:y, 11)
-    @test Meta.parse(" x#==#", 1) == (:x, 7)
+        @test Meta.parse("x + 1") == :(x + 1)
+        @test Meta.parse("x + 1", 1) == (:(x + 1), 6)
 
-    # Check that Meta.parse throws the JuliaSyntax.ParseError rather than
-    # Meta.ParseError when Core integration is enabled.
-    @test_throws JuliaSyntax.ParseError Meta.parse("[x")
+        # Test that parsing statements incrementally works and stops after
+        # whitespace / comment trivia
+        @test Meta.parse("x + 1\n(y)\n", 1) == (:(x + 1), 7)
+        @test Meta.parse("x + 1\n(y)\n", 7) == (:y, 11)
+        @test Meta.parse(" x#==#", 1) == (:x, 7)
 
-    JuliaSyntax.enable_in_core!(false)
+        # Check that Meta.parse throws the JuliaSyntax.ParseError rather than
+        # Meta.ParseError when Core integration is enabled.
+        @test_throws JuliaSyntax.ParseError Meta.parse("[x")
+
+        JuliaSyntax.enable_in_core!(false)
+    end
 end
diff --git a/test/parser.jl b/test/parser.jl
@@ -390,14 +390,14 @@ tests = [
         "export (\$f)"  =>  "(export (\$ f))"
     ],
     JuliaSyntax.parse_if_elseif => [
-        "if a xx elseif b yy else zz end" => "(if a (block xx) (elseif (block b) (block yy) (block zz)))"
+        "if a xx elseif b yy else zz end" => "(if a (block xx) (elseif b (block yy) (block zz)))"
         "if end"        =>  "(if (error) (block))"
         "if \n end"     =>  "(if (error) (block))"
         "if a end"      =>  "(if a (block))"
         "if a xx end"   =>  "(if a (block xx))"
         "if a \n\n xx \n\n end"   =>  "(if a (block xx))"
-        "if a xx elseif b yy end"   =>  "(if a (block xx) (elseif (block b) (block yy)))"
-        "if a xx else if b yy end"  =>  "(if a (block xx) (error-t) (elseif (block b) (block yy)))"
+        "if a xx elseif b yy end"   =>  "(if a (block xx) (elseif b (block yy)))"
+        "if a xx else if b yy end"  =>  "(if a (block xx) (error-t) (elseif b (block yy)))"
         "if a xx else yy end"   =>  "(if a (block xx) (block yy))"
     ],
     JuliaSyntax.parse_const_local_global => [
diff --git a/test/parser_api.jl b/test/parser_api.jl
@@ -1,7 +1,12 @@
 @testset "parser API" begin
     @testset "String and buffer input" begin
         # String
-        @test parse(Expr, "x+y\nz") == (Expr(:toplevel, :(x+y), :z), [], 6)
+        let
+            ex,diag,pos = parse(Expr, "x+y\nz")
+            @test JuliaSyntax.remove_linenums!(ex) == Expr(:toplevel, :(x+y), :z)
+            @test diag == []
+            @test pos == 6
+        end
         @test parse(Expr, "x+y\nz", rule=:statement) == (:(x+y), [], 4)
         @test parse(Expr, "x+y\nz", rule=:atom) == (:x, [], 2)
         @test parse(Expr, "x+y\nz", 5, rule=:atom) == (:z, [], 6)
@@ -56,7 +61,7 @@
     end
 
     @testset "parseall" begin
-        @test parseall(Expr, " x ") == Expr(:toplevel, :x)
+        @test JuliaSyntax.remove_linenums!(parseall(Expr, " x ")) == Expr(:toplevel, :x)
         @test parseall(Expr, " x ", rule=:statement) == :x
         @test parseall(Expr, " x ", rule=:atom) == :x
         # TODO: Fix this situation with trivia here; the brackets are trivia, but
diff --git a/test/runtests.jl b/test/runtests.jl
diff --git a/test/syntax_tree.jl b/test/syntax_tree.jl
diff --git a/test/test_utils.jl b/test/test_utils.jl
