Tweak macro name representation

The current representation for macro names is a bit peculiar. When
the parser encounters `@a`, it treats `@` as notation for the
macrocall and then `reset_node!`'s (which itself may be considerd
a bit of a code smell) the `a` to a special MacroName
token kind that (when translated back to julia Expr) implicitly
adds back the `@`. Things get even more preculiar with `@var"a"`
where only the token inside the string macro gets reset.

One particular consequence of this is JuliaLang/julia#58885,
because our translation back to Expr does not check the RAW_STRING_FLAG
(whereas the translation for K"Identifier" does).

A second issue is that we currently parse `@A.b.c` and `A.b.@c` to the
same SyntaxTree (of course the green tree is different). We aren't
currently being super precise about the required invariants for syntax
trees, but in general it would be desirable for non-trivia notation
(like `@`) to be precisely recoverable from the tree, which is not
the case here. This is especially annoying because there are syntax
cases that are errors for one of these, but not the other (e.g.
`@A.b.$` is an error, but `A.B.@$` is allowed). Now, I think the wisdom
of some of those syntax choices can be debated, but that is the situation
we face.

So this PR tries to clean that all up a bit by:
- Replacing the terminal K"MacroName" by a non-terminal K"macro_name".
  With this form, `@A.c` parses as `(macro_name (. A c))` while
  `A.@c` parses as `(. A (macro_name c))`.
- (In particular the `@` notation is now always associated with the
   macro_name).
- Emitting the dots in `@..` and `@...` as direct identifier tokens
  rather than having to reset them back.
- Adjusting everything else accordingly.

Partially written by Claude Code, though it had some trouble with
the actual code changes.

Author: Keno

src/integration/expr.jl

