From 501d4c080766b0c03fdabd5d04813d8b727c76e3 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Sat, 17 Dec 2016 21:26:20 -0800 Subject: [PATCH] add rule to detect throwing strings (#1878) --- docs/rules/no-string-throw/index.html | 11 +++++ src/rules/noStringThrowRule.ts | 61 +++++++++++++++++++++++++ test/rules/no-string-throw/test.js.lint | 30 ++++++++++++ test/rules/no-string-throw/test.ts.lint | 30 ++++++++++++ test/rules/no-string-throw/tslint.json | 8 ++++ 5 files changed, 140 insertions(+) create mode 100644 docs/rules/no-string-throw/index.html create mode 100644 src/rules/noStringThrowRule.ts create mode 100644 test/rules/no-string-throw/test.js.lint create mode 100644 test/rules/no-string-throw/test.ts.lint create mode 100644 test/rules/no-string-throw/tslint.json diff --git a/docs/rules/no-string-throw/index.html b/docs/rules/no-string-throw/index.html new file mode 100644 index 00000000000..a5e36309ab5 --- /dev/null +++ b/docs/rules/no-string-throw/index.html @@ -0,0 +1,11 @@ +--- +ruleName: no-string-throw +description: Flags throwing plain strings or concatenations of strings because only Errors produce proper stack traces. +options: null +optionsDescription: '' +type: functionality +typescriptOnly: false +layout: rule +title: 'Rule: no-string-throw' +optionsJSON: 'null' +--- \ No newline at end of file diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts new file mode 100644 index 00000000000..18cd7551466 --- /dev/null +++ b/src/rules/noStringThrowRule.ts @@ -0,0 +1,61 @@ +import * as Lint from "tslint"; +import * as ts from "typescript"; + +export class Rule extends Lint.Rules.AbstractRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "no-string-throw", + description: `Flags throwing plain strings or concatenations of strings ` + + `because only Errors produce proper stack traces.`, + options: null, + optionsDescription: "", + type: "functionality", + typescriptOnly: false, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING = + "Throwing plain strings (not instances of Error) gives no stack traces"; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + public visitThrowStatement(node: ts.ThrowStatement) { + const {expression} = node; + if (this.stringConcatRecursive(expression)) { + const fix = new Lint.Fix( + Rule.metadata.ruleName, + [new Lint.Replacement( + expression.getStart(), + expression.getEnd() - expression.getStart(), + `new Error(${expression.getText()})`)]); + this.addFailure(this.createFailure( + node.getStart(), node.getWidth(), Rule.FAILURE_STRING, fix)); + } + + super.visitThrowStatement(node); + } + + private stringConcatRecursive(node: ts.Node): boolean { + switch (node.kind) { + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.NoSubstitutionTemplateLiteral: + case ts.SyntaxKind.TemplateExpression: + return true; + case ts.SyntaxKind.BinaryExpression: + const n = node as ts.BinaryExpression; + const op = n.operatorToken.kind; + return op === ts.SyntaxKind.PlusToken && + (this.stringConcatRecursive(n.left) || + this.stringConcatRecursive(n.right)); + case ts.SyntaxKind.ParenthesizedExpression: + return this.stringConcatRecursive( + (node as ts.ParenthesizedExpression).expression); + default: + return false; + } + } +} diff --git a/test/rules/no-string-throw/test.js.lint b/test/rules/no-string-throw/test.js.lint new file mode 100644 index 00000000000..e6723ce1a83 --- /dev/null +++ b/test/rules/no-string-throw/test.js.lint @@ -0,0 +1,30 @@ +let a = () => throw 'bla'; + ~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let b = () => throw new Error('bla'); + +let c = () => throw 'string1' + 'string2' + 'string3'; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let d = () => throw 'string' + 1; + ~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let e = () => throw 'string1' + 1 + {}; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let f = () => throw ('string'); + ~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let g = () => throw 1 + 2 + ('string'); + ~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +// no warning because rule does not check for toString() +const one = 1; +let h = () => throw one.toString(); + +let i = () => throw `some template string`; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +const someVariable = 123; +let j = () => throw `template with ${someVariable} string`; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] diff --git a/test/rules/no-string-throw/test.ts.lint b/test/rules/no-string-throw/test.ts.lint new file mode 100644 index 00000000000..569de072840 --- /dev/null +++ b/test/rules/no-string-throw/test.ts.lint @@ -0,0 +1,30 @@ +let a: never = () => throw 'bla'; + ~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let b: never = () => throw new Error('bla'); + +let c: never = () => throw 'string1' + 'string2' + 'string3'; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let d: never = () => throw 'string' + 1; + ~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let e: never = () => throw 'string1' + 1 + {}; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let f: never = () => throw ('string'); + ~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +let g: never = () => throw 1 + 2 + ('string'); + ~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +// no warning because rule does not check for toString() +const one = 1; +let h: never = () => throw one.toString(); + +let i: never = () => throw `some template string`; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +const someVariable = 123; +let j: never = () => throw `template with ${someVariable} string`; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] diff --git a/test/rules/no-string-throw/tslint.json b/test/rules/no-string-throw/tslint.json new file mode 100644 index 00000000000..06fdacc07ab --- /dev/null +++ b/test/rules/no-string-throw/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "no-string-throw": true + }, + "jsRules": { + "no-string-throw": true + } +} \ No newline at end of file