From b58f7b08e0568abacc7fa65819a7d89b5fff5ac5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 29 Nov 2023 21:52:02 +0100 Subject: [PATCH 1/2] docs(eslint-plugin): enforce a heading for each rule option --- .cspell.json | 6 +- .../rules/explicit-member-accessibility.md | 4 +- .../docs/rules/no-empty-interface.md | 15 +-- .../docs/rules/no-explicit-any.md | 7 ++ .../rules/no-meaningless-void-operator.md | 2 + .../docs/rules/no-misused-promises.md | 6 +- .../eslint-plugin/docs/rules/no-this-alias.md | 69 ++++++++++++++ .../docs/rules/parameter-properties.md | 4 +- .../docs/rules/prefer-literal-enum-member.md | 4 +- .../docs/rules/prefer-nullish-coalescing.md | 10 ++ .../docs/rules/promise-function-async.md | 68 ++++++++++++++ .../docs/rules/sort-type-constituents.md | 56 +++++++++++ .../docs/rules/triple-slash-reference.md | 94 ++++++++++++++----- packages/eslint-plugin/tests/docs.test.ts | 42 +++++++-- 14 files changed, 336 insertions(+), 51 deletions(-) diff --git a/.cspell.json b/.cspell.json index ec11ebf5018d..dd9c1d912d7f 100644 --- a/.cspell.json +++ b/.cspell.json @@ -72,6 +72,7 @@ "declarators", "destructure", "destructured", + "destructures", "discoverability", "dprint", "errored", @@ -144,7 +145,10 @@ "overrides": [ { "filename": "**/*.{ts,mts,cts,js,cjs,mjs,tsx,jsx}", - "ignoreRegExpList": ["/@[a-z]+/", "/#(end)?region/"], + "ignoreRegExpList": [ + "/@[a-z]+/", + "/#(end)?region/" + ], "includeRegExpList": [ "/\\/\\*[\\s\\S]*?\\*\\/|([^\\\\:]|^)\\/\\/.*$/", "/(\\/\\/[^\\n\\r]*[\\n\\r]+)/" diff --git a/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md b/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md index d41f85c09c40..55c28dc2eec0 100644 --- a/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md +++ b/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md @@ -164,7 +164,7 @@ class Animal { } ``` -### Overrides +### `overrides` There are three ways in which an override can be used. @@ -312,7 +312,7 @@ class Animal { } ``` -### Except specific methods +### `ignoredMethodNames` If you want to ignore some specific methods, you can do it by specifying method names. Note that this option does not care for the context, and will ignore every method with these names, which could lead to it missing some cases. You should use this sparingly. e.g. `[ { ignoredMethodNames: ['specificMethod', 'whateverMethod'] } ]` diff --git a/packages/eslint-plugin/docs/rules/no-empty-interface.md b/packages/eslint-plugin/docs/rules/no-empty-interface.md index 64bf24ffe1fa..f665acd4ae9a 100644 --- a/packages/eslint-plugin/docs/rules/no-empty-interface.md +++ b/packages/eslint-plugin/docs/rules/no-empty-interface.md @@ -50,20 +50,9 @@ interface Baz extends Foo, Bar {} ## Options -This rule accepts a single object option with the following default configuration: - -```json -{ - "@typescript-eslint/no-empty-interface": [ - "error", - { - "allowSingleExtends": false - } - ] -} -``` +### `allowSingleExtends` -- `allowSingleExtends: true` will silence warnings about extending a single interface without adding additional members +`allowSingleExtends: true` will silence warnings about extending a single interface without adding additional members ## When Not To Use It diff --git a/packages/eslint-plugin/docs/rules/no-explicit-any.md b/packages/eslint-plugin/docs/rules/no-explicit-any.md index 5694ebf709a1..58ee9b79e215 100644 --- a/packages/eslint-plugin/docs/rules/no-explicit-any.md +++ b/packages/eslint-plugin/docs/rules/no-explicit-any.md @@ -94,6 +94,13 @@ function greet(param: Array): Array {} ## Options +### `fixToUnknown` + +By default, this rule will not provide automatic ESLint _fixes_: only opt-in _suggestions_. +Switching types to `unknown` is safer but is likely to cause additional type errors. + +Enabling `{ "fixToUnknown": true }` gives the rule an auto-fixer to replace `: any` with `: unknown`. + ### `ignoreRestArgs` A boolean to specify if arrays from the rest operator are considered okay. `false` by default. diff --git a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md index f904c473d667..dda276543d4a 100644 --- a/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md +++ b/packages/eslint-plugin/docs/rules/no-meaningless-void-operator.md @@ -44,4 +44,6 @@ void bar(); // discarding a number ## Options +### `checkNever` + `checkNever: true` will suggest removing `void` when the argument has type `never`. diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md index b1cddcb6d68d..667e15074a4e 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-promises.md +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -17,7 +17,7 @@ See [`no-floating-promises`](./no-floating-promises.md) for detecting unhandled ## Options -### `"checksConditionals"` +### `checksConditionals` If you don't want to check conditionals, you can configure the rule with `"checksConditionals": false`: @@ -73,7 +73,7 @@ while (await promise) { -### `"checksVoidReturn"` +### `checksVoidReturn` Likewise, if you don't want functions that return promises where a void return is expected to be checked, your configuration will look like this: @@ -182,7 +182,7 @@ eventEmitter.on('some-event', () => { -### `"checksSpreads"` +### `checksSpreads` If you don't want to check object spreads, you can add this configuration: diff --git a/packages/eslint-plugin/docs/rules/no-this-alias.md b/packages/eslint-plugin/docs/rules/no-this-alias.md index 640d5a6a2b54..fa74f580f0c6 100644 --- a/packages/eslint-plugin/docs/rules/no-this-alias.md +++ b/packages/eslint-plugin/docs/rules/no-this-alias.md @@ -33,6 +33,75 @@ setTimeout(() => { ## Options +### `allowDestructuring` + +It can sometimes be useful to destructure properties from a class instance, such as retrieving multiple properties from the instance in one of its methods. +`allowDestructuring` allows those destructures and is `true` by default. +You can explicitly disallow them by setting `allowDestructuring` to `false`. + +Examples of code for the `{ "allowDestructuring": false }` option: + + + +#### ❌ Incorrect + +```ts option='{ "allowDestructuring": false }' +class ComponentLike { + props: unknown; + state: unknown; + + render() { + const { props, state } = this; + + console.log(props); + console.log(state); + } +} +``` + +#### ✅ Correct + +```ts option='{ "allowDestructuring": false }' +class ComponentLike { + props: unknown; + state: unknown; + + render() { + console.log(this.props); + console.log(this.state); + } +} +``` + +### `allowedNames` + +`no-this-alias` can alternately be used to allow only a specific list of names as `this` aliases. +We recommend against this except as a transitory step towards fixing all rule violations. + +Examples of code for the `{ "allowedNames": ["self"] }` option: + + + +#### ❌ Incorrect + +```ts option='{ "allowedNames": ["self"] }' +class Example { + method() { + const that = this; + } +} +``` + +#### ✅ Correct + +```ts option='{ "allowedNames": ["self"] }' +class Example { + method() { + const self = this; + } +} +``` + ## When Not To Use It If you need to assign `this` to variables, you shouldn’t use this rule. diff --git a/packages/eslint-plugin/docs/rules/parameter-properties.md b/packages/eslint-plugin/docs/rules/parameter-properties.md index 6d970076957c..437d243a718e 100644 --- a/packages/eslint-plugin/docs/rules/parameter-properties.md +++ b/packages/eslint-plugin/docs/rules/parameter-properties.md @@ -19,7 +19,7 @@ It may take an options object containing either or both of: - `"allow"`: allowing certain kinds of properties to be ignored - `"prefer"`: either `"class-property"` _(default)_ or `"parameter-property"` -### `"allow"` +### `allow` If you would like to ignore certain kinds of properties then you may pass an object containing `"allow"` as an array of any of the following options: @@ -45,7 +45,7 @@ For example, to ignore `public` properties: } ``` -### `"prefer"` +### `prefer` By default, the rule prefers class property (`"class-property"`). You can switch it to instead preferring parameter property with (`"parameter-property"`). diff --git a/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md b/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md index ea276761c9ed..2e525edb9ca3 100644 --- a/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md +++ b/packages/eslint-plugin/docs/rules/prefer-literal-enum-member.md @@ -61,7 +61,9 @@ enum Valid { ## Options -- `allowBitwiseExpressions` set to `true` will allow you to use bitwise expressions in enum initializer (Default: `false`). +### `allowBitwiseExpressions` + +When set to `true` will allow you to use bitwise expressions in enum initializer (default: `false`). Examples of code for the `{ "allowBitwiseExpressions": true }` option: diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md index 9377117fba44..0db387ab9a21 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md @@ -167,6 +167,16 @@ foo ?? 'a string'; Also, if you would like to ignore all primitives types, you can set `ignorePrimitives: true`. It is equivalent to `ignorePrimitives: { string: true, number: true, bigint: true, boolean: true }`. +### `allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing` + +If this is set to `false`, then the rule will error on every file whose `tsconfig.json` does _not_ have the `strictNullChecks` compiler option (or `strict`) set to `true`. + +Without `strictNullChecks`, TypeScript essentially erases `undefined` and `null` from the types. This means when this rule inspects the types from a variable, **it will not be able to tell that the variable might be `null` or `undefined`**, which essentially makes this rule useless. + +You should be using `strictNullChecks` to ensure complete type-safety in your codebase. + +If for some reason you cannot turn on `strictNullChecks`, but still want to use this rule - you can use this option to allow it - but know that the behavior of this rule is _undefined_ with the compiler option turned off. We will not accept bug reports if you are using this option. + ## When Not To Use It If you are not using TypeScript 3.7 (or greater), then you will not be able to use this rule, as the operator is not supported. diff --git a/packages/eslint-plugin/docs/rules/promise-function-async.md b/packages/eslint-plugin/docs/rules/promise-function-async.md index f3095ed1808c..297ec2fe0126 100644 --- a/packages/eslint-plugin/docs/rules/promise-function-async.md +++ b/packages/eslint-plugin/docs/rules/promise-function-async.md @@ -57,3 +57,71 @@ async function functionReturnsUnionWithPromiseImplicitly(p: boolean) { return p ? 'value' : Promise.resolve('value'); } ``` + +## Options + +### `allowAny` + +Whether to ignore functions that return `any` and `unknown`. +If you want additional safety, consider turning this option off, as it makes the rule less able to catch incorrect Promise behaviors. + +Examples of code with `{ "allowAny": false }`: + + + +#### ❌ Incorrect + +```ts option='{ "allowAny": false }' +const returnsAny = () => ({}) as any; +``` + +#### ✅ Correct + +```ts option='{ "allowAny": false }' +const returnsAny = async () => ({}) as any; +``` + +### `allowedPromiseNames` + +For projects that use constructs other than the global built-in `Promise` for asynchronous code. +This option allows specifying string names of classes or interfaces that cause a function to be checked as well. + +Examples of code with `{ "allowedPromiseNames": ["Bluebird"] }`: + + + +#### ❌ Incorrect + +```ts option='{ "allowedPromiseNames": ["Bluebird"] }' +import { Bluebird } from 'bluebird'; + +const returnsBluebird = () => new Bluebird(() => {}); +``` + +#### ✅ Correct + +```ts option='{ "allowedPromiseNames": ["Bluebird"] }' +import { Bluebird } from 'bluebird'; + +const returnsBluebird = async () => new Bluebird(() => {}); +``` + +### `checkArrowFunctions` + +Whether to check arrow functions. +`true` by default, but can be set to `false` to ignore them. + +### `checkFunctionDeclarations` + +Whether to check standalone function declarations. +`true` by default, but can be set to `false` to ignore them. + +### `checkFunctionExpressions` + +Whether to check inline function expressions. +`true` by default, but can be set to `false` to ignore them. + +### `checkMethodDeclarations` + +Whether to check methods on classes and object literals +`true` by default, but can be set to `false` to ignore them. diff --git a/packages/eslint-plugin/docs/rules/sort-type-constituents.md b/packages/eslint-plugin/docs/rules/sort-type-constituents.md index 025e31ccad4f..97c79efff536 100644 --- a/packages/eslint-plugin/docs/rules/sort-type-constituents.md +++ b/packages/eslint-plugin/docs/rules/sort-type-constituents.md @@ -82,6 +82,46 @@ type T4 = ## Options +### `checkIntersections` + +Whether to check intersection types (`&`). + +Examples of code with `{ "checkIntersections": true }` (the default): + + + +#### ❌ Incorrect + +```ts option='{ "checkIntersections": true }' +type ExampleIntersection = B & A; +``` + +#### ✅ Correct + +```ts option='{ "checkIntersections": true }' +type ExampleIntersection = A & B; +``` + +### `checkUnions` + +Whether to check union types (`|`). + +Examples of code with `{ "checkUnions": true }` (the default): + + + +#### ❌ Incorrect + +```ts option='{ "checkUnions": true }' +type ExampleUnion = B | A; +``` + +#### ✅ Correct + +```ts option='{ "checkUnions": true }' +type ExampleUnion = A | B; +``` + ### `groupOrder` Each constituent of the type is placed into a group, and then the rule sorts alphabetically within each group. @@ -99,3 +139,19 @@ The ordering of groups is determined by this option. - `tuple` - Tuple types (`[A, B, C]`) - `union` - Union types (`A | B`) - `nullish` - `null` and `undefined` + +For example, configuring the rule with `{ "groupOrder": ["literal", "nullish" ]}`: + + + +#### ❌ Incorrect + +```ts option='{ "groupOrder": ["literal", "nullish" ]}' +type ExampleGroup = null | 123; +``` + +#### ✅ Correct + +```ts option='{ "groupOrder": ["literal", "nullish" ]}' +type ExampleGroup = 123 | null; +``` diff --git a/packages/eslint-plugin/docs/rules/triple-slash-reference.md b/packages/eslint-plugin/docs/rules/triple-slash-reference.md index 538c319059ed..e0dd81847aa7 100644 --- a/packages/eslint-plugin/docs/rules/triple-slash-reference.md +++ b/packages/eslint-plugin/docs/rules/triple-slash-reference.md @@ -8,46 +8,94 @@ description: 'Disallow certain triple slash directives in favor of ES6-style imp TypeScript's `///` triple-slash references are a way to indicate that types from another module are available in a file. Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module `import`s. -This rule reports on the use of `/// `, `/// `, or `/// ` directives. +This rule reports on the use of `/// `, `/// `, or `/// ` directives. ## Options -With `{ "path": "never", "types": "never", "lib": "never" }` options set, the following will all be **incorrect** usage: +Any number of the three kinds of references can be specified as an option. +Specifying `'always'` disables this lint rule for that kind of reference. -```ts option='{ "path": "never", "types": "never", "lib": "never" }' showPlaygroundButton -/// -/// -/// +### `lib` + +When set to `'never'`, bans `/// ` and enforces using an `import` instead: + + + +#### ❌ Incorrect + +```ts option='{ "lib": "never" }' +/// + +globalThis.value; +``` + +#### ✅ Correct + +```ts option='{ "lib": "never" }' +import { value } from 'code'; +``` + +### `path` + +When set to `'never'`, bans `/// ` and enforces using an `import` instead: + + + +#### ❌ Incorrect + +```ts option='{ "path": "never" }' +/// + +globalThis.value; ``` -Examples of **incorrect** code for the `{ "types": "prefer-import" }` option. Note that these are only errors when **both** styles are used for the **same** module: +#### ✅ Correct -```ts option='{ "types": "prefer-import" }' showPlaygroundButton -/// -import * as foo from 'foo'; +```ts option='{ "path": "never" }' +import { value } from 'code'; ``` -```ts option='{ "types": "prefer-import" }' showPlaygroundButton -/// -import foo = require('foo'); +### `types` + +When set to `'never'`, bans `/// ` and enforces using an `import` instead: + + + +#### ❌ Incorrect + +```ts option='{ "types": "never" }' +/// + +globalThis.value; ``` -With `{ "path": "always", "types": "always", "lib": "always" }` options set, the following will all be **correct** usage: +#### ✅ Correct -```ts option='{ "path": "always", "types": "always", "lib": "always" }' showPlaygroundButton -/// -/// -/// +```ts option='{ "types": "never" }' +import { value } from 'code'; ``` -Examples of **correct** code for the `{ "types": "prefer-import" }` option: + -```ts option='{ "types": "prefer-import" }' showPlaygroundButton -import * as foo from 'foo'; +The `types` option may alternately be given a `"prefer-import"` value. +Doing so indicates the rule should only report if there is already an `import` from the same location: + + + +#### ❌ Incorrect + +```ts option='{ "types": "prefer-import" }' +/// + +import { valueA } from 'code'; + +globalThis.valueB; ``` -```ts option='{ "types": "prefer-import" }' showPlaygroundButton -import foo = require('foo'); +#### ✅ Correct + +```ts option='{ "types": "prefer-import" }' +import { valueA, valueB } from 'code'; ``` ## When To Use It diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts index ce9f3115a02f..82d39aa9ff4a 100644 --- a/packages/eslint-plugin/tests/docs.test.ts +++ b/packages/eslint-plugin/tests/docs.test.ts @@ -47,6 +47,9 @@ describe('Validating rule docs', () => { 'camelcase.md', 'no-duplicate-imports.md', ]); + + const rulesWithComplexOptions = new Set(['array-type', 'member-ordering']); + it('All rules must have a corresponding rule doc', () => { const files = fs .readdirSync(docsRoot) @@ -93,13 +96,13 @@ describe('Validating rule docs', () => { test(`headers must be title-cased`, () => { // Get all H2 headers objects as the other levels are variable by design. - const headers = tokens.filter(tokenIsH2); + const h2s = tokens.filter(tokenIsH2); - headers.forEach(header => - expect(header.text).toBe(titleCase(header.text)), - ); + h2s.forEach(h2 => expect(h2.text).toBe(titleCase(h2.text))); }); + const headers = tokens.filter(tokenIsHeading); + const importantHeadings = new Set([ 'How to Use', 'Options', @@ -108,14 +111,41 @@ describe('Validating rule docs', () => { ]); test('important headings must be h2s', () => { - const headers = tokens.filter(tokenIsHeading); - for (const header of headers) { if (importantHeadings.has(header.raw.replace(/#/g, '').trim())) { expect(header.depth).toBe(2); } } }); + + const { schema } = rule.meta; + if ( + !rulesWithComplexOptions.has(ruleName) && + Array.isArray(schema) && + !rule.meta.docs?.extendsBaseRule && + rule.meta.type !== 'layout' + ) { + test('each rule option should be mentioned in a header', () => { + const headerTextAfterOptions = headers + .slice(headers.findIndex(header => header.text === 'Options')) + .map(header => header.text) + .join('\n'); + + for (const schemaItem of schema) { + if (schemaItem.type === 'object') { + for (const property of Object.keys( + schemaItem.properties as object, + )) { + if (!headerTextAfterOptions.includes(`\`${property}\``)) { + throw new Error( + `At least one header should include \`${property}\`.`, + ); + } + } + } + } + }); + } }); } }); From 0f297be0e06d6596d888c4330628f7ffabb4d9e3 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 24 Dec 2023 15:11:45 -0500 Subject: [PATCH 2/2] Fix lint --- packages/eslint-plugin/tests/docs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts index dc117ea4b437..228d97aa2ef2 100644 --- a/packages/eslint-plugin/tests/docs.test.ts +++ b/packages/eslint-plugin/tests/docs.test.ts @@ -193,7 +193,7 @@ describe('Validating rule metadata', () => { } for (const [ruleName, rule] of rulesData) { - describe(`${ruleName}`, () => { + describe(ruleName, () => { it('`name` field in rule must match the filename', () => { // validate if rule name is same as url // there is no way to access this field but its used only in generation of docs url