From 2b3b5d6d144f1df152385d8460b23b514c7c518b Mon Sep 17 00:00:00 2001 From: Alexander T Date: Mon, 18 Nov 2019 20:02:11 +0200 Subject: [PATCH 01/23] feat(eslint-plugin): add no-extra-non-null-assertion (#1183) Co-authored-by: Brad Zacher --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-extra-non-null-assertion.md | 37 +++++++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-extra-non-null-assertion.ts | 25 +++++++ .../rules/no-extra-non-null-assertion.test.ts | 66 +++++++++++++++++++ 6 files changed, 132 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-extra-non-null-assertion.md create mode 100644 packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts create mode 100644 packages/eslint-plugin/tests/rules/no-extra-non-null-assertion.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 4a7557f6f46c..76b68e32a92f 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -164,6 +164,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | :heavy_check_mark: | | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-extra-non-null-assertion`](./docs/rules/no-extra-non-null-assertion.md) | Disallow extra non-null assertion | | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | | [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately. | | | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/no-extra-non-null-assertion.md b/packages/eslint-plugin/docs/rules/no-extra-non-null-assertion.md new file mode 100644 index 000000000000..37f4d2e3a52f --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-extra-non-null-assertion.md @@ -0,0 +1,37 @@ +# Disallow extra non-null assertion + +## Rule Details + +Examples of **incorrect** code for this rule: + +```ts +const foo: { bar: number } | null = null; +const bar = foo!!!.bar; +``` + +```ts +function foo(bar: number | undefined) { + const bar: number = bar!!!; +} +``` + +Examples of **correct** code for this rule: + +```ts +const foo: { bar: number } | null = null; +const bar = foo!.bar; +``` + +```ts +function foo(bar: number | undefined) { + const bar: number = bar!; +} +``` + +## How to use + +```json +{ + "@typescript-eslint/no-extra-non-null-assertion": ["error"] +} +``` diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 53d1cf8a87f6..96d264106cc5 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -32,6 +32,7 @@ "@typescript-eslint/no-empty-interface": "error", "@typescript-eslint/no-explicit-any": "error", "no-extra-parens": "off", + "@typescript-eslint/no-extra-non-null-assertion": "error", "@typescript-eslint/no-extra-parens": "error", "@typescript-eslint/no-extraneous-class": "error", "@typescript-eslint/no-floating-promises": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d706378e9340..d28b23edca6b 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -22,6 +22,7 @@ import noDynamicDelete from './no-dynamic-delete'; import noEmptyFunction from './no-empty-function'; import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; +import noExtraNonNullAssertion from './no-extra-non-null-assertion'; import noExtraParens from './no-extra-parens'; import noExtraneousClass from './no-extraneous-class'; import noFloatingPromises from './no-floating-promises'; @@ -93,6 +94,7 @@ export default { 'no-empty-function': noEmptyFunction, 'no-empty-interface': noEmptyInterface, 'no-explicit-any': noExplicitAny, + 'no-extra-non-null-assertion': noExtraNonNullAssertion, 'no-extra-parens': noExtraParens, 'no-extraneous-class': noExtraneousClass, 'no-floating-promises': noFloatingPromises, diff --git a/packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts b/packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts new file mode 100644 index 000000000000..cd116a3a4d7b --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts @@ -0,0 +1,25 @@ +import * as util from '../util'; + +export default util.createRule({ + name: 'no-extra-non-null-assertion', + meta: { + type: 'problem', + docs: { + description: 'Disallow extra non-null assertion', + category: 'Stylistic Issues', + recommended: false, + }, + schema: [], + messages: { + noExtraNonNullAssertion: 'Forbidden extra non-null assertion.', + }, + }, + defaultOptions: [], + create(context) { + return { + 'TSNonNullExpression > TSNonNullExpression'(node): void { + context.report({ messageId: 'noExtraNonNullAssertion', node }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-extra-non-null-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-extra-non-null-assertion.test.ts new file mode 100644 index 000000000000..a851fe9a0604 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-extra-non-null-assertion.test.ts @@ -0,0 +1,66 @@ +import rule from '../../src/rules/no-extra-non-null-assertion'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-extra-non-null-assertion', rule, { + valid: [ + { + code: ` +const foo: { bar: number } | null = null; +const bar = foo!.bar; + `, + }, + { + code: ` +function foo(bar: number | undefined) { + const bar: number = bar!; +} `, + }, + ], + invalid: [ + { + code: ` +const foo: { bar: number } | null = null; +const bar = foo!!!!.bar; + `, + errors: [ + { + messageId: 'noExtraNonNullAssertion', + endColumn: 19, + column: 13, + line: 3, + }, + { + messageId: 'noExtraNonNullAssertion', + endColumn: 18, + column: 13, + line: 3, + }, + { + messageId: 'noExtraNonNullAssertion', + endColumn: 17, + column: 13, + line: 3, + }, + ], + }, + { + code: ` +function foo(bar: number | undefined) { + const bar: number = bar!!; +} + `, + errors: [ + { + messageId: 'noExtraNonNullAssertion', + endColumn: 27, + column: 23, + line: 3, + }, + ], + }, + ], +}); From 45500155a653db502d059d8dc444161309e461b7 Mon Sep 17 00:00:00 2001 From: Jamison Dance Date: Mon, 18 Nov 2019 18:29:47 -0700 Subject: [PATCH 02/23] docs: tweak grammar (#1227) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f296ee034eff..1f6d7c3cac1d 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ This is not valid JavaScript code, because it contains a so-called type annotati However, we can leverage the fact that ESLint has been designed with these use-cases in mind! -It turns out that ESLint is not just comprised of one library, instead it is comprised of a few important moving parts. One of those moving parts is **the parser**. ESLint ships with a parser built in (called [`espree`](https://github.com/eslint/espree)), and so if you only ever write standard JavaScript, you don't need to care about this implementation detail. +It turns out that ESLint is not just one library. Instead it is composed of a few important moving parts. One of those moving parts is **the parser**. ESLint ships with a parser built in (called [`espree`](https://github.com/eslint/espree)), and so if you only ever write standard JavaScript, you don't need to care about this implementation detail. The great thing is, though, if we want to support non-standard JavaScript syntax, all we need to do is provide ESLint with an alternative parser to use - that is a first-class use-case offered by ESLint. From dd9f58ccfcfe982d299f0de639b9eee26b27d1bc Mon Sep 17 00:00:00 2001 From: garyking Date: Tue, 19 Nov 2019 01:01:26 -0500 Subject: [PATCH 03/23] docs(eslint-plugin): fix title in no-unused-expressions docs (#1230) --- packages/eslint-plugin/docs/rules/no-unused-expressions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unused-expressions.md b/packages/eslint-plugin/docs/rules/no-unused-expressions.md index 7da998ab2c6c..e1a86449181e 100644 --- a/packages/eslint-plugin/docs/rules/no-unused-expressions.md +++ b/packages/eslint-plugin/docs/rules/no-unused-expressions.md @@ -1,4 +1,4 @@ -# require or disallow semicolons instead of ASI (semi) +# Disallow Unused Expressions (no-unused-expressions) This rule aims to eliminate unused expressions which have no effect on the state of the program. From 56c00b32e47967ec0fb065e49063317e02181d0b Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 19 Nov 2019 09:09:23 -0800 Subject: [PATCH 04/23] fix(eslint-plugin): [req-await] crash on nonasync promise return (#1228) --- .../eslint-plugin/src/rules/require-await.ts | 19 +- .../tests/rules/require-await.test.ts | 181 ++++++++---------- .../eslint-plugin/typings/eslint-rules.d.ts | 2 +- 3 files changed, 90 insertions(+), 112 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 5f27f8ad8a1d..4db47f9e075b 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -42,13 +42,12 @@ export default util.createRule({ } return { - 'FunctionDeclaration[async = true]': rules.FunctionDeclaration, - 'FunctionExpression[async = true]': rules.FunctionExpression, + FunctionDeclaration: rules.FunctionDeclaration, + FunctionExpression: rules.FunctionExpression, + ArrowFunctionExpression: rules.ArrowFunctionExpression, 'ArrowFunctionExpression[async = true]'( node: TSESTree.ArrowFunctionExpression, ): void { - rules.ArrowFunctionExpression(node); - // If body type is not BlockStatment, we need to check the return type here if (node.body.type !== AST_NODE_TYPES.BlockStatement) { const expression = parserServices.esTreeNodeToTSNodeMap.get( @@ -56,15 +55,13 @@ export default util.createRule({ ); if (expression && isThenableType(expression)) { // tell the base rule to mark the scope as having an await so it ignores it - rules.AwaitExpression(node as never); + rules.AwaitExpression(); } } }, - 'FunctionDeclaration[async = true]:exit': - rules['FunctionDeclaration:exit'], - 'FunctionExpression[async = true]:exit': rules['FunctionExpression:exit'], - 'ArrowFunctionExpression[async = true]:exit': - rules['ArrowFunctionExpression:exit'], + 'FunctionDeclaration:exit': rules['FunctionDeclaration:exit'], + 'FunctionExpression:exit': rules['FunctionExpression:exit'], + 'ArrowFunctionExpression:exit': rules['ArrowFunctionExpression:exit'], AwaitExpression: rules.AwaitExpression, ForOfStatement: rules.ForOfStatement, @@ -74,7 +71,7 @@ export default util.createRule({ >(node); if (expression && isThenableType(expression)) { // tell the base rule to mark the scope as having an await so it ignores it - rules.AwaitExpression(node as never); + rules.AwaitExpression(); } }, }; diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 09b54bd3a99d..fff294f7ccb5 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -28,106 +28,87 @@ const noAwaitAsyncFunctionExpression: any = { ruleTester.run('require-await', rule, { valid: [ - { - // Non-async function declaration - code: `function numberOne(): number { - return 1; - }`, - }, - { - // Non-async function expression - code: `const numberOne = function(): number { - return 1; - }`, - }, - { - // Non-async arrow function expression (concise-body) - code: `const numberOne = (): number => 1;`, - }, - { - // Non-async arrow function expression (block-body) - code: `const numberOne = (): number => { - return 1; - };`, - }, - { - // Async function declaration with await - code: `async function numberOne(): Promise { - return await 1; - }`, - }, - { - // Async function expression with await - code: `const numberOne = async function(): Promise { - return await 1; - }`, - }, - { - // Async arrow function expression with await (concise-body) - code: `const numberOne = async (): Promise => await 1;`, - }, - { - // Async arrow function expression with await (block-body) - code: `const numberOne = async (): Promise => { - return await 1; - };`, - }, - { - // Async function declaration with promise return - code: `async function numberOne(): Promise { - return Promise.resolve(1); - }`, - }, - { - // Async function expression with promise return - code: `const numberOne = async function(): Promise { - return Promise.resolve(1); - }`, - }, - { - // Async arrow function with promise return (concise-body) - code: `const numberOne = async (): Promise => Promise.resolve(1);`, - }, - { - // Async arrow function with promise return (block-body) - code: `const numberOne = async (): Promise => { - return Promise.resolve(1); - };`, - }, - { - // Async function declaration with async function return - code: `async function numberOne(): Promise { - return getAsyncNumber(1); - } - async function getAsyncNumber(x: number): Promise { - return Promise.resolve(x); - }`, - }, - { - // Async function expression with async function return - code: `const numberOne = async function(): Promise { - return getAsyncNumber(1); - } - const getAsyncNumber = async function(x: number): Promise { - return Promise.resolve(x); - }`, - }, - { - // Async arrow function with async function return (concise-body) - code: `const numberOne = async (): Promise => getAsyncNumber(1); - const getAsyncNumber = async function(x: number): Promise { - return Promise.resolve(x); - }`, - }, - { - // Async arrow function with async function return (block-body) - code: `const numberOne = async (): Promise => { - return getAsyncNumber(1); - }; - const getAsyncNumber = async function(x: number): Promise { - return Promise.resolve(x); - }`, - }, + // Non-async function declaration + `function numberOne(): number { + return 1; + }`, + // Non-async function expression + `const numberOne = function(): number { + return 1; + }`, + // Non-async arrow function expression (concise-body) + `const numberOne = (): number => 1;`, + // Non-async arrow function expression (block-body) + `const numberOne = (): number => { + return 1; + };`, + // Non-async function that returns a promise + // https://github.com/typescript-eslint/typescript-eslint/issues/1226 + ` +function delay() { + return Promise.resolve(); +} + `, + ` +const delay = () => { + return Promise.resolve(); +} + `, + `const delay = () => Promise.resolve();`, + // Async function declaration with await + `async function numberOne(): Promise { + return await 1; + }`, + // Async function expression with await + `const numberOne = async function(): Promise { + return await 1; + }`, + // Async arrow function expression with await (concise-body) + `const numberOne = async (): Promise => await 1;`, + // Async arrow function expression with await (block-body) + `const numberOne = async (): Promise => { + return await 1; + };`, + // Async function declaration with promise return + `async function numberOne(): Promise { + return Promise.resolve(1); + }`, + // Async function expression with promise return + `const numberOne = async function(): Promise { + return Promise.resolve(1); + }`, + // Async arrow function with promise return (concise-body) + `const numberOne = async (): Promise => Promise.resolve(1);`, + // Async arrow function with promise return (block-body) + `const numberOne = async (): Promise => { + return Promise.resolve(1); + };`, + // Async function declaration with async function return + `async function numberOne(): Promise { + return getAsyncNumber(1); + } + async function getAsyncNumber(x: number): Promise { + return Promise.resolve(x); + }`, + // Async function expression with async function return + `const numberOne = async function(): Promise { + return getAsyncNumber(1); + } + const getAsyncNumber = async function(x: number): Promise { + return Promise.resolve(x); + }`, + // Async arrow function with async function return (concise-body) + `const numberOne = async (): Promise => getAsyncNumber(1); + const getAsyncNumber = async function(x: number): Promise { + return Promise.resolve(x); + }`, + // Async arrow function with async function return (block-body) + `const numberOne = async (): Promise => { + return getAsyncNumber(1); + }; + const getAsyncNumber = async function(x: number): Promise { + return Promise.resolve(x); + }`, // https://github.com/typescript-eslint/typescript-eslint/issues/1188 ` async function testFunction(): Promise { diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 8037e624c4f2..2ec44b6dbc20 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -451,7 +451,7 @@ declare module 'eslint/lib/rules/require-await' { node: TSESTree.ArrowFunctionExpression, ): void; ReturnStatement(node: TSESTree.ReturnStatement): void; - AwaitExpression(node: TSESTree.AwaitExpression): void; + AwaitExpression(): void; ForOfStatement(node: TSESTree.ForOfStatement): void; } >; From f1ab9a248c986e3ceda40ec385a104cae3d2955f Mon Sep 17 00:00:00 2001 From: Alexander T Date: Tue, 19 Nov 2019 19:28:15 +0200 Subject: [PATCH 05/23] feat(eslint-plugin): [no-extran-class] add allowWithDecorator opt (#886) Co-authored-by: Brad Zacher --- .../docs/rules/no-extraneous-class.md | 22 ++++++++++++++--- .../src/rules/no-extraneous-class.ts | 24 ++++++++++++++++--- .../tests/rules/no-extraneous-class.test.ts | 19 +++++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-extraneous-class.md b/packages/eslint-plugin/docs/rules/no-extraneous-class.md index 882d7e26a554..3c1fa0528351 100644 --- a/packages/eslint-plugin/docs/rules/no-extraneous-class.md +++ b/packages/eslint-plugin/docs/rules/no-extraneous-class.md @@ -50,9 +50,25 @@ const StaticOnly = { This rule accepts a single object option. -- `allowConstructorOnly: true` will silence warnings about classes containing only a constructor. -- `allowEmpty: true` will silence warnings about empty classes. -- `allowStaticOnly: true` will silence warnings about classes containing only static members. +```ts +type Options = { + // allow extraneous classes if they only contain a constructor + allowConstructorOnly?: boolean; + // allow extraneous classes if they have no body (i.e. are empty) + allowEmpty?: boolean; + // allow extraneous classes if they only contain static members + allowStaticOnly?: boolean; + // allow extraneous classes if they are have a decorator + allowWithDecorator?: boolean; +}; + +const defaultOptions: Options = { + allowConstructorOnly: false, + allowEmpty: false, + allowStaticOnly: false, + allowWithDecorator: false, +}; +``` ## When Not To Use It diff --git a/packages/eslint-plugin/src/rules/no-extraneous-class.ts b/packages/eslint-plugin/src/rules/no-extraneous-class.ts index 4756b5928f33..996dd0ea71cd 100644 --- a/packages/eslint-plugin/src/rules/no-extraneous-class.ts +++ b/packages/eslint-plugin/src/rules/no-extraneous-class.ts @@ -9,6 +9,7 @@ type Options = [ allowConstructorOnly?: boolean; allowEmpty?: boolean; allowStaticOnly?: boolean; + allowWithDecorator?: boolean; }, ]; type MessageIds = 'empty' | 'onlyStatic' | 'onlyConstructor'; @@ -36,6 +37,9 @@ export default util.createRule({ allowStaticOnly: { type: 'boolean', }, + allowWithDecorator: { + type: 'boolean', + }, }, }, ], @@ -50,9 +54,24 @@ export default util.createRule({ allowConstructorOnly: false, allowEmpty: false, allowStaticOnly: false, + allowWithDecorator: false, }, ], - create(context, [{ allowConstructorOnly, allowEmpty, allowStaticOnly }]) { + create( + context, + [{ allowConstructorOnly, allowEmpty, allowStaticOnly, allowWithDecorator }], + ) { + const isAllowWithDecorator = ( + node: TSESTree.ClassDeclaration | TSESTree.ClassExpression | undefined, + ): boolean => { + return !!( + allowWithDecorator && + node && + node.decorators && + node.decorators.length + ); + }; + return { ClassBody(node): void { const parent = node.parent as @@ -65,9 +84,8 @@ export default util.createRule({ } const reportNode = 'id' in parent && parent.id ? parent.id : parent; - if (node.body.length === 0) { - if (allowEmpty) { + if (allowEmpty || isAllowWithDecorator(parent)) { return; } diff --git a/packages/eslint-plugin/tests/rules/no-extraneous-class.test.ts b/packages/eslint-plugin/tests/rules/no-extraneous-class.test.ts index 1d2742ed0402..e009aa153ae8 100644 --- a/packages/eslint-plugin/tests/rules/no-extraneous-class.test.ts +++ b/packages/eslint-plugin/tests/rules/no-extraneous-class.test.ts @@ -65,6 +65,13 @@ export class Bar { }, // https://github.com/typescript-eslint/typescript-eslint/issues/170 'export default class { hello() { return "I am foo!"; } }', + { + code: ` +@FooDecorator +class Foo {} + `, + options: [{ allowWithDecorator: true }], + }, ], invalid: [ @@ -125,5 +132,17 @@ export class AClass { }, ], }, + { + code: ` +@FooDecorator +class Foo {} + `, + options: [{ allowWithDecorator: false }], + errors: [ + { + messageId: 'empty', + }, + ], + }, ], }); From 6cfd468f79df4edd3c2e2a082004361bfd37da36 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 19 Nov 2019 09:57:12 -0800 Subject: [PATCH 06/23] fix(eslint-plugin): [no-untyped-pub-sig] constructor return (#1231) --- .../src/rules/no-untyped-public-signature.ts | 5 +- .../rules/no-untyped-public-signature.test.ts | 52 ++++++++++++++++--- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts b/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts index 2ecb1cbd62c6..39a635b850e7 100644 --- a/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts +++ b/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts @@ -106,7 +106,10 @@ export default util.createRule({ }); } - if (!isReturnTyped(node.value.returnType)) { + if ( + node.kind !== 'constructor' && + !isReturnTyped(node.value.returnType) + ) { context.report({ node, messageId: 'noReturnType', diff --git a/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts b/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts index 1a167d64e30d..164f11b4bd76 100644 --- a/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts +++ b/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts @@ -28,7 +28,7 @@ ruleTester.run('no-untyped-public-signature', rule, { code: ` class A { public b(c: string):void { - + } }`, }, @@ -36,7 +36,7 @@ ruleTester.run('no-untyped-public-signature', rule, { code: ` class A { public b(...c):void { - + } }`, }, @@ -44,7 +44,7 @@ ruleTester.run('no-untyped-public-signature', rule, { code: ` class A { b(c):void { - + } }`, options: [{ ignoredMethods: ['b'] }], @@ -71,22 +71,43 @@ ruleTester.run('no-untyped-public-signature', rule, { code: ` class A { b(...c):void { - + } - + d(c):void { - + } }`, options: [{ ignoredMethods: ['b', 'd'] }], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1229 + ` +class Foo { + constructor() {} +} + `, + ` +class Foo { + abstract constructor() {} +} + `, + ` +class Foo { + constructor(c: string) {} +} + `, + ` +class Foo { + abstract constructor(c: string) {} +} + `, ], invalid: [ //untyped parameter { code: `class A { public b(c):void { - + } }`, errors: [{ messageId: 'untypedParameter' }], @@ -206,5 +227,22 @@ ruleTester.run('no-untyped-public-signature', rule, { }`, errors: [{ messageId: 'noReturnType' }], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1229 + { + code: ` +class Foo { + constructor(c) {} +} + `, + errors: [{ messageId: 'untypedParameter' }], + }, + { + code: ` +class Foo { + abstract constructor(c) {} +} + `, + errors: [{ messageId: 'untypedParameter' }], + }, ], }); From 5b1300da3c5f3346bc9d74a59be67efab283b6f9 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 21 Nov 2019 01:39:36 +0200 Subject: [PATCH 07/23] fix(eslint-plugin): [no-dynamic-delete] correct invalid fixer for identifiers (#1244) --- .../src/rules/no-dynamic-delete.ts | 8 ++----- .../tests/rules/no-dynamic-delete.test.ts | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts index 0ee1636ea643..6fbdba1058fc 100644 --- a/packages/eslint-plugin/src/rules/no-dynamic-delete.ts +++ b/packages/eslint-plugin/src/rules/no-dynamic-delete.ts @@ -33,14 +33,10 @@ export default util.createRule({ ) { return createPropertyReplacement( member.property, - member.property.value, + `.${member.property.value}`, ); } - if (member.property.type === AST_NODE_TYPES.Identifier) { - return createPropertyReplacement(member.property, member.property.name); - } - return undefined; } @@ -69,7 +65,7 @@ export default util.createRule({ replacement: string, ) { return (fixer: TSESLint.RuleFixer): TSESLint.RuleFix => - fixer.replaceTextRange(getTokenRange(property), `.${replacement}`); + fixer.replaceTextRange(getTokenRange(property), replacement); } function getTokenRange(property: TSESTree.Expression): [number, number] { diff --git a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts index e3b4e5366e8b..aa12c33c9e50 100644 --- a/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts +++ b/packages/eslint-plugin/tests/rules/no-dynamic-delete.test.ts @@ -52,6 +52,8 @@ ruleTester.run('no-dynamic-delete', rule, { code: `const container: { [i: string]: 0 } = {}; delete container['aa' + 'b'];`, errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container['aa' + 'b'];`, }, { code: `const container: { [i: string]: 0 } = {}; @@ -64,16 +66,22 @@ ruleTester.run('no-dynamic-delete', rule, { code: `const container: { [i: string]: 0 } = {}; delete container[-Infinity];`, errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container[-Infinity];`, }, { code: `const container: { [i: string]: 0 } = {}; delete container[+Infinity];`, errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container[+Infinity];`, }, { code: `const container: { [i: string]: 0 } = {}; delete container[NaN];`, errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + delete container[NaN];`, }, { code: `const container: { [i: string]: 0 } = {}; @@ -94,11 +102,26 @@ ruleTester.run('no-dynamic-delete', rule, { const name = 'name'; delete container[name];`, errors: [{ messageId: 'dynamicDelete' }], + output: `const container: { [i: string]: 0 } = {}; + const name = 'name'; + delete container[name];`, }, { code: `const container: { [i: string]: 0 } = {}; const getName = () => 'aaa'; delete container[getName()];`, + output: `const container: { [i: string]: 0 } = {}; + const getName = () => 'aaa'; + delete container[getName()];`, + errors: [{ messageId: 'dynamicDelete' }], + }, + { + code: `const container: { [i: string]: 0 } = {}; + const name = { foo: { bar: 'bar' } }; + delete container[name.foo.bar];`, + output: `const container: { [i: string]: 0 } = {}; + const name = { foo: { bar: 'bar' } }; + delete container[name.foo.bar];`, errors: [{ messageId: 'dynamicDelete' }], }, ], From d97f809c673c57be7e41b2121c4a1c8408d3f47c Mon Sep 17 00:00:00 2001 From: Eric Wang Date: Fri, 22 Nov 2019 04:13:20 +1100 Subject: [PATCH 08/23] fix(typescript-estree): fix synthetic default import (#1245) --- packages/typescript-estree/src/visitor-keys.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/typescript-estree/src/visitor-keys.ts b/packages/typescript-estree/src/visitor-keys.ts index e594fb240e05..d7ee3ed356fc 100644 --- a/packages/typescript-estree/src/visitor-keys.ts +++ b/packages/typescript-estree/src/visitor-keys.ts @@ -1,4 +1,4 @@ -import eslintVisitorKeys from 'eslint-visitor-keys'; +import * as eslintVisitorKeys from 'eslint-visitor-keys'; export const visitorKeys = eslintVisitorKeys.unionWith({ // Additional estree nodes. From fa88cb940af87c371946d83fbd31fb0b8007ff06 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 21 Nov 2019 19:16:18 +0200 Subject: [PATCH 09/23] feat(eslint-plugin): [restrict-plus-operands] check += (#892) Co-authored-by: Brad Zacher --- .../docs/rules/restrict-plus-operands.md | 31 +++++++ .../src/rules/restrict-plus-operands.ts | 89 ++++++++++++------- .../rules/restrict-plus-operands.test.ts | 58 ++++++++++++ 3 files changed, 148 insertions(+), 30 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/restrict-plus-operands.md b/packages/eslint-plugin/docs/rules/restrict-plus-operands.md index 1efffa83e69e..7b5af55a84ff 100644 --- a/packages/eslint-plugin/docs/rules/restrict-plus-operands.md +++ b/packages/eslint-plugin/docs/rules/restrict-plus-operands.md @@ -16,6 +16,37 @@ var foo = 1n + 1; ## Options +This rule has an object option: + +- `"checkCompoundAssignments": false`: (default) does not check compound assignments (`+=`) +- `"checkCompoundAssignments": true` + +### checkCompoundAssignments + +Examples of **incorrect** code for the `{ "checkCompoundAssignments": true }` option: + +```ts +/*eslint @typescript-eslint/restrict-plus-operands: ["error", { "checkCompoundAssignments": true }]*/ + +let foo: string | undefined; +foo += 'some data'; + +let bar: string = ''; +bar += 0; +``` + +Examples of **correct** code for the `{ "checkCompoundAssignments": true }` option: + +```ts +/*eslint @typescript-eslint/restrict-plus-operands: ["error", { "checkCompoundAssignments": true }]*/ + +let foo: number = 0; +foo += 1; + +let bar = ''; +bar += 'test'; +``` + ```json { "@typescript-eslint/restrict-plus-operands": "error" diff --git a/packages/eslint-plugin/src/rules/restrict-plus-operands.ts b/packages/eslint-plugin/src/rules/restrict-plus-operands.ts index 1f715f1f9022..c41587a169ac 100644 --- a/packages/eslint-plugin/src/rules/restrict-plus-operands.ts +++ b/packages/eslint-plugin/src/rules/restrict-plus-operands.ts @@ -2,7 +2,14 @@ import { TSESTree } from '@typescript-eslint/experimental-utils'; import ts from 'typescript'; import * as util from '../util'; -export default util.createRule({ +type Options = [ + { + checkCompoundAssignments?: boolean; + }, +]; +type MessageIds = 'notNumbers' | 'notStrings' | 'notBigInts'; + +export default util.createRule({ name: 'restrict-plus-operands', meta: { type: 'problem', @@ -20,12 +27,25 @@ export default util.createRule({ "Operands of '+' operation must either be both strings or both numbers. Consider using a template literal.", notBigInts: "Operands of '+' operation must be both bigints.", }, - schema: [], + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + checkCompoundAssignments: { + type: 'boolean', + }, + }, + }, + ], }, - defaultOptions: [], - create(context) { + defaultOptions: [ + { + checkCompoundAssignments: false, + }, + ], + create(context, [{ checkCompoundAssignments }]) { const service = util.getParserServices(context); - const typeChecker = service.program.getTypeChecker(); type BaseLiteral = 'string' | 'number' | 'bigint' | 'invalid'; @@ -83,32 +103,41 @@ export default util.createRule({ return getBaseTypeOfLiteralType(type); } - return { - "BinaryExpression[operator='+']"(node: TSESTree.BinaryExpression): void { - const leftType = getNodeType(node.left); - const rightType = getNodeType(node.right); + function checkPlusOperands( + node: TSESTree.BinaryExpression | TSESTree.AssignmentExpression, + ): void { + const leftType = getNodeType(node.left); + const rightType = getNodeType(node.right); - if ( - leftType === 'invalid' || - rightType === 'invalid' || - leftType !== rightType - ) { - if (leftType === 'string' || rightType === 'string') { - context.report({ - node, - messageId: 'notStrings', - }); - } else if (leftType === 'bigint' || rightType === 'bigint') { - context.report({ - node, - messageId: 'notBigInts', - }); - } else { - context.report({ - node, - messageId: 'notNumbers', - }); - } + if ( + leftType === 'invalid' || + rightType === 'invalid' || + leftType !== rightType + ) { + if (leftType === 'string' || rightType === 'string') { + context.report({ + node, + messageId: 'notStrings', + }); + } else if (leftType === 'bigint' || rightType === 'bigint') { + context.report({ + node, + messageId: 'notBigInts', + }); + } else { + context.report({ + node, + messageId: 'notNumbers', + }); + } + } + } + + return { + "BinaryExpression[operator='+']": checkPlusOperands, + "AssignmentExpression[operator='+=']"(node): void { + if (checkCompoundAssignments) { + checkPlusOperands(node); } }, }; diff --git a/packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts b/packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts index 44583a202a1d..5d979706f84d 100644 --- a/packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts +++ b/packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts @@ -81,6 +81,28 @@ function foo(a: T) { return a + 1; } `, + { + code: ` +let foo: number = 0; +foo += 1; + `, + options: [ + { + checkCompoundAssignments: false, + }, + ], + }, + { + code: ` +let foo: number = 0; +foo += "string"; + `, + options: [ + { + checkCompoundAssignments: false, + }, + ], + }, ], invalid: [ { @@ -383,5 +405,41 @@ function foo(a: T) { }, ], }, + { + code: ` +let foo: string | undefined; +foo += "some data"; + `, + options: [ + { + checkCompoundAssignments: true, + }, + ], + errors: [ + { + messageId: 'notStrings', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +let foo = ''; +foo += 0; + `, + options: [ + { + checkCompoundAssignments: true, + }, + ], + errors: [ + { + messageId: 'notStrings', + line: 3, + column: 1, + }, + ], + }, ], }); From ceb6f1ce225ad663111dd9d19083d20e1a45d30f Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 21 Nov 2019 19:17:50 +0200 Subject: [PATCH 10/23] feat(eslint-plugin): [no-unnece-cond] Add allowConstantLoopConditions (#1029) Co-authored-by: Brad Zacher --- .../docs/rules/no-unnecessary-condition.md | 10 +++ .../src/rules/no-unnecessary-condition.ts | 63 ++++++++++++++----- .../rules/no-unnecessary-condition.test.ts | 21 +++++++ 3 files changed, 77 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md index 48b98849c41e..4bc160e68ccd 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md @@ -53,6 +53,16 @@ function head(items: T[]) { } ``` +- `allowConstantLoopConditions` (default `false`) - allows constant expressions in loops. + +Example of correct code for when `allowConstantLoopConditions` is `true`: + +```ts +while (true) {} +for (; true; ) {} +do {} while (true); +``` + ## When Not To Use It The main downside to using this rule is the need for type information. diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 8e75f7adb3fd..420271a23b86 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -41,15 +41,9 @@ const isLiteral = (type: ts.Type): boolean => isLiteralType(type); // #endregion -type ExpressionWithTest = - | TSESTree.ConditionalExpression - | TSESTree.DoWhileStatement - | TSESTree.ForStatement - | TSESTree.IfStatement - | TSESTree.WhileStatement; - export type Options = [ { + allowConstantLoopConditions?: boolean; ignoreRhs?: boolean; }, ]; @@ -74,6 +68,9 @@ export default createRule({ { type: 'object', properties: { + allowConstantLoopConditions: { + type: 'boolean', + }, ignoreRhs: { type: 'boolean', }, @@ -91,10 +88,11 @@ export default createRule({ }, defaultOptions: [ { + allowConstantLoopConditions: false, ignoreRhs: false, }, ], - create(context, [{ ignoreRhs }]) { + create(context, [{ allowConstantLoopConditions, ignoreRhs }]) { const service = getParserServices(context); const checker = service.program.getTypeChecker(); @@ -165,14 +163,13 @@ export default createRule({ * Filters all LogicalExpressions to prevent some duplicate reports. */ function checkIfTestExpressionIsNecessaryConditional( - node: ExpressionWithTest, + node: TSESTree.ConditionalExpression | TSESTree.IfStatement, ): void { - if ( - node.test !== null && - node.test.type !== AST_NODE_TYPES.LogicalExpression - ) { - checkNode(node.test); + if (node.test.type === AST_NODE_TYPES.LogicalExpression) { + return; } + + checkNode(node.test); } /** @@ -187,14 +184,46 @@ export default createRule({ } } + /** + * Checks that a testable expression of a loop is necessarily conditional, reports otherwise. + */ + function checkIfLoopIsNecessaryConditional( + node: + | TSESTree.DoWhileStatement + | TSESTree.ForStatement + | TSESTree.WhileStatement, + ): void { + if ( + node.test === null || + node.test.type === AST_NODE_TYPES.LogicalExpression + ) { + return; + } + + /** + * Allow: + * while (true) {} + * for (;true;) {} + * do {} while (true) + */ + if ( + allowConstantLoopConditions && + isBooleanLiteralType(getNodeType(node.test), true) + ) { + return; + } + + checkNode(node.test); + } + return { BinaryExpression: checkIfBinaryExpressionIsNecessaryConditional, ConditionalExpression: checkIfTestExpressionIsNecessaryConditional, - DoWhileStatement: checkIfTestExpressionIsNecessaryConditional, - ForStatement: checkIfTestExpressionIsNecessaryConditional, + DoWhileStatement: checkIfLoopIsNecessaryConditional, + ForStatement: checkIfLoopIsNecessaryConditional, IfStatement: checkIfTestExpressionIsNecessaryConditional, - WhileStatement: checkIfTestExpressionIsNecessaryConditional, LogicalExpression: checkLogicalExpressionForUnnecessaryConditionals, + WhileStatement: checkIfLoopIsNecessaryConditional, }; }, }); diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index c5d90fd01efa..30e511ac119d 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -97,6 +97,14 @@ declare const b2: true; if(b1 && b2) {}`, options: [{ ignoreRhs: true }], }, + { + code: ` +while(true) {} +for (;true;) {} +do {} while(true) + `, + options: [{ allowConstantLoopConditions: true }], + }, ], invalid: [ // Ensure that it's checking in all the right places @@ -201,5 +209,18 @@ const t1 = (b1 && b2) ? 'yes' : 'no'`, ruleError(9, 13, 'alwaysTruthy'), ], }, + { + code: ` +while(true) {} +for (;true;) {} +do {} while(true) + `, + options: [{ allowConstantLoopConditions: false }], + errors: [ + ruleError(2, 7, 'alwaysTruthy'), + ruleError(3, 7, 'alwaysTruthy'), + ruleError(4, 13, 'alwaysTruthy'), + ], + }, ], }); From d785c61f9e518ebefdd09614a0dc69494bdf2e85 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 21 Nov 2019 19:25:22 +0200 Subject: [PATCH 11/23] feat(eslint-plugin): [camelcase] add genericType option (#925) --- .../eslint-plugin/docs/rules/camelcase.md | 100 ++++++++- packages/eslint-plugin/src/rules/camelcase.ts | 25 ++- .../tests/rules/camelcase.test.ts | 206 ++++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 1 + 4 files changed, 329 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/camelcase.md b/packages/eslint-plugin/docs/rules/camelcase.md index 84c4d9d13b5d..5fa0c8e530ff 100644 --- a/packages/eslint-plugin/docs/rules/camelcase.md +++ b/packages/eslint-plugin/docs/rules/camelcase.md @@ -30,8 +30,10 @@ variable that will be imported into the local module scope. This rule has an object option: -- `"properties": "always"` (default) enforces camelcase style for property names -- `"properties": "never"` does not check property names +- `"properties": "never"` (default) does not check property names +- `"properties": "always"` enforces camelcase style for property names +- `"genericType": "never"` (default) does not check generic identifiers +- `"genericType": "always"` enforces camelcase style for generic identifiers - `"ignoreDestructuring": false` (default) enforces camelcase style for destructured identifiers - `"ignoreDestructuring": true` does not check destructured identifiers - `allow` (`string[]`) list of properties to accept. Accept regex. @@ -129,6 +131,100 @@ var obj = { }; ``` +### genericType: "always" + +Examples of **incorrect** code for this rule with the default `{ "genericType": "always" }` option: + +```typescript +/* eslint @typescript-eslint/camelcase: ["error", { "genericType": "always" }] */ + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} +``` + +Examples of **correct** code for this rule with the default `{ "genericType": "always" }` option: + +```typescript +/* eslint @typescript-eslint/camelcase: ["error", { "genericType": "always" }] */ + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} +``` + +### genericType: "never" + +Examples of **correct** code for this rule with the `{ "genericType": "never" }` option: + +```typescript +/* eslint @typescript-eslint/camelcase: ["error", { "genericType": "never" }] */ + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} + +interface Foo {} +function foo() {} +class Foo {} +type Foo = {}; +class Foo { + method() {} +} +``` + ### ignoreDestructuring: false Examples of **incorrect** code for this rule with the default `{ "ignoreDestructuring": false }` option: diff --git a/packages/eslint-plugin/src/rules/camelcase.ts b/packages/eslint-plugin/src/rules/camelcase.ts index 0928484b629d..a2be8a1c6958 100644 --- a/packages/eslint-plugin/src/rules/camelcase.ts +++ b/packages/eslint-plugin/src/rules/camelcase.ts @@ -8,6 +8,19 @@ import * as util from '../util'; type Options = util.InferOptionsTypeFromRule; type MessageIds = util.InferMessageIdsTypeFromRule; +const schema = util.deepMerge( + Array.isArray(baseRule.meta.schema) + ? baseRule.meta.schema[0] + : baseRule.meta.schema, + { + properties: { + genericType: { + enum: ['always', 'never'], + }, + }, + }, +); + export default util.createRule({ name: 'camelcase', meta: { @@ -17,7 +30,7 @@ export default util.createRule({ category: 'Stylistic Issues', recommended: 'error', }, - schema: baseRule.meta.schema, + schema: [schema], messages: baseRule.meta.messages, }, defaultOptions: [ @@ -25,6 +38,7 @@ export default util.createRule({ allow: ['^UNSAFE_'], ignoreDestructuring: false, properties: 'never', + genericType: 'never', }, ], create(context, [options]) { @@ -36,6 +50,7 @@ export default util.createRule({ AST_NODE_TYPES.TSAbstractClassProperty, ]; + const genericType = options.genericType; const properties = options.properties; const allow = (options.allow || []).map(entry => ({ name: entry, @@ -117,6 +132,14 @@ export default util.createRule({ return; } + if (parent && parent.type === AST_NODE_TYPES.TSTypeParameter) { + if (genericType === 'always' && isUnderscored(name)) { + report(node); + } + + return; + } + if (parent && parent.type === AST_NODE_TYPES.OptionalMemberExpression) { // Report underscored object names if ( diff --git a/packages/eslint-plugin/tests/rules/camelcase.test.ts b/packages/eslint-plugin/tests/rules/camelcase.test.ts index d2d3287ce662..617cdf3e40f7 100644 --- a/packages/eslint-plugin/tests/rules/camelcase.test.ts +++ b/packages/eslint-plugin/tests/rules/camelcase.test.ts @@ -79,6 +79,100 @@ ruleTester.run('camelcase', rule, { code: 'abstract class Foo { abstract bar: number = 0; }', options: [{ properties: 'always' }], }, + { + code: 'interface Foo {}', + options: [{ genericType: 'never' }], + }, + { + code: 'interface Foo {}', + options: [{ genericType: 'always' }], + }, + { + code: 'interface Foo {}', + options: [{ genericType: 'always' }], + }, + { + code: 'function fn() {}', + options: [{ genericType: 'never' }], + }, + { + code: 'function fn() {}', + options: [{ genericType: 'always' }], + }, + { + code: 'function fn() {}', + options: [{ genericType: 'always' }], + }, + { + code: 'class Foo {}', + options: [{ genericType: 'never' }], + }, + { + code: 'class Foo {}', + options: [{ genericType: 'always' }], + }, + { + code: 'class Foo {}', + options: [{ genericType: 'always' }], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'never' }], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'always' }], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'always' }], + }, + { + code: ` +type Foo = {} + `, + options: [{ genericType: 'always' }], + }, + { + code: ` +type Foo = {} + `, + options: [{ genericType: 'always' }], + }, + { + code: ` +type Foo = {} + `, + options: [{ genericType: 'never' }], + }, + { + code: ` +class Foo { + FOO_method() {} +} + `, + options: [{ allow: ['^FOO'] }], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{}], + }, { code: 'const foo = foo?.baz;', }, @@ -238,5 +332,117 @@ ruleTester.run('camelcase', rule, { }, ], }, + { + code: 'interface Foo {}', + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 1, + column: 15, + }, + ], + }, + { + code: 'function fn() {}', + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 1, + column: 13, + }, + ], + }, + { + code: 'class Foo {}', + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 1, + column: 11, + }, + ], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 3, + column: 10, + }, + ], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 3, + column: 10, + }, + { + messageId: 'notCamelCase', + data: { + name: 't_bar', + }, + line: 3, + column: 24, + }, + ], + }, + { + code: ` +class Foo { + method() {} +} + `, + options: [{ genericType: 'always' }], + errors: [ + { + messageId: 'notCamelCase', + data: { + name: 't_foo', + }, + line: 3, + column: 10, + }, + { + messageId: 'notCamelCase', + data: { + name: 't_bar', + }, + line: 3, + column: 18, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 2ec44b6dbc20..62f45419b188 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -35,6 +35,7 @@ declare module 'eslint/lib/rules/camelcase' { allow?: string[]; ignoreDestructuring?: boolean; properties?: 'always' | 'never'; + genericType?: 'never' | 'always'; }, ], { From 1bd863efd09f754a54aa36b829f51ebe39ec7145 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 23 Nov 2019 16:02:25 +1300 Subject: [PATCH 12/23] docs(eslint-plugin): correct typo in configs README (#1251) --- packages/eslint-plugin/src/configs/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/configs/README.md b/packages/eslint-plugin/src/configs/README.md index 95accb694903..28747608efb7 100644 --- a/packages/eslint-plugin/src/configs/README.md +++ b/packages/eslint-plugin/src/configs/README.md @@ -1,6 +1,6 @@ # Premade configs -These configs exist for your convenience. They contain configuration intended to save you time and effort when configuring your project by disabling rules known to conflict with this repository, or cause issues in typesript codebases. +These configs exist for your convenience. They contain configuration intended to save you time and effort when configuring your project by disabling rules known to conflict with this repository, or cause issues in TypeScript codebases. ## All @@ -26,7 +26,7 @@ The recommended set is an **_opinionated_** set of rules that we think you shoul 1. They help you adhere to TypeScript best practices. 2. They help catch probable issue vectors in your code. -That being said, it is not the only way to use `@typescript-eslint/eslint-plugin`, nor is it the way that will necesasrily work 100% for your project/company. It has been built based off of two main things: +That being said, it is not the only way to use `@typescript-eslint/eslint-plugin`, nor is it the way that will necessarily work 100% for your project/company. It has been built based off of two main things: 1. TypeScript best practices collected and collated from places like: - [TypeScript repo](https://github.com/Microsoft/TypeScript). From 1d56c8247deb67edc37d671311af9e8ab8614dec Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 23 Nov 2019 16:02:35 +1300 Subject: [PATCH 13/23] docs(eslint-plugin): fix typo in no-this-alias (#1252) --- packages/eslint-plugin/docs/rules/no-this-alias.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-this-alias.md b/packages/eslint-plugin/docs/rules/no-this-alias.md index b2c891fa0e6d..4a512be31eb8 100644 --- a/packages/eslint-plugin/docs/rules/no-this-alias.md +++ b/packages/eslint-plugin/docs/rules/no-this-alias.md @@ -1,6 +1,6 @@ # Disallow aliasing `this` (no-this-alias) -This rule prohibts assigning variables to `this`. +This rule prohibits assigning variables to `this`. ## Rule Details From b16a4b6a79637d0ac7c61526da6777a0ac3dddd5 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 24 Nov 2019 14:11:04 -0800 Subject: [PATCH 14/23] feat: suggestion types, suggestions for no-explicit-any (#1250) --- package.json | 2 +- .../src/rules/no-explicit-any.ts | 31 +++- .../tests/rules/no-explicit-any.test.ts | 163 +++++++++++++++++- .../experimental-utils/src/ts-eslint/Rule.ts | 22 ++- .../src/ts-eslint/RuleTester.ts | 12 ++ tests/integration/README.md | 2 +- .../fixtures/markdown/test.js.snap | 96 +++++++++++ .../integration/fixtures/vue-jsx/test.js.snap | 24 +++ .../integration/fixtures/vue-sfc/test.js.snap | 24 +++ yarn.lock | 44 +++-- 10 files changed, 394 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index 111bf02de8ba..aca3b80aee45 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "@types/node": "^12.12.7", "all-contributors-cli": "^6.11.0", "cz-conventional-changelog": "^3.0.2", - "eslint": "^6.6.0", + "eslint": "^6.7.0", "eslint-plugin-eslint-comments": "^3.1.2", "eslint-plugin-eslint-plugin": "^2.1.0", "eslint-plugin-import": "^2.18.2", diff --git a/packages/eslint-plugin/src/rules/no-explicit-any.ts b/packages/eslint-plugin/src/rules/no-explicit-any.ts index 249265a7683c..29fb16651a2c 100644 --- a/packages/eslint-plugin/src/rules/no-explicit-any.ts +++ b/packages/eslint-plugin/src/rules/no-explicit-any.ts @@ -11,7 +11,7 @@ export type Options = [ ignoreRestArgs?: boolean; }, ]; -export type MessageIds = 'unexpectedAny'; +export type MessageIds = 'unexpectedAny' | 'suggestUnknown' | 'suggestNever'; export default util.createRule({ name: 'no-explicit-any', @@ -25,6 +25,10 @@ export default util.createRule({ fixable: 'code', messages: { unexpectedAny: 'Unexpected any. Specify a different type.', + suggestUnknown: + 'Use `unknown` instead, this will force you to explicitly, and safely assert the type is correct.', + suggestNever: + "Use `never` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", }, schema: [ { @@ -172,17 +176,36 @@ export default util.createRule({ return; } - let fix: TSESLint.ReportFixFunction | null = null; + const fixOrSuggest: { + fix: TSESLint.ReportFixFunction | null; + suggest: TSESLint.ReportSuggestionArray | null; + } = { + fix: null, + suggest: [ + { + messageId: 'suggestUnknown', + fix(fixer): TSESLint.RuleFix { + return fixer.replaceText(node, 'unknown'); + }, + }, + { + messageId: 'suggestNever', + fix(fixer): TSESLint.RuleFix { + return fixer.replaceText(node, 'never'); + }, + }, + ], + }; if (fixToUnknown) { - fix = (fixer => + fixOrSuggest.fix = (fixer => fixer.replaceText(node, 'unknown')) as TSESLint.ReportFixFunction; } context.report({ node, messageId: 'unexpectedAny', - fix, + ...fixOrSuggest, }); }, }; diff --git a/packages/eslint-plugin/tests/rules/no-explicit-any.test.ts b/packages/eslint-plugin/tests/rules/no-explicit-any.test.ts index c5624ba06351..0bc8e5f3438f 100644 --- a/packages/eslint-plugin/tests/rules/no-explicit-any.test.ts +++ b/packages/eslint-plugin/tests/rules/no-explicit-any.test.ts @@ -3,6 +3,7 @@ import { RuleTester } from '../RuleTester'; import { TSESLint } from '@typescript-eslint/experimental-utils'; type InvalidTestCase = TSESLint.InvalidTestCase; +type SuggestionOutput = TSESLint.SuggestionOutput; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -306,11 +307,31 @@ type obj = { messageId: 'unexpectedAny', line: 1, column: 31, + suggestions: [ + { + messageId: 'suggestUnknown', + output: 'function generic(param: Array): Array {}', + }, + { + messageId: 'suggestNever', + output: 'function generic(param: Array): Array {}', + }, + ], }, { messageId: 'unexpectedAny', line: 1, column: 44, + suggestions: [ + { + messageId: 'suggestUnknown', + output: 'function generic(param: Array): Array {}', + }, + { + messageId: 'suggestNever', + output: 'function generic(param: Array): Array {}', + }, + ], }, ], }, @@ -705,11 +726,31 @@ type obj = { messageId: 'unexpectedAny', line: 1, column: 15, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `class Foo extends Bar {}`, + }, + { + messageId: 'suggestNever', + output: `class Foo extends Bar {}`, + }, + ], }, { messageId: 'unexpectedAny', line: 1, column: 32, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `class Foo extends Bar {}`, + }, + { + messageId: 'suggestNever', + output: `class Foo extends Bar {}`, + }, + ], }, ], }, @@ -720,11 +761,31 @@ type obj = { messageId: 'unexpectedAny', line: 1, column: 24, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `abstract class Foo extends Bar {}`, + }, + { + messageId: 'suggestNever', + output: `abstract class Foo extends Bar {}`, + }, + ], }, { messageId: 'unexpectedAny', line: 1, column: 41, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `abstract class Foo extends Bar {}`, + }, + { + messageId: 'suggestNever', + output: `abstract class Foo extends Bar {}`, + }, + ], }, ], }, @@ -735,16 +796,46 @@ type obj = { messageId: 'unexpectedAny', line: 1, column: 24, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `abstract class Foo implements Bar, Baz {}`, + }, + { + messageId: 'suggestNever', + output: `abstract class Foo implements Bar, Baz {}`, + }, + ], }, { messageId: 'unexpectedAny', line: 1, column: 44, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `abstract class Foo implements Bar, Baz {}`, + }, + { + messageId: 'suggestNever', + output: `abstract class Foo implements Bar, Baz {}`, + }, + ], }, { messageId: 'unexpectedAny', line: 1, column: 54, + suggestions: [ + { + messageId: 'suggestUnknown', + output: `abstract class Foo implements Bar, Baz {}`, + }, + { + messageId: 'suggestNever', + output: `abstract class Foo implements Bar, Baz {}`, + }, + ], }, ], }, @@ -771,19 +862,51 @@ type obj = { { // https://github.com/typescript-eslint/typescript-eslint/issues/64 code: ` - function test>() {} - const test = >() => {}; - `, +function test>() {} +const test = >() => {}; + `.trimRight(), errors: [ { messageId: 'unexpectedAny', line: 2, - column: 41, + column: 33, + suggestions: [ + { + messageId: 'suggestUnknown', + output: ` +function test>() {} +const test = >() => {}; + `.trimRight(), + }, + { + messageId: 'suggestNever', + output: ` +function test>() {} +const test = >() => {}; + `.trimRight(), + }, + ], }, { messageId: 'unexpectedAny', line: 3, - column: 41, + column: 33, + suggestions: [ + { + messageId: 'suggestUnknown', + output: ` +function test>() {} +const test = >() => {}; + `.trimRight(), + }, + { + messageId: 'suggestNever', + output: ` +function test>() {} +const test = >() => {}; + `.trimRight(), + }, + ], }, ], }, @@ -869,7 +992,23 @@ type obj = { ], }, ] as InvalidTestCase[]).reduce((acc, testCase) => { - acc.push(testCase); + const suggestions = (code: string): SuggestionOutput[] => [ + { + messageId: 'suggestUnknown', + output: code.replace(/any/, 'unknown'), + }, + { + messageId: 'suggestNever', + output: code.replace(/any/, 'never'), + }, + ]; + acc.push({ + ...testCase, + errors: testCase.errors.map(e => ({ + ...e, + suggestions: e.suggestions ?? suggestions(testCase.code), + })), + }); const options = testCase.options || []; const code = `// fixToUnknown: true\n${testCase.code}`; acc.push({ @@ -881,7 +1020,17 @@ type obj = { return err; } - return { ...err, line: err.line + 1 }; + return { + ...err, + line: err.line + 1, + suggestions: + err.suggestions?.map( + (s): SuggestionOutput => ({ + ...s, + output: `// fixToUnknown: true\n${s.output}`, + }), + ) ?? suggestions(code), + }; }), }); diff --git a/packages/experimental-utils/src/ts-eslint/Rule.ts b/packages/experimental-utils/src/ts-eslint/Rule.ts index dad91e6a8e09..5cf79e3e16ec 100644 --- a/packages/experimental-utils/src/ts-eslint/Rule.ts +++ b/packages/experimental-utils/src/ts-eslint/Rule.ts @@ -105,6 +105,9 @@ interface RuleFixer { type ReportFixFunction = ( fixer: RuleFixer, ) => null | RuleFix | RuleFix[] | IterableIterator; +type ReportSuggestionArray = ReportDescriptorBase< + TMessageIds +>[]; interface ReportDescriptorBase { /** @@ -119,7 +122,19 @@ interface ReportDescriptorBase { * The messageId which is being reported. */ messageId: TMessageIds; + // we disallow this because it's much better to use messageIds for reusable errors that are easily testable + // message?: string; + // suggestions instead have this property that works the same, but again it's much better to use messageIds + // desc?: string; +} +interface ReportDescriptorWithSuggestion + extends ReportDescriptorBase { + /** + * 6.7's Suggestions API + */ + suggest?: ReportSuggestionArray | null; } + interface ReportDescriptorNodeOptionalLoc { /** * The Node or AST Token which the report is being attached to @@ -136,9 +151,9 @@ interface ReportDescriptorLocOnly { */ loc: TSESTree.SourceLocation | TSESTree.LineAndColumnData; } -type ReportDescriptor = ReportDescriptorBase< - TMessageIds -> & +type ReportDescriptor< + TMessageIds extends string +> = ReportDescriptorWithSuggestion & (ReportDescriptorNodeOptionalLoc | ReportDescriptorLocOnly); interface RuleContext< @@ -416,6 +431,7 @@ interface RuleModule< export { ReportDescriptor, ReportFixFunction, + ReportSuggestionArray, RuleContext, RuleFix, RuleFixer, diff --git a/packages/experimental-utils/src/ts-eslint/RuleTester.ts b/packages/experimental-utils/src/ts-eslint/RuleTester.ts index 84da228fb81e..2a32daa5a9ec 100644 --- a/packages/experimental-utils/src/ts-eslint/RuleTester.ts +++ b/packages/experimental-utils/src/ts-eslint/RuleTester.ts @@ -19,6 +19,16 @@ interface ValidTestCase> { }; } +interface SuggestionOutput { + messageId: TMessageIds; + data?: Record; + /** + * NOTE: Suggestions will be applied as a stand-alone change, without triggering multipass fixes. + * Each individual error has its own suggestion, so you have to show the correct, _isolated_ output for each suggestion. + */ + output: string; +} + interface InvalidTestCase< TMessageIds extends string, TOptions extends Readonly @@ -35,6 +45,7 @@ interface TestCaseError { column?: number; endLine?: number; endColumn?: number; + suggestions?: SuggestionOutput[]; } interface RunTests< @@ -79,6 +90,7 @@ class RuleTester extends (ESLintRuleTester as { export { InvalidTestCase, + SuggestionOutput, RuleTester, RuleTesterConfig, RunTests, diff --git a/tests/integration/README.md b/tests/integration/README.md index eb33e8bd62df..c0aa0b976433 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -14,5 +14,5 @@ These tests are setup to run within docker containers to ensure that each test i 1. Copy+paste the `test.sh` from an existing fixture, and adjust the `eslint` command as required. 1. Add a new entry to `docker-compose.yml` by copy+pasting an existing section, and changing the name to match your new folder. 1. Add a new entry to `run-all-tests.sh` by copy+pasting an existing command, and changing the name to match your new folder. -1. Run your integration test by running the single command you copied in . +1. Run your integration test by running the single command you copied in the previous step. - If your test finishes successfully, a `test.js.snap` will be created. diff --git a/tests/integration/fixtures/markdown/test.js.snap b/tests/integration/fixtures/markdown/test.js.snap index 4fc96f79abc7..5f28d2474ea6 100644 --- a/tests/integration/fixtures/markdown/test.js.snap +++ b/tests/integration/fixtures/markdown/test.js.snap @@ -29,6 +29,30 @@ Array [ "nodeType": "TSAnyKeyword", "ruleId": "@typescript-eslint/no-explicit-any", "severity": 2, + "suggestions": Array [ + Object { + "desc": "Use \`unknown\` instead, this will force you to explicitly, and safely assert the type is correct.", + "fix": Object { + "range": Array [ + 51, + 54, + ], + "text": "unknown", + }, + "messageId": "suggestUnknown", + }, + Object { + "desc": "Use \`never\` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", + "fix": Object { + "range": Array [ + 51, + 54, + ], + "text": "never", + }, + "messageId": "suggestNever", + }, + ], }, Object { "column": 3, @@ -62,6 +86,30 @@ Array [ "nodeType": "TSAnyKeyword", "ruleId": "@typescript-eslint/no-explicit-any", "severity": 2, + "suggestions": Array [ + Object { + "desc": "Use \`unknown\` instead, this will force you to explicitly, and safely assert the type is correct.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "unknown", + }, + "messageId": "suggestUnknown", + }, + Object { + "desc": "Use \`never\` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "never", + }, + "messageId": "suggestNever", + }, + ], }, Object { "column": 3, @@ -84,6 +132,30 @@ Array [ "nodeType": "TSAnyKeyword", "ruleId": "@typescript-eslint/no-explicit-any", "severity": 2, + "suggestions": Array [ + Object { + "desc": "Use \`unknown\` instead, this will force you to explicitly, and safely assert the type is correct.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "unknown", + }, + "messageId": "suggestUnknown", + }, + Object { + "desc": "Use \`never\` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "never", + }, + "messageId": "suggestNever", + }, + ], }, Object { "column": 3, @@ -106,6 +178,30 @@ Array [ "nodeType": "TSAnyKeyword", "ruleId": "@typescript-eslint/no-explicit-any", "severity": 2, + "suggestions": Array [ + Object { + "desc": "Use \`unknown\` instead, this will force you to explicitly, and safely assert the type is correct.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "unknown", + }, + "messageId": "suggestUnknown", + }, + Object { + "desc": "Use \`never\` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", + "fix": Object { + "range": Array [ + 16, + 19, + ], + "text": "never", + }, + "messageId": "suggestNever", + }, + ], }, Object { "column": 3, diff --git a/tests/integration/fixtures/vue-jsx/test.js.snap b/tests/integration/fixtures/vue-jsx/test.js.snap index d4432e47428a..e0fbff509d49 100644 --- a/tests/integration/fixtures/vue-jsx/test.js.snap +++ b/tests/integration/fixtures/vue-jsx/test.js.snap @@ -18,6 +18,30 @@ Array [ "nodeType": "TSAnyKeyword", "ruleId": "@typescript-eslint/no-explicit-any", "severity": 2, + "suggestions": Array [ + Object { + "desc": "Use \`unknown\` instead, this will force you to explicitly, and safely assert the type is correct.", + "fix": Object { + "range": Array [ + 390, + 393, + ], + "text": "unknown", + }, + "messageId": "suggestUnknown", + }, + Object { + "desc": "Use \`never\` instead, this is useful when instantiating generic type parameters that you don't need to know the type of.", + "fix": Object { + "range": Array [ + 390, + 393, + ], + "text": "never", + }, + "messageId": "suggestNever", + }, + ], }, ], "source": "