From 00b4a5deecad426ff552791748f89eae51bb3e9d Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Wed, 3 Apr 2019 07:49:45 -0700 Subject: [PATCH 1/4] Include actual types for restrict-plus-operands. It can be difficult for users to find out why exactly they received the "restrict-plus-operands" lint error. In particular, users often end up with `any` types leaking into their code (e.g. in tests), and are then puzzled why this triggers. To fix, this change includes a textual representation of the type in the error message, e.g.: [Operands of '+' operation must either be both strings or both numbers, but found 5 + undefined[]] The type representation isn't always super pretty (e.g. the `undefined[]` bit above), but should still be helpful, and also matches TS compiler's representation of these. --- src/rules/restrictPlusOperandsRule.ts | 22 ++++++------- .../rules/restrict-plus-operands/test.ts.lint | 33 ++++++++++--------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index 207ec49a56f..ea6848e8e88 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/src/rules/restrictPlusOperandsRule.ts @@ -37,7 +37,7 @@ export class Rule extends Lint.Rules.TypedRule { public static INVALID_TYPES_ERROR = "Operands of '+' operation must either be both strings or both numbers"; - public static SUGGEST_TEMPLATE_LITERALS = ", consider using template literals"; + public static SUGGEST_TEMPLATE_LITERALS = ". Consider using template literals."; public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk, undefined, program.getTypeChecker()); @@ -47,17 +47,17 @@ export class Rule extends Lint.Rules.TypedRule { function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker) { return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void { if (isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.PlusToken) { - const leftType = getBaseTypeOfLiteralType(tc.getTypeAtLocation(node.left)); - const rightType = getBaseTypeOfLiteralType(tc.getTypeAtLocation(node.right)); - if (leftType === "invalid" || rightType === "invalid" || leftType !== rightType) { - if (leftType === "string" || rightType === "string") { - return ctx.addFailureAtNode( - node, - Rule.INVALID_TYPES_ERROR + Rule.SUGGEST_TEMPLATE_LITERALS, - ); - } else { - return ctx.addFailureAtNode(node, Rule.INVALID_TYPES_ERROR); + const leftType = tc.getTypeAtLocation(node.left); + const leftTypeStr = getBaseTypeOfLiteralType(leftType); + const rightType = tc.getTypeAtLocation(node.right); + const rightTypeStr = getBaseTypeOfLiteralType(rightType); + if (leftTypeStr === "invalid" || rightTypeStr === "invalid" || leftTypeStr !== rightTypeStr) { + const actualTypes = `, but found ${tc.typeToString(leftType, node)} + ${tc.typeToString(rightType, node)}`; + let message = Rule.INVALID_TYPES_ERROR + actualTypes; + if (leftTypeStr === "string" || rightTypeStr === "string") { + message += Rule.SUGGEST_TEMPLATE_LITERALS; } + return ctx.addFailureAtNode(node, message); } } return ts.forEachChild(node, cb); diff --git a/test/rules/restrict-plus-operands/test.ts.lint b/test/rules/restrict-plus-operands/test.ts.lint index 18ef4acdd28..44b5ffb8e51 100644 --- a/test/rules/restrict-plus-operands/test.ts.lint +++ b/test/rules/restrict-plus-operands/test.ts.lint @@ -17,35 +17,38 @@ var pair: NumberStringPair = { // bad var bad1 = 5 + "10"; - ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + "10". Consider using template literals.] var bad2 = [] + 5; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + 5] var bad3 = [] + {}; - ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + {}] var bad4 = [] + []; - ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + undefined[]] var bad4 = 5 + []; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + undefined[]] var bad5 = "5" + {}; - ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found "5" + {}. Consider using template literals.] var bad6 = 5.5 + "5"; - ~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5.5 + "5". Consider using template literals.] var bad7 = "5.5" + 5; - ~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found "5.5" + 5. Consider using template literals.] var bad8 = x + y; - ~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found number + string. Consider using template literals.] var bad9 = y + x; - ~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found string + number. Consider using template literals.] var bad10 = x + {}; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found number + {}] var bad11 = [] + y; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + string. Consider using template literals.] var bad12 = pair.first + "10"; - ~~~~~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found number + "10". Consider using template literals.] var bad13 = 5 + pair.second; - ~~~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, consider using template literals] + ~~~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + string. Consider using template literals.] var bad14 = pair + pair; - ~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers] + ~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found NumberStringPair + NumberStringPair] +var anyTyped: any = 5; +var bad15 = anyTyped + 12; + ~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found any + 12] // good var good1 = 5 + 10; From 9054ba1e95edd3cbd10ba3e4de7af928a6fd25f7 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Sun, 7 Apr 2019 21:45:06 +0200 Subject: [PATCH 2/4] Special case emitting undefined[] for empty arrays. --- src/rules/restrictPlusOperandsRule.ts | 11 ++++++++++- test/rules/restrict-plus-operands/test.ts.lint | 10 +++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index ea6848e8e88..8ad61e2090c 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/src/rules/restrictPlusOperandsRule.ts @@ -52,7 +52,7 @@ function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker) { const rightType = tc.getTypeAtLocation(node.right); const rightTypeStr = getBaseTypeOfLiteralType(rightType); if (leftTypeStr === "invalid" || rightTypeStr === "invalid" || leftTypeStr !== rightTypeStr) { - const actualTypes = `, but found ${tc.typeToString(leftType, node)} + ${tc.typeToString(rightType, node)}`; + const actualTypes = `, but found ${getTypeString(tc, node.left, leftType)} + ${getTypeString(tc, node.right, rightType)}`; let message = Rule.INVALID_TYPES_ERROR + actualTypes; if (leftTypeStr === "string" || rightTypeStr === "string") { message += Rule.SUGGEST_TEMPLATE_LITERALS; @@ -64,6 +64,15 @@ function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker) { }); } +function getTypeString(tc: ts.TypeChecker, node: ts.Node, type: ts.Type) { + const typeString = tc.typeToString(type, node); + if (typeString === "undefined[]" && ts.isArrayLiteralExpression(node) && !node.elements.length) { + // Special case literal "[]" arrays that would otherwise be emitted as undefined[]. + return "[]"; + } + return typeString; +} + function getBaseTypeOfLiteralType(type: ts.Type): "string" | "number" | "invalid" { if ( isTypeFlagSet(type, ts.TypeFlags.StringLiteral) || diff --git a/test/rules/restrict-plus-operands/test.ts.lint b/test/rules/restrict-plus-operands/test.ts.lint index 44b5ffb8e51..4899d42ced7 100644 --- a/test/rules/restrict-plus-operands/test.ts.lint +++ b/test/rules/restrict-plus-operands/test.ts.lint @@ -19,13 +19,13 @@ var pair: NumberStringPair = { var bad1 = 5 + "10"; ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + "10". Consider using template literals.] var bad2 = [] + 5; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + 5] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found [] + 5] var bad3 = [] + {}; - ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + {}] + ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found [] + {}] var bad4 = [] + []; - ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + undefined[]] + ~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found [] + []] var bad4 = 5 + []; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + undefined[]] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found 5 + []] var bad5 = "5" + {}; ~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found "5" + {}. Consider using template literals.] var bad6 = 5.5 + "5"; @@ -39,7 +39,7 @@ var bad9 = y + x; var bad10 = x + {}; ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found number + {}] var bad11 = [] + y; - ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found undefined[] + string. Consider using template literals.] + ~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found [] + string. Consider using template literals.] var bad12 = pair.first + "10"; ~~~~~~~~~~~~~~~~~ [Operands of '+' operation must either be both strings or both numbers, but found number + "10". Consider using template literals.] var bad13 = 5 + pair.second; From 1a169ae72277e42d1ca524a27125954b865ecf17 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 7 Apr 2019 17:01:52 -0400 Subject: [PATCH 3/4] Trivial change to kickstart CircleCI --- src/rules/restrictPlusOperandsRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index 8ad61e2090c..aaa1f1d953d 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/src/rules/restrictPlusOperandsRule.ts @@ -37,7 +37,7 @@ export class Rule extends Lint.Rules.TypedRule { public static INVALID_TYPES_ERROR = "Operands of '+' operation must either be both strings or both numbers"; - public static SUGGEST_TEMPLATE_LITERALS = ". Consider using template literals."; + public static SUGGEST_TEMPLATE_LITERALS = "; consider using template literals."; public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk, undefined, program.getTypeChecker()); From 8e3060233241f9b6c4f4236de3cf68eb90a96481 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 7 Apr 2019 17:02:22 -0400 Subject: [PATCH 4/4] Revert "Trivial change to kickstart CircleCI" --- src/rules/restrictPlusOperandsRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/restrictPlusOperandsRule.ts b/src/rules/restrictPlusOperandsRule.ts index aaa1f1d953d..8ad61e2090c 100644 --- a/src/rules/restrictPlusOperandsRule.ts +++ b/src/rules/restrictPlusOperandsRule.ts @@ -37,7 +37,7 @@ export class Rule extends Lint.Rules.TypedRule { public static INVALID_TYPES_ERROR = "Operands of '+' operation must either be both strings or both numbers"; - public static SUGGEST_TEMPLATE_LITERALS = "; consider using template literals."; + public static SUGGEST_TEMPLATE_LITERALS = ". Consider using template literals."; public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk, undefined, program.getTypeChecker());