Add support for error templates in tests (palantir#2481)

Add the ability to use placeholders in message substitutions. Syntax is inspired by python. It uses node's `util.format()` under the hood.

docs/develop/testing-rules/index.md

@@ -97,6 +97,49 @@ Then, at the bottom of our test file, we specify what full message each shortcut
 
 Again, we can run `grunt test` to make sure our rule is producing the output we expect. If it isn't we'll see the difference between the output from the rule and the output we marked.
 
+You can also use placeholders to format messages. That's useful if the error message contains non-static parts, e.g. variable names. But let's stick to the above example for now.
+
+``` 
+const octopus = 5;
+      ~~~~~~~     [no-animal]
+
+let giraffe: number, tiger: number;
+    ~~~~~~~                 [no-animal]
+                     ~~~~~  [no-animal]
+
+const tree = 5;
+      ~~~~      [error % ("plants")]
+const skyscraper = 100;
+
+[error] Variables named after %s are not allowed!
+[no-animal]: error % ('animals')
+```
+
+We created a message template called `error` which has one placeholder `%s`. For a complete list of supported placeholders, please refer to the documentation of node's [util.format()](https://nodejs.org/api/util.html#util_util_format_format_args).
+To use the template for formatting, you need to use a special syntax: `template_name % ('substitution1' [, "substitution2" [, ...]])`.
+Substitutions are passed as comma separated list of javascript string literals. The strings need to be wrapped in single or double quotes. Escaping characters works like you would expect in javascript.
+
+You may have noticed that the template is used for formatting in two different places. Use it in inline errors to pass another substitution every time.
+If you use formatting in another message shorthand (like we did for `[no-animal]`), you need to make sure the template is defined before its use. That means swapping the lines of `[error]` and `[no-animal]` will not work. There are no restrictions for the use of `[no-animal]` in inline errors, though.
+
+Now let's pretend the rule changed its error message to include the variable name at the end. The following example shows how to substitute multiple placeholders.
+
+``` 
+const octopus = 5;
+      ~~~~~~~     [no-animal % ('octopus')]
+
+let giraffe: number, tiger: number;
+    ~~~~~~~                 [no-animal % ('giraffe')]
+                     ~~~~~  [no-animal % ('tiger')]
+
+const tree = 5;
+      ~~~~      [error % ("plants", "tree")]
+const skyscraper = 100;
+
+[error] Variables named after %s are not allowed: '%s'
+[no-animal]: error % ('animals')
+```
+
 ##### Typescript version requirement #####
 
 Sometimes a rule requires a minimum version of the typescript compiler or your test contains syntax that older versions of the typescript parser cannot handle.

src/test/parse.ts

@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+import * as ts from "typescript";
+import {format} from "util";
+
 import {
     CodeLine,
     EndErrorLine,
@@ -26,6 +29,8 @@ import {
 } from "./lines";
 import {errorComparator, LintError, lintSyntaxError} from "./lintError";
 
+let scanner: ts.Scanner | undefined;
+
 export function getTypescriptVersionRequirement(text: string): string | undefined {
     const lines = text.split(/\r?\n/);
     const firstLine = parseLine(lines[0]);
@@ -60,8 +65,10 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
     }
 
     const messageSubstitutionLines = lines.filter((l) => l instanceof MessageSubstitutionLine) as MessageSubstitutionLine[];
-    const messageSubstitutions = new Map(messageSubstitutionLines.map(({ key, message }) =>
-        [key, message] as [string, string]));
+    const messageSubstitutions = new Map<string, string>();
+    for (const {key, message} of messageSubstitutionLines) {
+        messageSubstitutions.set(key, formatMessage(messageSubstitutions, message));
+    }
 
     // errorLineForCodeLine[5] contains all the ErrorLine objects associated with the 5th line of code, for example
     const errorLinesForCodeLines = createCodeLineNoToErrorsMap(lines);
@@ -71,7 +78,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
         lintErrors.push({
             startPos: errorStartPos,
             endPos: { line: lineNo, col: errorLine.endCol },
-            message: messageSubstitutions.get(errorLine.message) || errorLine.message,
+            message: substituteMessage(messageSubstitutions, errorLine.message),
         });
     }
     // for each line of code...
@@ -114,6 +121,74 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
     return lintErrors;
 }
 
