From 6f62399ab3565d7a1942d5d431a98a475b1a8e76 Mon Sep 17 00:00:00 2001
From: Claire Foster <aka.c42f@gmail.com>
Date: Fri, 2 Aug 2024 14:19:48 +1000
Subject: [PATCH] Make `function (@f(x)) body end` an ambiguity error

This case is ambiguous as it might be either one of the following;
require the user to explicitly disambiguate between them

```
function (@f(x),)
    body
end

function @f(x)
    body
end
```

For the same reasons, `function ($f) body end` is also ambiguous.

Also fix parsing of `function (f(x),) end` to correctly emit a tuple.
---
 src/parser.jl       | 14 +++++++++++---
 test/diagnostics.jl |  3 +++
 test/parser.jl      |  5 +++++
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/parser.jl b/src/parser.jl
index 6d2bd893..831887b2 100644
--- a/src/parser.jl
+++ b/src/parser.jl
@@ -2110,14 +2110,15 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
             # * The whole function declaration, in parens
             bump(ps, TRIVIA_FLAG)
             is_empty_tuple = peek(ps, skip_newlines=true) == K")"
-            opts = parse_brackets(ps, K")") do _, _, _, _
+            opts = parse_brackets(ps, K")") do had_commas, had_splat, num_semis, num_subexprs
                 _parsed_call = was_eventually_call(ps)
                 _needs_parse_call = peek(ps, 2) ∈ KSet"( ."
-                _is_anon_func = !_needs_parse_call && !_parsed_call
+                _is_anon_func = (!_needs_parse_call && !_parsed_call) || had_commas
                 return (needs_parameters = _is_anon_func,
                         is_anon_func     = _is_anon_func,
                         parsed_call      = _parsed_call,
-                        needs_parse_call = _needs_parse_call)
+                        needs_parse_call = _needs_parse_call,
+                        maybe_grouping_parens = !had_commas && !had_splat && num_semis == 0 && num_subexprs == 1)
             end
             is_anon_func = opts.is_anon_func
             parsed_call = opts.parsed_call
@@ -2128,7 +2129,14 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
                 # function (x,y) end    ==>  (function (tuple-p x y) (block))
                 # function (x=1) end    ==>  (function (tuple-p (= x 1)) (block))
                 # function (;x=1) end   ==>  (function (tuple-p (parameters (= x 1))) (block))
+                # function (f(x),) end  ==>  (function (tuple-p (call f x)) (block))
+                ambiguous_parens = opts.maybe_grouping_parens &&
+                                   peek_behind(ps).kind in KSet"macrocall $"
                 emit(ps, mark, K"tuple", PARENS_FLAG)
+                if ambiguous_parens
+                    # Got something like `(@f(x))`. Is it anon `(@f(x),)` or named sig `@f(x)` ??
+                    emit(ps, mark, K"error", error="Ambiguous signature. Add a trailing comma if this is a 1-argument anonymous function; remove parentheses if this is a macro call acting as function signature.")
+                end
             elseif is_empty_tuple
                 # Weird case which is consistent with parse_paren but will be
                 # rejected in lowering
diff --git a/test/diagnostics.jl b/test/diagnostics.jl
index 1d1f9e5d..97ee25a0 100644
--- a/test/diagnostics.jl
+++ b/test/diagnostics.jl
@@ -40,6 +40,9 @@ end
     @test diagnostic("\n+ (x, y)") ==
         Diagnostic(3, 3, :error, "whitespace not allowed between prefix function call and argument list")
 
+    @test diagnostic("function (\$f) body end") ==
+        Diagnostic(10, 13, :error, "Ambiguous signature. Add a trailing comma if this is a 1-argument anonymous function; remove parentheses if this is a macro call acting as function signature.")
+
 	@test diagnostic("A.@B.x", only_first=true) ==
         Diagnostic(3, 4, :error, "`@` must appear on first or last macro name component")
 	@test diagnostic("@M.(x)") ==
diff --git a/test/parser.jl b/test/parser.jl
index 80b7ffec..c5ab1d5e 100644
--- a/test/parser.jl
+++ b/test/parser.jl
@@ -563,6 +563,11 @@ tests = [
         "function (x,y) end"   =>  "(function (tuple-p x y) (block))"
         "function (x=1) end"   =>  "(function (tuple-p (= x 1)) (block))"
         "function (;x=1) end"  =>  "(function (tuple-p (parameters (= x 1))) (block))"
+        "function (f(x),) end" =>  "(function (tuple-p (call f x)) (block))"
+        "function (@f(x);) end" => "(function (tuple-p (macrocall-p @f x) (parameters)) (block))"
+        "function (@f(x)...) end" =>  "(function (tuple-p (... (macrocall-p @f x))) (block))"
+        "function (@f(x)) end" =>  "(function (error (tuple-p (macrocall-p @f x))) (block))"
+        "function (\$f) end"   =>  "(function (error (tuple-p (\$ f))) (block))"
         "function ()(x) end"   =>  "(function (call (tuple-p) x) (block))"
         "function (A).f() end" =>  "(function (call (. (parens A) f)) (block))"
         "function (:)() end"   =>  "(function (call (parens :)) (block))"