Formatter remove single quotes from macros containing dashes

[carlosedp]

Describe the bug

Formatter remove single quotes from macros containing dashes.

To Reproduce

The code below:

-define(CCR_INITIAL, ?'CC-REQUEST-TYPE_INITIAL_REQUEST').
-define(CCR_UPDATE, ?'CC-REQUEST-TYPE_UPDATE_REQUEST').
-define(CCR_TERMINATE, ?'CC-REQUEST-TYPE_TERMINATION_REQUEST').
-define(MSISDN, ?'SUBSCRIPTION-ID-TYPE_END_USER_E164').
-define(IMSI, ?'SUBSCRIPTION-ID-TYPE_END_USER_IMSI').

becomes:

-define(DCCA_APPLICATION_ID, 4).
-define(CCR_INITIAL, ?CC-REQUEST-TYPE_INITIAL_REQUEST).
-define(CCR_UPDATE, ?CC-REQUEST-TYPE_UPDATE_REQUEST).
-define(CCR_TERMINATE, ?CC-REQUEST-TYPE_TERMINATION_REQUEST).
-define(MSISDN, ?SUBSCRIPTION-ID-TYPE_END_USER_E164).
-define(IMSI, ?SUBSCRIPTION-ID-TYPE_END_USER_IMSI).

Removing the single quotes, making the code unable to compile.

Expected behavior

Do not remove quotes from macros that have dashes. This doesn't happen when using erlfmt external formatter.

Rebar3 Log Run rebar3 with DEBUG=1 and paste its output here.

Additional context

Below the format config in my rebar.config:

{format, [
    {files, ["apps/*/{src,include,test}/*.{erl,hrl,app.src}", "**/{rebar,sys}.config"]},
    % {formatter, erlfmt_formatter},
    {options, #{
        paper => 100,
        print_width => 100,
        ignore_pragma => true
    }}
]}.

[elbrujohalcon]

First of all, as usual with macros…

😱 OMG! Do people really do that thing? 🤦🏻

Now… Thanks, @carlosedp for the report.

I'll eventually check it out, but I have to tell you: This will not be fixed soon…

…unless you feel like sending a PR to fix it 😉

And also…

😱 OMG! Do people really do that thing? 🤦🏻

[carlosedp]

Hahaha, it's an internal macro that is compiled from Diameter files... I do not have control of its generated name.

I was looking at the code and laughed about the macro comments.. 😂 Any pointers I can look at to fix this?

[elbrujohalcon]

You should probably start by checking what ktn_dodger:parse_file("your_file.erl", […the same options that rebar3_format uses when calling it…]) returns in your case…

If I'm not mistaken… it's very very likely that ktn_dodger puts a variable named CC-REQUEST-TYPE_INITIAL_REQUEST in that place… If that's so, you can patch the default_formatter so that when it is about to write a variable, if the variable has strange characters (i.e. it's not really a variable), it's quoted.

[carlosedp]

I created a small test file as:

-module(test_format).

-include_lib("rfc4006_cc_Gy.hrl").

-define(WORK, abc).
-define(CCR_INITIAL, ?'CC-REQUEST-TYPE_INITIAL_REQUEST').
-define(CCR_UPDATE, ?'WORK').

parsing it gave me:

1> ktn_dodger:parse_file("apps/dccaserver/src/test_format.erl").
{ok,[{tree,attribute,
           {attr,1,[],none},
           {attribute,{tree,atom,{attr,1,[],none},module},
                      [{tree,atom,{attr,1,[],none},test_format}]}},
     {tree,attribute,
           {attr,3,[],none},
           {attribute,{atom,3,include_lib},
                      [{string,3,"rfc4006_cc_Gy.hrl"}]}},
     {tree,attribute,
           {attr,5,[],none},
           {attribute,{tree,atom,{attr,5,[],none},define},
                      [{var,5,'WORK'},{tree,text,{attr,5,[],none},"abc"}]}},
     {tree,attribute,
           {attr,6,[],none},
           {attribute,{tree,atom,{attr,6,[],none},define},
                      [{var,6,'CCR_INITIAL'},
                       {tree,text,
                             {attr,6,[],none},
                             "?CC - REQUEST - TYPE_INITIAL_REQUEST"}]}},
     {tree,attribute,
           {attr,7,[],none},
           {attribute,{tree,atom,{attr,7,[],none},define},
                      [{var,7,'CCR_UPDATE'},
                       {tree,text,{attr,7,[],none},"?WORK"}]}}]}

As seen, ktn_dodger separated the word in the dashes but it's quoted(the attr 6)... should I open the issue on katana_code so?

[elbrujohalcon]

Yeah… it parsed it as raw text. That means it doesn't know how to parse it. So, please do open a ticket there.

[shapovalovts]

Same issue with standard erlang module public_key. It contains many macros like ?'id-ce-subjectAltName' that are used when parsing certificates. Any chance to fix this in the the plugin?

[elbrujohalcon]

@shapovalovts To be honest, nobody is planning to work on this any time soon. The fix probably requires a bunch of non-trivial changes. All for not a huge gain, unless you are telling me that the OTP team is planning to start using this formatter for its code. In that case, I would drop everything and start working on the fix this very second.

On the other hand, pull requests are more than welcomed both here and in katana-code repositories ;)

[shapovalovts]

Thank you for the clarification. To make it clear: I have no idea about OTP team plans regarding their code re-formatting (though would be cool if they start using your plugin). The macros like the one I mentioned are used outside OTP code in different projects when certificates meta data is parsed.