prefer-template: Allow "a" + "b" + "c", and enable rule (#2741)

src/configuration.ts

@@ -168,7 +168,7 @@ function findup(filename: string, directory: string): string | undefined {
         const filenameLower = filename.toLowerCase();
         const result = fs.readdirSync(cwd).find((entry) => entry.toLowerCase() === filenameLower);
         if (result !== undefined) {
-            showWarningOnce(`Using mixed case tslint.json is deprecated. Found: ` + path.join(cwd, result));
+            showWarningOnce(`Using mixed case tslint.json is deprecated. Found: ${path.join(cwd, result)}`);
         }
         return result;
     }
@@ -243,6 +243,7 @@ function resolveConfigurationPath(filePath: string, relativeTo?: string) {
         try {
             return require.resolve(filePath);
         } catch (err) {
+            // tslint:disable-next-line prefer-template (fixed in 5.3)
             throw new Error(`Invalid "extends" configuration value - could not require "${filePath}". ` +
                 "Review the Node lookup algorithm (https://nodejs.org/api/modules.html#modules_all_together) " +
                 "for the approximate method TSLint uses to find the referenced configuration file.");

src/formatters/checkstyleFormatter.ts

@@ -54,14 +54,14 @@ export class Formatter extends AbstractFormatter {
                         output += "</file>";
                     }
                     previousFilename = failure.getFileName();
-                    output += "<file name=\"" + this.escapeXml(failure.getFileName()) + "\">";
+                    output += `<file name="${this.escapeXml(failure.getFileName())}">`;
                 }
-                output += "<error line=\"" + (failure.getStartPosition().getLineAndCharacter().line + 1) + "\" ";
-                output += "column=\"" + (failure.getStartPosition().getLineAndCharacter().character + 1) + "\" ";
-                output += "severity=\"" + severity + "\" ";
-                output += "message=\"" + this.escapeXml(failure.getFailure()) + "\" ";
+                output += `<error line="${failure.getStartPosition().getLineAndCharacter().line + 1}" `;
+                output += `column="${failure.getStartPosition().getLineAndCharacter().character + 1}" `;
+                output += `severity="${severity}" `;
+                output += `message="${this.escapeXml(failure.getFailure())}" `;
                 // checkstyle parser wants "source" to have structure like <anything>dot<category>dot<type>
-                output += "source=\"failure.tslint." + this.escapeXml(failure.getRuleName()) + "\" />";
+                output += `source="failure.tslint.${this.escapeXml(failure.getRuleName())}" />`;
             }
             if (previousFilename !== null) {
                 output += "</file>";

src/formatters/codeFrameFormatter.ts

@@ -95,6 +95,6 @@ export class Formatter extends AbstractFormatter {
             outputLines.shift();
         }
 
-        return outputLines.join("\n") + "\n";
+        return `${outputLines.join("\n")}\n`;
     }
 }

src/formatters/fileslistFormatter.ts

@@ -45,6 +45,6 @@ export class Formatter extends AbstractFormatter {
             }
         }
 
-        return files.join("\n") + "\n";
+        return `${files.join("\n")}\n`;
     }
 }

src/formatters/msbuildFormatter.ts

@@ -47,6 +47,6 @@ export class Formatter extends AbstractFormatter {
             return `${fileName}${positionTuple}: ${severity} ${camelizedRule}: ${failureString}`;
         });
 
-        return outputLines.join("\n") + "\n";
+        return `${outputLines.join("\n")}\n`;
     }
 }

src/formatters/pmdFormatter.ts

@@ -51,11 +51,11 @@ export class Formatter extends AbstractFormatter {
             const lineAndCharacter = failure.getStartPosition().getLineAndCharacter();
             const priority = failure.getRuleSeverity() === "warning" ? 4 : 3;
 
-            output += "<file name=\"" + failure.getFileName();
-            output += "\"><violation begincolumn=\"" + (lineAndCharacter.character + 1);
-            output += "\" beginline=\"" + (lineAndCharacter.line + 1);
-            output += "\" priority=\"" + priority + "\"";
-            output += " rule=\"" + failureString + "\"> </violation></file>";
+            output += `<file name="${failure.getFileName()}`;
+            output += `"><violation begincolumn="${lineAndCharacter.character + 1}`;
+            output += `" beginline="${lineAndCharacter.line + 1}`;
+            output += `" priority="${priority}"`;
+            output += ` rule="${failureString}"></violation></file>`;
         }
 
         output += "</pmd>";

