Merge pull request palantir#4454 from palantir/ad/prep-5.12.1

Prepare 5.12.1 release

package.json

@@ -1,6 +1,6 @@
 {
     "name": "tslint",
-    "version": "5.12.0",
+    "version": "5.12.1",
     "description": "An extensible static analysis linter for the TypeScript language",
     "bin": {
         "tslint": "./bin/tslint"

src/configs/latest.ts

@@ -65,6 +65,10 @@ export const rules = {
     "no-duplicate-switch-case": true,
     "no-implicit-dependencies": true,
     "no-return-await": true,
+
+    // added in v5.12
+    "function-constructor": true,
+    "unnecessary-bind": true,
 };
 // tslint:enable object-literal-sort-keys
 

src/configs/recommended.ts

@@ -47,7 +47,6 @@ export const rules = {
     "cyclomatic-complexity": false,
     eofline: true,
     forin: true,
-    "function-constructor": true,
     "import-spacing": true,
     indent: {
         options: ["spaces"],

src/linter.ts

@@ -42,7 +42,7 @@ import { arrayify, dedent, flatMap, mapDefined } from "./utils";
  * Linter that can lint multiple files in consecutive runs.
  */
 export class Linter {
-    public static VERSION = "5.12.0";
+    public static VERSION = "5.12.1";
 
     public static findConfiguration = findConfiguration;
     public static findConfigurationPath = findConfigurationPath;
@@ -104,13 +104,16 @@ export class Linter {
 
     /**
      * Returns a list of source file names from a TypeScript program. This includes all referenced
-     * files and excludes declaration (".d.ts") files.
+     * files and excludes declaration (".d.ts") files, as well as JSON files, to avoid problems with
+     * `resolveJsonModule`.
      */
     public static getFileNames(program: ts.Program): string[] {
         return mapDefined(
             program.getSourceFiles(),
             file =>
-                file.fileName.endsWith(".d.ts") || program.isSourceFileFromExternalLibrary(file)
+                file.fileName.endsWith(".d.ts") ||
+                file.fileName.endsWith(".json") ||
+                program.isSourceFileFromExternalLibrary(file)
                     ? undefined
                     : file.fileName,
         );

src/rules/incrementDecrementRule.ts

@@ -80,7 +80,14 @@ function walk(context: Lint.WalkContext<Options>) {
 
     function complainOnNode(node: ts.PostfixUnaryExpression | ts.PrefixUnaryExpression) {
         const newOperatorText = node.operator === ts.SyntaxKind.PlusPlusToken ? "+= 1" : "-= 1";
-        const replacement = createReplacement(node, newOperatorText);
+        let replacement: Lint.Replacement | undefined;
+
+        if (
+            tsutils.isPrefixUnaryExpression(node) ||
+            node.parent.kind === ts.SyntaxKind.ExpressionStatement
+        ) {
+            replacement = createReplacement(node, newOperatorText);
+        }
 
         const failure = Rule.FAILURE_STRING_FACTORY(newOperatorText);
 

src/rules/quotemarkRule.ts

@@ -27,12 +27,12 @@ const OPTION_JSX_DOUBLE = "jsx-double";
 const OPTION_AVOID_TEMPLATE = "avoid-template";
 const OPTION_AVOID_ESCAPE = "avoid-escape";
 
-type QUOTE_MARK = "'" | '"' | "`";
-type JSX_QUOTE_MARK = "'" | '"';
+type QUOTEMARK = "'" | '"' | "`";
+type JSX_QUOTEMARK = "'" | '"';
 
 interface Options {
-    quoteMark: QUOTE_MARK;
-    jsxQuoteMark: JSX_QUOTE_MARK;
+    quotemark: QUOTEMARK;
+    jsxQuotemark: JSX_QUOTEMARK;
     avoidEscape: boolean;
     avoidTemplate: boolean;
 }
@@ -52,9 +52,11 @@ export class Rule extends Lint.Rules.AbstractRule {
             * \`"${OPTION_JSX_SINGLE}"\` enforces single quotes for JSX attributes.
             * \`"${OPTION_JSX_DOUBLE}"\` enforces double quotes for JSX attributes.
             * \`"${OPTION_AVOID_TEMPLATE}"\` forbids single-line untagged template strings that do not contain string interpolations.
+                Note that backticks may still be used if \`"${OPTION_AVOID_ESCAPE}"\` is enabled and both single and double quotes are
+                present in the string (the latter option takes precedence).
             * \`"${OPTION_AVOID_ESCAPE}"\` allows you to use the "other" quotemark in cases where escaping would normally be required.
-            For example, \`[true, "${OPTION_DOUBLE}", "${OPTION_AVOID_ESCAPE}"]\` would not report a failure on the string literal
-            \`'Hello "World"'\`.`,
+                For example, \`[true, "${OPTION_DOUBLE}", "${OPTION_AVOID_ESCAPE}"]\` would not report a failure on the string literal
+                \`'Hello "World"'\`.`,
         options: {
             type: "array",
             items: {
@@ -87,15 +89,14 @@ export class Rule extends Lint.Rules.AbstractRule {
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         const args = this.ruleArguments;
-
-        const quoteMark = getQuotemarkPreference(args);
-        const jsxQuoteMark = getJSXQuotemarkPreference(args);
+        const quotemark = getQuotemarkPreference(args);
+        const jsxQuotemark = getJSXQuotemarkPreference(args, quotemark);
 
         return this.applyWithFunction(sourceFile, walk, {
             avoidEscape: hasArg(OPTION_AVOID_ESCAPE),
             avoidTemplate: hasArg(OPTION_AVOID_TEMPLATE),
-            jsxQuoteMark,
-            quoteMark,
+            jsxQuotemark,
+            quotemark,
         });
 
         function hasArg(name: string): boolean {
@@ -114,108 +115,98 @@ function walk(ctx: Lint.WalkContext<Options>) {
                 node.parent.kind !== ts.SyntaxKind.TaggedTemplateExpression &&
                 isSameLine(sourceFile, node.getStart(sourceFile), node.end))
         ) {
-            const expectedQuoteMark =
+            const expectedQuotemark =
                 node.parent.kind === ts.SyntaxKind.JsxAttribute
-                    ? options.jsxQuoteMark
-                    : options.quoteMark;
-            const actualQuoteMark = sourceFile.text[node.end - 1];
+                    ? options.jsxQuotemark
+                    : options.quotemark;
+            const actualQuotemark = sourceFile.text[node.end - 1];
 
-            if (actualQuoteMark === expectedQuoteMark) {
+            if (actualQuotemark === expectedQuotemark) {
                 return;
             }
 
-            let fixQuoteMark = expectedQuoteMark;
-
-            const needsQuoteEscapes = node.text.includes(expectedQuoteMark);
+            let fixQuotemark = expectedQuotemark;
+            const needsQuoteEscapes = node.text.includes(expectedQuotemark);
 
             // This string requires escapes to use the expected quote mark, but `avoid-escape` was passed
             if (needsQuoteEscapes && options.avoidEscape) {
                 if (node.kind === ts.SyntaxKind.StringLiteral) {
                     return;
                 }
 
-                // If we are expecting double quotes, use single quotes to avoid
-                //   escaping. Otherwise, just use double quotes.
-                fixQuoteMark = expectedQuoteMark === '"' ? "'" : '"';
-
-                // It also includes the fixQuoteMark. Let's try to use single
-                //   quotes instead, unless we originally expected single
-                //   quotes, in which case we will try to use backticks. This
-                //   means that we may use backtick even with avoid-template
-                //   in trying to avoid escaping. What is the desired priority
-                //   here?
-                if (node.text.includes(fixQuoteMark)) {
-                    fixQuoteMark = expectedQuoteMark === "'" ? "`" : "'";
-
-                    // It contains all of the other kinds of quotes. Escaping is
-                    //   unavoidable, sadly.
-                    if (node.text.includes(fixQuoteMark)) {
+                // If we are expecting double quotes, use single quotes to avoid escaping.
+                // Otherwise, just use double quotes.
+                const alternativeFixQuotemark = expectedQuotemark === '"' ? "'" : '"';
+
+                if (node.text.includes(alternativeFixQuotemark)) {
+                    // It also includes the alternative fix quote mark. Let's try to use single quotes instead,
+                    // unless we originally expected single quotes, in which case we will try to use backticks.
+                    // This means that we may use backtick even with avoid-template in trying to avoid escaping.
+                    fixQuotemark = expectedQuotemark === "'" ? "`" : "'";
+
+                    if (fixQuotemark === actualQuotemark) {
+                        // We were already using the best quote mark for this scenario
+                        return;
+                    } else if (node.text.includes(fixQuotemark)) {
+                        // It contains all of the other kinds of quotes. Escaping is unavoidable, sadly.
                         return;
                     }
+                } else {
+                    fixQuotemark = alternativeFixQuotemark;
                 }
             }
 
             const start = node.getStart(sourceFile);
             let text = sourceFile.text.substring(start + 1, node.end - 1);
 
             if (needsQuoteEscapes) {
-                text = text.replace(new RegExp(fixQuoteMark, "g"), `\\${fixQuoteMark}`);
+                text = text.replace(new RegExp(fixQuotemark, "g"), `\\${fixQuotemark}`);
             }
 
-            text = text.replace(new RegExp(`\\\\${actualQuoteMark}`, "g"), actualQuoteMark);
+            text = text.replace(new RegExp(`\\\\${actualQuotemark}`, "g"), actualQuotemark);
 
             return ctx.addFailure(
                 start,
                 node.end,
-                Rule.FAILURE_STRING(actualQuoteMark, fixQuoteMark),
-                new Lint.Replacement(start, node.end - start, fixQuoteMark + text + fixQuoteMark),
+                Rule.FAILURE_STRING(actualQuotemark, fixQuotemark),
+                new Lint.Replacement(start, node.end - start, fixQuotemark + text + fixQuotemark),
             );
         }
         ts.forEachChild(node, cb);
     });
 }
 
-function getQuotemarkPreference(args: any[]): QUOTE_MARK {
-    type QUOTE_PREF = typeof OPTION_SINGLE | typeof OPTION_DOUBLE | typeof OPTION_BACKTICK;
-
-    const quoteFromOption = {
-        [OPTION_SINGLE]: "'",
-        [OPTION_DOUBLE]: '"',
-        [OPTION_BACKTICK]: "`",
-    };
-
-    for (const arg of args) {
+function getQuotemarkPreference(ruleArguments: any[]): QUOTEMARK {
+    for (const arg of ruleArguments) {
         switch (arg) {
             case OPTION_SINGLE:
+                return "'";
             case OPTION_DOUBLE:
+                return '"';
             case OPTION_BACKTICK:
-                return quoteFromOption[arg as QUOTE_PREF] as QUOTE_MARK;
+                return "`";
+            default:
+                continue;
         }
     }
 
     // Default to double quotes if no pref is found.
     return '"';
 }
 
-function getJSXQuotemarkPreference(args: any[]): JSX_QUOTE_MARK {
-    type JSX_QUOTE_PREF = typeof OPTION_JSX_SINGLE | typeof OPTION_JSX_DOUBLE;
-
-    const jsxQuoteFromOption = {
-        [OPTION_JSX_SINGLE]: "'",
-        [OPTION_JSX_DOUBLE]: '"',
-    };
-
-    for (const arg of args) {
+function getJSXQuotemarkPreference(ruleArguments: any[], regularQuotemarkPreference: QUOTEMARK): JSX_QUOTEMARK {
+    for (const arg of ruleArguments) {
         switch (arg) {
             case OPTION_JSX_SINGLE:
+                return "'";
             case OPTION_JSX_DOUBLE:
-                return jsxQuoteFromOption[arg as JSX_QUOTE_PREF] as JSX_QUOTE_MARK;
+                return '"';
+            default:
+                continue;
         }
     }
 
     // The JSX preference was not found, so try to use the regular preference.
     //   If the regular pref is backtick, use double quotes instead.
-    const regularQuotemark = getQuotemarkPreference(args);
-
-    return regularQuotemark !== "`" ? regularQuotemark : '"';
+    return regularQuotemarkPreference !== "`" ? regularQuotemarkPreference : '"';
 }

src/rules/strictTypePredicatesRule.ts

@@ -99,7 +99,12 @@ function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void {
 
         const exprType = checker.getTypeAtLocation(exprPred.expression);
         // TODO: could use checker.getBaseConstraintOfType to help with type parameters, but it's not publicly exposed.
-        if (isTypeFlagSet(exprType, ts.TypeFlags.Any | ts.TypeFlags.TypeParameter)) {
+        if (
+            isTypeFlagSet(
+                exprType,
+                ts.TypeFlags.Any | ts.TypeFlags.TypeParameter | ts.TypeFlags.Unknown,
+            )
+        ) {
             return;
         }
 

src/test.ts

@@ -205,8 +205,16 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
             errorsFromMarkup,
             fixesFromLinter: newFileText,
             fixesFromMarkup: fixedFileText,
-            markupFromLinter: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromMarkup),
-            markupFromMarkup: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromLinter),
+            markupFromLinter: parse.createMarkupFromErrors(
+                fileToLint,
+                fileTextWithoutMarkup,
+                errorsFromMarkup,
+            ),
+            markupFromMarkup: parse.createMarkupFromErrors(
+                fileToLint,
+                fileTextWithoutMarkup,
+                errorsFromLinter,
+            ),
             skipped: false,
         };
     }

src/verify/lines.ts

@@ -87,14 +87,17 @@ export function parseLine(text: string): Line {
  * Maps a Line object to a matching line of text that could be in a .lint file.
  * This is almost the inverse of parseLine.
  * If you ran `printLine(parseLine(someText), code)`, the whitespace in the result may be different than in someText
+ * @param fileName - File name containing the line and code.
  * @param line - A Line object to convert to text
  * @param code - If line represents error markup, this is the line of code preceding the markup.
  *               Otherwise, this parameter is not required.
  */
-export function printLine(line: Line, code?: string): string | undefined {
+export function printLine(fileName: string, line: Line, code?: string): string | undefined {
     if (line instanceof ErrorLine) {
         if (code === undefined) {
-            throw new Error("Must supply argument for code parameter when line is an ErrorLine");
+            throw new Error(
+                `${fileName}: Must supply argument for code parameter when line is an ErrorLine`,
+            );
         }
 
         const leadingSpaces = " ".repeat(line.startCol);
@@ -111,7 +114,7 @@ export function printLine(line: Line, code?: string): string | undefined {
             let tildes = "~".repeat(line.endCol - line.startCol);
             if (code.length < line.endCol) {
                 // Better than crashing in String.repeat
-                throw new Error(`Bad error marker at ${JSON.stringify(line)}`);
+                throw new Error(`Bad error marker in ${fileName} at ${JSON.stringify(line)}`);
             }
             let endSpaces = " ".repeat(code.length - line.endCol);
             if (tildes.length === 0) {

src/verify/parse.ts

@@ -255,7 +255,7 @@ function parseFormatArguments(text: string): string[] | undefined {
     return result.length === 0 ? undefined : result;
 }
 
-export function createMarkupFromErrors(code: string, lintErrors: LintError[]) {
+export function createMarkupFromErrors(fileName: string, code: string, lintErrors: LintError[]) {
     lintErrors.sort(errorComparator);
 
     const codeText = code.split("\n");
@@ -281,7 +281,7 @@ export function createMarkupFromErrors(code: string, lintErrors: LintError[]) {
 
     return flatMap(codeText, (line, i) => [
         line,
-        ...mapDefined(errorLinesForCodeText[i], err => printLine(err, line)),
+        ...mapDefined(errorLinesForCodeText[i], err => printLine(fileName, err, line)),
     ]).join("\n");
 }
 /* tslint:enable:object-literal-sort-keys */

test/executable/executableTests.ts

@@ -19,8 +19,10 @@ import * as cp from "child_process";
 import * as fs from "fs";
 import * as os from "os";
 import * as path from "path";
+import * as semver from "semver";
 import { Logger, Options, run, Status } from "../../src/runner";
 import { denormalizeWinPath } from "../../src/utils";
+import { getNormalizedTypescriptVersion } from "../../src/verify/parse";
 import { createTempFile } from "../utils";
 
 // when tests are run with mocha from npm scripts CWD points to project root
@@ -41,6 +43,8 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
     this.slow(3000); // the executable is JIT-ed each time it runs; avoid showing slowness warnings
     this.timeout(10000);
 
+    const tsVersion = getNormalizedTypescriptVersion();
+
     describe("Files", () => {
         it("exits with code 1 if no arguments passed", done => {
             execCli([], (err, stdout, stderr) => {
@@ -596,6 +600,13 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
                 fs.readFileSync("test/files/project-multiple-fixes/after.test.ts", "utf-8"),
             );
         }).timeout(8000);
+
+        if (semver.satisfies(tsVersion, ">=2.9")) {
+            it("does not try to parse JSON files with --resolveJsonModule with TS >= 2.9", async () => {
+                const status = await execRunner({project: "test/files/tsconfig-resolve-json-module/tsconfig.json"});
+                assert.equal(status, Status.Ok, "process should exit without an error");
+            });
+        }
     });
 
     describe("--type-check", () => {

test/files/tsconfig-resolve-json-module/index.ts

@@ -0,0 +1,3 @@
+import settings from "./test.json";
+// tslint:disable-next-line:no-console
+console.log(settings.dry);

test/files/tsconfig-resolve-json-module/test.json

@@ -0,0 +1,4 @@
+{
+  "dry": false,
+  "debug": false
+}