+/**
+ * Process `message` as follows:
+ * - search `substitutions` for an exact match and return the substitution
+ * - try to format the message when it looks like: name % ('substitution1' [, "substitution2" [, ...]])
+ * - or return it unchanged
+ */
+function substituteMessage(templates: Map<string, string>, message: string): string {
+    const substitution = templates.get(message);
+    if (substitution !== undefined) {
+        return substitution;
+    }
+    return formatMessage(templates, message);
+}
+
+/**
+ * Tries to format the message when it has the correct format or returns it unchanged:  name % ('substitution1' [, "substitution2" [, ...]])
+ * Where `name` is the name of a message substitution that is used as template.
+ * If `name` is not found in `templates`, `message` is returned unchanged.
+ */
+function formatMessage(templates: Map<string, string>, message: string): string {
+    const formatMatch = /^([\w_]+) % \((.+)\)$/.exec(message);
+    if (formatMatch !== null) {
+        const template = templates.get(formatMatch[1]);
+        if (template !== undefined) {
+            const formatArgs = parseFormatArguments(formatMatch[2]);
+            if (formatArgs !== undefined) {
+                message = format(template, ...formatArgs);
+            }
+        }
+    }
+    return message;
+}
+
+/**
+ * Parse a list of comma separated string literals.
+ * This function bails out if it sees something unexpected.
+ * Whitespace between tokens is ignored.
+ * Trailing comma is allowed.
+ */
+function parseFormatArguments(text: string): string[] | undefined {
+    if (scanner === undefined) {
+        // once the scanner is created, it is cached for subsequent calls
+        scanner = ts.createScanner(ts.ScriptTarget.Latest, false);
+    }
+    scanner.setText(text);
+    const result: string[] = [];
+    let expectValue = true;
+    for (let token = scanner.scan(); token !== ts.SyntaxKind.EndOfFileToken; token = scanner.scan()) {
+        if (token === ts.SyntaxKind.StringLiteral) {
+            if (!expectValue) {
+                return undefined;
+            }
+            result.push(scanner.getTokenValue());
+            expectValue = false;
+        } else if (token === ts.SyntaxKind.CommaToken) {
+            if (expectValue) {
+                return undefined;
+            }
+            expectValue = true;
+        } else if (token !== ts.SyntaxKind.WhitespaceTrivia) {
+            // only ignore whitespace, other trivia like comments makes this function bail out
+            return undefined;
+        }
+    }
+
+    return result.length === 0 ? undefined : result;
+}
+
 export function createMarkupFromErrors(code: string, lintErrors: LintError[]) {
     lintErrors.sort(errorComparator);
 

test/rules/_integration/error-format/test.ts.lint

@@ -0,0 +1,37 @@
+import {} from 'foo';
+
+let noSubstitution;
+    ~~~~~~~~~~~~~~ [Identifier 'noSubstitution' is never reassigned; use 'const' instead of 'let'.]
+let noFormat;
+    ~~~~~~~~ [no_format]
+let format;
+    ~~~~~~ [let_base % ('format')]
+let formatMultiArgs;
+    ~~~~~~~~~~~~~~~ [base % ('formatMultiArgs', 'let')]
+var formatMultiArgsTrailingComma;
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [base % ('formatMultiArgsTrailingComma', 'var', )]
+let indirection;
+    ~~~~~~~~~~~ [let % ('indirection')]
+var indirection2;
+    ~~~~~~~~~~~~ [var % ('indirection2')]
+let indirection3;
+    ~~~~~~~~~~~~ [partial_format % ('let')]
+var formatInSubstitution;
+    ~~~~~~~~~~~~~~~~~~~~ [preformatted]
+let worksWithEscape;
+    ~~~~~~~~~~~~~~~ [full_substitution % ('Identifier \'worksWithEscape\' is never reassigned; use \'const\' instead of \'let\'.')]
+var worksWithDoubleQuotes;
+    ~~~~~~~~~~~~~~~~~~~~~ [full_substitution % ("Identifier 'worksWithDoubleQuotes' is never reassigned; use 'const' instead of 'var'.")]
+let doesntTrimSpaces;
+    ~~~~~~~~~~~~~~~~ [base3 % ("Identifier 'doesntTrimSpaces' is never reassigned; ", " 'let'")]
+
+[no_format]: Identifier 'noFormat' is never reassigned; use 'const' instead of 'let'.
+[base]: Identifier '%s' is never reassigned; use 'const' instead of '%s'.
+[let_base]: Identifier '%s' is never reassigned; use 'const' instead of 'let'.
+[base2]: Identifier '%%s' is never reassigned; use 'const' instead of '%s'.
+[let]: base2 % ('let')
+[var]: base2 % ('var')
+[partial_format]: base % ('indirection3')
+[preformatted]: base % ('formatInSubstitution', 'var')
+[full_substitution]: %s
+[base3]: %suse 'const' instead of%s.

test/rules/_integration/error-format/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "prefer-const": true
+  }
+}

test/rules/prefer-const/default/test.ts.fix

@@ -194,3 +194,4 @@ declare let ambient = 0;
 declare namespace ambient {
     let foo = 0;
 }
+