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
Support ESLint 8.x #302
Support ESLint 8.x #302
Conversation
Update packages to support eslint 8 and fix unsupported code error that results in 'Module specifier must be a string literal' parse error. Update @typescript-eslint/experimental-utils, to be in line with eslint 8.x types. Update typescript to 4.4.4, after updating of @typescript-eslint/experimental-utils, because it kept failing due to errors in 'node_modules/@typescript-eslint/types/dist/ast-spec.d.ts' and node_modules/typescript/lib/typescript.d.ts. Remove typings/eslint.d.ts in favor of @types/eslint. Remove all categories, since that's removed in eslint 8. Fix spacing after/before imports, to fix failing eslint rule. Replace 'message' with 'messageId', since 'message' has been removed. Remove support for eslint <7, because the ESLint class does not exist for eslint <7.
Was generated with outdated npm version
I don't have permission to commit changes on release.yml, and here in particular:
|
package.json
Outdated
@@ -30,31 +30,32 @@ | |||
"check-format": "prettier --list-different \"{src,tests}/**/*.ts\"" | |||
}, | |||
"peerDependencies": { | |||
"eslint": "^5.0.0 || ^6.0.0 || ^7.0.0" | |||
"eslint": "^8.0.0" |
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.
why can't we keep compatibility with other older versions?
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.
Well, I tried using our plugin with eslint 5.16.0 and I got some type errors on linting. Not sure how I managed to do that, but I am no longer able to reproduce them. I will revert that change.
ruling/index.ts
Outdated
import * as lodash from "lodash"; | ||
import * as minimist from "minimist"; | ||
|
||
const rulesPath = path.join(__dirname, "../lib/rules"); | ||
|
||
run(); | ||
run().catch((error) => { |
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.
do we need this catch
? won't it throw anyway?
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.
I missed that; I would say it was added for debugging purpose. I will remove it.
type: 'suggestion', | ||
docs: { | ||
description: 'Cognitive Complexity of functions should not be too high', | ||
category: 'Best Practices', |
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.
why does this go away?
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 was dropped (see eslint/eslint#13398)
@@ -30,31 +30,32 @@ | |||
"check-format": "prettier --list-different \"{src,tests}/**/*.ts\"" | |||
}, | |||
"peerDependencies": { | |||
"eslint": "^5.0.0 || ^6.0.0 || ^7.0.0" | |||
"eslint": "^5.0.0 || ^6.0.0 || ^7.0.0|| ^8.0.0" |
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.
package-lock should be updated
src/rules/cognitive-complexity.ts
Outdated
context.report({ node, message: fileComplexity.toString() }); | ||
context.report({ | ||
node, | ||
messageId: 'refactorFunction', |
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.
we should use another message id here, to have final message to contain only fileComplexity
value (https://github.com/SonarSource/SonarJS/blob/master/eslint-bridge/src/linter.ts#L389)
@yassin-kammoun-sonarsource I am not satisfied with the way |
@@ -650,7 +650,7 @@ class TopLevel { | |||
} | |||
`, | |||
options: [0, 'metric'], | |||
errors: [complexity(25)], | |||
errors: [{ messageId: 'fileComplexity', data: { complexityAmount: complexity.toString() } }], |
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.
@vilchik-elena Something is fishy here: we are calling toString
on a function symbol.
Kudos, SonarCloud Quality Gate passed! |
Fixes #286