Tweak some ESLint in the 2.0dev branch #41
Conversation
| extends: | ||
| - eslint:recommended | ||
|
|
||
| root: true |
There was a problem hiding this comment.
Nothing too exciting in this file. I added an ".yml" extension to the .eslintrc file, per latest ESLint recommendations (plus why not).
Also changed all the "0", "1", "2" codes to the newer verbose syntax of "off", "warn", "error", respectively. Makes for better reading.
| @@ -152,8 +152,10 @@ class DistanceMatrix { | |||
| const clusters = elements.map(el => [el]); | |||
|
|
|||
| // Init matrix: | |||
| // eslint-disable-next-line prefer-const | |||
There was a problem hiding this comment.
Oh, I also added the "prefer-const" rule, because it's a solid rule. The downside is that we seem to use for (let foo of bar) {..}. Not sure if we want to change those to "const" or keep as "let". I vaguely recall Firefox had issues w/ for (const foo of bar) {...} which is why we use let, depending on the target runtime.
TL;DR: I like prefer-const, so I enabled it globally, and explicitly disabled that error for each for (let ...) loop.
There was a problem hiding this comment.
I like prefer-const as well, but my dislike of linter pragmas in my code outweighs it. :-P My hopes are pinned on FF fixing that, and then we can prefer-const to our hearts' content.
| @@ -0,0 +1,2 @@ | |||
| env: | |||
| mocha: true | |||
There was a problem hiding this comment.
Since we're only using mocha env in test/, I removed the env from the root .eslintrc.yml file and moved it into this nested test/.eslintrc.yml file.
There was a problem hiding this comment.
Whizzy! I didn't know you could do that.
| describe('LHS tests', function () { | ||
| it('makes a dom() LHS that rule() tolerates', function () { | ||
| const lhs = dom('smoo'); | ||
| const rhs = type('bar'); | ||
| const theRule = rule(lhs, rhs); | ||
| rule(lhs, rhs); |
There was a problem hiding this comment.
We weren't using theRule, so I removed it. 🤷
|
Not very interesting PR, and you're very unimpressed. Things get messier if you actually want to use/extend the eslint-config-airbnb-base rules. I did a bit of additional rules tweaking and ran diff --git a/.eslintrc.yml b/.eslintrc.yml
index 447af7c..ea14d89 100644
--- a/.eslintrc.yml
+++ b/.eslintrc.yml
@@ -4,18 +4,23 @@ env:
extends:
- eslint:recommended
+ - airbnb-base
root: true
rules:
+ import/no-extraneous-dependencies: off
+
generator-star-spacing: error
guard-for-in: warn # There's nothing wrong with for..in if you know what you're doing. This is here just to keep me from accidentally saying "for..in" when I mean "for..of". Delete this and come up with a better solution if we ever need to use "for..in".
indent: [error, 4]
+ max-len: [error, {code: 100, ignoreComments: true, ignoreStrings: true, ignoreTemplateLiterals: true}]
no-dupe-class-members: error
no-loop-func: error
no-new-func: error # equivalent to eval()
no-throw-literal: error
no-trailing-spaces: error
+ no-underscore-dangle: off
no-unused-vars: [warn, {vars: all, args: none}]
no-use-before-define: [error, {functions: false, classes: false}]
no-useless-escape: errordiff --git a/package.json b/package.json
index d3b7cc0..7a97c49 100644
--- a/package.json
+++ b/package.json
@@ -13,6 +13,8 @@
"chai": "^3.5.0",
"coveralls": "^2.11.11",
"eslint": "^3.7.1",
+ "eslint-config-airbnb-base": "^8.0.0",
+ "eslint-plugin-import": "^2.0.0",
"istanbul": "^0.4.4",
"jsdom": "^8.3.0",
"mocha": "^2.4.5",
$ ./node_modules/.bin/eslint . (click-o, expand-o)/Users/pdehaan/dev/github/mozilla/fathom/clusters.js
21:9 error Unexpected assignment within an 'if' statement no-cond-assign
92:9 error Unexpected use of '&' no-bitwise
94:25 error Unexpected use of '&' no-bitwise
97:16 error Unexpected use of '&' no-bitwise
99:25 error Unexpected use of '&' no-bitwise
152:15 error 'clusters' is already declared in the upper scope no-shadow
282:5 error Unexpected assignment within a 'while' statement no-cond-assign
/Users/pdehaan/dev/github/mozilla/fathom/fnode.js
63:9 error Unexpected assignment within an 'if' statement no-cond-assign
/Users/pdehaan/dev/github/mozilla/fathom/lhs.js
33:16 error Unnecessary 'else' after 'return' no-else-return
45:14 error Expected 'this' to be used by class method 'checkFact' class-methods-use-this
50:19 error Expected 'this' to be used by class method 'guaranteedType' class-methods-use-this
67:45 error Unary operator '++' used no-plusplus
74:14 error Expected 'this' to be used by class method 'checkFact' class-methods-use-this
145:19 error Unexpected block statement surrounding arrow body arrow-body-style
/Users/pdehaan/dev/github/mozilla/fathom/rhs.js
54:33 error Do not access Object.prototype method 'hasOwnProperty' from target object no-prototype-builtins
55:25 error Assignment to property of function parameter 'result' no-param-reassign
84:13 error Assignment to property of function parameter 'result' no-param-reassign
133:13 error Assignment to property of function parameter 'result' no-param-reassign
153:13 error Assignment to property of function parameter 'result' no-param-reassign
170:13 error Assignment to property of function parameter 'result' no-param-reassign
201:37 error Do not access Object.prototype method 'hasOwnProperty' from target object no-prototype-builtins
/Users/pdehaan/dev/github/mozilla/fathom/ruleset.js
37:18 error 'rule' is already declared in the upper scope no-shadow
37:18 error 'rule' is already declared in the upper scope no-shadow
93:22 error 'rule' is already declared in the upper scope no-shadow
93:22 error 'rule' is already declared in the upper scope no-shadow
105:16 error Unnecessary 'else' after 'return' no-else-return
124:23 error 'rule' is already declared in the upper scope no-shadow
151:13 error 'ruleset' is already declared in the upper scope no-shadow
173:1 error Line 173 exceeds the maximum line length of 100 max-len
236:13 error 'ruleset' is already declared in the upper scope no-shadow
/Users/pdehaan/dev/github/mozilla/fathom/side.js
32:20 error 'score' is already declared in the upper scope no-shadow
77:15 error 'score' is already declared in the upper scope no-shadow
101:12 error Expected 'this' to be used by class method '_asSide' class-methods-use-this
104:13 error Assignment to function parameter 'side' no-param-reassign
/Users/pdehaan/dev/github/mozilla/fathom/utils.js
95:9 error Unary operator '++' used no-plusplus
137:49 error 'element' is already declared in the upper scope no-shadow
142:30 error 'element' is already declared in the upper scope no-shadow
142:60 error Unexpected mix of '||' and '&&' no-mixed-operators
143:72 error Unexpected mix of '||' and '&&' no-mixed-operators
158:53 error 'element' is already declared in the upper scope no-shadow
172:11 error 'length' is already declared in the upper scope no-shadow
188:21 error 'map' is already declared in the upper scope no-shadow
199:21 error 'map' is already declared in the upper scope no-shadow
209:44 error Unary operator '--' used no-plusplus
✖ 44 problems (44 errors, 0 warnings) |
|
As far as actually extending the airbnb config, I don't think I want to go that far. They have a lot of good ideas I'm willing to steal, but I don't agree with everything. In my view, a linter that flags only things over which you'd smack your forehead and say "Of course I want to fix that!" is the goal. But one that finds more errors yet whose nitpicking drives you to quit is a bad tradeoff. :-) |
| root: true | ||
|
|
||
| rules: | ||
| generator-star-spacing: error |
There was a problem hiding this comment.
Yay, words are so much better than numbers.
|
Disabled the |
|
Lovely; thank you! |
No description provided.