-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Migrate the repo to ESLint #31777
Migrate the repo to ESLint #31777
Conversation
Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary |
37ac82c
to
5995ca2
Compare
Hi @a-tarasyuk, thanks so much for taking this project on! In order to be able to review this PR, I think it's best to set out a couple guiding principles on what we value in this conversion:
In a perfect world, these principles would require there to be an exact equivalent in ESLint of every TSLint rule we run. However, as we found and you pointed out, this isn't exactly the case, and the number of changes in your PR certainly demonstrates this. In order to make sure everyone's on the same page about these changes, I think the best plan will be to enable as many rules as possible that don't require code changes, and then for those that do, leave them commented out in the eslint config. We can then start a review thread rooted on that line to decide whether we want to make the necessary changes, fix the rule, or drop the rule entirely. I can do one to start off, since I know that the Regarding the set of rules and the formatter from Furthermore, it's difficult for us to take on external dependencies for these because they're very specific to this particular repo, and we definitely do not want to give the impression that we are suggesting a particular style guide for TypeScript code (for an example of just how much we want to avoid that impression, see this page). I think the best course of action for the two packages you created is to deprecate them and move the code into this repo as mentioned above. Please let me know if you have any questions about my recommendations. |
@uniqueiniquity Thanks for the feedback. 👍 Correct me if I'm wrong
Maybe that makes sense only for me and need to drop this public repository. To port the About |
@a-tarasyuk Sure thing! Regarding the ported rules from The formatter might actually be ok as it is. I'll talk to some others on the team and decide what makes the most sense for that piece. |
921824b
to
3c2c181
Compare
3c2c181
to
55b8a38
Compare
TODO:
TODO:
Ignored folders /built/local/**
/tests/**
/lib/** tslint.json(`tslint.json` + `tslint:latest` + `built/local/tslint/rules`){
"adjacent-overload-signatures": true,
"align": false,
"array-type": [
true,
"array"
],
"arrow-parens": false,
"arrow-return-shorthand": false,
"ban-types": false,
"callable-types": true,
"class-name": true,
"comment-format": [
true,
"check-space"
],
"curly": [
true,
"ignore-same-line"
],
"cyclomatic-complexity": false,
"eofline": false,
"forin": false,
"import-spacing": true,
"indent": [
true,
"spaces"
],
"interface-name": [
true,
"never-prefix"
],
"interface-over-type-literal": true,
"jsdoc-format": true,
"label-position": true,
"max-classes-per-file": false,
"max-line-length": false,
"member-access": false,
"member-ordering": false,
"new-parens": true,
"no-angle-bracket-type-assertion": false,
"no-any": false,
"no-arg": true,
"no-bitwise": false,
"no-conditional-assignment": false,
"no-consecutive-blank-lines": false,
"no-console": false,
"no-construct": true,
"no-debugger": false,
"no-duplicate-super": true,
"no-empty": true,
"no-empty-interface": false,
"no-eval": true,
"no-internal-module": true,
"no-invalid-this": false,
"no-misused-new": true,
"no-namespace": false,
"no-parameter-properties": false,
"no-reference": false,
"no-reference-import": true,
"no-shadowed-variable": false,
"no-string-literal": true,
"no-string-throw": true,
"no-switch-case-fall-through": true,
"no-trailing-whitespace": [
true,
"ignore-template-strings"
],
"no-unnecessary-initializer": true,
"no-unsafe-finally": true,
"no-unused-expression": true,
"no-use-before-declare": false,
"no-var-keyword": true,
"no-var-requires": false,
"object-literal-key-quotes": [
true,
"consistent-as-needed"
],
"object-literal-shorthand": true,
"object-literal-sort-keys": false,
"one-line": [
true,
"check-open-brace",
"check-whitespace"
],
"one-variable-per-declaration": false,
"only-arrow-functions": {
"options": [
"allow-declarations",
"allow-named-functions"
]
},
"ordered-imports": false,
"prefer-const": true,
"prefer-for-of": true,
"quotemark": [
true,
"double",
"avoid-escape"
],
"radix": false,
"semicolon": [
true,
"always",
"ignore-bound-class-methods"
],
"space-before-function-paren": false,
"trailing-comma": false,
"triple-equals": true,
"typedef": false,
"typedef-whitespace": [
true,
{
"call-signature": "nospace",
"index-signature": "nospace",
"parameter": "nospace",
"property-declaration": "nospace",
"variable-declaration": "nospace"
},
{
"call-signature": "onespace",
"index-signature": "onespace",
"parameter": "onespace",
"property-declaration": "onespace",
"variable-declaration": "onespace"
}
],
"typeof-compare": false,
"unified-signatures": true,
"use-isnan": true,
"variable-name": [
true,
"ban-keywords",
"check-format",
"allow-leading-underscore"
],
"whitespace": [
true,
"check-branch",
"check-decl",
"check-operator",
"check-module",
"check-separator",
"check-type"
],
"no-invalid-template-strings": true,
"no-sparse-arrays": true,
"no-object-literal-type-assertion": false,
"prefer-conditional-expression": false,
"prefer-object-spread": true,
"no-duplicate-variable": {
"options": "check-parameters"
},
"no-this-assignment": true,
"no-duplicate-imports": true,
"space-within-parens": true,
"no-submodule-imports": false,
"ban-comma-operator": false,
"no-duplicate-switch-case": true,
"no-implicit-dependencies": [
true,
"dev"
],
"no-return-await": true,
"function-constructor": true,
"unnecessary-bind": true,
"no-unnecessary-type-assertion": true,
"ban": [
true,
"setInterval",
"setTimeout"
],
"linebreak-style": [
true,
"CRLF"
],
"next-line": [
true,
"check-catch",
"check-else"
],
"no-bom": true,
"no-increment-decrement": true,
"no-inferrable-types": true,
"no-null-keyword": true,
"no-unnecessary-qualifier": true,
"boolean-trivia": true,
"debug-assert": true,
"no-double-space": true,
"no-in-operator": true,
"no-type-assertion-whitespace": true,
"object-literal-surrounding-space": true,
"type-operator-spacing": 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.
I think it's ready!
Fixes #30553
scripts/eslint
- TODO:no-increment-decrement
-> no-plusplus (completely disallows++
,--
, however, customno-increment-decrement
rule behaves differently. maybe better to implementno-increment-decrement
as it was)I added debug information tono-increment-decrement
, for both cases:case ts.SyntaxKind.PrefixUnaryExpression:
andcase ts.SyntaxKind.PostfixUnaryExpression:
. and the result was empty.Does the TypeScript project not contain the code that will be processed by this rule? Is this rule still applied?
not used/disabledalign
import-spacing
no-reference-import
- change to@typescript-eslint/triple-slash-reference
not used/disabledtypedef
whitespace
not used/disabledprefer-conditional-expression
one-line
->"brace-style": ["error", "stroustrup", { "allowSingleLine": true }]
jsdoc-format
-> eslint-plugin-jsdoconly-arrow-functions
-> prefer-arrow/prefer-arrow-functions - is not a strict alternative, need to check, maybe better to implement this rule as part of microsoft-typescriptvariable-name
-camelcase
,no-underscore-dangle
,id-blacklist
, and/orid-match
. Seeno-duplicate-imports
-> import/no-duplicates or no-duplicate-importsno-submodule-imports
-> import/no-internal-modules (slightly different) not used/disabledno-implicit-dependencies
- import/no-extraneous-dependenciesno-null-keyword
-> eslint-plugin-no-null (doesn’t handle null type)cc @DanielRosenwasser