Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

scribbles: attempt to add eslint #325

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Mar 29, 2021

This PR collects the notes about switching to ESLint.

So far:

  1. Add dependencies in package.json
  2. Hack lint.ts to call eslint instead of tslint.
  3. Collect a list of tslint overrides that will need to be ported.

I've also got an in-progress .eslintrc.js[on] in my clone of DT which I will add here in a comment. I used tslint-to-eslint-config to produce the initial version, which didn't seem to be super accurate.

I haven't started porting any of our custom rules.

@sandersn
Copy link
Member Author

.eslintrc.js:

/*
👋 Hi! This file was autogenerated by tslint-to-eslint-config.
https://github.com/typescript-eslint/tslint-to-eslint-config

It represents the closest reasonable ESLint configuration to this
project's original TSLint configuration.

We recommend eventually switching this configuration to extend from
the recommended rulesets in typescript-eslint. 
https://github.com/typescript-eslint/tslint-to-eslint-config/blob/master/docs/FAQs.md

Happy linting! 💖
*/
module.exports = {
    "env": {
        "browser": true,
        "es6": true
    },
    "parser": "@typescript-eslint/parser",
    "parserOptions": {
        "project": "../DefinitelyTyped/types/aframe/tsconfig.json",
        "sourceType": "module"
    },
    "plugins": [
        "eslint-plugin-prefer-arrow",
        "eslint-plugin-import",
        "eslint-plugin-no-null",
        "eslint-plugin-unicorn",
        "eslint-plugin-jsdoc",
        "@typescript-eslint",
        "@typescript-eslint/tslint"
    ],
    "rules": {
        "@typescript-eslint/adjacent-overload-signatures": "error",
        "@typescript-eslint/array-type": [
            "error",
            {
                "default": "array-simple"
            }
        ],
        "@typescript-eslint/await-thenable": "error",
        "@typescript-eslint/ban-ts-comment": "error",
        "@typescript-eslint/ban-types": [
            "error",
            {
                "types": {
                    "object": false,
                    "{}": false,
                    "Object": {
                        "message": "Avoid using the `Object` type. Did you mean `object`?"
                    },
                    "Function": {
                        "message": "Avoid using the `Function` type. Prefer a specific function type, like `() => void`."
                    },
                    "Boolean": {
                        "message": "Avoid using the `Boolean` type. Did you mean `boolean`?"
                    },
                    "Number": {
                        "message": "Avoid using the `Number` type. Did you mean `number`?"
                    },
                    "String": {
                        "message": "Avoid using the `String` type. Did you mean `string`?"
                    },
                    "Symbol": {
                        "message": "Avoid using the `Symbol` type. Did you mean `symbol`?"
                    }
                }
            }
        ],
        "@typescript-eslint/consistent-type-assertions": "error",
        "@typescript-eslint/consistent-type-definitions": "error",
        "@typescript-eslint/dot-notation": "off",
        "@typescript-eslint/explicit-member-accessibility": [
            "error",
            {
                "accessibility": "no-public"
            }
        ],
        "@typescript-eslint/indent": "off",
        "@typescript-eslint/member-delimiter-style": [
            "error",
            {
                "multiline": {
                    "delimiter": "semi",
                    "requireLast": true
                },
                "singleline": {
                    "delimiter": "semi",
                    "requireLast": false
                }
            }
        ],
        "@typescript-eslint/member-ordering": "off",
        "@typescript-eslint/naming-convention": "off",
        "@typescript-eslint/no-empty-function": "off",
        "@typescript-eslint/no-empty-interface": "error",
        "@typescript-eslint/no-explicit-any": "off",
        "@typescript-eslint/no-extraneous-class": "error",
        "@typescript-eslint/no-floating-promises": "off",
        "@typescript-eslint/no-for-in-array": "error",
        "@typescript-eslint/no-inferrable-types": [
            "error",
            {
                "ignoreParameters": true
            }
        ],
        "@typescript-eslint/no-misused-new": "error",
        "@typescript-eslint/no-namespace": "error",
        "@typescript-eslint/no-non-null-assertion": "off",
        "@typescript-eslint/no-param-reassign": "off",
        "@typescript-eslint/no-parameter-properties": "off",
        "@typescript-eslint/no-require-imports": "off",
        "@typescript-eslint/no-shadow": [
            "off",
            {
                "hoist": "all"
            }
        ],
        "@typescript-eslint/no-this-alias": "off",
        "@typescript-eslint/no-unnecessary-boolean-literal-compare": "error",
        "@typescript-eslint/no-unnecessary-qualifier": "error",
        "@typescript-eslint/no-unnecessary-type-arguments": "error",
        "@typescript-eslint/no-unnecessary-type-assertion": "error",
        "@typescript-eslint/no-unused-expressions": "off",
        "@typescript-eslint/no-unused-vars": "off",
        "@typescript-eslint/no-use-before-define": "off",
        "@typescript-eslint/no-var-requires": "error",
        "@typescript-eslint/prefer-for-of": "error",
        "@typescript-eslint/prefer-function-type": "off",
        "@typescript-eslint/prefer-namespace-keyword": "error",
        "@typescript-eslint/prefer-readonly": "error",
        "@typescript-eslint/promise-function-async": "off",
        "@typescript-eslint/quotes": "off",
        "@typescript-eslint/require-await": "error",
        "@typescript-eslint/restrict-plus-operands": "off",
        "@typescript-eslint/semi": [
            "error",
            "always"
        ],
        "@typescript-eslint/strict-boolean-expressions": "off",
        "@typescript-eslint/triple-slash-reference": [
            "off",
            {
                "path": "always",
                "types": "prefer-import",
                "lib": "always"
            }
        ],
        "@typescript-eslint/type-annotation-spacing": "error",
        "@typescript-eslint/unbound-method": "off",
        "@typescript-eslint/unified-signatures": "error",
        "arrow-body-style": "error",
        "arrow-parens": [
            "off",
            "always"
        ],
        "brace-style": [
            "error",
            "1tbs"
        ],
        "class-methods-use-this": "off",
        "comma-dangle": "off",
        "complexity": "off",
        "constructor-super": "error",
        "curly": "off",
        "default-case": "off",
        "eol-last": "error",
        "eqeqeq": [
            "error",
            "smart"
        ],
        "guard-for-in": "off",
        "id-blacklist": "off",
        "id-match": "off",
        "import/no-default-export": "off",
        "import/no-deprecated": "off",
        "import/no-extraneous-dependencies": "off",
        "import/no-internal-modules": "off",
        "import/no-unassigned-import": "off",
        "import/order": "off",
        "jsdoc/check-alignment": "error",
        "jsdoc/check-indentation": "error",
        "jsdoc/newline-after-description": "error",
        "jsdoc/no-types": "off",
        "linebreak-style": "off",
        "max-classes-per-file": "off",
        "max-len": [
            "error",
            {
                "code": 200
            }
        ],
        "max-lines": "off",
        "new-parens": "error",
        "newline-per-chained-call": "off",
        "no-bitwise": "off",
        "no-caller": "error",
        "no-cond-assign": "error",
        "no-console": "off",
        "no-debugger": "error",
        "no-duplicate-case": "error",
        "no-duplicate-imports": "error",
        "no-empty": "off",
        "no-eval": "error",
        "no-extra-bind": "error",
        "no-irregular-whitespace": "error",
        "no-magic-numbers": "off",
        "no-multiple-empty-lines": "error",
        "no-new-func": "error",
        "no-new-wrappers": "error",
        "no-null/no-null": "off",
        "no-plusplus": [
            "off",
            {
                "allowForLoopAfterthoughts": true
            }
        ],
        "no-redeclare": "off",
        "no-return-await": "error",
        "no-sequences": "error",
        "no-sparse-arrays": "error",
        "no-template-curly-in-string": "error",
        "no-throw-literal": "error",
        "no-trailing-spaces": "error",
        "no-undef-init": "error",
        "no-underscore-dangle": "off",
        "no-unsafe-finally": "error",
        "no-unused-labels": "error",
        "no-useless-constructor": "off",
        "no-var": "error",
        "no-void": "error",
        "object-shorthand": "error",
        "one-var": [
            "error",
            "never"
        ],
        "padding-line-between-statements": [
            "off",
            {
                "blankLine": "always",
                "prev": "*",
                "next": "return"
            }
        ],
        "prefer-arrow/prefer-arrow-functions": "error",
        "prefer-const": "error",
        "prefer-object-spread": "error",
        "prefer-template": "error",
        "quote-props": [
            "error",
            "as-needed"
        ],
        "radix": "error",
        "space-before-function-paren": [
            "error",
            {
                "anonymous": "never",
                "asyncArrow": "always",
                "named": "never"
            }
        ],
        "space-in-parens": [
            "error",
            "never"
        ],
        "spaced-comment": [
            "error",
            "always",
            {
                "markers": [
                    "/"
                ]
            }
        ],
        "unicorn/filename-case": "off",
        "use-isnan": "error",
        "yoda": "off",
        "@typescript-eslint/tslint/config": [
            "error",
            {
                "rules": {
                    "comment-type": [
                        true,
                        "singleline",
                        "multiline",
                        "doc",
                        "directive"
                    ],
                    "dt-header": true,
                    "encoding": true,
                    "expect": true,
                    "export-just-namespace": true,
                    "import-spacing": true,
                    "no-any-union": true,
                    "no-bad-reference": true,
                    "no-const-enum": true,
                    "no-dead-reference": true,
                    "no-declare-current-package": true,
                    "no-dynamic-delete": true,
                    "no-import-default-of-export-equals": true,
                    "no-mergeable-namespace": true,
                    "no-null-undefined-union": true,
                    "no-outside-dependencies": true,
                    "no-padding": true,
                    "no-redundant-jsdoc-2": true,
                    "no-redundant-undefined": true,
                    "no-relative-import-in-test": true,
                    "no-self-import": true,
                    "no-single-declare-module": true,
                    "no-unnecessary-callback-wrapper": true,
                    "no-unnecessary-generics": true,
                    "no-useless-files": true,
                    "npm-naming": [
                        true,
                        {
                            "mode": "code",
                            "errors": [
                                [
                                    "NeedsExportEquals",
                                    false
                                ]
                            ]
                        }
                    ],
                    "prefer-conditional-expression": true,
                    "prefer-declare-function": true,
                    "prefer-switch": true,
                    "prefer-while": true,
                    "static-this": true,
                    "strict-comparisons": true,
                    "strict-export-declare-modifiers": true,
                    "trim-file": true,
                    "void-return": true,
                    "whitespace": [
                        true,
                        "check-branch",
                        "check-decl",
                        "check-operator",
                        "check-module",
                        "check-separator",
                        "check-type",
                        "check-typecast"
                    ]
                }
            }
        ]
    }
};

