Re: Feature request: a new warning option

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Wed, Oct 9, 2019 at 6:03 AM Sebastian Gniazdowski
<sgniazdowski@xxxxxxxxx> wrote:
>
> Hello,
> how about detecting situations in the code like the following:
>
> fun() { some code possibly returning false }
> (( condition )) && fun || print "Some 'else'-instruction"

I've made a quick-and-dirty search over powerlevel10k source code to
see how often this warning would have false positives. Here are a few
examples of `x && y || z` constructs where `y` can fail and yet the
code is correct.

  (( $+GITSTATUS_DAEMON_PID_POWERLEVEL9K )) &&
     gitstatus_start POWERLEVEL9K ||
     _p9k_gitstatus_disabled=1

  mkdir -p ${dst:h} && cp -f $src $dst || return

  _p9k_cached_cmd_stdout node -v && [[ $_p9k_ret == v?* ]] || return

`x && y || z` is not `x ? y : z` even though it's often abused as
such. I think the solution is to use `x && y || z` when you mean it,
which is to say *only* when both `x` and `y` may be falsy. The ternary
can be expressed with an `if`.

Compare:

  (( condition )) && fun || print "Some 'else'-instruction"

vs

  if (( condition )) { fun } else { print "Some 'else'-instruction" }

The latter is longer but is at least as clear. It also comes with
curlies that make it easy to split the statement into multiple lines
if necessary, or to add extra statements inside the branches.

I myself am guilty of using `x && y || z` in place of a ternary
and this leads to bugs that your proposed warning is meant to flag.
However, if I were to enable this warning, I wouldn't be able to
use `x && y || z` when it is the best tool for the job. Hence I'm
leaning towards unlearning my current habbit and starting to use
`if-else` more.

What do you think?

Roman.