src/formatters/proseFormatter.ts

@@ -58,6 +58,6 @@ export class Formatter extends AbstractFormatter {
             return `${failure.getRuleSeverity().toUpperCase()}: ${fileName}${positionTuple}: ${failureString}`;
         });
 
-        return fixLines.concat(errorLines).join("\n") + "\n";
+        return `${fixLines.concat(errorLines).join("\n")}\n`;
     }
 }

src/formatters/stylishFormatter.ts

@@ -46,7 +46,7 @@ export class Formatter extends AbstractFormatter {
             outputLines.shift();
         }
 
-        return outputLines.join("\n") + "\n";
+        return `${outputLines.join("\n")}\n`;
     }
 
     private mapToMessages(failures: RuleFailure[]): string[] {
@@ -84,9 +84,9 @@ export class Formatter extends AbstractFormatter {
             positionTuple = this.pad(positionTuple, positionMaxSize);
 
             if (failure.getRuleSeverity() === "warning") {
-                positionTuple = colors.blue(failure.getRuleSeverity().toUpperCase() + ": " + positionTuple);
+                positionTuple = colors.blue(`${failure.getRuleSeverity().toUpperCase()}: ${positionTuple}`);
             } else {
-                positionTuple = colors.red(failure.getRuleSeverity().toUpperCase() + ": " + positionTuple);
+                positionTuple = colors.red(`${failure.getRuleSeverity().toUpperCase()}: ${positionTuple}`);
             }
 
             // Output

src/formatters/tapFormatter.ts

@@ -57,7 +57,7 @@ export class Formatter extends AbstractFormatter {
             output = output.concat([`1..${failures.length}`]).concat(this.mapToMessages(failures));
         }
 
-        return output.join("\n") + "\n";
+        return `${output.join("\n")}\n`;
     }
 
     private mapToMessages(failures: RuleFailure[]): string[] {

src/formatters/verboseFormatter.ts

@@ -31,9 +31,7 @@ export class Formatter extends AbstractFormatter {
     /* tslint:enable:object-literal-sort-keys */
 
     public format(failures: RuleFailure[]): string {
-
-        return this.mapToMessages(failures)
-            .join("\n") + "\n";
+        return `${this.mapToMessages(failures).join("\n")}\n`;
     }
 
     private mapToMessages(failures: RuleFailure[]): string[] {
@@ -43,7 +41,7 @@ export class Formatter extends AbstractFormatter {
             const ruleName = failure.getRuleName();
 
             const lineAndCharacter = failure.getStartPosition().getLineAndCharacter();
-            const positionTuple = "[" + (lineAndCharacter.line + 1) + ", " + (lineAndCharacter.character + 1) + "]";
+            const positionTuple = `[${lineAndCharacter.line + 1}, ${lineAndCharacter.character + 1}]`;
 
             return `${failure.getRuleSeverity().toUpperCase()}: (${ruleName}) ${fileName}${positionTuple}: ${failureString}`;
         });

src/formatters/vsoFormatter.ts

@@ -49,6 +49,6 @@ export class Formatter extends AbstractFormatter {
             return `##vso[task.logissue type=warning;${properties}]${failureString}`;
         });
 
-        return outputLines.join("\n") + "\n";
+        return `${outputLines.join("\n")}\n`;
     }
 }

src/linter.ts