@sandersn
Copy link
Member Author

12 ESLint rules behave differently from their TSLint counterparts:

  • @typescript-eslint/ban-ts-comment:
    • The typescript-eslint now bans @ts- comments from being used
  • prefer-arrow/prefer-arrow-functions:
    • ESLint does not support allowing standalone function declarations.
    • ESLint does not support allowing named functions defined with the function keyword.
  • no-redeclare:
    • ESLint does not support check-parameters.
  • @typescript-eslint/no-unused-expressions:
    • The TSLint optional config "allow-new" is the default ESLint behavior and will no longer be ignored.
  • no-void:
    • ESLint does not support ignoring arrow function shorthand.
  • eqeqeq:
    • Option "smart" allows for comparing two literal values, evaluating the value of typeof and null comparisons.
  • arrow-body-style:
    • ESLint will throw an error if the function body is multiline yet has a one-line return on it.
  • no-plusplus:
    • ++ and -- operators will now only be allowed in for loops.
  • prefer-template:
    • Single concatenations will no longer be ignored.
  • space-before-function-paren:
    • Option "constructor" is not supported by ESLint.
    • Option "method" is not supported by ESLint.
  • no-underscore-dangle:
    • Leading or trailing underscores (_) on identifiers will now be forbidden.
  • @typescript-eslint/no-unused-vars:

