diff --git a/.eslintrc.js b/.eslintrc.js index c789421b024..1c18917bbcb 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -221,6 +221,15 @@ module.exports = { '@typescript-eslint/internal/plugin-test-formatting': 'error', }, }, + // files which list all the things + { + files: ['packages/eslint-plugin/src/rules/index.ts'], + rules: { + // enforce alphabetical ordering + 'sort-keys': 'error', + 'import/order': ['error', { alphabetize: { order: 'asc' } }], + }, + }, // tools and tests { files: ['**/tools/**/*.ts', '**/tests/**/*.ts'], diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 9213a8d5273..3dcb079be5a 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -117,6 +117,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: | | [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md) | Requires that `.toString()` is only called on objects which provide useful information when stringified | | | :thought_balloon: | | [`@typescript-eslint/no-confusing-non-null-assertion`](./docs/rules/no-confusing-non-null-assertion.md) | Disallow non-null assertion in locations that may be confusing | | :wrench: | | +| [`@typescript-eslint/no-confusing-void-expression`](./docs/rules/no-confusing-void-expression.md) | Requires expressions of type void to appear in statement position | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index aece78f3357..749ba2bdf82 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -96,7 +96,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`no-unused-variable`] | 🌓 | [`@typescript-eslint/no-unused-vars`] | | [`no-use-before-declare`] | ✅ | [`@typescript-eslint/no-use-before-define`] | | [`no-var-keyword`] | 🌟 | [`no-var`][no-var] | -| [`no-void-expression`] | 🛑 | N/A (unrelated to the similarly named ESLint rule `no-void`) | +| [`no-void-expression`] | ✅ | [`@typescript-eslint/no-confusing-void-expression`] | | [`prefer-conditional-expression`] | 🛑 | N/A | | [`prefer-object-spread`] | 🌟 | [`prefer-object-spread`][prefer-object-spread] | | [`radix`] | 🌟 | [`radix`][radix] | @@ -650,6 +650,8 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/no-floating-promises`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md [`@typescript-eslint/no-magic-numbers`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-magic-numbers.md [`@typescript-eslint/no-unsafe-member-access`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md +[`@typescript-eslint/restrict-template-expressions`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/restrict-template-expressions.md +[`@typescript-eslint/no-confusing-void-expression`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md diff --git a/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md new file mode 100644 index 00000000000..8a00255170d --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md @@ -0,0 +1,149 @@ +# Requires expressions of type void to appear in statement position (`no-confusing-void-expression`) + +Returning the results of an expression whose type is void can be misleading. +Attempting to do so is likely a symptom of expecting a different return type from a function. +Even if used correctly, it can be misleading for other developers, +who don't know what a particular function does and if its result matters. + +This rule provides automatic fixes for most common cases. + +## Examples + +Examples of **incorrect** code for this rule: + +```ts +// somebody forgot that `alert` doesn't return anything +const response = alert('Are you sure?'); +console.log(alert('Are you sure?')); + +// it's not obvious whether the chained promise will contain the response (fixable) +promise.then(value => window.postMessage(value)); + +// it looks like we are returning the result of `console.error` (fixable) +function doSomething() { + if (!somethingToDo) { + return console.error('Nothing to do!'); + } + + console.log('Doing a thing...'); +} +``` + +Examples of **correct** code for this rule: + +```ts +// just a regular void function in a statement position +alert('Hello, world!'); + +// this function returns a boolean value so it's ok +const response = confirm('Are you sure?'); +console.log(confirm('Are you sure?')); + +// now it's obvious that `postMessage` doesn't return any response +promise.then(value => { + window.postMessage(value); +}); + +// now it's explicit that we want to log the error and return early +function doSomething() { + if (!somethingToDo) { + console.error('Nothing to do!'); + return; + } + + console.log('Doing a thing...'); +} + +// using logical expressions for their side effects is fine +cond && console.log('true'); +cond || console.error('false'); +cond ? console.log('true') : console.error('false'); +``` + +## Options + +An object option can be specified. Each boolean flag makes the rule less strict. + +```ts +type Options = { + ignoreArrowShorthand?: boolean; + ignoreVoidOperator?: boolean; +}; + +const defaults: Options = { + ignoreArrowShorthand: false, + ignoreVoidOperator: false, +}; +``` + +### `ignoreArrowShorthand` + +`false` by default. + +```json +{ + "@typescript-eslint/no-confusing-void-expression": [ + "error", + { "ignoreArrowShorthand": true } + ] +} +``` + +It might be undesirable to wrap every arrow function shorthand expression with braces. +Especially when using Prettier formatter, which spreads such code across 3 lines instead of 1. + +Examples of additional **correct** code with this option enabled: + +```ts +promise.then(value => window.postMessage(value)); +``` + +### `ignoreVoidOperator` + +`false` by default. + +```json +{ + "@typescript-eslint/no-confusing-void-expression": [ + "error", + { "ignoreVoidOperator": true } + ] +} +``` + +It might be preferable to only use some distinct syntax +to explicitly mark the confusing but valid usage of void expressions. +This option allows void expressions which are explicitly wrapped in the `void` operator. +This can help avoid confusion among other developers as long as they are made aware of this code style. + +This option also changes the automatic fixes for common cases to use the `void` operator. +It also enables a suggestion fix to wrap the void expression with `void` operator for every problem reported. + +Examples of additional **correct** code with this option enabled: + +```ts +// now it's obvious that we don't expect any response +promise.then(value => void window.postMessage(value)); + +// now it's explicit that we don't want to return anything +function doSomething() { + if (!somethingToDo) { + return void console.error('Nothing to do!'); + } + + console.log('Doing a thing...'); +} + +// we are sure that we want to always log `undefined` +console.log(void alert('Hello, world!')); +``` + +## When Not To Use It + +The return type of a function can be inspected by going to its definition or hovering over it in an IDE. +If you don't care about being explicit about the void type in actual code then don't use this rule. +Also, if you prefer concise coding style then also don't use it. + +## Related to + +- TSLint: ['no-void-expression'](https://palantir.github.io/tslint/rules/no-void-expression/) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index d847322d986..d17db11d0b5 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -47,6 +47,7 @@ export = { '@typescript-eslint/no-array-constructor': 'error', '@typescript-eslint/no-base-to-string': 'error', '@typescript-eslint/no-confusing-non-null-assertion': 'error', + '@typescript-eslint/no-confusing-void-expression': 'error', 'no-dupe-class-members': 'off', '@typescript-eslint/no-dupe-class-members': 'error', 'no-duplicate-imports': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index a4540aa5788..de982a91d87 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -8,14 +8,12 @@ import braceStyle from './brace-style'; import classLiteralPropertyStyle from './class-literal-property-style'; import commaDangle from './comma-dangle'; import commaSpacing from './comma-spacing'; -import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion'; import consistentIndexedObjectStyle from './consistent-indexed-object-style'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; import consistentTypeImports from './consistent-type-imports'; import defaultParamLast from './default-param-last'; import dotNotation from './dot-notation'; -import enumMembersSpacing from './space-infix-ops'; import explicitFunctionReturnType from './explicit-function-return-type'; import explicitMemberAccessibility from './explicit-member-accessibility'; import explicitModuleBoundaryTypes from './explicit-module-boundary-types'; @@ -30,25 +28,27 @@ import methodSignatureStyle from './method-signature-style'; import namingConvention from './naming-convention'; import noArrayConstructor from './no-array-constructor'; import noBaseToString from './no-base-to-string'; +import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion'; +import noConfusingVoidExpression from './no-confusing-void-expression'; import noDupeClassMembers from './no-dupe-class-members'; +import noDuplicateImports from './no-duplicate-imports'; 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 noImplicitAnyCatch from './no-implicit-any-catch'; -import noExtraneousClass from './no-extraneous-class'; import noExtraNonNullAssertion from './no-extra-non-null-assertion'; import noExtraParens from './no-extra-parens'; import noExtraSemi from './no-extra-semi'; +import noExtraneousClass from './no-extraneous-class'; import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; -import preferLiteralEnumMember from './prefer-literal-enum-member'; +import noImplicitAnyCatch from './no-implicit-any-catch'; import noImpliedEval from './no-implied-eval'; import noInferrableTypes from './no-inferrable-types'; import noInvalidThis from './no-invalid-this'; import noInvalidVoidType from './no-invalid-void-type'; -import noLossOfPrecision from './no-loss-of-precision'; import noLoopFunc from './no-loop-func'; +import noLossOfPrecision from './no-loss-of-precision'; import noMagicNumbers from './no-magic-numbers'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; @@ -83,6 +83,7 @@ import preferEnumInitializers from './prefer-enum-initializers'; import preferForOf from './prefer-for-of'; import preferFunctionType from './prefer-function-type'; import preferIncludes from './prefer-includes'; +import preferLiteralEnumMember from './prefer-literal-enum-member'; import preferNamespaceKeyword from './prefer-namespace-keyword'; import preferNullishCoalescing from './prefer-nullish-coalescing'; import preferOptionalChain from './prefer-optional-chain'; @@ -101,6 +102,7 @@ import restrictTemplateExpressions from './restrict-template-expressions'; import returnAwait from './return-await'; import semi from './semi'; import spaceBeforeFunctionParen from './space-before-function-paren'; +import spaceInfixOps from './space-infix-ops'; import strictBooleanExpressions from './strict-boolean-expressions'; import switchExhaustivenessCheck from './switch-exhaustiveness-check'; import tripleSlashReference from './triple-slash-reference'; @@ -108,7 +110,6 @@ import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; -import noDuplicateImports from './no-duplicate-imports'; export default { 'adjacent-overload-signatures': adjacentOverloadSignatures, @@ -127,11 +128,11 @@ export default { 'consistent-type-imports': consistentTypeImports, 'default-param-last': defaultParamLast, 'dot-notation': dotNotation, - 'space-infix-ops': enumMembersSpacing, 'explicit-function-return-type': explicitFunctionReturnType, 'explicit-member-accessibility': explicitMemberAccessibility, 'explicit-module-boundary-types': explicitModuleBoundaryTypes, 'func-call-spacing': funcCallSpacing, + indent: indent, 'init-declarations': initDeclarations, 'keyword-spacing': keywordSpacing, 'lines-between-class-members': linesBetweenClassMembers, @@ -142,7 +143,9 @@ export default { 'no-array-constructor': noArrayConstructor, 'no-base-to-string': noBaseToString, 'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual, + 'no-confusing-void-expression': noConfusingVoidExpression, 'no-dupe-class-members': noDupeClassMembers, + 'no-duplicate-imports': noDuplicateImports, 'no-dynamic-delete': noDynamicDelete, 'no-empty-function': noEmptyFunction, 'no-empty-interface': noEmptyInterface, @@ -184,8 +187,8 @@ export default { 'no-unsafe-member-access': noUnsafeMemberAccess, 'no-unsafe-return': noUnsafeReturn, 'no-unused-expressions': noUnusedExpressions, - 'no-unused-vars-experimental': noUnusedVarsExperimental, 'no-unused-vars': noUnusedVars, + 'no-unused-vars-experimental': noUnusedVarsExperimental, 'no-use-before-define': noUseBeforeDefine, 'no-useless-constructor': noUselessConstructor, 'no-var-requires': noVarRequires, @@ -198,28 +201,27 @@ export default { 'prefer-namespace-keyword': preferNamespaceKeyword, 'prefer-nullish-coalescing': preferNullishCoalescing, 'prefer-optional-chain': preferOptionalChain, - 'prefer-readonly-parameter-types': preferReadonlyParameterTypes, 'prefer-readonly': preferReadonly, + 'prefer-readonly-parameter-types': preferReadonlyParameterTypes, 'prefer-reduce-type-parameter': preferReduceTypeParameter, 'prefer-regexp-exec': preferRegexpExec, 'prefer-string-starts-ends-with': preferStringStartsEndsWith, 'prefer-ts-expect-error': preferTsExpectError, 'promise-function-async': promiseFunctionAsync, + quotes: quotes, 'require-array-sort-compare': requireArraySortCompare, 'require-await': requireAwait, 'restrict-plus-operands': restrictPlusOperands, 'restrict-template-expressions': restrictTemplateExpressions, 'return-await': returnAwait, + semi: semi, 'space-before-function-paren': spaceBeforeFunctionParen, + 'space-infix-ops': spaceInfixOps, 'strict-boolean-expressions': strictBooleanExpressions, 'switch-exhaustiveness-check': switchExhaustivenessCheck, 'triple-slash-reference': tripleSlashReference, 'type-annotation-spacing': typeAnnotationSpacing, + typedef: typedef, 'unbound-method': unboundMethod, 'unified-signatures': unifiedSignatures, - 'no-duplicate-imports': noDuplicateImports, - indent: indent, - quotes: quotes, - semi: semi, - typedef: typedef, }; diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts new file mode 100644 index 00000000000..a9b715350e9 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -0,0 +1,331 @@ +import { + AST_NODE_TYPES, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; +import * as util from '../util'; + +export type Options = [ + { + ignoreArrowShorthand?: boolean; + ignoreVoidOperator?: boolean; + }, +]; + +export type MessageId = + | 'invalidVoidExpr' + | 'invalidVoidExprWrapVoid' + | 'invalidVoidExprArrow' + | 'invalidVoidExprArrowWrapVoid' + | 'invalidVoidExprReturn' + | 'invalidVoidExprReturnLast' + | 'invalidVoidExprReturnWrapVoid' + | 'voidExprWrapVoid'; + +export default util.createRule({ + name: 'no-confusing-void-expression', + meta: { + docs: { + description: + 'Requires expressions of type void to appear in statement position', + category: 'Best Practices', + recommended: false, + requiresTypeChecking: true, + }, + messages: { + invalidVoidExpr: + 'Placing a void expression inside another expression is forbidden. ' + + 'Move it to its own statement instead.', + invalidVoidExprWrapVoid: + 'Void expressions used inside another expression ' + + 'must be moved to its own statement ' + + 'or marked explicitly with the `void` operator.', + invalidVoidExprArrow: + 'Returning a void expression from an arrow function shorthand is forbidden. ' + + 'Please add braces to the arrow function.', + invalidVoidExprArrowWrapVoid: + 'Void expressions returned from an arrow function shorthand ' + + 'must be marked explicitly with the `void` operator.', + invalidVoidExprReturn: + 'Returning a void expression from a function is forbidden. ' + + 'Please move it before the `return` statement.', + invalidVoidExprReturnLast: + 'Returning a void expression from a function is forbidden. ' + + 'Please remove the `return` statement.', + invalidVoidExprReturnWrapVoid: + 'Void expressions returned from a function ' + + 'must be marked explicitly with the `void` operator.', + voidExprWrapVoid: 'Mark with an explicit `void` operator', + }, + schema: [ + { + type: 'object', + properties: { + ignoreArrowShorthand: { type: 'boolean' }, + ignoreVoidOperator: { type: 'boolean' }, + }, + additionalProperties: false, + }, + ], + type: 'problem', + fixable: 'code', + }, + defaultOptions: [{}], + + create(context, [options]) { + return { + 'AwaitExpression, CallExpression, TaggedTemplateExpression'( + node: + | TSESTree.AwaitExpression + | TSESTree.CallExpression + | TSESTree.TaggedTemplateExpression, + ): void { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const type = util.getConstrainedTypeAtLocation(checker, tsNode); + if (!tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike)) { + // not a void expression + return; + } + + const invalidAncestor = findInvalidAncestor(node); + if (invalidAncestor == null) { + // void expression is in valid position + return; + } + + const sourceCode = context.getSourceCode(); + const wrapVoidFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix => { + const nodeText = sourceCode.getText(node); + const newNodeText = `void ${nodeText}`; + return fixer.replaceText(node, newNodeText); + }; + + if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) { + // handle arrow function shorthand + + if (options.ignoreVoidOperator) { + // handle wrapping with `void` + return context.report({ + node, + messageId: 'invalidVoidExprArrowWrapVoid', + fix: wrapVoidFix, + }); + } + + // handle wrapping with braces + const arrowFunction = invalidAncestor; + return context.report({ + node, + messageId: 'invalidVoidExprArrow', + fix(fixer) { + const arrowBody = arrowFunction.body; + const arrowBodyText = sourceCode.getText(arrowBody); + const newArrowBodyText = `{ ${arrowBodyText}; }`; + if (util.isParenthesized(arrowBody, sourceCode)) { + const bodyOpeningParen = sourceCode.getTokenBefore( + arrowBody, + util.isOpeningParenToken, + )!; + const bodyClosingParen = sourceCode.getTokenAfter( + arrowBody, + util.isClosingParenToken, + )!; + return fixer.replaceTextRange( + [bodyOpeningParen.range[0], bodyClosingParen.range[1]], + newArrowBodyText, + ); + } + return fixer.replaceText(arrowBody, newArrowBodyText); + }, + }); + } + + if (invalidAncestor.type === AST_NODE_TYPES.ReturnStatement) { + // handle return statement + + if (options.ignoreVoidOperator) { + // handle wrapping with `void` + return context.report({ + node, + messageId: 'invalidVoidExprReturnWrapVoid', + fix: wrapVoidFix, + }); + } + + const returnStmt = invalidAncestor; + + if (isFinalReturn(returnStmt)) { + // remove the `return` keyword + return context.report({ + node, + messageId: 'invalidVoidExprReturnLast', + fix(fixer) { + const returnValue = returnStmt.argument!; + const returnValueText = sourceCode.getText(returnValue); + let newReturnStmtText = `${returnValueText};`; + if (isPreventingASI(returnValue, sourceCode)) { + // put a semicolon at the beginning of the line + newReturnStmtText = `;${newReturnStmtText}`; + } + return fixer.replaceText(returnStmt, newReturnStmtText); + }, + }); + } + + // move before the `return` keyword + return context.report({ + node, + messageId: 'invalidVoidExprReturn', + fix(fixer) { + const returnValue = returnStmt.argument!; + const returnValueText = sourceCode.getText(returnValue); + let newReturnStmtText = `${returnValueText}; return;`; + if (isPreventingASI(returnValue, sourceCode)) { + // put a semicolon at the beginning of the line + newReturnStmtText = `;${newReturnStmtText}`; + } + if (returnStmt.parent?.type !== AST_NODE_TYPES.BlockStatement) { + // e.g. `if (cond) return console.error();` + // add braces if not inside a block + newReturnStmtText = `{ ${newReturnStmtText} }`; + } + return fixer.replaceText(returnStmt, newReturnStmtText); + }, + }); + } + + // handle generic case + if (options.ignoreVoidOperator) { + // this would be reported by this rule btw. such irony + return context.report({ + node, + messageId: 'invalidVoidExprWrapVoid', + suggest: [{ messageId: 'voidExprWrapVoid', fix: wrapVoidFix }], + }); + } + context.report({ + node, + messageId: 'invalidVoidExpr', + }); + }, + }; + + /** + * Inspects the void expression's ancestors and finds closest invalid one. + * By default anything other than an ExpressionStatement is invalid. + * Parent expressions which can be used for their short-circuiting behavior + * are ignored and their parents are checked instead. + * @param node The void expression node to check. + * @returns Invalid ancestor node if it was found. `null` otherwise. + */ + function findInvalidAncestor(node: TSESTree.Node): TSESTree.Node | null { + const parent = util.nullThrows( + node.parent, + util.NullThrowsReasons.MissingParent, + ); + + if (parent.type === AST_NODE_TYPES.ExpressionStatement) { + // e.g. `{ console.log("foo"); }` + // this is always valid + return null; + } + + if (parent.type === AST_NODE_TYPES.LogicalExpression) { + if (parent.right === node) { + // e.g. `x && console.log(x)` + // this is valid only if the next ancestor is valid + return findInvalidAncestor(parent); + } + } + + if (parent.type === AST_NODE_TYPES.ConditionalExpression) { + if (parent.consequent === node || parent.alternate === node) { + // e.g. `cond ? console.log(true) : console.log(false)` + // this is valid only if the next ancestor is valid + return findInvalidAncestor(parent); + } + } + + if (parent.type === AST_NODE_TYPES.ArrowFunctionExpression) { + // e.g. `() => console.log("foo")` + // this is valid with an appropriate option + if (options.ignoreArrowShorthand) { + return null; + } + } + + if (parent.type === AST_NODE_TYPES.UnaryExpression) { + if (parent.operator === 'void') { + // e.g. `void console.log("foo")` + // this is valid with an appropriate option + if (options.ignoreVoidOperator) { + return null; + } + } + } + + // any other parent is invalid + return parent; + } + + /** Checks whether the return statement is the last statement in a function body. */ + function isFinalReturn(node: TSESTree.ReturnStatement): boolean { + // the parent must be a block + const block = util.nullThrows( + node.parent, + util.NullThrowsReasons.MissingParent, + ); + if (block.type !== AST_NODE_TYPES.BlockStatement) { + // e.g. `if (cond) return;` (not in a block) + return false; + } + + // the block's parent must be a function + const blockParent = util.nullThrows( + block.parent, + util.NullThrowsReasons.MissingParent, + ); + if ( + ![ + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.ArrowFunctionExpression, + ].includes(blockParent.type) + ) { + // e.g. `if (cond) { return; }` + // not in a top-level function block + return false; + } + + // must be the last child of the block + if (block.body.indexOf(node) < block.body.length - 1) { + // not the last statement in the block + return false; + } + + return true; + } + + /** + * Checks whether the given node, if placed on its own line, + * would prevent automatic semicolon insertion on the line before. + * + * This happens if the line begins with `(`, `[` or `` ` `` + */ + function isPreventingASI( + node: TSESTree.Expression, + sourceCode: Readonly, + ): boolean { + const startToken = util.nullThrows( + sourceCode.getFirstToken(node), + util.NullThrowsReasons.MissingToken('first token', node.type), + ); + + return ['(', '[', '`'].includes(startToken.value); + } + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts new file mode 100644 index 00000000000..3586c3388f4 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts @@ -0,0 +1,291 @@ +import rule, { + MessageId, + Options, +} from '../../src/rules/no-confusing-void-expression'; +import { + batchedSingleLineTests, + getFixturesRootDir, + noFormat, + RuleTester, +} from '../RuleTester'; + +const rootPath = getFixturesRootDir(); +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('no-confusing-void-expression', rule, { + valid: [ + ...batchedSingleLineTests({ + code: ` + () => Math.random(); + console.log('foo'); + foo && console.log(foo); + foo || console.log(foo); + foo ? console.log(true) : console.log(false); + `, + }), + + ...batchedSingleLineTests({ + options: [{ ignoreArrowShorthand: true }], + code: ` + () => console.log('foo'); + foo => foo && console.log(foo); + foo => foo || console.log(foo); + foo => (foo ? console.log(true) : console.log(false)); + `, + }), + + ...batchedSingleLineTests({ + options: [{ ignoreVoidOperator: true }], + code: ` + !void console.log('foo'); + +void (foo && console.log(foo)); + -void (foo || console.log(foo)); + () => void ((foo && void console.log(true)) || console.log(false)); + const x = void (foo ? console.log(true) : console.log(false)); + !(foo && void console.log(foo)); + !!(foo || void console.log(foo)); + const x = (foo && void console.log(true)) || void console.log(false); + () => (foo ? void console.log(true) : void console.log(false)); + return void console.log('foo'); + `, + }), + ], + + invalid: [ + ...batchedSingleLineTests({ + code: ` + const x = console.log('foo'); + console.error(console.log('foo')); + [console.log('foo')]; + ({ x: console.log('foo') }); + void console.log('foo'); + console.log('foo') ? true : false; + (console.log('foo') && true) || false; + (cond && console.log('ok')) || console.log('error'); + !console.log('foo'); + `, + errors: [ + { line: 2, column: 11, messageId: 'invalidVoidExpr' }, + { line: 3, column: 23, messageId: 'invalidVoidExpr' }, + { line: 4, column: 10, messageId: 'invalidVoidExpr' }, + { line: 5, column: 15, messageId: 'invalidVoidExpr' }, + { line: 6, column: 14, messageId: 'invalidVoidExpr' }, + { line: 7, column: 9, messageId: 'invalidVoidExpr' }, + { line: 8, column: 10, messageId: 'invalidVoidExpr' }, + { line: 9, column: 18, messageId: 'invalidVoidExpr' }, + { line: 10, column: 10, messageId: 'invalidVoidExpr' }, + ], + }), + + { + code: "() => console.log('foo');", + errors: [{ line: 1, column: 7, messageId: 'invalidVoidExprArrow' }], + output: noFormat`() => { console.log('foo'); };`, + }, + { + code: 'foo => foo && console.log(foo);', + errors: [{ line: 1, column: 15, messageId: 'invalidVoidExprArrow' }], + output: noFormat`foo => { foo && console.log(foo); };`, + }, + { + code: 'foo => foo || console.log(foo);', + errors: [{ line: 1, column: 15, messageId: 'invalidVoidExprArrow' }], + output: noFormat`foo => { foo || console.log(foo); };`, + }, + { + code: 'foo => (foo ? console.log(true) : console.log(false));', + errors: [ + { line: 1, column: 15, messageId: 'invalidVoidExprArrow' }, + { line: 1, column: 35, messageId: 'invalidVoidExprArrow' }, + ], + output: noFormat`foo => { foo ? console.log(true) : console.log(false); };`, + }, + { + code: ` + function f() { + return console.log('foo'); + console.log('bar'); + } + `, + errors: [{ line: 3, column: 18, messageId: 'invalidVoidExprReturn' }], + output: noFormat` + function f() { + console.log('foo'); return; + console.log('bar'); + } + `, + }, + { + code: noFormat` + function f() { + console.log('foo') + return ['bar', 'baz'].forEach(console.log) + console.log('quux') + } + `, + errors: [{ line: 4, column: 18, messageId: 'invalidVoidExprReturn' }], + output: noFormat` + function f() { + console.log('foo') + ;['bar', 'baz'].forEach(console.log); return; + console.log('quux') + } + `, + }, + { + code: ` + function f() { + console.log('foo'); + return console.log('bar'); + } + `, + errors: [{ line: 4, column: 18, messageId: 'invalidVoidExprReturnLast' }], + output: ` + function f() { + console.log('foo'); + console.log('bar'); + } + `, + }, + { + code: noFormat` + function f() { + console.log('foo') + return ['bar', 'baz'].forEach(console.log) + } + `, + errors: [{ line: 4, column: 18, messageId: 'invalidVoidExprReturnLast' }], + output: noFormat` + function f() { + console.log('foo') + ;['bar', 'baz'].forEach(console.log); + } + `, + }, + { + code: ` + const f = () => { + if (cond) { + return console.error('foo'); + } + console.log('bar'); + }; + `, + errors: [{ line: 4, column: 20, messageId: 'invalidVoidExprReturn' }], + output: noFormat` + const f = () => { + if (cond) { + console.error('foo'); return; + } + console.log('bar'); + }; + `, + }, + { + code: ` + const f = function () { + if (cond) return console.error('foo'); + console.log('bar'); + }; + `, + errors: [{ line: 3, column: 28, messageId: 'invalidVoidExprReturn' }], + output: noFormat` + const f = function () { + if (cond) { console.error('foo'); return; } + console.log('bar'); + }; + `, + }, + + { + options: [{ ignoreVoidOperator: true }], + code: "return console.log('foo');", + errors: [ + { line: 1, column: 8, messageId: 'invalidVoidExprReturnWrapVoid' }, + ], + output: "return void console.log('foo');", + }, + { + options: [{ ignoreVoidOperator: true }], + code: "console.error(console.log('foo'));", + errors: [ + { + line: 1, + column: 15, + messageId: 'invalidVoidExprWrapVoid', + suggestions: [ + { + messageId: 'voidExprWrapVoid', + output: "console.error(void console.log('foo'));", + }, + ], + }, + ], + }, + { + options: [{ ignoreVoidOperator: true }], + code: "console.log('foo') ? true : false;", + errors: [ + { + line: 1, + column: 1, + messageId: 'invalidVoidExprWrapVoid', + suggestions: [ + { + messageId: 'voidExprWrapVoid', + output: "void console.log('foo') ? true : false;", + }, + ], + }, + ], + }, + { + options: [{ ignoreVoidOperator: true }], + code: "const x = foo ?? console.log('foo');", + errors: [ + { + line: 1, + column: 18, + messageId: 'invalidVoidExprWrapVoid', + suggestions: [ + { + messageId: 'voidExprWrapVoid', + output: "const x = foo ?? void console.log('foo');", + }, + ], + }, + ], + }, + { + options: [{ ignoreVoidOperator: true }], + code: 'foo => foo || console.log(foo);', + errors: [ + { line: 1, column: 15, messageId: 'invalidVoidExprArrowWrapVoid' }, + ], + output: 'foo => foo || void console.log(foo);', + }, + { + options: [{ ignoreVoidOperator: true }], + code: "!!console.log('foo');", + errors: [ + { + line: 1, + column: 3, + messageId: 'invalidVoidExprWrapVoid', + suggestions: [ + { + messageId: 'voidExprWrapVoid', + output: "!!void console.log('foo');", + }, + ], + }, + ], + }, + ], +});