Ets fun2ms without ms transform

[bormilan]

Description

It detects if a module that does not include ms_transform.hrl has any ets:fun2ms function call.

Closes #313;

• I have read and understood the contributing guidelines

[elbrujohalcon]

For performance (and semantics) sake, I would do the check in the opposite direction. In other words, instead of first looking for all files that don't include ms_transform and checking each one to see if they use ets:fun2ms… I would look for all files that use ets:fun2ms (which are surely far less than the ones that don't include ms_transform) and then check if they include the hrl or not.

You should also add a test with a file that has the -include but not the usage of fun2ms, for completeness sake. Like…

-module(pass_ms_transform_included_too).

-include_lib("stdlib/include/ms_transform.hrl").

-export([test/0]).

test() -> ok.

And maybe another one like…

-module(pass_ms_transform_included_as_well).

-export([test/0]).

test() -> {this, is, "not", ets, my:fun2ms("It's my own")}.

[bormilan]

Oh yes, that is a way better approach. Thanks!

[bormilan]

I made some changes based on your suggestions. However, I believe this rule could be more generic. Currently, it only checks for a specific scenario, but we can expand it.

We could create a rule that detects any function call that requires an include but does not have it.
We can add this scenario to the default configuration, and users can also add their own pairs of {function_call, lib_file_name} if they wish.

[elbrujohalcon]

@bormilan We can do that in another ticket. Please write a ticket for that.
For now, I think we're fine. There are very few (if any) other cases of functions that require a header file. The only one I can think of is lager:debug(…) and other functions like that. But that's a third party library that's being obsoleted by OTP's logger, anyway. And it's not exactly an hrl what you need… it's a parse_transform. So, it's not 100% the same.

[elbrujohalcon]

A few tiny improvements in code and docs… and do consider adding the tests I requested in my previous review, @bormilan.
Thanks.

[bormilan]

Okay, it sounds

A few tiny improvements in code and docs… and do consider adding the tests I requested in my previous review, @bormilan. Thanks.

Yes, I will add that test, I didn't forget.

[bormilan]

@bormilan We can do that in another ticket. Please write a ticket for that. For now, I think we're fine. There are very few (if any) other cases of functions that require a header file. The only one I can think of is lager:debug(…) and other functions like that. But that's a third party library that's being obsoleted by OTP's logger, anyway. And it's not exactly an hrl what you need… it's a parse_transform. So, it's not 100% the same.

Okay, It sounds good!

[elbrujohalcon]

A little bit of nit-picking, but overall the code is great.
If you feel like ignoring the nit-picky comments, @bormilan … just let me know and I'll merge the PR as-is.

[bormilan]

If you feel like ignoring the nit-picky comments, @bormilan … just let me know and I'll merge the PR as-is.

No, I'm grateful for your time, and I want to bring home as much as I can from these, so please be as nit-picky as you want, I really appreciate it! 🙌

Some say, "God is in the details."
I want to appreciate this project with the best work I can do for the opportunities it provides me on my journey.

[elbrujohalcon]

I don't think this file belongs to this PR.

[paulo-ferraz-oliveira]

@elbrujohalcon, I wonder if, in the future, to ease maintenance, this list should be build like a sub-set of _strict, for example...

[elbrujohalcon]

That's a good idea, yeah.