@@ -246,8 +246,6 @@ function node_to_expr(cursor, source, txtbuf::Vector{UInt8}, txtbuf_offset::UInt
                         val isa UInt128 ? Symbol("@uint128_str") :
                         Symbol("@big_str")
                 return Expr(:macrocall, GlobalRef(Core, macname), nothing, str)
-            elseif k == K"MacroName" && val === Symbol("@.")
-                return Symbol("@__dot__")
             else
                 return val
             end
@@ -296,6 +294,30 @@ function node_to_expr(cursor, source, txtbuf::Vector{UInt8}, txtbuf_offset::UInt
                          nodehead, source)
 end
 
+function adjust_macro_name!(retexpr::Union{Expr, Symbol}, k::Kind)
+    if !(retexpr isa Symbol)
+        retexpr::Expr
+        # can happen for incomplete or errors
+        (length(retexpr.args) < 2 || retexpr.head != :(.)) && return retexpr
+        arg2 = retexpr.args[2]
+        isa(arg2, QuoteNode) || return retexpr
+        retexpr.args[2] = QuoteNode(adjust_macro_name!(arg2.value, k))
+        return retexpr
+    end
+    if k == K"macro_name"
+        if retexpr === Symbol(".")
+            return Symbol("@__dot__")
+        else
+            return Symbol("@$retexpr")
+        end
+    elseif k == K"macro_name_cmd"
+        return Symbol("@$(retexpr)_cmd")
+    else
+        @assert k == K"macro_name_str"
+        return Symbol("@$(retexpr)_str")
+    end
+end
+
 # Split out from the above for codesize reasons, to avoid specialization on multiple
 # tree types.
 @noinline function _node_to_expr(retexpr::Expr, loc::LineNumberNode,
@@ -312,6 +334,8 @@ end
         # However, errors can add additional errors tokens which we represent
         # as e.g. `Expr(:var, ..., Expr(:error))`.
         return retexpr.args[1]
+    elseif k in KSet"macro_name macro_name_cmd macro_name_str"
+        return adjust_macro_name!(retexpr.args[1], k)
     elseif k == K"?"
         retexpr.head = :if
     elseif k == K"op=" && length(args) == 3
@@ -331,7 +355,7 @@ end
     elseif k == K"macrocall"
         if length(args) >= 2
             a2 = args[2]
-            if @isexpr(a2, :macrocall) && kind(firstchildhead) == K"CmdMacroName"
+            if @isexpr(a2, :macrocall) && kind(firstchildhead) == K"macro_name_cmd"
                 # Fix up for custom cmd macros like foo`x`
                 args[2] = a2.args[3]
             end

src/julia/kinds.jl

@@ -194,15 +194,6 @@ register_kinds!(JuliaSyntax, 0, [
     "BEGIN_IDENTIFIERS"
         "Identifier"
         "Placeholder" # Used for empty catch variables, and all-underscore identifiers in lowering
-        # Macro names are modelled as special kinds of identifiers because the full
-        # macro name may not appear as characters in the source: The `@` may be
-        # detached from the macro name as in `@A.x` (ugh!!), or have a _str or _cmd
-        # suffix appended.
-        "BEGIN_MACRO_NAMES"
-            "MacroName"
-            "StringMacroName"
-            "CmdMacroName"
-        "END_MACRO_NAMES"
     "END_IDENTIFIERS"
 
     "BEGIN_KEYWORDS"
@@ -1048,6 +1039,10 @@ register_kinds!(JuliaSyntax, 0, [
         "iteration"
         "comprehension"
         "typed_comprehension"
+        # Macro names
+        "macro_name"
+        "macro_name_cmd"
+        "macro_name_str"
         # Container for a single statement/atom plus any trivia and errors
         "wrapper"
     "END_SYNTAX_KINDS"
@@ -1111,10 +1106,6 @@ const _nonunique_kind_names = Set([
     K"String"
     K"Char"
     K"CmdString"
-
-    K"MacroName"
-    K"StringMacroName"
-    K"CmdMacroName"
 ])
 
 """
@@ -1201,7 +1192,6 @@ is_prec_unicode_ops(x) = K"BEGIN_UNICODE_OPS" <= kind(x) <= K"END_UNICODE_OPS"
 is_prec_pipe_lt(x)     = kind(x) == K"<|"
 is_prec_pipe_gt(x)     = kind(x) == K"|>"
 is_syntax_kind(x)      = K"BEGIN_SYNTAX_KINDS"<= kind(x) <= K"END_SYNTAX_KINDS"
-is_macro_name(x)       = K"BEGIN_MACRO_NAMES" <= kind(x) <= K"END_MACRO_NAMES"
 is_syntactic_assignment(x) = K"BEGIN_SYNTACTIC_ASSIGNMENTS" <= kind(x) <= K"END_SYNTACTIC_ASSIGNMENTS"
 
 function is_string_delim(x)

src/julia/literal_parsing.jl

@@ -430,12 +430,6 @@ function parse_julia_literal(txtbuf::Vector{UInt8}, head::SyntaxHead, srcrange)
             Symbol(normalize_identifier(val_str))
     elseif k == K"error"
         ErrorVal()
-    elseif k == K"MacroName"
-        Symbol("@$(normalize_identifier(val_str))")
-    elseif k == K"StringMacroName"
-        Symbol("@$(normalize_identifier(val_str))_str")
-    elseif k == K"CmdMacroName"
-        Symbol("@$(normalize_identifier(val_str))_cmd")
     elseif is_syntax_kind(head)
         nothing
     elseif is_keyword(k)

src/julia/parser.jl

@@ -1505,6 +1505,7 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
         # 2(x) ==> (* 2 x)
         return
     end
+    is_macrocall_on_entry = is_macrocall
     # source range of the @-prefixed part of a macro
     macro_atname_range = nothing
     # $A.@x  ==>  (macrocall (. ($ A) @x))
@@ -1533,7 +1534,7 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
             # A.@var"#" a ==> (macrocall (. A (var @#)) a)
             # @+x y       ==> (macrocall @+ x y)
             # [email protected]       ==> (macrocall (. A @.) x)
-            fix_macro_name_kind!(ps, macro_name_position)
+            is_macrocall_on_entry && emit(ps, mark, K"macro_name")
             let ps = with_space_sensitive(ps)
                 # Space separated macro arguments
                 # A.@foo a b    ==> (macrocall (. A @foo) a b)
@@ -1566,6 +1567,7 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
             # f(a; b; c)  ==> (call f a (parameters b) (parameters c))
             # (a=1)()  ==>  (call (parens (= a 1)))
             # f (a)    ==>  (call f (error-t) a)
+            is_macrocall_on_entry && emit(ps, mark, K"macro_name")
             bump_disallowed_space(ps)
             bump(ps, TRIVIA_FLAG)
             opts = parse_call_arglist(ps, K")")
@@ -1580,11 +1582,13 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 # @x(a, b)   ==>  (macrocall-p @x a b)
                 # A.@x(y)    ==>  (macrocall-p (. A @x) y)
                 # A.@x(y).z  ==>  (. (macrocall-p (. A @x) y) z)
-                fix_macro_name_kind!(ps, macro_name_position)
                 is_macrocall = false
+                # @f()()     ==>  (call (macrocall-p (macro_name f)))
+                is_macrocall_on_entry = false
                 macro_atname_range = nothing
             end
         elseif k == K"["
+            is_macrocall_on_entry && emit(ps, mark, K"macro_name")
             m = position(ps)
             # a [i]  ==>  (ref a (error-t) i)
             bump_disallowed_space(ps)
@@ -1599,7 +1603,6 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 # @S[a].b  ==>  (. (macrocall @S (vect a)) b)
                 #v1.7: @S[a ;; b]  ==>  (macrocall @S (ncat-2 a b))
                 #v1.6: @S[a ;; b]  ==>  (macrocall @S (error (ncat-2 a b)))
-                fix_macro_name_kind!(ps, macro_name_position)
                 emit(ps, m, ckind, cflags | set_numeric_flags(dim))
                 check_ncat_compat(ps, m, ckind)
                 emit(ps, mark, K"macrocall")
@@ -1643,12 +1646,18 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 emit_diagnostic(ps, macro_atname_range...,
                     error="`@` must appear on first or last macro name component")
                 bump(ps, TRIVIA_FLAG, error="Unexpected `.` after macro name")
+                # Recover by treating the `@` as if it had been on the wole thing
+                reset_node!(ps, macro_atname_range[2], kind=K"TOMBSTONE")
+                is_macrocall_on_entry = true
             else
                 bump(ps, TRIVIA_FLAG)
             end
             k = peek(ps)
             if k == K"("
                 if is_macrocall
+                    is_macrocall_on_entry && emit(ps, mark, K"macro_name")
+                    # Recover by pretending we do have the syntax
+                    is_macrocall_on_entry = false
                     # @M.(x)  ==> (macrocall (dotcall @M (error-t) x))
                     bump_invisible(ps, K"error", TRIVIA_FLAG)
                     emit_diagnostic(ps, mark,
@@ -1677,7 +1686,11 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 m = position(ps)
                 bump(ps, TRIVIA_FLAG)
                 parse_atom(ps)
-                emit(ps, m, K"$")
+                if is_macrocall
+                    emit(ps, m, K"error", error="invalid macro name")
+                else
+                    emit(ps, m, K"$")
+                end
                 macro_name_position = position(ps)
                 emit(ps, mark, K".")
             elseif k == K"@"
@@ -1690,11 +1703,12 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                     bump(ps, TRIVIA_FLAG, error="repeated `@` in macro module path")
                 else
                     bump(ps, TRIVIA_FLAG)
-                    is_macrocall = true
                 end
                 parse_macro_name(ps)
                 macro_name_position = position(ps)
+                !is_macrocall && emit(ps, m, K"macro_name")
                 macro_atname_range = (m, position(ps))
+                is_macrocall = true
                 emit(ps, mark, K".")
             elseif k == K"'"
                 # f.'  =>  (dotcall-post f (error '))
@@ -1717,6 +1731,7 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
             bump(ps, remap_kind=K"Identifier")
             emit(ps, mark, K"call", POSTFIX_OP_FLAG)
         elseif k == K"{"
+            is_macrocall_on_entry && emit(ps, mark, K"macro_name")
             # Type parameter curlies and macro calls
             m = position(ps)
             # S {a} ==> (curly S (error-t) a)
@@ -1727,7 +1742,6 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
                 # @S{a,b} ==> (macrocall S (braces a b))
                 # A.@S{a}  ==> (macrocall (. A @S) (braces a))
                 # @S{a}.b  ==> (. (macrocall @S (braces a)) b)
-                fix_macro_name_kind!(ps, macro_name_position)
                 emit(ps, m, K"braces", opts.delim_flags)
                 emit(ps, mark, K"macrocall")
                 min_supported_version(v"1.6", ps, mark, "macro call without space before `{}`")
@@ -1754,8 +1768,8 @@ function parse_call_chain(ps::ParseState, mark, is_macrocall=false)
             #
             # Use a special token kind for string and cmd macro names so the
             # names can be expanded later as necessary.
-            outk = is_string_delim(k) ? K"StringMacroName" : K"CmdMacroName"
-            fix_macro_name_kind!(ps, macro_name_position, outk)
+            outk = is_string_delim(k) ? K"macro_name_str" : K"macro_name_cmd"
+            emit(ps, mark, outk)
             parse_string(ps, true)
             t = peek_token(ps)
             k = kind(t)
@@ -2039,7 +2053,7 @@ function parse_resword(ps::ParseState)
         # export \n a      ==>  (export a)
         # export \$a, \$(a*b) ==> (export (\$ a) (\$ (parens (call-i a * b))))
         bump(ps, TRIVIA_FLAG)
-        parse_comma_separated(ps, x->parse_atsym(x, false))
+        parse_comma_separated(ps, x->parse_import_atsym(x, false))
         emit(ps, mark, word)
     elseif word in KSet"import using"
         parse_imports(ps)
@@ -2372,34 +2386,6 @@ function _is_valid_macro_name(peektok)
     return !is_error(peektok.kind) && (peektok.is_leaf || peektok.kind == K"var")
 end
 
-function fix_macro_name_kind!(ps::ParseState, macro_name_position, name_kind=nothing)
-    k = peek_behind(ps, macro_name_position).kind
-    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"parens"
-        # @(A) x  ==>  (macrocall (parens @A) x)
-        macro_name_position = first_child_position(ps, macro_name_position)
-        if macro_name_position == NO_POSITION
-            return
-        end
-        k = peek_behind(ps, macro_name_position).kind
-    elseif k == K"error"
-        # Error already reported in parse_macro_name
-        return
-    end
-    if isnothing(name_kind)
-        name_kind = _is_valid_macro_name(peek_behind(ps, macro_name_position)) ?
-                    K"MacroName" : K"error"
-        if name_kind == K"error"
-            # TODO: This isn't quite accurate
-            emit_diagnostic(ps, macro_name_position, macro_name_position,
-                            error="invalid macro name")
-        end
-    end
-    reset_node!(ps, macro_name_position, kind=name_kind)
-end
-
 # If remap_kind is false, the kind will be remapped by parse_call_chain after
 # it discovers which component of the macro's module path is the macro name.
 #
@@ -2425,15 +2411,16 @@ end
 # Parse an identifier, interpolation or @-prefixed symbol
 #
 # flisp: parse-atsym
-function parse_atsym(ps::ParseState, allow_quotes=true)
+function parse_import_atsym(ps::ParseState, allow_quotes=true)
     bump_trivia(ps)
     if peek(ps) == K"@"
+        mark = position(ps)
         # export @a       ==>  (export @a)
         # export @var"'"  ==>  (export (var @'))
         # export a, \n @b ==>  (export a @b)
         bump(ps, TRIVIA_FLAG)
         parse_macro_name(ps)
-        fix_macro_name_kind!(ps, position(ps))
+        emit(ps, mark, K"macro_name")
     else
         # export a  ==>  (export a)
         # export \n a  ==>  (export a)
@@ -2538,7 +2525,7 @@ function parse_import(ps::ParseState, word, has_import_prefix)
         # import A: x as y  ==>  (import (: (importpath A) (as (importpath x) y)))
         # using  A: x as y  ==>  (using (: (importpath A) (as (importpath x) y)))
         bump(ps, TRIVIA_FLAG)
-        parse_atsym(ps, false)
+        parse_import_atsym(ps, false)
         emit(ps, mark, K"as")
         if word == K"using" && !has_import_prefix
             # using A as B     ==>  (using (error (as (importpath A) B)))
@@ -2589,7 +2576,7 @@ function parse_import_path(ps::ParseState)
     end
     # import @x     ==>  (import (importpath @x))
     # import $A     ==>  (import (importpath ($ A)))
-    parse_atsym(ps, false)
+    parse_import_atsym(ps, false)
     while true
         t = peek_token(ps)
         k = kind(t)
@@ -2611,7 +2598,7 @@ function parse_import_path(ps::ParseState)
                 bump_disallowed_space(ps)
             end
             bump(ps, TRIVIA_FLAG)
-            parse_atsym(ps)
+            parse_import_atsym(ps)
         elseif k == K"..."
             # Import the .. operator
             # import A...  ==>  (import (importpath A ..))

src/julia/tokenize.jl

@@ -1168,30 +1168,21 @@ end
 function lex_dot(l::Lexer)
     if accept(l, '.')
         if accept(l, '.')
+            l.last_token == K"@" && return emit(l, K"Identifier")
             return emit(l, K"...")
         else
             if is_dottable_operator_start_char(peekchar(l))
                 readchar(l)
                 return emit(l, K"ErrorInvalidOperator")
             else
+                l.last_token == K"@" && return emit(l, K"Identifier")
                 return emit(l, K"..")
             end
         end
     elseif Base.isdigit(peekchar(l))
         return lex_digit(l, K"Float")
     else
-        pc, dpc = dpeekchar(l)
-        # When we see a dot followed by an operator, we want to emit just the dot
-        # and let the next token be the operator
-        if is_operator_start_char(pc) || (pc == '!' && dpc == '=')
-            return emit(l, K".")
-        elseif pc == '÷'
-            return emit(l, K".")
-        elseif pc == '=' && dpc == '>'
-            return emit(l, K".")
-        elseif is_dottable_operator_start_char(pc)
-            return emit(l, K".")
-        end
+        l.last_token == K"@" && return emit(l, K"Identifier")
         return emit(l, K".")
     end
 end

test/diagnostics.jl

@@ -53,7 +53,7 @@ end
         Diagnostic(1, 9, :error, "try without catch or finally")
     # TODO: better range
 	@test diagnostic("@A.\$x a") ==
-        Diagnostic(6, 5, :error, "invalid macro name")
+        Diagnostic(4, 5, :error, "invalid macro name")
 
 	@test diagnostic("a, , b") ==
         Diagnostic(4, 4, :error, "unexpected `,`")

test/expr.jl

@@ -554,6 +554,7 @@
 
         # var""
         @test parsestmt("@var\"#\" a") == Expr(:macrocall, Symbol("@#"), LineNumberNode(1), :a)
+        @test parsestmt("@var\"\\\"\" a") == Expr(:macrocall, Symbol("@\""), LineNumberNode(1), :a)
         @test parsestmt("A.@var\"#\" a") == Expr(:macrocall, Expr(:., :A, QuoteNode(Symbol("@#"))), LineNumberNode(1), :a)
 
         # Square brackets