inaka / elvis_core

The core of an Erlang linter
Other
61 stars 56 forks source link

Using ets:fun2ms without including ms_transform.hrl should be an error #313

Closed rlipscombe closed 4 days ago

rlipscombe commented 1 year ago

Using ets:fun2ms without including ms_transform.hrl raises a runtime error, rather than a compile error:

The parse transform is provided in the ms_transform module and the source must include file ms_transform.hrl in STDLIB for this pseudo function to work. Failing to include the hrl file in the source results in a runtime error, not a compile time error. The include file is easiest included by adding line -include_lib("stdlib/include/ms_transform.hrl"). to the source file.

Describe the solution you'd like

If elvis could spot this, and raise an error, that'd save some headaches. I understand that it probably can't be exhaustive, but it'd be nice.

paulo-ferraz-oliveira commented 1 year ago

šŸ‘

Should it pertain only to cases where the function is called explicitly?

I'm thinking you can do

M = ets,
M:fun2ms(_)

in which case it would be harder to detect. Or even if using apply, for example...

elbrujohalcon commented 1 year ago

We can add a flag, likeā€¦ method :: aggressive | naive.

With naive (the default), Elvis will only emit a warning if it finds ets:fun2ms and no include for ms_transform.hrl. With aggressive, Elvis will emit a warning if it finds the atom fun2ms somewhere in the code and no include for ms_transform.hrl.

rlipscombe commented 1 year ago

The parse transform only works on explicit calls to ets:fun2ms and dbg:fun2ms, based on a (very) brief skim of the code, so I don't see the need for complicating it beyond that.

See https://github.com/erlang/otp/blob/OTP-25.2/lib/stdlib/src/ms_transform.erl#L394-L399