Skip to content

Commit

Permalink
add rule to detect throwing strings (palantir#1878)
Browse files Browse the repository at this point in the history
  • Loading branch information
atscott authored and nchen63 committed Dec 18, 2016
1 parent 552ad84 commit 501d4c0
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 0 deletions.
11 changes: 11 additions & 0 deletions docs/rules/no-string-throw/index.html
Original file line number Diff line number Diff line change
@@ -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'
---
61 changes: 61 additions & 0 deletions src/rules/noStringThrowRule.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
30 changes: 30 additions & 0 deletions test/rules/no-string-throw/test.js.lint
Original file line number Diff line number Diff line change
@@ -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]
30 changes: 30 additions & 0 deletions test/rules/no-string-throw/test.ts.lint
Original file line number Diff line number Diff line change
@@ -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]
8 changes: 8 additions & 0 deletions test/rules/no-string-throw/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"rules": {
"no-string-throw": true
},
"jsRules": {
"no-string-throw": true
}
}

0 comments on commit 501d4c0

Please sign in to comment.