Closed
Bug 1294621
Opened 8 years ago
Closed 8 years ago
Enable the no-lonely-if rule for eslint
Categories
(Toolkit :: General, defect)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: jaws, Assigned: jaws)
Details
Attachments
(1 file)
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 hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
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
Attachment #8780380 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6d1a4462e4898a7c270a98bdb01b3a7893a2fd06
Bug 1294621 - Enable the no-lonely-if rule for eslint. r=gijs
Comment 4•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•