Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parentheses not required warning in using A: (..) #350

Closed
LilithHafner opened this issue Sep 5, 2023 · 6 comments · Fixed by #477
Closed

Parentheses not required warning in using A: (..) #350

LilithHafner opened this issue Sep 5, 2023 · 6 comments · Fixed by #477
Labels
bug Something isn't working

Comments

@LilithHafner
Copy link
Member

In this case, parentheses are requires, but the warning says they are not:

julia> JuliaSyntax.parse(Expr, "using A: (..)")
ERROR: ParseError:
# Warning @ line 1:10
using A: (..)
#        └──┘ ── parentheses are not required here
Stacktrace:
 [1] _parse(rule::Symbol, need_eof::Bool, ::Type{…}, text::String, index::Int64; version::VersionNumber, ignore_trivia::Bool, filename::Nothing, first_line::Int64, ignore_errors::Bool, ignore_warnings::Bool, kws::@Kwargs{})
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:93
 [2] _parse (repeats 2 times)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:77 [inlined]
 [3] parsestmt(::Type{Expr}, text::String)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:140
 [4] parse(::Type, ::Vararg{Any}; kwargs::@Kwargs{})
   @ JuliaSyntax ./deprecated.jl:116
 [5] parse(::Type, ::Vararg{Any})
   @ JuliaSyntax ./deprecated.jl:113
 [6] top-level scope
   @ REPL[19]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> JuliaSyntax.parse(Expr, "using A: ..")
ERROR: ParseError:
# Error @ line 1:12
using A: ..
#          └ ── premature end of input
Stacktrace:
 [1] _parse(rule::Symbol, need_eof::Bool, ::Type{…}, text::String, index::Int64; version::VersionNumber, ignore_trivia::Bool, filename::Nothing, first_line::Int64, ignore_errors::Bool, ignore_warnings::Bool, kws::@Kwargs{})
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:93
 [2] _parse (repeats 2 times)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:77 [inlined]
 [3] parsestmt(::Type{Expr}, text::String)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:140
 [4] parse(::Type, ::Vararg{Any}; kwargs::@Kwargs{})
   @ JuliaSyntax ./deprecated.jl:116
 [5] parse(::Type, ::Vararg{Any})
   @ JuliaSyntax ./deprecated.jl:113
 [6] top-level scope
   @ REPL[20]:1
Some type information was truncated. Use `show(err)` to see complete types.
@LilithHafner LilithHafner added the bug Something isn't working label Sep 5, 2023
@c42f
Copy link
Member

c42f commented Sep 8, 2023

Ah this is unfortunate. I really wanted to eventually deprecate allowing parentheses in using (arguably the reference parser allows parens in many places where they add visual noise).

We do have some icky inconsistent syntax for dealing with these kind of things. For example:

julia> module A
           .. = 1
       end

There's various inconsistent forms which allow importing .. from A:

julia> import .A.(..)

julia> import .A.var".."

julia> import .A.:(..)

Only some of which are consistent with accessing a field of A

julia> A.:(..)
1

julia> A.var".."
1

# A dotcall instead :(
julia> A.(..)
ERROR: MethodError: objects of type Module are not callable

I'm quite tempted to say that the long game is "just use var for this" and to really deprecate parentheses in these cases. Because var syntax is a completely general way to construct identifiers regardless of whether they're keywords or other special syntax which can't be parenthesized.

@ericphanson
Copy link
Collaborator

ericphanson commented Feb 25, 2024

Is there a workaround that doesn't involve changing the source code? I again want to parse Makie (this time for ExplicitImports.jl) and cannot :(.

Also, why does the Base version work? E.g. I can load Makie on Julia v1.10. Is it that it falls back to the flisp parser?

edit: fyi SparseArrays also hits this

@ericphanson
Copy link
Collaborator

Is there a workaround that doesn't involve changing the source code?

There is! It turns out this is a "warning" (despite only using the word "error" in the message), so ignore_warnings=true works:

julia> JuliaSyntax.parseall(JuliaSyntax.GreenNode, "import Makie: (..)")
ERROR: ParseError:
# Warning @ line 1:15
import Makie: (..)
#             └──┘ ── parentheses are not required here
Stacktrace:
 [1] _parse(rule::Symbol, need_eof::Bool, ::Type{…}, text::String, index::Int64; version::VersionNumber, ignore_trivia::Bool, filename::Nothing, first_line::Int64, ignore_errors::Bool, ignore_warnings::Bool, kws::@Kwargs{})
   @ JuliaSyntax ~/.julia/packages/JuliaSyntax/eqCSU/src/parser_api.jl:93
 [2] _parse (repeats 2 times)
   @ ~/.julia/packages/JuliaSyntax/eqCSU/src/parser_api.jl:77 [inlined]
 [3] parseall(::Type{JuliaSyntax.GreenNode}, text::String)
   @ JuliaSyntax ~/.julia/packages/JuliaSyntax/eqCSU/src/parser_api.jl:143
 [4] top-level scope
   @ REPL[18]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> JuliaSyntax.parseall(JuliaSyntax.GreenNode, "import Makie: (..)", ignore_warnings=true)
     1:18     │[toplevel]
     1:18     │  [import]
     1:18     │    [:]
     1:6import
     7:12     │      [importpath]
     7:7      │        Whitespace
     8:12     │        Identifier       ✔
    13:13:
    14:18     │      [importpath]
    14:14     │        Whitespace
    15:18     │        [parens]
    15:15     │          (
    16:17..18:18     │          )

@LilithHafner
Copy link
Member Author

why does the Base version work?

Base uses ignore_warnings=true

@ericphanson
Copy link
Collaborator

It turns out this is a "warning" (despite only using the word "error" in the message)

My apologies, this is wrong, it does say warning! It is just very light and I missed it.

@c42f
Copy link
Member

c42f commented Jul 26, 2024

Is .. the only thing where parens are really required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants