1294621 - Enable the no-lonely-if rule for eslint

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

no-lonely-if: disallow if statements as the only statement in else blocks

If an if statement is the only statement in the else block, it is often clearer to use an else if form.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1294621 - Enable the no-lonely-if rule for eslint.

Review commit: https://reviewboard.mozilla.org/r/71110/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71110/

Comment 2

[:Gijs (he/him)]

Comment on attachment 8780380 [details]
Bug 1294621 - Enable the no-lonely-if rule for eslint.

https://reviewboard.mozilla.org/r/71110/#review68694

::: browser/extensions/pocket/content/panels/js/saved.js:270
(Diff revision 1)
>                      e.preventDefault();
>                      e.stopImmediatePropagation();
>                      inputwrapper.find('.pkt_ext_tag_input').tokenInput('remove', {name:selected.find('p').text()});
>                  }
>              }
> -            else {
> +            else if ($(e.target).parent().hasClass('token-input-input-token')) {

Note that the pocket stuff is in an external repo as well... dunno if you have access, if not, prod mkaply on IRC.

::: toolkit/mozapps/extensions/content/extensions.js:688
(Diff revision 1)
> -      } else {
> +      } else if (gHistory.canGoForward)
> -        if (gHistory.canGoForward)
> -          gHistory.forward();
> +        gHistory.forward();
> -        else
> +      else
> -          gViewController.replaceView(gViewDefault);
> +        gViewController.replaceView(gViewDefault);
> -      }
> +    }

Please add braces around the else if / else clauses here.

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:6112
(Diff revision 1)
> -      else {
> +      else if (aRequest instanceof Ci.nsIHttpChannel)
> -        if (aRequest instanceof Ci.nsIHttpChannel)
> -          this.downloadFailed(AddonManager.ERROR_NETWORK_FAILURE,
> +        this.downloadFailed(AddonManager.ERROR_NETWORK_FAILURE,
> -                              aRequest.responseStatus + " " +
> +                            aRequest.responseStatus + " " +
> -                              aRequest.responseStatusText);
> +                            aRequest.responseStatusText);
> -        else
> +      else
> -          this.downloadFailed(AddonManager.ERROR_NETWORK_FAILURE, aStatus);
> +        this.downloadFailed(AddonManager.ERROR_NETWORK_FAILURE, aStatus);

The if is braced, so the else if and else should be braced too. In fact, this looks strange...:

```
}
else if ()
  ...
else
  ...
} // <--- what is this closing? The outer if that's not in the patch context? Extra braces would make this more readable.
else {
```

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:7512
(Diff revision 1)
> -    else {
> +    else if (!addon.userDisabled)
>        // Only set softDisabled if not already disabled
> -      if (!addon.userDisabled)
> -        addon.softDisabled = val;
> +      addon.softDisabled = val;
> -    }

Also wants moar braces.

::: toolkit/mozapps/extensions/test/xpcshell/test_bug299716.js:108
(Diff revision 1)
> -  } else {
> +  } else if (aItem != null)
> -    if (aItem != null)
> -      do_throw("Addon " + aAddonsEntry.id + " was detected");
> +    do_throw("Addon " + aAddonsEntry.id + " was detected");

Some more braces here.

::: toolkit/mozapps/extensions/test/xpcshell/test_system_update.js:420
(Diff revision 1)
> -  else {
> +  else if (finalState[0])
>      // If the new state is using the profile then that directory will exist.
> -    if (finalState[0])
> -      expectedDirs++;
> +    expectedDirs++;

And here

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/6d1a4462e4898a7c270a98bdb01b3a7893a2fd06
Bug 1294621 - Enable the no-lonely-if rule for eslint. r=gijs

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6d1a4462e489