Boolean shorthand predicate should return `false` when not matching

[arnodirlam]

Currently, when using the boolean shorthand predicate, then the condition doesn't match, nil is returned.

Instead, false should be returned.

Example

defmodule Todo.List do
  infer :archived?, when: %{archived_at: {:not, nil}}
end

iex> %Todo.List{archived_at: nil}
...> |> Infer.get!(:archived?)

# current result
nil

# new result
false

Migration

This change is backward-incompatible.

To migrate, users must go through all conditions matching boolean shorthand predicates with nil, and replace that with false.

[Zurga]

I think I managed to do this. The process is as follows:

1. Filter out rules that have a boolean value,
2. Group these rules by their key,
3. Select only rules that have only one possible value (true or false),
4. Create a rule with the inverse value and insert it after the last rule with the same key.

Example of the result:

rules = [
  %Dx.Rule{key: :test, value: false, when: %{something: true}},
  %Dx.Rule{key: :test, value: false, when: %{something_else: true}},
  #Rules below should not be affected
  %Dx.Rule{key: :test2, value: false, when: %{something: true}},
  %Dx.Rule{key: :test2, value: true, when: %{something_else: true}},
]
# After injecting the fallback
[
  %Dx.Rule{key: :test, value: false, when: %{something: true}},
  %Dx.Rule{key: :test, value: false, when: %{something_else: true}},
  %Dx.Rule{key: :test, value: true, when: %{}},

  %Dx.Rule{key: :test2, value: false, when: %{something: true}},
  %Dx.Rule{key: :test2, value: true, when: %{something_else: true}},
]

[arnodirlam]

Yes, that's a great approach, @Zurga, thanks a lot for looking into this.

It goes in the right direction, but I suggest making it more strict:

1. Only boolean shorthands should imply the other boolean rule. As soon as you write out an explicit result, it should not be modified. Example:

   infer archived?: true, when: %{archived_at: {:not, nil}}

2. Multiple boolean shorthands for the same predicate should be disallowed, I think. When there are multiple conditions, on which it can match, one can use a list (logical OR) to combine them. For example:

   # allowed
   infer :archived?,
     when: [
       %{archived_at: {:not, nil}},
       %{created_at: {:on_or_before, ~D[2022-01-01]}}
     ]
   
   # not allowed
   infer :archived?, when: %{archived_at: {:not, nil}}
   infer :archived?, when: %{created_at: {:on_or_before, ~D[2022-01-01]}}

What do you think?

[Zurga]

Thanks, I really like the idea of this project!

On point 1: I am a lazy, forgetful person who does not want to write fallbacks :p, so I figured it would be good to generate the inverse even if the value is set. The DSL would be 'helpful'. It does also touch on a fundamental thing in my opinion: can an inference have different types as values? I find it logical that if a boolean is used, the value can then only be boolean and thus an inverse can safely be applied if only one possible value is given.

On point two:
I would allow both to be honest, unless it becomes a perfomance issue, but even then the parser could provide optimizations. What would be the reason to enforce a strict style?

Thanks for the swift response!

[arnodirlam]

Thanks, I really like the idea of this project!

Thank you, glad to hear that. How did you hear of it initally, if I may ask?

On point 1: I am a lazy, forgetful person who does not want to write fallbacks :p, so I figured it would be good to generate the inverse even if the value is set. The DSL would be 'helpful'.

That's a fair point. It saves time while writing it. However, as a "lazy, forgetful person" you would probably save much more time and energy in total, because you only write these rules once (or whenever they change) but read them a dozen times after. This is where being explicit saves time IMO, because you don't have to think as much when you clearly see what the fallback is. That's one aspect why Elixir generally favours "explicit over implicit".

can an inference have different types as values?

Yes, this is currently the case, and it's by design. Dx should be flexible, so you can use it to return knowledge from your schema+logic and then do things with this knowledge. My idea was to not limit what can be returned, especially not when Dx is not widely used (yet) so that any usage patterns or best practices have emerged. It's more of an experimentation ground for now. It works, but how exactly it can and should be utilized is still open.

These are my main reasons for favouring a stricter approach when using explicit result values (and no boolean shorthand).

On point two:
I would allow both to be honest, unless it becomes a perfomance issue, but even then the parser could provide optimizations. What would be the reason to enforce a strict style?

There's no performance issue. It's just my idea of how the boolean shorthand feature would be used. It's also easier to implement, and loosening a strict version - if done in the future - is backward-compatible, whereas making things stricter in the future would not be.

That being said, I'm fine with allowing multiple boolean shorthands for the same predicate, if you think it makes sense 👍

If you'd like to give it a shot, most code that needs to be changed (apart from docs) should be in lib/dx/parser.ex. My initial approach would be to add a boolean_shorthand? field (or similar) to the Dx.Rule struct and take it from there.