From 3877b1896f46e64b2a4ef04833dad93b6941d3e2 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 14 May 2017 16:21:47 +0200 Subject: [PATCH 1/4] Simplify type checking, return early --- package.json | 2 +- src/rules/noInferredEmptyObjectTypeRule.ts | 39 ++++++++-------------- yarn.lock | 6 ++-- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 2f383dfaf67..7072b07603e 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "resolve": "^1.3.2", "semver": "^5.3.0", "tslib": "^1.6.0", - "tsutils": "^1.8.0" + "tsutils": "^1.9.1" }, "peerDependencies": { "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev" diff --git a/src/rules/noInferredEmptyObjectTypeRule.ts b/src/rules/noInferredEmptyObjectTypeRule.ts index 7ee5e91a782..adc5e448fdb 100644 --- a/src/rules/noInferredEmptyObjectTypeRule.ts +++ b/src/rules/noInferredEmptyObjectTypeRule.ts @@ -15,9 +15,9 @@ * limitations under the License. */ +import { isObjectFlagSet, isObjectType, isTypeReference } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; -import { isObjectFlagSet, isTypeFlagSet } from "../language/utils"; export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ @@ -60,42 +60,31 @@ class NoInferredEmptyObjectTypeRule extends Lint.AbstractWalker { private checkNewExpression(node: ts.NewExpression): void { if (node.typeArguments === undefined) { - const objType = this.checker.getTypeAtLocation(node) as ts.TypeReference; - if (isTypeFlagSet(objType, ts.TypeFlags.Object) && objType.typeArguments !== undefined) { - const typeArgs = objType.typeArguments as ts.ObjectType[]; - if (typeArgs.some((a) => this.isEmptyObjectInterface(a))) { - this.addFailureAtNode(node, Rule.EMPTY_INTERFACE_INSTANCE); - } + const type = this.checker.getTypeAtLocation(node); + if (isTypeReference(type) && type.typeArguments !== undefined && + type.typeArguments.some((a) => isObjectType(a) && this.isEmptyObjectInterface(a))) { + this.addFailureAtNode(node, Rule.EMPTY_INTERFACE_INSTANCE); } } } private checkCallExpression(node: ts.CallExpression): void { if (node.typeArguments === undefined) { - const callSig = this.checker.getResolvedSignature(node); - const retType = this.checker.getReturnTypeOfSignature(callSig) as ts.TypeReference; - if (this.isEmptyObjectInterface(retType)) { + const retType = this.checker.getReturnTypeOfSignature(this.checker.getResolvedSignature(node)); + if (isObjectType(retType) && this.isEmptyObjectInterface(retType)) { this.addFailureAtNode(node, Rule.EMPTY_INTERFACE_FUNCTION); } } } private isEmptyObjectInterface(objType: ts.ObjectType): boolean { - const isAnonymous = isObjectFlagSet(objType, ts.ObjectFlags.Anonymous); - let hasProblematicCallSignatures = false; - const hasProperties = (objType.getProperties() !== undefined && objType.getProperties().length > 0); - const hasNumberIndexType = objType.getNumberIndexType() !== undefined; - const hasStringIndexType = objType.getStringIndexType() !== undefined; - const callSig = objType.getCallSignatures(); - if (callSig !== undefined && callSig.length > 0) { - const isClean = callSig.every((sig) => { - const csigRetType = this.checker.getReturnTypeOfSignature(sig) as ts.TypeReference; - return this.isEmptyObjectInterface(csigRetType); + return isObjectFlagSet(objType, ts.ObjectFlags.Anonymous) && + objType.getProperties().length === 0 && + objType.getNumberIndexType() === undefined && + objType.getStringIndexType() === undefined && + objType.getCallSignatures().every((signature) => { + const type = this.checker.getReturnTypeOfSignature(signature); + return isObjectType(type) && this.isEmptyObjectInterface(type); }); - if (!isClean) { - hasProblematicCallSignatures = true; - } - } - return (isAnonymous && !hasProblematicCallSignatures && !hasProperties && !hasNumberIndexType && !hasStringIndexType); } } diff --git a/yarn.lock b/yarn.lock index 7467ca1cebf..a44df488abc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1441,9 +1441,9 @@ tslint@latest: tslib "^1.6.0" tsutils "^1.8.0" -tsutils@^1.8.0: - version "1.8.0" - resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.8.0.tgz#bf8118ed8e80cd5c9fc7d75728c7963d44ed2f52" +tsutils@^1.8.0, tsutils@^1.9.1: + version "1.9.1" + resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.9.1.tgz#b9f9ab44e55af9681831d5f28d0aeeaf5c750cb0" type-detect@0.1.1: version "0.1.1" From c9a712ef113f240e3f170b4694faab4bd686911b Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 14 May 2017 16:22:34 +0200 Subject: [PATCH 2/4] Enable module loading in tests and add regression test for #2646 --- src/test.ts | 26 +++++++++---------- .../test.ts.lint | 4 +++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/test.ts b/src/test.ts index 74eb88e2a6b..44fcb368714 100644 --- a/src/test.ts +++ b/src/test.ts @@ -89,8 +89,7 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ for (const fileToLint of filesToLint) { const isEncodingRule = path.basename(testDirectory) === "encoding"; - const fileBasename = path.basename(fileToLint, MARKUP_FILE_EXTENSION); - const fileCompileName = fileBasename.replace(/\.lint$/, ""); + const fileCompileName = path.resolve(fileToLint.replace(/\.lint$/, "")); let fileText = isEncodingRule ? readBufferWithDetectedEncoding(fs.readFileSync(fileToLint)) : fs.readFileSync(fileToLint, "utf-8"); const tsVersionRequirement = parse.getTypescriptVersionRequirement(fileText); if (tsVersionRequirement !== undefined) { @@ -117,26 +116,25 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ let program: ts.Program | undefined; if (tslintConfig !== undefined && tslintConfig.linterOptions !== undefined && tslintConfig.linterOptions.typeCheck === true) { const compilerHost: ts.CompilerHost = { - fileExists: () => true, - getCanonicalFileName: (filename: string) => filename, - getCurrentDirectory: () => "", + fileExists: (file) => + file === fileCompileName || fs.existsSync(file), + getCanonicalFileName: (filename) => filename, + getCurrentDirectory: () => process.cwd(), getDefaultLibFileName: () => ts.getDefaultLibFileName(compilerOptions), - getDirectories: (_path: string) => [], + getDirectories: (path) => fs.readdirSync(path), getNewLine: () => "\n", - getSourceFile(filenameToGet: string) { + getSourceFile(filenameToGet) { const target = compilerOptions.target === undefined ? ts.ScriptTarget.ES5 : compilerOptions.target; if (filenameToGet === ts.getDefaultLibFileName(compilerOptions)) { const fileContent = fs.readFileSync(ts.getDefaultLibFilePath(compilerOptions), "utf8"); return ts.createSourceFile(filenameToGet, fileContent, target); } else if (filenameToGet === fileCompileName) { - return ts.createSourceFile(fileBasename, fileTextWithoutMarkup, target, true); - } else if (fs.existsSync(path.resolve(path.dirname(fileToLint), filenameToGet))) { - const text = fs.readFileSync(path.resolve(path.dirname(fileToLint), filenameToGet), "utf8"); - return ts.createSourceFile(filenameToGet, text, target, true); + return ts.createSourceFile(filenameToGet, fileTextWithoutMarkup, target, true); } - throw new Error(`Couldn't get source file '${filenameToGet}'`); + const text = fs.readFileSync(filenameToGet, "utf8"); + return ts.createSourceFile(filenameToGet, text, target, true); }, - readFile: (x: string) => x, + readFile: (x) => x, useCaseSensitiveFileNames: () => true, writeFile: () => null, }; @@ -154,7 +152,7 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ }; const linter = new Linter(lintOptions, program); // Need to use the true path (ending in '.lint') for "encoding" rule so that it can read the file. - linter.lint(isEncodingRule ? fileToLint : fileBasename, fileTextWithoutMarkup, tslintConfig); + linter.lint(isEncodingRule ? fileToLint : fileCompileName, fileTextWithoutMarkup, tslintConfig); const failures = linter.getResult().failures; const errorsFromLinter: LintError[] = failures.map((failure) => { const startLineAndCharacter = failure.getStartPosition().getLineAndCharacter(); diff --git a/test/rules/no-inferred-empty-object-type/test.ts.lint b/test/rules/no-inferred-empty-object-type/test.ts.lint index 2967deb4193..120b965fbda 100644 --- a/test/rules/no-inferred-empty-object-type/test.ts.lint +++ b/test/rules/no-inferred-empty-object-type/test.ts.lint @@ -84,3 +84,7 @@ new MultiParamsClass<() => void, () => {}>(); new MultiParamsClass<() => void, () => void>(); new MultiParamsClass<{ [x: number]: string }, () => void>(); new MultiParamsClass<{ [x: string]: string }, number>(); + +// don't crash with stack overflow +import {expect} from "chai"; +expect(1).to.eq(1); From 6cd967a24b2daf91b29254b608a3aeffd3ae8726 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Sun, 14 May 2017 21:22:15 +0200 Subject: [PATCH 3/4] Fix windows test failures --- src/test.ts | 6 +++--- src/utils.ts | 5 +++++ test/executable/executableTests.ts | 3 ++- test/utils.ts | 5 ----- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/test.ts b/src/test.ts index 44fcb368714..31472842ff5 100644 --- a/src/test.ts +++ b/src/test.ts @@ -27,7 +27,7 @@ import {Replacement} from "./language/rule/rule"; import * as Linter from "./linter"; import {LintError} from "./test/lintError"; import * as parse from "./test/parse"; -import {mapDefined, readBufferWithDetectedEncoding} from "./utils"; +import {denormalizeWinPath, mapDefined, readBufferWithDetectedEncoding} from "./utils"; const MARKUP_FILE_EXTENSION = ".lint"; const FIXES_FILE_EXTENSION = ".fix"; @@ -89,7 +89,7 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ for (const fileToLint of filesToLint) { const isEncodingRule = path.basename(testDirectory) === "encoding"; - const fileCompileName = path.resolve(fileToLint.replace(/\.lint$/, "")); + const fileCompileName = denormalizeWinPath(path.resolve(fileToLint.replace(/\.lint$/, ""))); let fileText = isEncodingRule ? readBufferWithDetectedEncoding(fs.readFileSync(fileToLint)) : fs.readFileSync(fileToLint, "utf-8"); const tsVersionRequirement = parse.getTypescriptVersionRequirement(fileText); if (tsVersionRequirement !== undefined) { @@ -128,7 +128,7 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ if (filenameToGet === ts.getDefaultLibFileName(compilerOptions)) { const fileContent = fs.readFileSync(ts.getDefaultLibFilePath(compilerOptions), "utf8"); return ts.createSourceFile(filenameToGet, fileContent, target); - } else if (filenameToGet === fileCompileName) { + } else if (denormalizeWinPath(filenameToGet) === fileCompileName) { return ts.createSourceFile(filenameToGet, fileTextWithoutMarkup, target, true); } const text = fs.readFileSync(filenameToGet, "utf8"); diff --git a/src/utils.ts b/src/utils.ts index a93efa72e8b..75c1734a4aa 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -208,3 +208,8 @@ export function detectBufferEncoding(buffer: Buffer, length = buffer.length): En return "utf8"; } + +// converts Windows normalized paths (with backwards slash `\`) to paths used by TypeScript (with forward slash `/`) +export function denormalizeWinPath(path: string): string { + return path.replace(/\\/g, "/"); +} diff --git a/test/executable/executableTests.ts b/test/executable/executableTests.ts index b11046416e4..c73f43aa1f6 100644 --- a/test/executable/executableTests.ts +++ b/test/executable/executableTests.ts @@ -19,7 +19,8 @@ import * as cp from "child_process"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; -import { createTempFile, denormalizeWinPath } from "../utils"; +import { denormalizeWinPath } from "../../src/utils"; +import { createTempFile } from "../utils"; // when tests are run with mocha from npm scripts CWD points to project root const EXECUTABLE_DIR = path.resolve(process.cwd(), "test", "executable"); diff --git a/test/utils.ts b/test/utils.ts index fecf7456b6c..9c5faa6fc47 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -47,8 +47,3 @@ export function createTempFile(extension: string) { } return tmpfile; } - -// converts Windows normalized paths (with backwards slash `\`) to paths used by TypeScript (with forward slash `/`) -export function denormalizeWinPath(path: string): string { - return path.replace(/\\/g, "/"); -} From e53a4e9ccf06836e5b7f4ec40af8bca62faeb34f Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 15 May 2017 22:02:18 +0200 Subject: [PATCH 4/4] revert unnecessary whitspace change --- src/test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test.ts b/src/test.ts index 31472842ff5..404cc31f4e1 100644 --- a/src/test.ts +++ b/src/test.ts @@ -116,8 +116,7 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[ let program: ts.Program | undefined; if (tslintConfig !== undefined && tslintConfig.linterOptions !== undefined && tslintConfig.linterOptions.typeCheck === true) { const compilerHost: ts.CompilerHost = { - fileExists: (file) => - file === fileCompileName || fs.existsSync(file), + fileExists: (file) => file === fileCompileName || fs.existsSync(file), getCanonicalFileName: (filename) => filename, getCurrentDirectory: () => process.cwd(), getDefaultLibFileName: () => ts.getDefaultLibFileName(compilerOptions),