@@ -81,7 +81,7 @@ class Linter {
 
     constructor(private options: ILinterOptions, private program?: ts.Program) {
         if (typeof options !== "object") {
-            throw new Error("Unknown Linter options type: " + typeof options);
+            throw new Error(`Unknown Linter options type: ${typeof options}`);
         }
         if ((options as any).configuration != null) {
             throw new Error("ILinterOptions does not contain the property `configuration` as of version 4. " +

src/ruleLoader.ts

@@ -96,10 +96,10 @@ function transformName(name: string): string {
     // camelize strips out leading and trailing underscores and dashes, so make sure they aren't passed to camelize
     // the regex matches the groups (leading underscores and dashes)(other characters)(trailing underscores and dashes)
     const nameMatch = name.match(/^([-_]*)(.*?)([-_]*)$/);
-    if (nameMatch == null) {
-        return name + "Rule";
+    if (nameMatch === null) {
+        return `${name}Rule`;
     }
-    return nameMatch[1] + camelize(nameMatch[2]) + nameMatch[3] + "Rule";
+    return `${nameMatch[1]}${camelize(nameMatch[2])}${nameMatch[3]}Rule`;
 }
 
 /**
@@ -108,7 +108,7 @@ function transformName(name: string): string {
  */
 function loadRule(directory: string, ruleName: string): RuleConstructor | "not-found" {
     const fullPath = path.join(directory, ruleName);
-    if (fs.existsSync(fullPath + ".js")) {
+    if (fs.existsSync(`${fullPath}.js`)) {
         const ruleModule = require(fullPath) as { Rule: RuleConstructor } | undefined;
         if (ruleModule !== undefined) {
             return ruleModule.Rule;

src/rules/arrayTypeRule.ts

@@ -81,7 +81,7 @@ function walk(ctx: Lint.WalkContext<Option>): void {
         // Add a space if the type is preceded by 'as' and the node has no leading whitespace
         const space = parens === 0 && parent!.kind === ts.SyntaxKind.AsExpression && node.getStart() === node.getFullStart();
         const fix = [
-            new Lint.Replacement(elementType.getStart(), parens, (space ? " " : "") + "Array<"),
+            new Lint.Replacement(elementType.getStart(), parens, `${space ? " " : ""}Array<`),
             // Delete the square brackets and replace with an angle bracket
             Lint.Replacement.replaceFromTo(elementType.getEnd() - parens, node.getEnd(), ">"),
         ];

src/rules/banRule.ts

@@ -50,7 +50,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:enable:object-literal-sort-keys */
 
     public static FAILURE_STRING_FACTORY(expression: string, messageAddition?: string) {
-        return `Calls to '${expression}' are not allowed.${messageAddition !== undefined ? " " + messageAddition : ""}`;
+        return `Calls to '${expression}' are not allowed.${messageAddition !== undefined ? ` ${messageAddition}` : ""}`;
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {

src/rules/banTypesRule.ts

@@ -51,8 +51,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:enable:object-literal-sort-keys */
 
     public static FAILURE_STRING_FACTORY(typeName: string, messageAddition?: string) {
-        return `Don't use '${typeName}' as a type.` +
-            (messageAddition !== undefined ? " " + messageAddition : "");
+        return `Don't use '${typeName}' as a type.${messageAddition !== undefined ? ` ${messageAddition}` : ""}`;
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {

src/rules/callableTypesRule.ts

@@ -82,7 +82,7 @@ function renderSuggestion(call: ts.CallSignatureDeclaration,
     const start = call.getStart(sourceFile);
     const colonPos = call.type!.pos - 1 - start;
     const text = sourceFile.text.substring(start, call.end);
-    const suggestion = text.substr(0, colonPos) + " =>" + text.substr(colonPos + 1);
+    const suggestion = `${text.substr(0, colonPos)} =>${text.substr(colonPos + 1)}`;
 
     if (parent.kind === ts.SyntaxKind.InterfaceDeclaration) {
         if (parent.typeParameters !== undefined) {

src/rules/completedDocsRule.ts

@@ -425,10 +425,10 @@ class CompletedDocsWalker extends Lint.ProgramAwareRuleWalker {
         let description = Rule.FAILURE_STRING_EXIST;
 
         if (node.modifiers !== undefined) {
-            description += node.modifiers.map((modifier) => this.describeModifier(modifier.kind)) + " ";
+            description += `${node.modifiers.map((modifier) => this.describeModifier(modifier.kind)).join(",")} `;
         }
 
-        return description + nodeType + ".";
+        return `${description}${nodeType}.`;
     }
 
     private describeModifier(kind: ts.SyntaxKind) {

src/rules/cyclomaticComplexityRule.ts

@@ -56,7 +56,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:enable:object-literal-sort-keys */
 
     public static FAILURE_STRING(expected: number, actual: number, name?: string): string {
-        return `The function${name === undefined ? "" : " " + name} has a cyclomatic complexity of ` +
+        return `The function${name === undefined ? "" : ` ${name}`} has a cyclomatic complexity of ` +
             `${actual} which is higher than the threshold of ${expected}`;
     }
 

src/rules/deprecationRule.ts

@@ -86,7 +86,7 @@ class Walker extends Lint.ProgramAwareRuleWalker {
       for (const {pos, end} of range) {
         const jsDocText = commentNode.getFullText().substring(pos, end);
         if (jsDocText.includes("@deprecated")) {
-            this.addFailureAtNode(node, node.text + " is deprecated.");
+            this.addFailureAtNode(node, `${node.text} is deprecated.`);
         }
       }
     }

src/rules/memberAccessRule.ts

@@ -134,6 +134,6 @@ function memberType(node: ts.ClassElement): string {
         case ts.SyntaxKind.SetAccessor:
             return "set property accessor";
         default:
-            throw new Error("unhandled node type " + ts.SyntaxKind[node.kind]);
+            throw new Error(`unhandled node type ${ts.SyntaxKind[node.kind]}`);
     }
 }

src/rules/memberOrderingRule.ts

@@ -92,11 +92,11 @@ const PRESET_NAMES = Array.from(PRESETS.keys());
 
 const allMemberKindNames = mapDefined(Object.keys(MemberKind), (key) => {
     const mk = (MemberKind as any)[key];
-    return typeof mk === "number" ? MemberKind[mk].replace(/[A-Z]/g, (cap) => "-" + cap.toLowerCase()) : undefined;
+    return typeof mk === "number" ? MemberKind[mk].replace(/[A-Z]/g, (cap) => `-${cap.toLowerCase()}`) : undefined;
 });
 
 function namesMarkdown(names: string[]): string {
-    return names.map((name) => "* `" + name + "`").join("\n    ");
+    return names.map((name) => `* \`${name}\``).join("\n    ");
 }
 
 const optionsDescription = Lint.Utils.dedent`
@@ -296,7 +296,7 @@ function caseInsensitiveLess(a: string, b: string) {
 }
 
 function memberKindForConstructor(access: Access): MemberKind {
-    return (MemberKind as any)[access + "Constructor"] as MemberKind;
+    return (MemberKind as any)[`${access}Constructor`] as MemberKind;
 }
 
 function memberKindForMethodOrField(access: Access, membership: "Static" | "Instance", kind: "Method" | "Field"): MemberKind {
@@ -310,7 +310,7 @@ function memberKindFromName(name: string): MemberKind[] {
     return typeof kind === "number" ? [kind as MemberKind] : allAccess.map(addModifier);
 
     function addModifier(modifier: string) {
-        const modifiedKind = (MemberKind as any)[Lint.Utils.camelize(modifier + "-" + name)];
+        const modifiedKind = (MemberKind as any)[Lint.Utils.camelize(`${modifier}-${name}`)];
         if (typeof modifiedKind !== "number") {
             throw new Error(`Bad member kind: ${name}`);
         }

src/rules/noAnyRule.ts

@@ -34,6 +34,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     };
     /* tslint:enable:object-literal-sort-keys */
 
+    // tslint:disable-next-line prefer-template (fixed in 5.3)
     public static FAILURE_STRING = "Type declaration of 'any' loses type-safety. " +
         "Consider replacing it with a more precise type, the empty type ('{}'), " +
         "or suppress this occurrence.";

src/rules/noMagicNumbersRule.ts

@@ -76,7 +76,7 @@ class NoMagicNumbersWalker extends Lint.AbstractWalker<Set<string>> {
             if (isPrefixUnaryExpression(node) &&
                 node.operator === ts.SyntaxKind.MinusToken &&
                 node.operand.kind === ts.SyntaxKind.NumericLiteral) {
-                return this.checkNumericLiteral(node, "-" + (node.operand as ts.NumericLiteral).text);
+                return this.checkNumericLiteral(node, `-${(node.operand as ts.NumericLiteral).text}`);
             }
             return ts.forEachChild(node, cb);
         };

src/rules/noStringLiteralRule.ts

@@ -51,7 +51,7 @@ function walk(ctx: Lint.WalkContext<void>) {
                     argument,
                     Rule.FAILURE_STRING,
                     // expr['foo'] -> expr.foo
-                    Lint.Replacement.replaceFromTo(node.expression.end, node.end, "." + argument.text),
+                    Lint.Replacement.replaceFromTo(node.expression.end, node.end, `.${argument.text}`),
                 );
             }
         }

src/rules/objectLiteralShorthandRule.ts

@@ -53,7 +53,7 @@ class ObjectLiteralShorthandWalker extends Lint.RuleWalker {
                 // Delete from name start up to value to include the ':'.
                 const lengthToValueStart = value.getStart() - name.getStart();
                 const fix = this.deleteText(name.getStart(), lengthToValueStart);
-                this.addFailureAtNode(node, Rule.LONGHAND_PROPERTY + `('{${name.getText()}}').`, fix);
+                this.addFailureAtNode(node, `${Rule.LONGHAND_PROPERTY}('{${name.getText()}}').`, fix);
         }
 
         if (value.kind === ts.SyntaxKind.FunctionExpression) {
@@ -62,7 +62,7 @@ class ObjectLiteralShorthandWalker extends Lint.RuleWalker {
                 return;  // named function expressions are OK.
             }
             const star = fnNode.asteriskToken !== undefined ? fnNode.asteriskToken.getText() : "";
-            this.addFailureAtNode(node, Rule.LONGHAND_METHOD + `('{${name.getText()}${star}() {...}}').`);
+            this.addFailureAtNode(node, `${Rule.LONGHAND_METHOD}('{${name.getText()}${star}() {...}}').`);
         }
 
         super.visitPropertyAssignment(node);

src/rules/preferTemplateRule.ts

@@ -78,14 +78,15 @@ function getError(node: ts.Node, allowSingleConcat: boolean): string | undefined
         // Otherwise ignore. ("a" + "b", probably writing a long newline-less string on many lines.)
         return containsNewline(left as StringLike) || containsNewline(right as StringLike) ? Rule.FAILURE_STRING_MULTILINE : undefined;
     } else if (!l && !r) {
-        // Watch out for `"a" + b + c`.
+        // Watch out for `"a" + b + c`. Parsed as `("a" + b) + c`.
         return containsAnyStringLiterals(left) ? Rule.FAILURE_STRING : undefined;
     } else if (l) {
         // `"x" + y`
         return !allowSingleConcat ? Rule.FAILURE_STRING : undefined;
     } else {
         // `? + "b"`
-        return !allowSingleConcat || isPlusExpression(left) ? Rule.FAILURE_STRING : undefined;
+        // If LHS consists of only string literals (as in `"a" + "b" + "c"`, allow it.)
+        return !containsOnlyStringLiterals(left) && (!allowSingleConcat || isPlusExpression(left))  ? Rule.FAILURE_STRING : undefined;
     }
 }
 
@@ -99,13 +100,12 @@ function containsNewline(node: StringLike): boolean {
     }
 }
 
-function containsAnyStringLiterals(node: ts.Expression): boolean {
-    if (!isPlusExpression(node)) {
-        return false;
-    }
+function containsOnlyStringLiterals(node: ts.Expression): boolean {
+    return isPlusExpression(node) && isStringLike(node.right) && (isStringLike(node.left) || containsAnyStringLiterals(node.left));
+}
 
-    const { left, right } = node;
-    return isStringLike(right) || isStringLike(left) || containsAnyStringLiterals(left);
+function containsAnyStringLiterals(node: ts.Expression): boolean {
+    return isPlusExpression(node) && (isStringLike(node.right) || isStringLike(node.left) || containsAnyStringLiterals(node.left));
 }
 
 function isPlusExpression(node: ts.Node): node is ts.BinaryExpression {

src/rules/strictBooleanExpressionsRule.ts

@@ -342,13 +342,13 @@ function stringOr(parts: string[]): string {
         case 1:
             return parts[0];
         case 2:
-            return parts[0] + " or " + parts[1];
+            return `${parts[0]} or ${parts[1]}`;
         default:
             let res = "";
             for (let i = 0; i < parts.length - 1; i++) {
-                res += parts[i] + ", ";
+                res += `${parts[i]}, `;
             }
-            return res + "or " + parts[parts.length - 1];
+            return `${res}or ${parts[parts.length - 1]}`;
     }
 }
 

src/rules/typedefRule.ts

@@ -223,7 +223,7 @@ class TypedefWalker extends Lint.RuleWalker {
                                 typeAnnotation: ts.TypeNode | undefined,
                                 name?: ts.Node) {
         if (this.hasOption(option) && typeAnnotation == null) {
-            this.addFailureAt(location, 1, "expected " + option + getName(name, ": '", "'") + " to have a typedef");
+            this.addFailureAt(location, 1, `expected ${option}${getName(name, ": '", "'")} to have a typedef`);
         }
     }
 }