Skip to content

Improve error_info() type#324

Closed
gomoripeti wants to merge 1 commit into
WhatsApp:mainfrom
gomoripeti:error_info_type
Closed

Improve error_info() type#324
gomoripeti wants to merge 1 commit into
WhatsApp:mainfrom
gomoripeti:error_info_type

Conversation

@gomoripeti

Copy link
Copy Markdown
Contributor

erlfmt_scan returns erl_anno:location() in errors however erlfmt_parse mostly returns full erlfmt_scan:anno() maps. (I'm not sure if the parser should/can be changed to also return location() as the code generated by yecc just takes the second element of the token tuple, without any transformation, if it is not an erl_anno:anno() https://github.com/erlang/otp/blob/maint/lib/parsetools/include/yeccpre.hrl#L147) The errors from the parser also surface in the return value of erlfmt:read_nodes[_string]

I noticed this via dialyzer when calling erlfmt_parse directly.

`erlfmt_scan` returns `erl_anno:location()` in errors however
`erlfmt_parse` mostly returns full `erlfmt_scan:anno()` maps. The errors
from the parser also surface in the return value of
`erlfmt:read_nodes[_string]`
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 26, 2021
Comment thread src/erlfmt_parse.yrl
%% XXX. To be refined.
-type error_description() :: term().
-type error_info() :: {erl_anno:line(), module(), error_description()}.
-type error_info() :: {erl_anno:location() | erlfmt_scan:anno(), module(), error_description()}.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret_err/2 throws a proper erl_anno:location(), not sure if it can be returned by parse_node or if it always returns erlfmt_scan:anno()

@gomoripeti

Copy link
Copy Markdown
Contributor Author

Closing in favour of #325

@gomoripeti gomoripeti closed this Dec 4, 2021
@gomoripeti

Copy link
Copy Markdown
Contributor Author

without this fix I get dialyzer warnings when calling erlfmt_parse:parse_node/1 directly

case erlfmt_parse:parse_node(Tokens) of
  {error, {ErrorLoc, erlfmt_parse, _Reason}} ->
    case ErrorLoc of
      #{'location' := Loc} -> ...
      Loc = {Line, Col} -> ...
The pattern #{'location':=Loc} can never match the type non_neg_integer()
The pattern Loc = {Line, Col} can never match the type non_neg_integer()

@gomoripeti

Copy link
Copy Markdown
Contributor Author

(hm, I intended to reopen when adding a comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants