add forbidden regex

[bormilan]

Description

I added the `forbidden_regex' to all "naming_convention" rules.
I made tests for a simple case about making numbers forbidden.

I can't decide in these situations that making a "case in case" is a good implementation. I feel that moving these into another function is not much cleaner, and maybe reading is more frustrating if you need to look into another function just for another case. What do you think about it?

Closes #151.

• I have read and understood the contributing guidelines

[bormilan]

But what's the problem with andalso? or why do we want to avoid it?

[elbrujohalcon]

But what's the problem with andalso? or why do we want to avoid it?

The implicit specification for andalso (giving its desired semantics) is boolean() andalso boolean() -> boolean(), but the actual way in which andalso works is (as you noticed in your code) boolean() andalso Type -> Type. In other words, while it's semantically expected for the second expression to be boolean… it works anyway if it's not because that's how the VM optimizes this code.

Taking advantage of an optimization to mess up with the expected semantics of an expression is not a great thing to do. First of all, nothing prevents the OTP team from changing that behaviour in the future and crash if the second part is not a boolean (that would be semantically correct, anyway). But more importantly… for developers reading that code… finding something that's not a boolean there is unexpected and surprising future devs is not usually a good thing in terms of maintainability.

[elbrujohalcon]

I'll leave the PR open for another day so @paulo-ferraz-oliveira can review… and if he doesn't request changes by then, I'll merge it.

[paulo-ferraz-oliveira]

All of these should refer "name" instead of format, because function format is odd to read. I'll probably not be able to comment on the other warnings because they're outside the scope of the pull request, but you could change them all to e.g. "Function ~p's name does not...", "Variable ~p's name, on line ~p, does not...", etc.

[paulo-ferraz-oliveira]

Generally approving, but did comment on a few things that could be improved.

[bormilan]

But what's the problem with andalso? or why do we want to avoid it?

The implicit specification for andalso (giving its desired semantics) is boolean() andalso boolean() -> boolean(), but the actual way in which andalso works is (as you noticed in your code) boolean() andalso Type -> Type. In other words, while it's semantically expected for the second expression to be boolean… it works anyway if it's not because that's how the VM optimizes this code.

Taking advantage of an optimization to mess up with the expected semantics of an expression is not a great thing to do. First of all, nothing prevents the OTP team from changing that behaviour in the future and crash if the second part is not a boolean (that would be semantically correct, anyway). But more importantly… for developers reading that code… finding something that's not a boolean there is unexpected and surprising future devs is not usually a good thing in terms of maintainability.

Thank you for this explanation. I truly appreciate these "small"(but important) things that I can't learn anywhere else. Thanks again for the effort you put into these reviews!

[paulo-ferraz-oliveira]

@bormilan, it's also one of the reasons behind https://github.com/inaka/elvis_core/blob/main/doc_rules/elvis_style/no_block_expressions.md. I was seeing (and writing, since Erlang didn't forbid me) code like:

Connected andalso begin
  NumberOfEvents = num_events(...)
  NumberOfEvents > 10 orelse log(...)
end

While that works, it's generally messy to maintain.

[bormilan]

@bormilan, it's also one of the reasons behind https://github.com/inaka/elvis_core/blob/main/doc_rules/elvis_style/no_block_expressions.md. I was seeing (and writing, since Erlang didn't forbid me) code like:

Connected andalso begin
  NumberOfEvents = num_events(...)
  NumberOfEvents > 10 orelse log(...)
end

While that works, it's generally messy to maintain.

Thank you for the hint and all the reviews, too!

[bormilan]

If it's okay, I will rebase it to prepare for the merge.