36 rules are not known by tslint-to-eslint-config to have ESLint equivalents:

  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "comment-type".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "dt-header".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "encoding".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "expect".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "export-just-namespace".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "import-spacing".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-any-union".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-bad-reference".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-const-enum".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-dead-reference".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-declare-current-package".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-dynamic-delete".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-import-default-of-export-equals".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-mergeable-namespace".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-null-undefined-union".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-outside-dependencies".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-padding".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-redundant-jsdoc-2".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-redundant-undefined".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-relative-import-in-test".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-self-import".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-single-declare-module".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-unnecessary-callback-wrapper".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-unnecessary-generics".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "no-useless-files".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "npm-naming".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "prefer-conditional-expression".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "prefer-declare-function".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "prefer-switch".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "prefer-while".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "static-this".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "strict-comparisons".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "strict-export-declare-modifiers".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "trim-file".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "void-return".
  • tslint-to-eslint-config does not know the ESLint equivalent for TSLint's "whitespace".

@sandersn
Copy link
Member Author

@43081j you might be interested in this

@sandersn
Copy link
Member Author

sandersn commented Mar 29, 2021

DT also added the following devDependencies:

        "@typescript-eslint/eslint-plugin-tslint": "^4.19.0",
        "@typescript-eslint/parser": "^4.19.0",
        "danger": "^10.1.1",
        "dtslint": "latest",
        "eslint": "^7.22.0",
        "eslint-plugin-import": "^2.22.1",
        "eslint-plugin-jsdoc": "^32.3.0",
        "eslint-plugin-no-null": "^1.0.2",
        "eslint-plugin-prefer-arrow": "^1.2.3",
        "eslint-plugin-unicorn": "^29.0.0",

