From 4b2e2e2a0c9ff8ebb027cf19cb471638b232eefd Mon Sep 17 00:00:00 2001 From: rrogowski Date: Sun, 24 Feb 2019 11:07:52 -0500 Subject: [PATCH 1/6] fix for #4530 --- src/rules/noStringThrowRule.ts | 14 ++++++++++++-- test/rules/no-string-throw/test.ts.fix | 24 ++++++++++++++++++++++++ test/rules/no-string-throw/test.ts.lint | 3 +++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/rules/no-string-throw/test.ts.fix diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index a970d890a74..50262e73836 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -73,10 +73,20 @@ function walk(ctx: Lint.WalkContext): void { if (isThrowStatement(node)) { const { expression } = node; if (expression !== undefined && isString(expression)) { - ctx.addFailureAtNode(node, Rule.FAILURE_STRING, [ + const fix = [ Lint.Replacement.appendText(expression.getStart(sourceFile), "new Error("), Lint.Replacement.appendText(expression.getEnd(), ")"), - ]); + ]; + + const hasWhitespaceAfterThrow = /^throw\s+/.test(node.getText()); + if (!hasWhitespaceAfterThrow) { + const token = node.getFirstToken() as ts.Node; + if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { + fix.push(Lint.Replacement.appendText(token.getEnd(), " ")); + } + } + + ctx.addFailureAtNode(node, Rule.FAILURE_STRING, fix); } } return ts.forEachChild(node, cb); diff --git a/test/rules/no-string-throw/test.ts.fix b/test/rules/no-string-throw/test.ts.fix new file mode 100644 index 00000000000..39451aff0bb --- /dev/null +++ b/test/rules/no-string-throw/test.ts.fix @@ -0,0 +1,24 @@ +let a: never = () => throw new Error('bla'); + +let b: never = () => throw new Error('bla'); + +let c: never = () => throw new Error('string1' + 'string2' + 'string3'); + +let d: never = () => throw new Error('string' + 1); + +let e: never = () => throw new Error('string1' + 1 + {}); + +let f: never = () => throw new Error(('string')); + +let g: never = () => throw new Error(1 + 2 + ('string')); + +// no warning because rule does not check for toString() +const one = 1; +let h: never = () => throw one.toString(); + +let i: never = () => throw new Error(`some template string`); + +const someVariable = 123; +let j: never = () => throw new Error(`template with ${someVariable} string`); + +throw new Error(('component requires CSS height property')); diff --git a/test/rules/no-string-throw/test.ts.lint b/test/rules/no-string-throw/test.ts.lint index 569de072840..bb109d0c81c 100644 --- a/test/rules/no-string-throw/test.ts.lint +++ b/test/rules/no-string-throw/test.ts.lint @@ -28,3 +28,6 @@ let i: never = () => throw `some template string`; const someVariable = 123; let j: never = () => throw `template with ${someVariable} string`; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +throw('component requires CSS height property'); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] From aa80967d2cfde614164f7cf13bb32ed9e17e0bab Mon Sep 17 00:00:00 2001 From: rrogowski Date: Mon, 25 Feb 2019 09:53:10 -0500 Subject: [PATCH 2/6] optimization + new test cases --- src/rules/noStringThrowRule.ts | 7 +++++-- test/rules/no-string-throw/test.ts.fix | 6 ++++++ test/rules/no-string-throw/test.ts.lint | 9 +++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index 50262e73836..57e9aa2e315 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -78,8 +78,11 @@ function walk(ctx: Lint.WalkContext): void { Lint.Replacement.appendText(expression.getEnd(), ")"), ]; - const hasWhitespaceAfterThrow = /^throw\s+/.test(node.getText()); - if (!hasWhitespaceAfterThrow) { + // To prevent this fix from creating invalid syntax, we must ensure that the "throw" + // token is succeeded by a space if no other characters precede the string literal. + const offset = expression.getStart() - node.getStart(); + const numCharactersBetweenTokens = offset - "throw".length; + if (numCharactersBetweenTokens === 0) { const token = node.getFirstToken() as ts.Node; if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { fix.push(Lint.Replacement.appendText(token.getEnd(), " ")); diff --git a/test/rules/no-string-throw/test.ts.fix b/test/rules/no-string-throw/test.ts.fix index 39451aff0bb..92fc925fa7b 100644 --- a/test/rules/no-string-throw/test.ts.fix +++ b/test/rules/no-string-throw/test.ts.fix @@ -22,3 +22,9 @@ const someVariable = 123; let j: never = () => throw new Error(`template with ${someVariable} string`); throw new Error(('component requires CSS height property')); + +throw new Error('component...') + +throw new Error((('component...'))) + +throw/**/new Error('component') diff --git a/test/rules/no-string-throw/test.ts.lint b/test/rules/no-string-throw/test.ts.lint index bb109d0c81c..788fb9af842 100644 --- a/test/rules/no-string-throw/test.ts.lint +++ b/test/rules/no-string-throw/test.ts.lint @@ -31,3 +31,12 @@ let j: never = () => throw `template with ${someVariable} string`; throw('component requires CSS height property'); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +throw'component...' +~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +throw(('component...')) +~~~~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] + +throw/**/'component' +~~~~~~~~~~~~~~~~~~~~ [Throwing plain strings (not instances of Error) gives no stack traces] From b106d29e8f764fd9c24f89149e82d89f674fd932 Mon Sep 17 00:00:00 2001 From: rrogowski Date: Mon, 25 Feb 2019 09:58:12 -0500 Subject: [PATCH 3/6] make fix more uniform --- src/rules/noStringThrowRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index 57e9aa2e315..42b6eae449f 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -85,7 +85,7 @@ function walk(ctx: Lint.WalkContext): void { if (numCharactersBetweenTokens === 0) { const token = node.getFirstToken() as ts.Node; if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { - fix.push(Lint.Replacement.appendText(token.getEnd(), " ")); + fix.push(Lint.Replacement.appendText(expression.getStart(sourceFile), " ")); } } From 72885ee6dccddb3a09740703990ebe79d19d05d3 Mon Sep 17 00:00:00 2001 From: rrogowski Date: Tue, 5 Mar 2019 13:32:46 -0500 Subject: [PATCH 4/6] use .pos instead of .getStart --- src/rules/noStringThrowRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index 42b6eae449f..cc1148810a7 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -85,7 +85,7 @@ function walk(ctx: Lint.WalkContext): void { if (numCharactersBetweenTokens === 0) { const token = node.getFirstToken() as ts.Node; if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { - fix.push(Lint.Replacement.appendText(expression.getStart(sourceFile), " ")); + fix.push(Lint.Replacement.appendText(expression.pos, " ")); } } From b23369e90ec1ae727f650696dbe9adb625939bfa Mon Sep 17 00:00:00 2001 From: rrogowski Date: Tue, 5 Mar 2019 13:38:25 -0500 Subject: [PATCH 5/6] Revert "use .pos instead of .getStart" This reverts commit 72885ee6dccddb3a09740703990ebe79d19d05d3. --- src/rules/noStringThrowRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index cc1148810a7..42b6eae449f 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -85,7 +85,7 @@ function walk(ctx: Lint.WalkContext): void { if (numCharactersBetweenTokens === 0) { const token = node.getFirstToken() as ts.Node; if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { - fix.push(Lint.Replacement.appendText(expression.pos, " ")); + fix.push(Lint.Replacement.appendText(expression.getStart(sourceFile), " ")); } } From d535fda67113cb6dac316d09f53cc225f4adf9c3 Mon Sep 17 00:00:00 2001 From: rrogowski Date: Thu, 21 Mar 2019 11:20:19 -0400 Subject: [PATCH 6/6] don't overlap fixes --- src/rules/noStringThrowRule.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/rules/noStringThrowRule.ts b/src/rules/noStringThrowRule.ts index 42b6eae449f..8c8cc7d91cc 100644 --- a/src/rules/noStringThrowRule.ts +++ b/src/rules/noStringThrowRule.ts @@ -73,23 +73,16 @@ function walk(ctx: Lint.WalkContext): void { if (isThrowStatement(node)) { const { expression } = node; if (expression !== undefined && isString(expression)) { - const fix = [ - Lint.Replacement.appendText(expression.getStart(sourceFile), "new Error("), - Lint.Replacement.appendText(expression.getEnd(), ")"), - ]; - // To prevent this fix from creating invalid syntax, we must ensure that the "throw" // token is succeeded by a space if no other characters precede the string literal. const offset = expression.getStart() - node.getStart(); const numCharactersBetweenTokens = offset - "throw".length; - if (numCharactersBetweenTokens === 0) { - const token = node.getFirstToken() as ts.Node; - if (token !== undefined && token.kind === ts.SyntaxKind.ThrowKeyword) { - fix.push(Lint.Replacement.appendText(expression.getStart(sourceFile), " ")); - } - } + const newError = numCharactersBetweenTokens === 0 ? ` new Error(` : `new Error(`; - ctx.addFailureAtNode(node, Rule.FAILURE_STRING, fix); + ctx.addFailureAtNode(node, Rule.FAILURE_STRING, [ + Lint.Replacement.appendText(expression.getStart(sourceFile), newError), + Lint.Replacement.appendText(expression.getEnd(), ")"), + ]); } } return ts.forEachChild(node, cb);