Add public keyword

[LilithHafner]

This PR is a part of JuliaLang/julia#50105.

[codecov]

Codecov Report

Merging #320 (a60facf) into main (747702b) will decrease coverage by 0.01%.
Report is 6 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
- Coverage   96.56%   96.56%   -0.01%     
==========================================
  Files          14       14              
  Lines        4138     4158      +20     
==========================================
+ Hits         3996     4015      +19     
- Misses        142      143       +1     

Files Changed	Coverage Δ
src/kinds.jl	80.59% <ø> (ø)
src/tokenize.jl	99.07% <ø> (ø)
src/parser.jl	98.03% <100.00%> (+<0.01%)	⬆️

... and 1 file with indirect coverage changes

[KristofferC]

Is this just to test stuff out? It seems from the discussion in JuliaLang/julia#50105 that the whole concept of a "scoped export" was not very liked, or?

[LilithHafner]

Yse this is just to test things out, and also I don't think we ever really got to discussing naming particularly thoroughly—most discussion was and still is focused on objections about supporting the notion of public symbols at all.

[c42f]

Very cool!

I know this is just a prototype but I added a couple of comments.

A general comment - one tricky thing about using peek is that it doesn't always do what you expect - it can be overly restrictive. For example, one would normally expect var"x" to work in Julia syntax anywhere that x works, and this is handled by parse_atom. However, with the peeks you've got here, you can't write

export var"scoped"=true A, B

Of course that's no loss for this particular syntax! But just a general note of caution that peeking into the coming tokens should be viewed with skepticism, especially if it's more than a couple of tokens ahead :-) It can be a sign that a parsing function may be doing work that should be done deeper into the tree of calls.

[c42f]

I don't think this is working as intended.

Did you intend to restrict parsing of this to top level and to module top level? In that case, you can't add the logic to parse_resword because that's called quite deep in the recursive descent. Instead, you'll need to add it near the top level. Also there's something super weird and seemingly broken with the AST which comes out of public at module scope:

julia> parsestmt(SyntaxNode, "public A, B")
line:col│ tree                                   │ file_name
   1:1  │[public]
   1:8  │  A
   1:11 │  B


julia> parsestmt(SyntaxNode, "begin \n public A, B \n end")
line:col│ tree                                   │ file_name
   1:1  │[block]
   2:2  │  [public]
   2:9  │    A
   2:12 │    B


julia> parsestmt(SyntaxNode, "module \n public A, B \n end")
line:col│ tree                                   │ file_name
   1:1  │[module]
   2:2  │  public
   2:8  │  [block]
   2:9  │    [tuple]
   2:9  │      A
   2:12 │      B

[c42f]

This is probably the source of your woes at JuliaLang/julia#50105 (comment)

[LilithHafner]

Thank you for looking at this, I definitely appreciate/need help figuring out those woes. However, I don't think this is the problem.

Deep in the callstack, parse_resword is gated by peek_initial_reserved_words and I didn't add "public" to the list of matched keywords, so parse_resword should not process public deep in the stack. Instead, it is called for "public" by parse_public, after parse_docstring and before parse_eq (though I may have misplaced it in the callstack).

The examples you showed all seem good to me. The module example is weird because public is the module name.

Here are some more examples that IMO have good behavior with respect to when public is treated as a keyword and when it is not:

julia> parsestmt(SyntaxNode, "function f(public)\n    public + 3\nend")
line:col│ tree                                   │ file_name
   1:1  │[function]
   1:10 │  [call]
   1:10 │    f
   1:12 │    public
   1:19 │  [block]
   2:5  │    [call-i]
   2:5  │      public
   2:12 │      +
   2:14 │      3


julia> parsestmt(SyntaxNode, "public A, B")
line:col│ tree                                   │ file_name
   1:1  │[public]
   1:8  │  A
   1:11 │  B


julia> parsestmt(SyntaxNode, "begin \n public A, B \n end")
line:col│ tree                                   │ file_name
   1:1  │[block]
   2:2  │  [public]
   2:9  │    A
   2:12 │    B


julia> parsestmt(SyntaxNode, "if true \n public A, B \n end") # throws
ERROR: ParseError:
# Error @ line 2:9
if true 
#       ┌────
 public A, B 
 end
┘ ── Expected `end`
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:87
 [2] _parse(rule::Symbol, need_eof::Bool, ::Type{SyntaxNode}, text::String) (repeats 2 times)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:71 [inlined]
 [3] parsestmt(::Type{SyntaxNode}, text::String; kws::@Kwargs{})
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:134 [inlined]
 [4] parsestmt(::Type{SyntaxNode}, text::String)
   @ JuliaSyntax ~/.julia/dev/JuliaSyntax/src/parser_api.jl:134
 [5] top-level scope
   @ REPL[77]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> parsestmt(SyntaxNode, "if true \n public *= 4 \n end")
line:col│ tree                                   │ file_name
   1:1  │[if]
   1:4  │  true
   1:8  │  [block]
   2:2  │    [*=]
   2:2  │      public
   2:12 │      4


julia> parsestmt(SyntaxNode, "module Mod\n public A, B \n end")
line:col│ tree                                   │ file_name
   1:1  │[module]
   1:8  │  Mod
   1:11 │  [block]
   2:2  │    [public]
   2:9  │      A
   2:12 │      B

[c42f]

The module example is weird because public is the module name

Of course. Yikes that's kind of obvious 😬

Did you consider adding public to the list of contextual keywords? If we have it with the other contextual keywords some things which depend on the is_contextual_keyword() predicate might automatically work better. For example, we currently have

julia> parsestmt(SyntaxNode, "struct abstract end")
line:col│ tree                                   │ file_name
   1:1  │[struct]
   1:8  │  abstract
   1:16 │  [block]

but

julia> parsestmt(SyntaxNode, "struct public end")
ERROR: ParseError:
# Error @ line 1:7
struct public end
#     └─────┘ ── Invalid type name `public`

[c42f]

Is the following intended?

julia> parsestmt(SyntaxNode, """
           function f()
               begin
                   public A,B
               end
           end
       """)
line:col│ tree                                   │ file_name
   1:5  │[function]
   1:14 │  [call]
   1:14 │    f
   1:17 │  [block]
   2:9  │    [block]
   3:13 │      [public]
   3:20 │        A
   3:22 │        B

[LilithHafner]

Is the following intended?

Nope :)

[c42f]

Nope :)

Hehe. In that case I think we've got two options.

• Disable parsing of public within begin ... end. This is simple but could be inconvenient in toplevel code when one is trying to do something like a conditional @eval begin #= some module toplevel stuff =# end.
• Add another state variable to ParseState. Maybe call it toplevel? Branch on that plus K"public" when deciding to enter parse_resword? Might be a bit of work to find all the places where it's necessary to set ps = ParseState(ps, toplevel=false).

[LilithHafner]

Add another state variable to ParseState. Maybe call it toplevel

That feels right.

[LilithHafner]

public is a keyword iff it is at the top-level of a file or module and it is followed by an identifier. This allows

public(::String) = true

to continue to parse the old way.

[c42f]

I think this is good to go!