-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade ESLint to v8 #3984
Upgrade ESLint to v8 #3984
Changes from 12 commits
396633f
10c7425
0143624
dbebd9c
9677e37
ea1a009
1518185
77bb5dd
6223aa4
0bc06f3
8dcd68c
cb014b7
424470b
1454334
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,15 @@ | ||
{ | ||
"rules":{ | ||
"no-console": 0, | ||
// Inherited from airbnb via eslint-config-amo | ||
"function-paren-newline": 0 | ||
"rules": { | ||
// This project uses `console.log()`. | ||
"no-console": "off" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👌 I also prefer the more explicit "off" |
||
}, | ||
"env": { | ||
"node": true, | ||
"browser": true, | ||
"es6": true, | ||
}, | ||
"extends": ["amo"], | ||
"parser": "babel-eslint", | ||
"extends": ["amo/base"], | ||
"parser": "@babel/eslint-parser", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha, yeah. That |
||
"plugins": ["async-await"], | ||
"settings": { | ||
"async-await/space-after-async": 2, | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,9 +43,10 @@ | |
}, | ||
"homepage": "https://github.com/mozilla/addons-linter#readme", | ||
"dependencies": { | ||
"@babel/eslint-parser": "7.15.8", | ||
willdurand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"@mdn/browser-compat-data": "4.0.8", | ||
"addons-moz-compare": "1.2.0", | ||
"addons-scanner-utils": "5.0.0", | ||
"addons-scanner-utils": "6.1.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed to configure ESLint with ES 2022. |
||
"ajv": "6.12.6", | ||
"ajv-merge-patch": "4.1.0", | ||
"chalk": "4.1.2", | ||
|
@@ -54,8 +55,8 @@ | |
"common-tags": "1.8.0", | ||
"deepmerge": "4.2.2", | ||
"dispensary": "0.62.0", | ||
"eslint": "7.32.0", | ||
"eslint-plugin-no-unsanitized": "3.2.0", | ||
"eslint": "8.1.0", | ||
"eslint-plugin-no-unsanitized": "4.0.0", | ||
"eslint-visitor-keys": "3.0.0", | ||
"espree": "9.0.0", | ||
"esprima": "4.0.1", | ||
|
@@ -87,15 +88,13 @@ | |
"@babel/preset-env": "7.16.0", | ||
"@babel/register": "7.16.0", | ||
"babel-core": "7.0.0-bridge.0", | ||
"babel-eslint": "10.1.0", | ||
"babel-gettext-extractor": "4.1.3", | ||
"babel-jest": "27.3.1", | ||
"babel-loader": "8.2.3", | ||
"comment-json": "4.1.1", | ||
"doctoc": "2.1.0", | ||
"eslint-config-amo": "4.11.0", | ||
"eslint-config-amo": "5.1.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed to use |
||
"eslint-plugin-async-await": "0.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am questioning the usefulness of this plugin: I don't think it is used in most of the other projects and we do use async/await everywhere now. I would like to remove this dev dependency: WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @willdurand it looks that it does only provides two "code formatting" rules, which I would guess that prettier may also be taking care of. Also the package version is v0.0.0 and last release 5 years ago, definitely 👍 on my side on getting rid of it. |
||
"eslint-plugin-promise": "5.1.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Edit: it is used for the tests actually, and it is likely coming from addons-frontend, see: https://github.com/mozilla/addons-frontend/issues/2065. I'll revert this change. |
||
"github-markdown-css": "5.0.0", | ||
"gunzip-maybe": "1.4.2", | ||
"hashish": "0.0.4", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,28 +3,24 @@ | |
"jest": true, | ||
"jest/globals": true, | ||
}, | ||
"extends": [ | ||
"plugin:promise/recommended", | ||
], | ||
"globals": { | ||
"assert": true, | ||
"sinon": true, | ||
}, | ||
"plugins": [ | ||
"jest", | ||
"promise", | ||
], | ||
"plugins": ["jest"], | ||
"rules": { | ||
"import/no-extraneous-dependencies": ["error", { | ||
// Allow dev-dependencies in this directory. | ||
"devDependencies": true | ||
}], | ||
"jest/expect-expect": ["warn", { | ||
// Allow Jest or Sinon assertions | ||
"assertFunctionNames": ["expect", "sinon.assert.*"] | ||
// Register the custom matchers we use in this project. | ||
"assertFunctionNames": [ | ||
"expect", | ||
"sinon.assert.*", | ||
"assertHasMatchingError", | ||
"checkMinNodeVersion", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought I'd go ahead and mute some more warnings. We only have 3 warnings (due to excluded files) now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, Thank you for taking care of this! ❤️ I did notice those warnings not long time ago and they were kind of bothering me but didn't had the time to look into them. |
||
], | ||
}], | ||
"promise/always-return": "off", | ||
"promise/avoid-new": "off", | ||
"promise/no-nesting": "off", | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2978,11 +2978,13 @@ describe('ManifestJSONParser', () => { | |
]; | ||
|
||
for (const invalidUrl of testInvalidUrls) { | ||
// eslint-disable-next-line jest/expect-expect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't update the eslint config for these test cases because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sgtm |
||
it(`should fail on forbidden homepage_url "${invalidUrl}"`, () => { | ||
return testHomepageUrl(invalidUrl, false); | ||
}); | ||
} | ||
|
||
// eslint-disable-next-line jest/expect-expect | ||
it('should not mark non forbidden homepage url as invalid', () => { | ||
return testHomepageUrl('http://test.org', true); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,9 +121,7 @@ describe('JavaScript Scanner', () => { | |
expect(linterMessages).toEqual([]); | ||
}); | ||
|
||
// See: https://github.com/tc39/proposal-class-fields | ||
// eslint-disable-next-line jest/no-disabled-tests | ||
it.skip('should support public class fields', async () => { | ||
it('should support public class fields', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 |
||
const code = 'class MyClass { a = 1; }'; | ||
|
||
const jsScanner = new JavaScriptScanner(code, 'code.js'); | ||
|
@@ -286,7 +284,7 @@ describe('JavaScript Scanner', () => { | |
// don't change behaviour on us. | ||
// https://github.com/mozilla/addons-linter/pull/98#issuecomment-158890847 | ||
it('ignores /*global foo*/', () => { | ||
const eslint = ESLint.linter; | ||
const eslint = new ESLint.Linter(); | ||
const config = { rules: { test: 2 } }; | ||
let ok = false; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -681,7 +681,7 @@ describe('Linter.textOutput()', () => { | |
addonLinter.textOutput(uselesslyTinyTerminalWidth); | ||
expect(addonLinter.output.summary.errors).toEqual(1); | ||
} catch (e) { | ||
// eslint-disable-next-line jest/no-conditional-expect, jest/no-try-expect | ||
// eslint-disable-next-line jest/no-conditional-expect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
expect(false).toBe(true); | ||
} | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is (and always was) part of the base config.