and the following dependencies (which should likely be devDeps since there were none before):

    "dependencies": {
        "@typescript-eslint/eslint-plugin": "^4.19.0",
        "tslint": "5.18"
    }

I think this was just to support tslint-to-eslint-config, though

@43081j
Copy link
Contributor

43081j commented Mar 29, 2021

This is great, good to see it picked up again :D

For the eslint config, we could probably trim it down a huge amount by extending eslint and typescript-eslint:

{
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/eslint-recommended",
    "plugin:@typescript-eslint/recommended"
  ]
}

which means your typescript-eslint rules would come down to these:

{
        "@typescript-eslint/array-type": [
            "error",
            {
                "default": "array-simple"
            }
        ],
        "@typescript-eslint/await-thenable": "error",
        "@typescript-eslint/consistent-type-assertions": "error",
        "@typescript-eslint/consistent-type-definitions": "error",
        "@typescript-eslint/explicit-member-accessibility": [
            "error",
            {
                "accessibility": "no-public"
            }
        ],
        "@typescript-eslint/indent": "off",
        "@typescript-eslint/member-delimiter-style": "error",
        "@typescript-eslint/no-empty-function": "off",
        "@typescript-eslint/no-explicit-any": "off",
        "@typescript-eslint/no-extraneous-class": "error",
        "@typescript-eslint/no-for-in-array": "error",
        "@typescript-eslint/no-inferrable-types": [
            "error",
            {
                "ignoreParameters": true
            }
        ],
        "@typescript-eslint/no-non-null-assertion": "off",
        "@typescript-eslint/no-shadow": "off",
        "@typescript-eslint/no-this-alias": "off",
        "@typescript-eslint/no-unnecessary-boolean-literal-compare": "error",
        "@typescript-eslint/no-unnecessary-qualifier": "error",
        "@typescript-eslint/no-unnecessary-type-arguments": "error",
        "@typescript-eslint/no-unnecessary-type-assertion": "error",
        "@typescript-eslint/no-unused-vars": "off",
        "@typescript-eslint/prefer-for-of": "error",
        "@typescript-eslint/prefer-readonly": "error",
        "@typescript-eslint/require-await": "error",
        "@typescript-eslint/triple-slash-reference": "off",
        "@typescript-eslint/type-annotation-spacing": "error",
        "@typescript-eslint/unified-signatures": "error"
}

i removed all the ones which are:

  • missing in the inherited config (which means they are already off)
  • the exact same default config than what we had

there will be a few you can remove due to being now inherited from eslint too, but i didn't have chance to diff those.

@43081j
Copy link
Contributor

43081j commented Mar 30, 2021

You can also drop the unnecessary "unicorn" plugin. It isn't used and will have been pulled in just to reference a disabled rule so can be removed entirely instead.

Is the plan to "all or nothing" it? Convert all the rules at once? Would be happy to help if needed

@sandersn
Copy link
Member Author

sandersn commented Jul 1, 2021

Yeah, I was planning for all-or-nothing. But it might be easier to double-lint to start, and switch over slowly.

The biggest problem for me is funding, since it's hard for me to spend time on code that works stably and reliably, even though its dependency is not being maintained anymore.

@danvk @JoshuaKGoldberg @OliverJAsh were discussing this on twitter, so maybe they have plans to contribute something.

@43081j
Copy link
Contributor

43081j commented Jul 1, 2021

thats fair enough

@JoshuaKGoldberg @danvk @OliverJAsh this is being tracked by #300, would be useful if you could give input there if you're planning on opening an alternative PR.

hilariously in that issue, i'd previously sent you here! 😂 the ol' switcheroo

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants