Warn about function definitions with grouping parens

[c42f]

Function definitions with grouping parentheses around the signature are ugly and somewhat syntactically ambiguous. We must support them for compatibility but I think they're a syntactic misfeature we should discourage with a warning (and eventually in the far future disallow).

---

A first example of ambiguity

# 0-arg function named `f`
function (f())
end

# 1-arg anonymous function
function (f)
end

# function declaration without methods
function f
end

Arguably, the first form could be seen as a 1-arg anonymous function (with invalid nested expression f() which would be caught at lowering).

Allowing grouping parentheses here isn't ever necessary but has been done several times in the package ecosystem. The only valid use case I can think of is wanting to put the where on a new line (https://github.com/JuliaLang/JuliaSyntax.jl/issues/116#issuecomment-1271096904) but this is not really necessary.

---

There's a further problem about the precedence of :: and where sufficies to the call expression. To copy some comments from https://github.com/JuliaLang/JuliaSyntax.jl/pull/131#issuecomment-1295364086

You can't "just add grouping parens" without changing the relative precedence of :: and where:

Ie, we can't go from the normal syntax

julia> function f(a::T)::T where T
           a
       end
f (generic function with 1 method)

julia> f(1)
1

To the same thing with parens without the precedence of :: and where changing:

julia> function (g(a::T)::T where T)
           a
       end
ERROR: UndefVarError: T not defined

Additional parens (like in your example) are required to manually fix the precedence:

julia> function ((h(a::T)::T) where T)
           a
       end
h (generic function with 1 method)

[gafter]

The trees are functional, and enable incremental parsing (even if the parser isn't incremental today). When we designed the green-tree model originally, in Roslyn for C#, we did indeed implement incremental parsing. It handles small changes in files of millions of lines blazingly fast. If files are usually small, which might be the case in the Julia world, then incremental parsing may not matter much.

However, it would be a good idea not to take any design decisions that conflict with the possibility of having an incremental parser in the future. One thing that would conflict with it is having the behavior of the parser have the side-effect of producing warnings to a side channel. There are several ways to avoid that.

1. Have every node contain a summary of warnings underneath it.
2. Have warnings produced by the consumer.

The first approach is a great way of dealing with (syntax) errors, but it gets awkward for warnings.

I recommend the second approach. Is this something that can be done when Expr nodes are built after parsing?

[c42f]

Hi @gafter, thanks for taking the time to comment!

Currently diagnostics (errors/warnings/etc) are not a side effect of the parser - they are returned as a result of parsing and the caller needs to decide what to do with them. The function parse!(::ParseStream) is the low-level API for this.

To be precise, all diagnostics are recorded in a field of the ParseStream data structure which also contains a compact (almost-allocation-free!) representation of our green tree. All detailed human-readable representations of diagnostics are kept outside the tree. Error markers are left within the tree to indicate the points at which the tree is broken and probably shouldn't be processed by downstream tooling.

Is this something that can be done when Expr nodes are built after parsing?

I think the current design does this? The high-level APIs like parsestmt() call into the low level parse!() function. Then they inspect the list of diagnostics and decide whether to throw an error depending on whether ignore_warnings is true.

So this all works fine within JuliaSyntax itself. However, one area we currently have trouble is integration into Julia itself - we don't have a convention for returning warnings as part of calling Meta.parse(), so users of the JuliaSyntax integration with Base unfortunately don't have a way to receive warnings.

If you're interested in this issue, please read and comment on issue https://github.com/JuliaLang/JuliaSyntax.jl/issues/276. I'd be more than happy to have your thoughts there!