Fix #4530: no-string-throw fixer creates syntax errors (#4540)

* fix for #4530

* optimization + new test cases

* make fix more uniform

* use .pos instead of .getStart

* Revert "use .pos instead of .getStart"

This reverts commit 72885ee.

* don't overlap fixes

src/rules/noStringThrowRule.ts

@@ -73,8 +73,14 @@ function walk(ctx: Lint.WalkContext): void {
         if (isThrowStatement(node)) {
             const { expression } = node;
             if (expression !== undefined && isString(expression)) {
+                // 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;
+                const newError = numCharactersBetweenTokens === 0 ? ` new Error(` : `new Error(`;
+
                 ctx.addFailureAtNode(node, Rule.FAILURE_STRING, [
-                    Lint.Replacement.appendText(expression.getStart(sourceFile), "new Error("),
+                    Lint.Replacement.appendText(expression.getStart(sourceFile), newError),
                     Lint.Replacement.appendText(expression.getEnd(), ")"),
                 ]);
             }

test/rules/no-string-throw/test.ts.fix

@@ -0,0 +1,30 @@
+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'));
+
+throw new Error('component...')
+
+throw new Error((('component...')))
+
+throw/**/new Error('component')

test/rules/no-string-throw/test.ts.lint

@@ -28,3 +28,15 @@ 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]
+
+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]