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

Parsing error #1290

Closed
robertoaloi opened this issue May 5, 2022 · 3 comments · Fixed by #1382
Closed

Parsing error #1290

robertoaloi opened this issue May 5, 2022 · 3 comments · Fixed by #1382

Comments

@robertoaloi
Copy link
Member

Parsing the following module:

-module(my_module).
-export([main/1]).
-record(my_record, {}).
main(Record) -> Record#my_record{
    %% TODO
}.

Triggers an error:

[warning] Please report error parsing form error:{case_clause,comment}:[{els_parser,record_field_name,3,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,698}]},{els_parser,'-record_expr_pois/3-lc$^0/1-0-',2,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,670}]},{els_parser,record_expr_pois,3,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,671}]},{els_parser,do_points_of_interest,1,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,198}]},{els_parser,'-points_of_interest/1-fun-0-',2,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,184}]},{els_parser,fold2,3,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,880}]},{els_parser,fold1,3,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,873}]},{els_parser,fold,3,[{file,".../erlang_ls-0.31.0/apps/els_lsp/src/els_parser.erl"},{line,867}]}], {function,#{end_location => {6,3},location => {4,1}},[{clause,#{end_location => {6,2},location => {4,1}},{call,#{end_location => {4,13},location => {4,1}},{atom,#{end_location => {4,5},location => {4,1},text => "main"},main},[{var,#{end_location => {4,12},location => {4,6},text => "Record"},'Record'}]},empty,[{record,#{end_location => {6,2},location => {4,17}},{var,#{end_location => {4,23},location => {4,17},text => "Record"},'Record'},{atom,#{end_location => {4,33},location => {4,24},text => "my_record"},my_record},[{comment,#{end_location => {5,12},location => {5,5}},["%% TODO"]}]}]}]} [els_parser:parse_forms/1 L78] <0.10014.1>

FYI @gomoripeti

@gomoripeti
Copy link
Contributor

thanks Roberto
I don't think els_parser handles standalone comments anywhere (https://github.com/WhatsApp/erlfmt/blob/main/src/erlfmt_recomment.erl#L98) at least I did not expect them :)

as an alternative of fixing els_parser:
@michalmuskala would it be possible to allow erlfmt:read_node_string/3 (with an additional option) to ignore comments and skip recommenting? Apart from making els_parser's life easier it could also spare some cpu cycles for erlang_ls (this is not useful for erlfmt itself ofc) Would comments be useful for erlang_ls in the future? (I dont think it uses them right now)

@robertoaloi
Copy link
Member Author

If possible, I would prefer fixing els_parser on our side. We don't use comments yet, but they could turn out useful in the future (e.g. if we deal with refactoring).

@gomoripeti
Copy link
Contributor

makes sense. Will need to test the parser on more "final comments in containers", there might be other problematic places then records.
(the record part is not a big change, but I cannot commit when I will have time to address this unfortunately)

(there are two erlang refactoring tools > erlang langauge server for unification > there are three erlang refactoring tools :P)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants