Skip to content
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

Port custom tslint rules to eslint #648

Closed
sandersn opened this issue Mar 22, 2023 · 8 comments · Fixed by #866
Closed

Port custom tslint rules to eslint #648

sandersn opened this issue Mar 22, 2023 · 8 comments · Fixed by #866

Comments

@sandersn
Copy link
Member

sandersn commented Mar 22, 2023

This is a tracking issue to co-ordinate work:

Done:

  • no-outside-dependencies Port no-outside-dependencies #650
  • no-import-default-of-export-equals
  • no-self-import
  • no-single-element-tuple-type
  • no-relative-import-in-test
  • no-redundant-jsdoc2 (though the new name should be no-redundant-jsdoc unless there's a built-in eslint rule already)

In-progress:

  • no-single-declare-module

Not started:

  • no-padding
  • strict-export-declare-modifier
  • npm-naming
  • expect

Comment on this issue to let me know that you're working on a rule and I'll avoid working on it myself.

Also dtslint uses lots of default tslint rules. These all need eslint equivalents. I hope most (all?) of them are already written, but they all need to be tested and added to DT's .eslintrc.json.

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Mar 24, 2023

no-self-import looks quick, tackling it now...
edit: no-single-element-tuple-type too...
edit: and single-declare-module

(editing to collapse comments)

@JoshuaKGoldberg
Copy link
Contributor

On no-redundant-jsdoc2: it's generally available in eslint-plugin-jsdoc as jsdoc/check-tag-names with { typed: true }.

@jakebailey
Copy link
Member

jakebailey commented Apr 11, 2023

If we're going to use eslint-plugin-jsdoc, we will definitely need to verify that it doesn't negatively impact performance. I removed this plugin from the TS repo for that reason; its parsing of JSDoc was extremely inefficient. See: microsoft/TypeScript#51438

@JoshuaKGoldberg
Copy link
Contributor

verify that it doesn't negatively impact performance

...indeed. In the DefinitelyTyped repo, after DefinitelyTyped/DefinitelyTyped#65445:

Variant Timing
Baseline 8.621 s ± 0.249 s
jsdoc 17.843 s ± 0.656 s
Getting performance measurements locally

After installing eslint-plugin-local-rules:

npm i -D eslint-plugin-local-rules

I made the following two files:

.eslint-local-rules.cjs
const dtHeader = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/dt-header");
const exportJustNamespace = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/export-just-namespace");
const noAnyUnion = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-any-union");
const noBadReference = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-bad-reference");
const noConstEnum = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-const-enum");
const noDeadReference = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-dead-reference");
const noSingleElementTupleType = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-single-element-tuple-type");
const noUselessFiles = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/no-useless-files");
const preferDeclareFunction = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/prefer-declare-function");
const redundantUndefined = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/redundant-undefined");
const trimFile = require("../DefinitelyTyped-tools/packages/dtslint/dist/rules/trim-file");

module.exports = {
  "dt-header": dtHeader,
  "export-just-namespace": exportJustNamespace,
  "no-any-union": noAnyUnion,
  "no-bad-reference": noBadReference,
  "no-const-enum": noConstEnum,
  "no-dead-reference": noDeadReference,
  "no-single-element-tuple-type": noSingleElementTupleType,
  "no-useless-files": noUselessFiles,
  "prefer-declare-function": preferDeclareFunction,
  "redundant-undefined": redundantUndefined,
  "trim-file": trimFile,
};
.eslintrc.json
{
  "root": true,
  "extends": [],
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "warnOnUnsupportedTypeScriptVersion": false
  },
  "plugins": ["@typescript-eslint", "jsdoc", "local-rules"],
  "rules": {
    "jsdoc/check-tag-names": [
      "error",
      {
        "definedTags": [
          "addVersion",
          "api",
          "author",
          "beta",
          "brief",
          "category",
          "cfg",
          "chainable",
          "check",
          "classDescription",
          "condparamprivilege",
          "constraint",
          "credits",
          "declaration",
          "defApiFeature",
          "defaultValue",
          "detail",
          "end",
          "eventproperty",
          "experimental",
          "export",
          "expose",
          "extendscript",
          "factory",
          "field",
          "final",
          "fixme",
          "fluent",
          "for",
          "governance",
          "header",
          "hidden-property",
          "hidden",
          "id",
          "label",
          "language",
          "link",
          "listen",
          "locus",
          "methodOf",
          "minVersion",
          "ngdoc",
          "nonstandard",
          "note",
          "npm",
          "observable",
          "option",
          "optionobject",
          "options",
          "packageDocumentation",
          "param",
          "parent",
          "platform",
          "plugin",
          "preserve",
          "privateRemarks",
          "privilegeLevel",
          "privilegeName",
          "proposed",
          "range",
          "readOnly",
          "related",
          "remark",
          "remarks",
          "required",
          "requires",
          "restriction",
          "returnType",
          "section",
          "see",
          "since",
          "const",
          "singleton",
          "source",
          "struct",
          "suppress",
          "targetfolder",
          "enum",
          "title",
          "record",
          "title",
          "TODO",
          "trigger",
          "triggers",
          "typeparam",
          "typeParam",
          "unsupported",
          "url",
          "usage",
          "warn",
          "warning",
          "version"
        ],
        "typed": true
      }
    ],
    "local-rules/dt-header": "error",
    "local-rules/export-just-namespace": "error",
    "local-rules/no-any-union": "error",
    "local-rules/no-bad-reference": "error",
    "local-rules/no-const-enum": "error",
    "local-rules/no-dead-reference": "error",
    "local-rules/no-single-element-tuple-type": "error",
    "local-rules/no-useless-files": "error",
    "local-rules/prefer-declare-function": "error",
    "local-rules/redundant-undefined": "error",
    "local-rules/trim-file": "error"
  },
  "settings": {
    "jsdoc": {
      "tagNamePreference": {
        "argument": "argument",
        "exception": "exception",
        "function": "function",
        "method": "method",
        "param": "param",
        "return": "return",
        "returns": "returns"
      }
    }
  }
}

