Merge pull request #226 from JuliaLang/c42f/various-fixes

Allow parentheses in import paths and macro names

Author: c42f

src/expr.jl

@@ -320,6 +320,21 @@ function _to_expr(node::SyntaxNode; iteration_spec=false, need_linenodes=true,
         pushfirst!(args, :*)
     elseif headsym === :struct
         pushfirst!(args, has_flags(node, MUTABLE_FLAG))
+    elseif headsym === :import || headsym == :using
+        # Permit nonsense additional quoting such as
+        # import A.(:b).:c
+        if !isempty(args) && Meta.isexpr(args[1], Symbol(":"))
+            imports = args[1].args
+        else
+            imports = args
+        end
+        for imp in imports
+            for i = 1:length(imp.args)
+                if imp.args[i] isa QuoteNode
+                    imp.args[i] = imp.args[i].value
+                end
+            end
+        end
     end
     return Expr(headsym, args...)
 end

src/parser.jl

@@ -1444,21 +1444,6 @@ function parse_unary_prefix(ps::ParseState)
     end
 end
 
-# Parse a symbol or interpolation syntax
-function parse_identifier_or_interpolate(ps::ParseState)
-    mark = position(ps)
-    parse_unary_prefix(ps)
-    b = peek_behind(ps)
-    # export (x::T) ==> (export (error (parens (::-i x T))))
-    # export outer  ==> (export outer)
-    # export ($f)   ==> (export ($ f))
-    ok = (b.is_leaf  && (b.kind == K"Identifier" || is_operator(b.kind))) ||
-         (!b.is_leaf && b.kind in KSet"$ var")
-    if !ok
-        emit(ps, mark, K"error", error="Expected identifier")
-    end
-end
-
 # Parses a chain of sufficies at function call precedence, leftmost binding
 # tightest. This handles
 #  * Bracketed calls like a() b[] c{}
@@ -2008,7 +1993,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, parse_atsym)
+        parse_comma_separated(ps, x->parse_atsym(x, false))
         emit(ps, mark, K"export")
     elseif word in KSet"import using"
         parse_imports(ps)
@@ -2098,7 +2083,7 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
     mark = position(ps)
     if !is_function
         # Parse macro name
-        parse_identifier_or_interpolate(ps)
+        parse_unary_prefix(ps)
         kb = peek_behind(ps).orig_kind
         if is_initial_reserved_word(ps, kb)
             # macro while(ex) end  ==> (macro (call (error while) ex) (block))
@@ -2111,7 +2096,9 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
             # macro ($f)()  end  ==>  (macro (call (parens ($ f))) (block))
         end
     else
-        if peek(ps) == K"("
+        if peek(ps) != K"("
+            parse_unary_prefix(ps)
+        else
             # When an initial parenthesis is present, we might either have
             # * the function name in parens, followed by (args...)
             # * an anonymous function argument list in parens
@@ -2151,14 +2138,12 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
                 # function (::T)() end  ==> (function (call (parens (::-pre T))) (block))
                 emit(ps, mark, K"parens", PARENS_FLAG)
             end
-        else
-            parse_unary_prefix(ps)
         end
         if !is_anon_func
             kb = peek_behind(ps).orig_kind
             if is_reserved_word(kb)
                 # function begin() end  ==>  (function (call (error begin)) (block))
-                emit(ps, mark, K"error", error="Invalid function name")
+                emit(ps, mark, K"error", error="invalid function name")
             else
                 # function f() end     ==>  (function (call f) (block))
                 # function type() end  ==>  (function (call type) (block))
@@ -2335,18 +2320,10 @@ 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-1)
-            b = peek_behind(ps, macro_name_position)
-            k = b.kind
-            if !has_flags(b.flags, TRIVIA_FLAG)
-                break
-            end
-        end
+    elseif k == K"parens"
+        # @(A) x  ==>  (macrocall (parens @A) x)
+        macro_name_position = first_child_position(ps, macro_name_position)
+        k = peek_behind(ps, macro_name_position).kind
     elseif k == K"error"
         # Error already reported in parse_macro_name
         return
@@ -2373,12 +2350,12 @@ function parse_macro_name(ps::ParseState)
     # @var"#" x   ==>  (macrocall (var #) @$ x)
     bump_disallowed_space(ps)
     mark = position(ps)
-    k = peek(ps)
     parse_atom(ps, false)
-    if k == K"("
+    kb = peek_behind(ps, position(ps)).kind
+    if kb == K"parens"
         emit_diagnostic(ps, mark,
             warning="parenthesizing macro names is unnecessary")
-    elseif !(peek_behind(ps).kind in KSet"Identifier var")
+    elseif !(kb in KSet"Identifier var")
         # @[x] y z  ==>  (macrocall (error (vect x)) y z)
         emit(ps, mark, K"error", error="invalid macro name")
     end
@@ -2387,7 +2364,7 @@ end
 # Parse an identifier, interpolation or @-prefixed symbol
 #
 # flisp: parse-atsym
-function parse_atsym(ps::ParseState)
+function parse_atsym(ps::ParseState, allow_quotes=true)
     bump_trivia(ps)
     if peek(ps) == K"@"
         # export @a       ==>  (export @a)
@@ -2400,7 +2377,30 @@ function parse_atsym(ps::ParseState)
         # export a  ==>  (export a)
         # export \n a  ==>  (export a)
         # export $a, $(a*b)  ==>  (export ($ a) (parens ($ (call * a b))))
