diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 3a504fe7d83a..69ea06c23082 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -169,7 +169,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/sort-type-union-intersection-members`](./docs/rules/sort-type-union-intersection-members.md) | Enforces that members of a type union/intersection are sorted alphabetically | | :wrench: | | -| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: | +| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | :wrench: | :thought_balloon: | | [`@typescript-eslint/switch-exhaustiveness-check`](./docs/rules/switch-exhaustiveness-check.md) | Exhaustiveness checking in switch with union type | | | :thought_balloon: | | [`@typescript-eslint/triple-slash-reference`](./docs/rules/triple-slash-reference.md) | Sets preference level for triple slash directives versus ES6-style import declarations | :heavy_check_mark: | | | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md index 9eb5712ab482..b1ad94d8b340 100644 --- a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md @@ -155,6 +155,38 @@ You should be using `strictNullChecks` to ensure complete type-safety in your co If for some reason you cannot turn on `strictNullChecks`, but still want to use this rule - you can use this option to allow it - but know that the behavior of this rule is _undefined_ with the compiler option turned off. We will not accept bug reports if you are using this option. +## Fixes and suggestions + +This rule provides following fixes and suggestions for particular types in boolean context: + +- `boolean` - Always allowed - no fix needed. +- `string` - (when `allowString` is `false`) - Provides following suggestions: + - Change condition to check string's length (`str` → `str.length > 0`) + - Change condition to check for empty string (`str` → `str !== ""`) + - Explicitly cast value to a boolean (`str` → `Boolean(str)`) +- `number` - (when `allowNumber` is `false`): + - For `array.length` - Provides **autofix**: + - Change condition to check for 0 (`array.length` → `array.length > 0`) + - For other number values - Provides following suggestions: + - Change condition to check for 0 (`num` → `num !== 0`) + - Change condition to check for NaN (`num` → `!Number.isNaN(num)`) + - Explicitly cast value to a boolean (`num` → `Boolean(num)`) +- `object | null | undefined` - (when `allowNullableObject` is `false`) - Provides **autofix**: + - Change condition to check for null/undefined (`maybeObj` → `maybeObj != null`) +- `boolean | null | undefined` - Provides following suggestions: + - Explicitly treat nullish value the same as false (`maybeBool` → `maybeBool ?? false`) + - Change condition to check for true/false (`maybeBool` → `maybeBool === true`) +- `string | null | undefined` - Provides following suggestions: + - Change condition to check for null/undefined (`maybeStr` → `maybeStr != null`) + - Explicitly treat nullish value the same as an empty string (`maybeStr` → `maybeStr ?? ""`) + - Explicitly cast value to a boolean (`maybeStr` → `Boolean(maybeStr)`) +- `number | null | undefined` - Provides following suggestions: + - Change condition to check for null/undefined (`maybeNum` → `maybeNum != null`) + - Explicitly treat nullish value the same as 0 (`maybeNum` → `maybeNum ?? 0`) + - Explicitly cast value to a boolean (`maybeNum` → `Boolean(maybeNum)`) +- `any` and `unknown` - Provides following suggestions: + - Explicitly cast value to a boolean (`value` → `Boolean(value)`) + ## Related To - TSLint: [strict-boolean-expressions](https://palantir.github.io/tslint/rules/strict-boolean-expressions) diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 51265feb1368..b8e25d4542a3 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -1,9 +1,10 @@ import { - TSESTree, AST_NODE_TYPES, + ParserServices, + TSESTree, } from '@typescript-eslint/experimental-utils'; -import * as ts from 'typescript'; import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; import * as util from '../util'; export type Options = [ @@ -30,12 +31,24 @@ export type MessageId = | 'conditionErrorNullableNumber' | 'conditionErrorObject' | 'conditionErrorNullableObject' - | 'noStrictNullCheck'; + | 'noStrictNullCheck' + | 'conditionFixDefaultFalse' + | 'conditionFixDefaultEmptyString' + | 'conditionFixDefaultZero' + | 'conditionFixCompareNullish' + | 'conditionFixCastBoolean' + | 'conditionFixCompareTrue' + | 'conditionFixCompareFalse' + | 'conditionFixCompareStringLength' + | 'conditionFixCompareEmptyString' + | 'conditionFixCompareZero' + | 'conditionFixCompareNaN'; export default util.createRule({ name: 'strict-boolean-expressions', meta: { type: 'suggestion', + fixable: 'code', docs: { description: 'Restricts the types allowed in boolean expressions', category: 'Best Practices', @@ -93,6 +106,29 @@ export default util.createRule({ 'An explicit null check is required.', noStrictNullCheck: 'This rule requires the `strictNullChecks` compiler option to be turned on to function correctly.', + + conditionFixDefaultFalse: + 'Explicitly treat nullish value the same as false (`value ?? false`)', + conditionFixDefaultEmptyString: + 'Explicitly treat nullish value the same as an empty string (`value ?? ""`)', + conditionFixDefaultZero: + 'Explicitly treat nullish value the same as 0 (`value ?? 0`)', + conditionFixCompareNullish: + 'Change condition to check for null/undefined (`value != null`)', + conditionFixCastBoolean: + 'Explicitly cast value to a boolean (`Boolean(value)`)', + conditionFixCompareTrue: + 'Change condition to check if true (`value === true`)', + conditionFixCompareFalse: + 'Change condition to check if false (`value === false`)', + conditionFixCompareStringLength: + "Change condition to check string's length (`value.length !== 0`)", + conditionFixCompareEmptyString: + 'Change condition to check for empty string (`value !== ""`)', + conditionFixCompareZero: + 'Change condition to check for 0 (`value !== 0`)', + conditionFixCompareNaN: + 'Change condition to check for NaN (`!Number.isNaN(value)`)', }, }, defaultOptions: [ @@ -108,9 +144,10 @@ export default util.createRule({ }, ], create(context, [options]) { - const service = util.getParserServices(context); - const checker = service.program.getTypeChecker(); - const compilerOptions = service.program.getCompilerOptions(); + const parserServices = util.getParserServices(context); + const typeChecker = parserServices.program.getTypeChecker(); + const compilerOptions = parserServices.program.getCompilerOptions(); + const sourceCode = context.getSourceCode(); const isStrictNullChecks = tsutils.isStrictCompilerOptionEnabled( compilerOptions, 'strictNullChecks', @@ -187,8 +224,8 @@ export default util.createRule({ return; } - const tsNode = service.esTreeNodeToTSNodeMap.get(node); - const type = util.getConstrainedTypeAtLocation(checker, tsNode); + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode); const types = inspectVariantTypes(tsutils.unionTypeParts(type)); const is = (...wantedTypes: readonly VariantType[]): boolean => @@ -217,7 +254,56 @@ export default util.createRule({ // nullable boolean if (is('nullish', 'boolean')) { if (!options.allowNullableBoolean) { - context.report({ node, messageId: 'conditionErrorNullableBoolean' }); + if (isLogicalNegationExpression(node.parent!)) { + // if (!nullableBoolean) + context.report({ + node, + messageId: 'conditionErrorNullableBoolean', + suggest: [ + { + messageId: 'conditionFixDefaultFalse', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? false`, + }), + }, + { + messageId: 'conditionFixCompareFalse', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} === false`, + }), + }, + ], + }); + } else { + // if (nullableBoolean) + context.report({ + node, + messageId: 'conditionErrorNullableBoolean', + suggest: [ + { + messageId: 'conditionFixDefaultFalse', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? false`, + }), + }, + { + messageId: 'conditionFixCompareTrue', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} === true`, + }), + }, + ], + }); + } } return; } @@ -225,7 +311,74 @@ export default util.createRule({ // string if (is('string')) { if (!options.allowString) { - context.report({ node, messageId: 'conditionErrorString' }); + if (isLogicalNegationExpression(node.parent!)) { + // if (!string) + context.report({ + node, + messageId: 'conditionErrorString', + suggest: [ + { + messageId: 'conditionFixCompareStringLength', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code}.length === 0`, + }), + }, + { + messageId: 'conditionFixCompareEmptyString', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} === ""`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `!Boolean(${code})`, + }), + }, + ], + }); + } else { + // if (string) + context.report({ + node, + messageId: 'conditionErrorString', + suggest: [ + { + messageId: 'conditionFixCompareStringLength', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code}.length > 0`, + }), + }, + { + messageId: 'conditionFixCompareEmptyString', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} !== ""`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `Boolean(${code})`, + }), + }, + ], + }); + } } return; } @@ -233,7 +386,73 @@ export default util.createRule({ // nullable string if (is('nullish', 'string')) { if (!options.allowNullableString) { - context.report({ node, messageId: 'conditionErrorNullableString' }); + if (isLogicalNegationExpression(node.parent!)) { + // if (!nullableString) + context.report({ + node, + messageId: 'conditionErrorNullableString', + suggest: [ + { + messageId: 'conditionFixCompareNullish', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} == null`, + }), + }, + { + messageId: 'conditionFixDefaultEmptyString', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? ""`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `!Boolean(${code})`, + }), + }, + ], + }); + } else { + // if (nullableString) + context.report({ + node, + messageId: 'conditionErrorNullableString', + suggest: [ + { + messageId: 'conditionFixCompareNullish', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} != null`, + }), + }, + { + messageId: 'conditionFixDefaultEmptyString', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? ""`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `Boolean(${code})`, + }), + }, + ], + }); + } } return; } @@ -241,7 +460,101 @@ export default util.createRule({ // number if (is('number')) { if (!options.allowNumber) { - context.report({ node, messageId: 'conditionErrorNumber' }); + if (isArrayLengthExpression(node, typeChecker, parserServices)) { + if (isLogicalNegationExpression(node.parent!)) { + // if (!array.length) + context.report({ + node, + messageId: 'conditionErrorNumber', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} === 0`, + }), + }); + } else { + // if (array.length) + context.report({ + node, + messageId: 'conditionErrorNumber', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} > 0`, + }), + }); + } + } else if (isLogicalNegationExpression(node.parent!)) { + // if (!number) + context.report({ + node, + messageId: 'conditionErrorNumber', + suggest: [ + { + messageId: 'conditionFixCompareZero', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + // TODO: we have to compare to 0n if the type is bigint + wrap: code => `${code} === 0`, + }), + }, + { + // TODO: don't suggest this for bigint because it can't be NaN + messageId: 'conditionFixCompareNaN', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `Number.isNaN(${code})`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `!Boolean(${code})`, + }), + }, + ], + }); + } else { + // if (number) + context.report({ + node, + messageId: 'conditionErrorNumber', + suggest: [ + { + messageId: 'conditionFixCompareZero', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} !== 0`, + }), + }, + { + messageId: 'conditionFixCompareNaN', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `!Number.isNaN(${code})`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `Boolean(${code})`, + }), + }, + ], + }); + } } return; } @@ -249,7 +562,73 @@ export default util.createRule({ // nullable number if (is('nullish', 'number')) { if (!options.allowNullableNumber) { - context.report({ node, messageId: 'conditionErrorNullableNumber' }); + if (isLogicalNegationExpression(node.parent!)) { + // if (!nullableNumber) + context.report({ + node, + messageId: 'conditionErrorNullableNumber', + suggest: [ + { + messageId: 'conditionFixCompareNullish', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} == null`, + }), + }, + { + messageId: 'conditionFixDefaultZero', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? 0`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `!Boolean(${code})`, + }), + }, + ], + }); + } else { + // if (nullableNumber) + context.report({ + node, + messageId: 'conditionErrorNullableNumber', + suggest: [ + { + messageId: 'conditionFixCompareNullish', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} != null`, + }), + }, + { + messageId: 'conditionFixDefaultZero', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} ?? 0`, + }), + }, + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `Boolean(${code})`, + }), + }, + ], + }); + } } return; } @@ -264,7 +643,30 @@ export default util.createRule({ // nullable object if (is('nullish', 'object')) { if (!options.allowNullableObject) { - context.report({ node, messageId: 'conditionErrorNullableObject' }); + if (isLogicalNegationExpression(node.parent!)) { + // if (!nullableObject) + context.report({ + node, + messageId: 'conditionErrorNullableObject', + fix: util.getWrappingFixer({ + sourceCode, + node: node.parent, + innerNode: node, + wrap: code => `${code} == null`, + }), + }); + } else { + // if (nullableObject) + context.report({ + node, + messageId: 'conditionErrorNullableObject', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `${code} != null`, + }), + }); + } } return; } @@ -272,7 +674,20 @@ export default util.createRule({ // any if (is('any')) { if (!options.allowAny) { - context.report({ node, messageId: 'conditionErrorAny' }); + context.report({ + node, + messageId: 'conditionErrorAny', + suggest: [ + { + messageId: 'conditionFixCastBoolean', + fix: util.getWrappingFixer({ + sourceCode, + node, + wrap: code => `Boolean(${code})`, + }), + }, + ], + }); } return; } @@ -370,3 +785,31 @@ export default util.createRule({ } }, }); + +function isLogicalNegationExpression( + node: TSESTree.Node, +): node is TSESTree.UnaryExpression { + return node.type === AST_NODE_TYPES.UnaryExpression && node.operator === '!'; +} + +function isArrayLengthExpression( + node: TSESTree.Node, + typeChecker: ts.TypeChecker, + parserServices: ParserServices, +): node is TSESTree.MemberExpressionNonComputedName { + if (node.type !== AST_NODE_TYPES.MemberExpression) { + return false; + } + if (node.computed) { + return false; + } + if (node.property.name !== 'length') { + return false; + } + const objectTsNode = parserServices.esTreeNodeToTSNodeMap.get(node.object); + const objectType = util.getConstrainedTypeAtLocation( + typeChecker, + objectTsNode, + ); + return util.isTypeArrayTypeOrUnionOfArrayTypes(objectType, typeChecker); +} diff --git a/packages/eslint-plugin/src/util/getWrappingFixer.ts b/packages/eslint-plugin/src/util/getWrappingFixer.ts new file mode 100644 index 000000000000..4a9edcfcc98b --- /dev/null +++ b/packages/eslint-plugin/src/util/getWrappingFixer.ts @@ -0,0 +1,109 @@ +import { + AST_NODE_TYPES, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +interface WrappingFixerParams { + /** Source code. */ + sourceCode: Readonly; + /** The node we want to modify. */ + node: TSESTree.Node; + /** + * Descendant of `node` we want to preserve. + * Use this to replace some code with another. + * By default it's the node we are modifying (so nothing is removed). + */ + innerNode?: TSESTree.Node; + /** + * The function which gets the code of the `innerNode` and returns some code around it. + * E.g. ``code => `${code} != null` `` + */ + wrap: (code: string) => string; +} + +/** + * Wraps node with some code. Adds parenthesis as necessary. + * @returns Fixer which adds the specified code and parens if necessary. + */ +export function getWrappingFixer( + params: WrappingFixerParams, +): TSESLint.ReportFixFunction { + const { sourceCode, node, innerNode = node, wrap } = params; + return (fixer): TSESLint.RuleFix => { + let code = sourceCode.getText(innerNode); + + // check the inner expression's precedence + if ( + innerNode.type !== AST_NODE_TYPES.Literal && + innerNode.type !== AST_NODE_TYPES.Identifier && + innerNode.type !== AST_NODE_TYPES.MemberExpression && + innerNode.type !== AST_NODE_TYPES.CallExpression + ) { + // we are wrapping something else than a simple variable or function call + // the code we are adding might have stronger precedence than our wrapped node + // let's wrap our node in parens in case it has a weaker precedence than the code we are wrapping it in + code = `(${code})`; + } + + // do the wrapping + code = wrap(code); + + let parent = util.nullThrows( + node.parent, + util.NullThrowsReasons.MissingParent, + ); + + // check the outer expression's precedence + if ( + parent.type !== AST_NODE_TYPES.IfStatement && + parent.type !== AST_NODE_TYPES.ForStatement && + parent.type !== AST_NODE_TYPES.WhileStatement && + parent.type !== AST_NODE_TYPES.DoWhileStatement + ) { + // the whole expression's parent is something else than condition of if/for/while + // we wrapped the node in some expression which very likely has a different precedence than original wrapped node + // let's wrap the whole expression in parens just in case + if (!util.isParenthesized(node, sourceCode)) { + code = `(${code})`; + } + } + + // check if we need to insert semicolon + for (;;) { + const prevParent = parent; + parent = parent.parent!; + if ( + parent.type === AST_NODE_TYPES.LogicalExpression || + parent.type === AST_NODE_TYPES.BinaryExpression + ) { + if (parent.left === prevParent) { + // the next parent is a binary expression and current node is on the left + continue; + } + } + if (parent.type === AST_NODE_TYPES.ExpressionStatement) { + const block = parent.parent!; + if ( + block.type === AST_NODE_TYPES.Program || + block.type === AST_NODE_TYPES.BlockStatement + ) { + // the next parent is an expression in a block + const statementIndex = block.body.indexOf(parent); + const previousStatement = block.body[statementIndex - 1]; + if ( + statementIndex > 0 && + sourceCode.getLastToken(previousStatement)!.value !== ';' + ) { + // the previous statement in a block doesn't end with a semicolon + code = `;${code}`; + } + } + } + break; + } + + return fixer.replaceText(node, code); + }; +} diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index fc5346f73466..e7bb53547fc8 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -4,6 +4,7 @@ export * from './astUtils'; export * from './collectUnusedVariables'; export * from './createRule'; export * from './getFunctionHeadLoc'; +export * from './getWrappingFixer'; export * from './isTypeReadonly'; export * from './misc'; export * from './nullThrows'; diff --git a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts index 0d697ebb905d..d24de92ac65f 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -1,13 +1,13 @@ import * as path from 'path'; import rule, { - Options, MessageId, + Options, } from '../../src/rules/strict-boolean-expressions'; import { batchedSingleLineTests, getFixturesRootDir, - RuleTester, noFormat, + RuleTester, } from '../RuleTester'; const rootPath = getFixturesRootDir(); @@ -142,13 +142,49 @@ if (x) { { allowString: false, allowNumber: false, allowNullableObject: false }, ], code: noFormat` - if (true && 1) {} - while (false || "a") {} + if (true && (1 + 1)) {} + while (false || "a" + "b") {} (x: object) => true || false || x ? true : false; `, errors: [ - { messageId: 'conditionErrorNumber', line: 2, column: 13 }, - { messageId: 'conditionErrorString', line: 3, column: 25 }, + { + messageId: 'conditionErrorNumber', + line: 2, + column: 14, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: 'if (true && ((1 + 1) !== 0)) {}', + }, + { + messageId: 'conditionFixCompareNaN', + output: 'if (true && (!Number.isNaN((1 + 1)))) {}', + }, + { + messageId: 'conditionFixCastBoolean', + output: 'if (true && (Boolean((1 + 1)))) {}', + }, + ], + }, + { + messageId: 'conditionErrorString', + line: 3, + column: 25, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: ' while (false || (("a" + "b").length > 0)) {}', + }, + { + messageId: 'conditionFixCompareEmptyString', + output: ' while (false || (("a" + "b") !== "")) {}', + }, + { + messageId: 'conditionFixCastBoolean', + output: ' while (false || (Boolean(("a" + "b")))) {}', + }, + ], + }, { messageId: 'conditionErrorObject', line: 4, column: 41 }, ], }), @@ -158,15 +194,48 @@ if (x) { options: [ { allowString: false, allowNumber: false, allowNullableObject: false }, ], - code: ` - if (('' && {}) || (0 && void 0)) { - } - `, + code: noFormat`if (('' && {}) || (0 && void 0)) { }`, errors: [ - { messageId: 'conditionErrorString', line: 2, column: 14 }, - { messageId: 'conditionErrorObject', line: 2, column: 20 }, - { messageId: 'conditionErrorNumber', line: 2, column: 28 }, - { messageId: 'conditionErrorNullish', line: 2, column: 33 }, + { + messageId: 'conditionErrorString', + line: 1, + column: 6, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: noFormat`if (((''.length > 0) && {}) || (0 && void 0)) { }`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: noFormat`if ((('' !== "") && {}) || (0 && void 0)) { }`, + }, + { + messageId: 'conditionFixCastBoolean', + output: noFormat`if (((Boolean('')) && {}) || (0 && void 0)) { }`, + }, + ], + }, + { messageId: 'conditionErrorObject', line: 1, column: 12 }, + { + messageId: 'conditionErrorNumber', + line: 1, + column: 20, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: noFormat`if (('' && {}) || ((0 !== 0) && void 0)) { }`, + }, + { + messageId: 'conditionFixCompareNaN', + output: noFormat`if (('' && {}) || ((!Number.isNaN(0)) && void 0)) { }`, + }, + { + messageId: 'conditionFixCastBoolean', + output: noFormat`if (('' && {}) || ((Boolean(0)) && void 0)) { }`, + }, + ], + }, + { messageId: 'conditionErrorNullish', line: 1, column: 25 }, ], }, @@ -213,15 +282,105 @@ if (x) { while ("") {} for (; "foo";) {} declare const x: string; if (x) {} - (x: string) => !x; + (x: string) => (!x); (x: T) => x ? 1 : 0; `, errors: [ - { messageId: 'conditionErrorString', line: 2, column: 8 }, - { messageId: 'conditionErrorString', line: 3, column: 16 }, - { messageId: 'conditionErrorString', line: 4, column: 38 }, - { messageId: 'conditionErrorString', line: 5, column: 25 }, - { messageId: 'conditionErrorString', line: 6, column: 37 }, + { + messageId: 'conditionErrorString', + line: 2, + column: 8, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: `while ("".length > 0) {}`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: `while ("" !== "") {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: `while (Boolean("")) {}`, + }, + ], + }, + { + messageId: 'conditionErrorString', + line: 3, + column: 16, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: ` for (; "foo".length > 0;) {}`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: ` for (; "foo" !== "";) {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` for (; Boolean("foo");) {}`, + }, + ], + }, + { + messageId: 'conditionErrorString', + line: 4, + column: 38, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: ` declare const x: string; if (x.length > 0) {}`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: ` declare const x: string; if (x !== "") {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` declare const x: string; if (Boolean(x)) {}`, + }, + ], + }, + { + messageId: 'conditionErrorString', + line: 5, + column: 26, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: ` (x: string) => (x.length === 0);`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: ` (x: string) => (x === "");`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` (x: string) => (!Boolean(x));`, + }, + ], + }, + { + messageId: 'conditionErrorString', + line: 6, + column: 37, + suggestions: [ + { + messageId: 'conditionFixCompareStringLength', + output: ` (x: T) => (x.length > 0) ? 1 : 0;`, + }, + { + messageId: 'conditionFixCompareEmptyString', + output: ` (x: T) => (x !== "") ? 1 : 0;`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` (x: T) => (Boolean(x)) ? 1 : 0;`, + }, + ], + }, ], }), @@ -233,15 +392,169 @@ if (x) { for (; 123;) {} declare const x: number; if (x) {} (x: bigint) => !x; - (x: T) => x ? 1 : 0; + (x: T) => (x) ? 1 : 0; + ![]["length"]; // doesn't count as array.length when computed + declare const a: any[] & { notLength: number }; if (a.notLength) {} + `, + errors: [ + { + messageId: 'conditionErrorNumber', + line: 2, + column: 8, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + // TODO: fix compare zero suggestion for bigint + output: `while (0n !== 0) {}`, + }, + { + // TODO: remove check NaN suggestion for bigint + messageId: 'conditionFixCompareNaN', + output: `while (!Number.isNaN(0n)) {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: `while (Boolean(0n)) {}`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 3, + column: 16, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: ` for (; 123 !== 0;) {}`, + }, + { + messageId: 'conditionFixCompareNaN', + output: ` for (; !Number.isNaN(123);) {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` for (; Boolean(123);) {}`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 4, + column: 38, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: ` declare const x: number; if (x !== 0) {}`, + }, + { + messageId: 'conditionFixCompareNaN', + output: ` declare const x: number; if (!Number.isNaN(x)) {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` declare const x: number; if (Boolean(x)) {}`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 5, + column: 25, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + // TODO: fix compare zero suggestion for bigint + output: ` (x: bigint) => (x === 0);`, + }, + { + // TODO: remove check NaN suggestion for bigint + messageId: 'conditionFixCompareNaN', + output: ` (x: bigint) => (Number.isNaN(x));`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` (x: bigint) => (!Boolean(x));`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 6, + column: 38, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: ` (x: T) => (x !== 0) ? 1 : 0;`, + }, + { + messageId: 'conditionFixCompareNaN', + output: ` (x: T) => (!Number.isNaN(x)) ? 1 : 0;`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` (x: T) => (Boolean(x)) ? 1 : 0;`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 7, + column: 10, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: ` ([]["length"] === 0); // doesn't count as array.length when computed`, + }, + { + messageId: 'conditionFixCompareNaN', + output: ` (Number.isNaN([]["length"])); // doesn't count as array.length when computed`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` (!Boolean([]["length"])); // doesn't count as array.length when computed`, + }, + ], + }, + { + messageId: 'conditionErrorNumber', + line: 8, + column: 61, + suggestions: [ + { + messageId: 'conditionFixCompareZero', + output: ` declare const a: any[] & { notLength: number }; if (a.notLength !== 0) {}`, + }, + { + messageId: 'conditionFixCompareNaN', + output: ` declare const a: any[] & { notLength: number }; if (!Number.isNaN(a.notLength)) {}`, + }, + { + messageId: 'conditionFixCastBoolean', + output: ` declare const a: any[] & { notLength: number }; if (Boolean(a.notLength)) {}`, + }, + ], + }, + ], + }), + + // number (array.length) in boolean context + ...batchedSingleLineTests({ + options: [{ allowNumber: false }], + code: noFormat` + if (![].length) {} + (a: number[]) => a.length && "..." + (...a: T) => a.length || "empty"; `, errors: [ - { messageId: 'conditionErrorNumber', line: 2, column: 8 }, - { messageId: 'conditionErrorNumber', line: 3, column: 16 }, - { messageId: 'conditionErrorNumber', line: 4, column: 38 }, - { messageId: 'conditionErrorNumber', line: 5, column: 25 }, - { messageId: 'conditionErrorNumber', line: 6, column: 37 }, + { messageId: 'conditionErrorNumber', line: 2, column: 6 }, + { messageId: 'conditionErrorNumber', line: 3, column: 26 }, + { messageId: 'conditionErrorNumber', line: 4, column: 43 }, ], + output: noFormat` + if ([].length === 0) {} + (a: number[]) => (a.length > 0) && "..." + (...a: T) => (a.length > 0) || "empty"; + `, }), // mixed `string | number` value in boolean context @@ -268,9 +581,51 @@ if (x) { (x: T) => x ? 1 : 0; `, errors: [ - { messageId: 'conditionErrorNullableBoolean', line: 2, column: 38 }, - { messageId: 'conditionErrorNullableBoolean', line: 3, column: 27 }, - { messageId: 'conditionErrorNullableBoolean', line: 4, column: 57 }, + { + messageId: 'conditionErrorNullableBoolean', + line: 2, + column: 38, + suggestions: [ + { + messageId: 'conditionFixDefaultFalse', + output: `declare const x: boolean | null; if (x ?? false) {}`, + }, + { + messageId: 'conditionFixCompareTrue', + output: `declare const x: boolean | null; if (x === true) {}`, + }, + ], + }, + { + messageId: 'conditionErrorNullableBoolean', + line: 3, + column: 27, + suggestions: [ + { + messageId: 'conditionFixDefaultFalse', + output: ` (x?: boolean) => !(x ?? false);`, + }, + { + messageId: 'conditionFixCompareFalse', + output: ` (x?: boolean) => (x === false);`, + }, + ], + }, + { + messageId: 'conditionErrorNullableBoolean', + line: 4, + column: 57, + suggestions: [ + { + messageId: 'conditionFixDefaultFalse', + output: ` (x: T) => (x ?? false) ? 1 : 0;`, + }, + { + messageId: 'conditionFixCompareTrue', + output: ` (x: T) => (x === true) ? 1 : 0;`, + }, + ], + }, ], }), @@ -287,6 +642,11 @@ if (x) { { messageId: 'conditionErrorNullableObject', line: 3, column: 33 }, { messageId: 'conditionErrorNullableObject', line: 4, column: 52 }, ], + output: noFormat` + declare const x: object | null; if (x != null) {} + (x?: { a: number }) => (x == null); + (x: T) => (x != null) ? 1 : 0; + `, }), // nullable string in boolean context @@ -297,9 +657,66 @@ if (x) { (x: T) => x ? 1 : 0; `, errors: [ - { messageId: 'conditionErrorNullableString', line: 2, column: 37 }, - { messageId: 'conditionErrorNullableString', line: 3, column: 26 }, - { messageId: 'conditionErrorNullableString', line: 4, column: 56 }, + { + messageId: 'conditionErrorNullableString', + line: 2, + column: 37, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: 'declare const x: string | null; if (x != null) {}', + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: 'declare const x: string | null; if (x ?? "") {}', + }, + { + messageId: 'conditionFixCastBoolean', + output: 'declare const x: string | null; if (Boolean(x)) {}', + }, + ], + }, + { + messageId: 'conditionErrorNullableString', + line: 3, + column: 26, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ' (x?: string) => (x == null);', + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: ' (x?: string) => !(x ?? "");', + }, + { + messageId: 'conditionFixCastBoolean', + output: ' (x?: string) => (!Boolean(x));', + }, + ], + }, + { + messageId: 'conditionErrorNullableString', + line: 4, + column: 56, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: + ' (x: T) => (x != null) ? 1 : 0;', + }, + { + messageId: 'conditionFixDefaultEmptyString', + output: + ' (x: T) => (x ?? "") ? 1 : 0;', + }, + { + messageId: 'conditionFixCastBoolean', + output: + ' (x: T) => (Boolean(x)) ? 1 : 0;', + }, + ], + }, ], }), @@ -311,9 +728,66 @@ if (x) { (x: T) => x ? 1 : 0; `, errors: [ - { messageId: 'conditionErrorNullableNumber', line: 2, column: 37 }, - { messageId: 'conditionErrorNullableNumber', line: 3, column: 26 }, - { messageId: 'conditionErrorNullableNumber', line: 4, column: 56 }, + { + messageId: 'conditionErrorNullableNumber', + line: 2, + column: 37, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: 'declare const x: number | null; if (x != null) {}', + }, + { + messageId: 'conditionFixDefaultZero', + output: 'declare const x: number | null; if (x ?? 0) {}', + }, + { + messageId: 'conditionFixCastBoolean', + output: 'declare const x: number | null; if (Boolean(x)) {}', + }, + ], + }, + { + messageId: 'conditionErrorNullableNumber', + line: 3, + column: 26, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: ' (x?: number) => (x == null);', + }, + { + messageId: 'conditionFixDefaultZero', + output: ' (x?: number) => !(x ?? 0);', + }, + { + messageId: 'conditionFixCastBoolean', + output: ' (x?: number) => (!Boolean(x));', + }, + ], + }, + { + messageId: 'conditionErrorNullableNumber', + line: 4, + column: 56, + suggestions: [ + { + messageId: 'conditionFixCompareNullish', + output: + ' (x: T) => (x != null) ? 1 : 0;', + }, + { + messageId: 'conditionFixDefaultZero', + output: + ' (x: T) => (x ?? 0) ? 1 : 0;', + }, + { + messageId: 'conditionFixCastBoolean', + output: + ' (x: T) => (Boolean(x)) ? 1 : 0;', + }, + ], + }, ], }), @@ -326,11 +800,43 @@ if (x) { (x: T) => x ? 1 : 0; `, errors: [ - { messageId: 'conditionErrorAny', line: 2, column: 5 }, - { messageId: 'conditionErrorAny', line: 3, column: 15 }, - { messageId: 'conditionErrorAny', line: 4, column: 34 }, + { + messageId: 'conditionErrorAny', + line: 2, + column: 5, + suggestions: [ + { + messageId: 'conditionFixCastBoolean', + output: 'if (Boolean(x)) {}', + }, + ], + }, + { + messageId: 'conditionErrorAny', + line: 3, + column: 15, + suggestions: [ + { + messageId: 'conditionFixCastBoolean', + output: ' x => !(Boolean(x));', + }, + ], + }, + { + messageId: 'conditionErrorAny', + line: 4, + column: 34, + suggestions: [ + { + messageId: 'conditionFixCastBoolean', + output: ' (x: T) => (Boolean(x)) ? 1 : 0;', + }, + ], + }, ], }), + + // noStrictNullCheck { code: ` declare const x: string[] | null; @@ -353,5 +859,27 @@ if (x) { tsconfigRootDir: path.join(rootPath, 'unstrict'), }, }, + + // automatic semicolon insertion test + { + options: [{ allowNullableObject: false }], + code: noFormat` + declare const obj: { x: number } | null; + !obj + obj || 0 + obj && 1 || 0 + `, + errors: [ + { messageId: 'conditionErrorNullableObject', line: 3, column: 10 }, + { messageId: 'conditionErrorNullableObject', line: 4, column: 9 }, + { messageId: 'conditionErrorNullableObject', line: 5, column: 9 }, + ], + output: noFormat` + declare const obj: { x: number } | null; + (obj == null) + ;(obj != null) || 0 + ;(obj != null) && 1 || 0 + `, + }, ], });