Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
Refactor and fix no-inferred-empty-object-type (#2762)
Browse files Browse the repository at this point in the history
[bugfix] no-inferred-empty-object-type: fix stack overflow
[develop]: rule tests now support imports of node_modules
  • Loading branch information
ajafff authored and adidahiya committed May 26, 2017
1 parent efd95c6 commit cf4659e
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 46 deletions.
38 changes: 14 additions & 24 deletions src/rules/noInferredEmptyObjectTypeRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -60,12 +60,10 @@ class NoInferredEmptyObjectTypeRule extends Lint.AbstractWalker<void> {

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);
}
}
}
Expand All @@ -80,28 +78,20 @@ class NoInferredEmptyObjectTypeRule extends Lint.AbstractWalker<void> {
return;
}

const retType = this.checker.getReturnTypeOfSignature(callSig) as ts.TypeReference;
if (this.isEmptyObjectInterface(retType)) {
const retType = this.checker.getReturnTypeOfSignature(callSig);
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);
}
}
29 changes: 13 additions & 16 deletions src/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -90,8 +90,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 = 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) {
Expand All @@ -114,26 +113,24 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
let program: ts.Program | undefined;
if (hasConfig) {
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);
} else if (denormalizeWinPath(filenameToGet) === fileCompileName) {
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,
};
Expand All @@ -149,7 +146,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();
Expand Down
5 changes: 5 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,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, "/");
}
3 changes: 2 additions & 1 deletion test/executable/executableTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 4 additions & 0 deletions test/rules/no-inferred-empty-object-type/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -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);
5 changes: 0 additions & 5 deletions test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "/");
}

0 comments on commit cf4659e

Please sign in to comment.