This rule is available from https://www.npmjs.com/package/eslint-plugin-mozilla, and is enforced in mozilla-central. Note that we have a polyfill for `ChildNode.remove()` and that most cases have already been fixed, see PRs 8056 and 8138.
Since multiple empty lines is virtually unused in the code-base, and the few cases that do exist look like "typos", let's enforce greater consistency here; please see https://eslint.org/docs/rules/no-multiple-empty-lines.
https://eslint.org/docs/rules/no-var
Please note that two files were excluded:
1. `web/debugger.js`, since there's code in other files that currently depend on the global availability of code in `web/debugger.js`. Furthermore, since that file isn't used in production, doing a ES6 conversion probably isn't a priority.
2. `web/grab_to_pan.js`, since that file could be considered to be "external" code. We have made smaller changes to that file over the years, however doing a full ES6 `class` conversion might be a step too far!?
See https://eslint.org/docs/rules/for-direction; helps avoid typos that would cause infinite `for` loops.
Also, updates `eslint` and `eslint-plugin-mozilla` to the latest available versions.
http://eslint.org/docs/rules/comma-danglehttp://eslint.org/docs/rules/object-curly-spacing
*Please note:* This patch was created automatically, using the ESLint `--fix` command line option. In a couple of places this caused lines to become too long, and I've fixed those manually; please refer to the interdiff below for the only hand-edits in this patch.
```diff
diff --git a/gulpfile.js b/gulpfile.js
index d18b9c58..7d47fd8d 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -1247,7 +1247,8 @@ gulp.task('gh-pages-git', ['gh-pages-prepare', 'wintersmith'], function () {
var reason = process.env['PDFJS_UPDATE_REASON'];
safeSpawnSync('git', ['init'], { cwd: GH_PAGES_DIR, });
- safeSpawnSync('git', ['remote', 'add', 'origin', REPO], { cwd: GH_PAGES_DIR, });
+ safeSpawnSync('git', ['remote', 'add', 'origin', REPO],
+ { cwd: GH_PAGES_DIR, });
safeSpawnSync('git', ['add', '-A'], { cwd: GH_PAGES_DIR, });
safeSpawnSync('git', [
'commit', '-am', 'gh-pages site created via gulpfile.js script',
```
Please see http://eslint.org/docs/rules/object-shorthand.
Unfortunately, based on commit 9276d1dcd9, it seems that we still need to maintain compatibility with old Node.js versions, hence certain files/directories that are executed in Node.js are currently exempt from this rule.
Furthermore, since the files specific to the Chromium extension are not run through Babel, the `/extensions/chromium/` directory is also exempt from this rule.
In various viewer files, there's a number of cases where we basically duplicate the functionality of `createPromiseCapability` manually.
As far as I can tell, a couple of these cases have existed for a very long time, and notable even before the `createPromiseCapability` utility function existed.
Also, since we can write ES6 code now, the patch also replaces a couple of `bind` usages with arrow functions in code that's touched in the patch.
See http://eslint.org/docs/rules/#ecmascript-6.
To try and enforce consistent rules and to help avoid some possible errors in ES6 code from the start, this patch adds a few basic ESLint rules.
Note that a two of the rules, `no-shadow` and `object-shorthand`, are currently disabled. While it'd certainly be nice to enable both of them, it's currently impossible since that would result in close to one thousand lint errors.
I happened to notice that some inequalities had the wrong order, and was surprised since I thought that the `yoda` rule should have caught that.
However, reading http://eslint.org/docs/rules/yoda#options a bit more closely than previously, it's quite obvious that the `onlyEquality` option does *exactly* what its name suggests. Hence I think that it makes sense to adjust the options such that only ranges are allowed instead.
This patch gets rid of the only case in the code-base where we're throwing a plain `string`, rather than an `Error`, which besides better/more consistent error handling also allows us to enable the [`no-throw-literal`](http://eslint.org/docs/rules/no-throw-literal) ESLint rule.
See http://eslint.org/docs/rules/brace-style.
Having the opening/closing braces on the same line can often make the code slightly more difficult to read, in particular for `if`/`else if` statements, compared to using new lines.
This patch also, for consistency with `mozilla-central`, enables the [`no-iterator`](http://eslint.org/docs/rules/no-iterator) rule. Note that this rule didn't require a single code change.
Please see http://eslint.org/docs/rules/no-unused-vars; note that this patch purposely uses the same rule options as in `mozilla-central`, such that it fixes part of issue 7957.
It wasn't, in my opinion, entirely straightforward to enable this rule compared to the already existing rules. In many cases a `var descriptiveName = ...` format was used (more or less) to document the code, and I choose to place the old variable name in a trailing comment to not lose that information.
I welcome feedback on these changes, since it wasn't always entirely easy to know what changes made the most sense in every situation.
*Please note: ignoring whitespace changes is most likely necessary for the diff to be readable.*
This patch addresses all the current, in `mozilla-central`, linting failures in the addon. It should thus be possible to change the `.eslintignore` entry for PDF.js in `mozilla-central` from `browser/extensions/pdfjs/**` to `browser/extensions/pdfjs/build/**` and `browser/extensions/pdfjs/web/**` instead.
Note that we cannot, for backwards compatibility reason of the general PDF.js library, at this time make similar changes for files residing in the `build` and `web` directories in `mozilla-central`.
The main changes in this patch are that we now use [classes](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes) instead of our previous "class-like" functions, and also use the more compact [object shorthand notation](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/Object_initializer#New_notations_in_ECMAScript_2015).
A couple of functions were also converted to [arrow functions](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions), to reduced usages of `bind(this)` and `var self = this`.
One caveat with ES6 classes is that it's not (yet) possible to define private constants/helper functions within them, which is why the `NetworkManagerClosure` was kept to not change the visibility of those constant/functions.
Besides testing in Firefox Nightly 53, this patch has also been tested in Firefox ESR 45 and SeaMonkey 2.46.
However, I'd gladly welcome help with testing the patch more, to ensure that nothing has gone wrong during the refactoring.
Fixes the first bullet point of issue 7957.
http://eslint.org/docs/rules/no-lone-blocks
Note that we currently have no code that violates this rule in the source files, but it seems that the built files are possibly affected (see issue 7957).
Please see http://eslint.org/docs/rules/spaced-comment.
Note that the exceptions added for `line` comments are intended to still allow use of the old preprocessor without linting errors.
Also, I took the opportunity to improve the grammar slightly (w.r.t. capitalization and punctuation) for comments touched in the patch.
Using `else` after `return` is not necessary, and can often lead to unnecessarily cluttered code. By using the `no-else-return` rule in ESLint we can avoid this pattern, see http://eslint.org/docs/rules/no-else-return.
*Please note that most of the necessary code adjustments were made in PR 7890.*
ESLint has a number of advantageous properties, compared to JSHint. Among those are:
- The ability to find subtle bugs, thanks to more rules (e.g. PR 7881).
- Much more customizable in general, and many rules allow fine-tuned behaviour rather than the just the on/off rules in JSHint.
- Many more rules that can help developers avoid bugs, and a lot of rules that can be used to enforce a consistent coding style. The latter should be particularily useful for new contributors (and reduce the amount of stylistic review comments necessary).
- The ability to easily specify exactly what rules to use/not to use, as opposed to JSHint which has a default set. *Note:* in future JSHint version some of the rules we depend on will be removed, according to warnings in http://jshint.com/docs/options/, so we wouldn't be able to update without losing lint coverage.
- More easily disable one, or more, rules temporarily. In JSHint this requires using a numeric code, which isn't very user friendly, whereas in ESLint the rule name is simply used instead.
By default there's no rules enabled in ESLint, but there are some default rule sets available. However, to prevent linting failures if we update ESLint in the future, it seemed easier to just explicitly specify what rules we want.
Obviously this makes the ESLint config file somewhat bigger than the old JSHint config file, but given how rarely that one has been updated over the years I don't think that matters too much.
I've tried, to the best of my ability, to ensure that we enable the same rules for ESLint that we had for JSHint. Furthermore, I've also enabled a number of rules that seemed to make sense, both to catch possible errors *and* various style guide violations.
Despite the ESLint README claiming that it's slower that JSHint, https://github.com/eslint/eslint#how-does-eslint-performance-compare-to-jshint, locally this patch actually reduces the runtime for `gulp` lint (by approximately 20-25%).
A couple of stylistic rules that would have been nice to enable, but where our code currently differs to much to make it feasible:
- `comma-dangle`, controls trailing commas in Objects and Arrays (among others).
- `object-curly-spacing`, controls spacing inside of Objects.
- `spaced-comment`, used to enforce spaces after `//` and `/*. (This is made difficult by the fact that there's still some usage of the old preprocessor left.)
Rules that I indend to look into possibly enabling in follow-ups, if it seems to make sense: `no-else-return`, `no-lonely-if`, `brace-style` with the `allowSingleLine` parameter removed.
Useful links:
- http://eslint.org/docs/user-guide/configuring
- http://eslint.org/docs/rules/