-        parse_identifier_or_interpolate(ps)
+        # export (x::T) ==> (export (error (parens (::-i x T))))
+        # export outer  ==> (export outer)
+        # export ($f)   ==> (export ($ f))
+        mark = position(ps)
+        if allow_quotes && peek(ps) == K":"
+            # import A.:+  ==>  (import (. A (quote +)))
+            emit_diagnostic(ps, warning="quoting with `:` is not required here")
+        end
+        parse_unary_prefix(ps)
+        pos = position(ps)
+        while peek_behind(ps, pos).kind == K"parens"
+            # import A.(:+)  ==>  (import (. A (parens (quote +))))
+            pos = first_child_position(ps, pos)
+            emit_diagnostic(ps, mark, warning="parentheses are not required here")
+        end
+        if allow_quotes && peek_behind(ps, pos).kind == K"quote"
+            pos = first_child_position(ps, pos)
+        end
+        b = peek_behind(ps, pos)
+        ok = (b.is_leaf  && (b.kind == K"Identifier" || is_operator(b.kind))) ||
+             (!b.is_leaf && b.kind in KSet"$ var")
+        if !ok
+            emit(ps, mark, K"error", error="expected identifier")
+        end
     end
 end
 
@@ -2532,12 +2532,6 @@ function parse_import_path(ps::ParseState)
             # import A.B.C  ==>  (import (. A B C))
             bump_disallowed_space(ps)
             bump(ps, TRIVIA_FLAG)
-            if peek(ps) == K":"
-                # import A.:+  ==>  (import (. A +))
-                bump_disallowed_space(ps)
-                emit_diagnostic(ps, warning="quoting with `:` in import is unnecessary")
-                bump(ps, TRIVIA_FLAG)
-            end
             parse_atsym(ps)
         elseif is_dotted(t)
             # Resolve tokenization ambiguity: In imports, dots are part of the

test/diagnostics.jl

@@ -36,7 +36,13 @@ end
 	@test diagnostic("import A .==") ==
         Diagnostic(9, 9, :warning, "space between dots in import path")
 	@test diagnostic("import A.:+") ==
-        Diagnostic(10, 10, :warning, "quoting with `:` in import is unnecessary")
+        Diagnostic(10, 10, :warning, "quoting with `:` is not required here")
+    @test diagnostic("import A.(:+)") ==
+        Diagnostic(10, 13, :warning, "parentheses are not required here")
+    @test diagnostic("export (x)") ==
+        Diagnostic(8, 10, :warning, "parentheses are not required here")
+    @test diagnostic("export :x") == 
+        Diagnostic(8, 9, :error, "expected identifier")
 end
 
 @testset "diagnostics for literal parsing" begin

test/expr.jl

@@ -327,11 +327,17 @@
             Expr(:module, false, :A, Expr(:block, LineNumberNode(1), LineNumberNode(1)))
     end
 
+
     @testset "errors" begin
         @test parse(Expr, "--", ignore_errors=true) ==
             Expr(:error, "invalid operator: `--`")
         @test parseall(Expr, "a b", ignore_errors=true) ==
             Expr(:toplevel, LineNumberNode(1), :a,
                  LineNumberNode(1), Expr(:error, :b))
     end
+
+    @testset "import" begin
+        @test parse(Expr, "import A.(:b).:c: x.:z", ignore_warnings=true) ==
+            Expr(:import, Expr(Symbol(":"), Expr(:., :A, :b, :c), Expr(:., :x, :z)))
+    end
 end

test/parser.jl

@@ -321,6 +321,7 @@ tests = [
         "[@foo x]"     =>  "(vect (macrocall @foo x))"
         "[@foo]"       =>  "(vect (macrocall @foo))"
         "@var\"#\" a"  =>  "(macrocall (var @#) a)"                => Expr(:macrocall, Symbol("@#"), LineNumberNode(1), :a)
+        "@(A) x"       =>  "(macrocall (parens @A) x)"
         "A.@x y"       =>  "(macrocall (. A (quote @x)) y)"
         "A.@var\"#\" a"=>  "(macrocall (. A (quote (var @#))) a)"  => Expr(:macrocall, Expr(:., :A, QuoteNode(Symbol("@#"))), LineNumberNode(1), :a)
         "@+x y"        =>  "(macrocall @+ x y)"
@@ -648,7 +649,8 @@ tests = [
         "import \$A.@x"  =>  "(import (. (\$ A) @x))"
         "import A.B"  =>  "(import (. A B))"
         "import A.B.C"  =>  "(import (. A B C))"
-        "import A.:+"  =>  "(import (. A +))"
+        "import A.:+"  =>  "(import (. A (quote +)))"
+        "import A.(:+)"=>  "(import (. A (parens (quote +))))"
         "import A.=="  =>  "(import (. A ==))"
         "import A.⋆.f" =>  "(import (. A ⋆ f))"
         "import A..."  =>  "(import (. A ..))"
@@ -941,7 +943,7 @@ parseall_test_specs = [
 
     # The following may not be ideal error recovery! But at least the parser
     # shouldn't crash
-    "@(x y)" => "(toplevel (macrocall (error x (error-t y))))"
+    "@(x y)" => "(toplevel (macrocall (parens @x (error-t y))))"
     "|(&\nfunction" => "(toplevel (call | (& (function (error (error)) (block (error)) (error-t))) (error-t)))"
 ]