Report bad macro names more clearly (#147)

This change prevents a crash for various cases of bad syntax involving
macro names. In particular things like `@[x] a` and `@A.x a`.

The latter case where the @ is separated from the actual macro name is a
pain to deal with and is only approximately correct in this PR. But I'm
not sure why we even support this syntax and maybe we could
warn about it or deprecate it. So unclear whether going to greater
lengths emitting K"TOMBSTONE" to be precise about error reporting in
that case is worth it.

Author: c42f

src/hooks.jl

@@ -186,6 +186,9 @@ function _core_parser_hook(code, filename, lineno, offset, options)
             end
             ex = options === :all ? Expr(:toplevel, error_ex) : error_ex
         else
+            # FIXME: Unilaterally showing any warnings to stdout here is far
+            # from ideal. But Meta.parse() has no API for communicating this.
+            show_diagnostics(stdout, stream.diagnostics, code)
             # FIXME: Add support to lineno to this tree build (via SourceFile?)
             ex = build_tree(Expr, stream; filename=filename, wrap_toplevel_as_kind=K"None")
             if Meta.isexpr(ex, :None)

src/parser.jl

@@ -1546,10 +1546,12 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 # f.$x      ==>  (. f (inert ($ x)))
                 # f.$(x+y)  ==>  (. f (inert ($ (call + x y))))
                 # A.$B.@x   ==>  (macrocall (. (. A (inert ($ B))) (quote @x)))
+                # @A.$x a   ==>  (macrocall (. A (inert (error x))) a)
                 m = position(ps)
                 bump(ps, TRIVIA_FLAG)
                 parse_atom(ps)
                 emit(ps, m, K"$")
+                macro_name_position = position(ps)
                 emit(ps, m, K"inert")
                 emit(ps, mark, K".")
             elseif k == K"@"
@@ -2235,10 +2237,31 @@ function fix_macro_name_kind!(ps::ParseState, macro_name_position, name_kind=not
     if k == K"var"
         macro_name_position = first_child_position(ps, macro_name_position)
         k = peek_behind(ps, macro_name_position).kind
+    elseif k == K")"
+        # @(A) x =>  (macrocall @A x)
+        # TODO: Clean this up when K"parens" is implemented
+        while true
+            macro_name_position = ParseStreamPosition(macro_name_position.token_index-1,
+                                                      macro_name_position.range_index)
+            b = peek_behind(ps, macro_name_position)
+            k = b.kind
+            if !has_flags(b.flags, TRIVIA_FLAG)
+                break
+            end
+        end
+    elseif k == K"error"
+        # Error already reported in parse_macro_name
+        return
     end
     if isnothing(name_kind)
-        name_kind = k == K"Identifier" ? K"MacroName" :
-                    internal_error("unrecognized source kind for macro name ", k)
+        name_kind = (k == K"Identifier") ? K"MacroName" : K"error"
+        if name_kind == K"error"
+            # Hack to handle bad but unusual syntax like `@A.$x a`
+            ri = macro_name_position.range_index
+            startpos = ParseStreamPosition(ps.stream.ranges[ri].first_token, ri)
+            # This isn't quite accurate
+            emit_diagnostic(ps, startpos, macro_name_position, error="Invalid macro name")
+        end
     end
     reset_node!(ps, macro_name_position, kind=name_kind)
 end
@@ -2253,8 +2276,14 @@ function parse_macro_name(ps::ParseState)
     # @$ x   ==>  (macrocall @$ x)
     # @var"#" x   ==>  (macrocall (var #) @$ x)
     bump_disallowed_space(ps)
-    let ps = with_space_sensitive(ps)
-        parse_atom(ps, false)
+    mark = position(ps)
+    k = peek(ps)
+    parse_atom(ps, false)
+    if k == K"("
+        emit_diagnostic(ps, mark, warning="parenthesizing macro names is unnecessary")
+    elseif !(peek_behind(ps).kind in KSet"Identifier var")
+        # @[x] y z  ==>  (macrocall (error (vect x)) y z)
+        emit(ps, mark, K"error", error="Invalid macro name")
     end
 end
 

test/parser.jl

@@ -261,6 +261,7 @@ tests = [
         "@! x"  => "(macrocall @! x)"
         "@.. x" => "(macrocall @.. x)"
         "@\$ y"  => "(macrocall @\$ y)"
+        "@[x] y z" => "(macrocall (error (vect x)) y z)"
         # Special @doc parsing rules
         "@doc x\ny"    =>  "(macrocall @doc x y)"
         "A.@doc x\ny"  =>  "(macrocall (. A (quote @doc)) x y)"
@@ -326,6 +327,7 @@ tests = [
         "f.\$x"     =>  "(. f (inert (\$ x)))"
         "f.\$(x+y)" =>  "(. f (inert (\$ (call-i x + y))))"
         "A.\$B.@x"  =>  "(macrocall (. (. A (inert (\$ B))) (quote @x)))"
+        "@A.\$x a"  =>  "(macrocall (. A (inert (error x))) a)"
         "A.@x"      =>  "(macrocall (. A (quote @x)))"
         "A.@x a"    =>  "(macrocall (. A (quote @x)) a)"
         "@A.B.@x a" =>  "(macrocall (. (. A (quote B)) (quote (error-t) @x)) a)"