Then I ran time npx eslint "types/a*/**/*.d.ts" with and without the jsdoc/check-tag-names entry.

I'll take a look at eslint-plugin-jsdoc performance when I have time. cc @brettz & @gajus - I haven't checked how much of this is parsing vs. rule logic vs. other factors. If you have any advice/input that'd be lovely!

@jakebailey
Copy link
Member

Yeah. It's basically loads of splitting by lines and other such stuff, compared to TS's parser.

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented May 20, 2023

The following DefinitelyTyped TSLint rules have rough ESLint equivalents in the wild:

...which leaves the following DefinitelyTyped TSLint rules:

...we're pretty close! 😄

@JoshuaKGoldberg
Copy link
Contributor

Ah, and good news on the performance! eslint/eslint#17066 significantly improved the performance of ESLint's utils.search, which is frequently used for JSDoc parsing by eslint-plugin-jsdoc via a getJSDocComment function.

Running on a Macbook Air M2 with eslint@8.38.0 (before the fix landed):

$ hyperfine "npx eslint \"types/a*/**/*.d.ts\"" --ignore-failure
Benchmark 1: npx eslint "types/a*/**/*.d.ts"
  Time (mean ± σ):     14.468 s ±  0.165 s    [User: 19.037 s, System: 1.023 s]
  Range (min … max):   14.060 s … 14.656 s    10 runs

Running on a Macbook Air M2 with eslint@8.39.0 (which includes the fix):

$ hyperfine "npx eslint \"types/a*/**/*.d.ts\"" --ignore-failure
Benchmark 1: npx eslint "types/a*/**/*.d.ts"
  Time (mean ± σ):      8.057 s ±  0.237 s    [User: 12.593 s, System: 0.955 s]
  Range (min … max):    7.709 s …  8.530 s    10 runs

Nicely done @fasttime! 👏

@mastrzyz
Copy link
Contributor

mastrzyz commented Aug 30, 2023

Nevermind, Please continue without me.

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

Successfully merging a pull request may close this issue.

4 participants