otp icon indicating copy to clipboard operation
otp copied to clipboard

Can we get a warning on obviously non-boolean part of a guard?

Open elbrujohalcon opened this issue 7 months ago • 2 comments

Is your feature request related to a problem? Please describe. It is kinda related to a real-life bug that I found in a system I'm working on. In a nutshell it goes more or less like this…

%% We get a warning for this one
a_function(#{clause := [with, Stuff]}) when Stuff == this or that ->
    "This string will never be the result of the function";
%% And this one
a_function(#{clause := [with, Stuff]}) when Stuff == this orelse that ->
    "This string will only be the result if Stuff == this";
%% But no warning for this one
a_function(#{clause := [with, Stuff]}) when Stuff == this; that ->
    "This string will only be the result if Stuff == this";
a_function(_) ->
    "The result for any other parameter".

We had our function written similarly to the third clause in the example (i.e., when Thing == something; other_something) and the code compiled without warnings. But, of course, our intention was to write when Thing == something; Thing == other_something.

Describe the solution you'd like I would like erl_lint to check ; and orelse just like it does with or, detecting clearly incorrect (i.e. non-boolean) values and emitting a warning about them. Of course, the same thing can be applied to , and andalso.

Describe alternatives you've considered The alternative that I considered, which is what I'll do if this ticket gets rejected, is to add a rule for this to Elvis. 🤘🏻

Additional context No additional context.

elbrujohalcon avatar May 29 '25 15:05 elbrujohalcon

For what is worth, there is actually a legit use case for this feature, which is to make something fail in the middle of a guard. For example, imagine that you wanted to implement something like is_function/2 in a parse transform (or a macro).

The semantics are a bit tricky because is_function/2 always returns a boolean regardless if the first argument is function or not but the guard always fails if the second argument is not an integer between 0..255:

4> is_function(123, a).
** exception error: bad argument
     in function  is_function/2
        called as is_function(123,a)
        *** argument 2: not an integer
5> is_function(123, 123).
false

The only way, as far as I know, to implement similar semantics in a guard written by hand would be by returning a non-boolean value somewhere, such as this:

-define(is_function(Fun, Arity),
  is_function(Fun) andalso
    ((is_integer(Arity) and (Arity >= 0) and (Arity =< 255)) orelse fail) andalso
    erlang:fun_info(Fun, arity) =:= Arity.

the above assumes fun_info/2 is valid in guards for demonstration purposes

In other words, if anything running on top of Erlang wants to introduce guard-like functionality, this feature would be important.


With all that said, I am actually in favor of this proposal. Elixir already warns if we detect guards to return anything other than true, false, or the atom fail which we reserved especially for the purposes above. Perhaps a explicit erlang:guard_fail/0 would be even better.

josevalim avatar Jun 02 '25 07:06 josevalim

Thanks for raising this issue! The warning for misuse of or/2 is an "opportunistic" one in compiler pass, not erl_lint, and we're rather hesitant to add more of those.

We'd be open to adding a linter warning for basic cases with ; and orelse however since that's purely syntactical, and we can (hopefully) keep Elixir happy by not raising a warning when their fail atom is marked as compiler-generated. Feel free to make a PR. :-)

jhogberg avatar Jun 09 '25 12:06 jhogberg