Skip to content

Commit

Permalink
Don't use glob with --project option (palantir#3313)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ajafff @adidahiya
ajafff authored and adidahiya committed Nov 28, 2017
1 parent 1bd48b5 commit 048be95
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 32 deletions.
86 changes: 57 additions & 29 deletions src/runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import * as fs from "fs";
import * as glob from "glob";
import { Minimatch } from "minimatch";
import { filter as createMinimatchFilter, Minimatch } from "minimatch";
import * as path from "path";
import * as ts from "typescript";

Expand All @@ -32,7 +32,7 @@ import {
import { FatalError } from "./error";
import { LintResult } from "./index";
import * as Linter from "./linter";
import { arrayify, flatMap } from "./utils";
import { flatMap } from "./utils";

export interface Options {
/**
Expand All @@ -48,7 +48,7 @@ export interface Options {
/**
* File paths to lint.
*/
files?: string[];
files: string[];

/**
* Whether to return status code 0 even if there are lint errors.
Expand Down Expand Up @@ -175,35 +175,61 @@ async function runLinter(options: Options, logger: Logger): Promise<LintResult>

function resolveFilesAndProgram({ files, project, exclude, outputAbsolutePaths }: Options): { files: string[]; program?: ts.Program } {
// remove single quotes which break matching on Windows when glob is passed in single quotes
const ignore = arrayify(exclude).map(trimSingleQuotes);
exclude = exclude.map(trimSingleQuotes);

if (project === undefined) {
return { files: resolveGlobs(files, ignore, outputAbsolutePaths) };
return { files: resolveGlobs(files, exclude, outputAbsolutePaths) };
}

const projectPath = findTsconfig(project);
if (projectPath === undefined) {
throw new FatalError(`Invalid option for project: ${project}`);
}

exclude = exclude.map((pattern) => path.resolve(pattern));
const program = Linter.createProgram(projectPath);
let filesFound: string[];
if (files === undefined || files.length === 0) {
filesFound = Linter.getFileNames(program);
if (ignore.length !== 0) {
const mm = ignore.map((pattern) => new Minimatch(path.resolve(pattern)));
filesFound = filesFound.filter((file) => !mm.some((matcher) => matcher.match(file)));
}
if (files.length === 0) {
filesFound = filterFiles(Linter.getFileNames(program), exclude, false);
} else {
filesFound = resolveGlobs(files, ignore, outputAbsolutePaths);
files = files.map((f) => path.resolve(f));
filesFound = filterFiles(program.getSourceFiles().map((f) => f.fileName), files, true);
filesFound = filterFiles(filesFound, exclude, false);

// find non-glob files that have no matching file in the project and are not excluded by any exclude pattern
for (const file of filterFiles(files, exclude, false)) {
if (!glob.hasMagic(file) && !filesFound.some(createMinimatchFilter(file))) {
if (fs.existsSync(file)) {
throw new FatalError(`'${file}' is not included in project.`);
}
console.warn(`'${file}' does not exist. This will be an error in TSLint 6.`); // TODO make this an error in v6.0.0
}
}
}
return { files: filesFound, program };
}

function resolveGlobs(files: string[] | undefined, ignore: string[], outputAbsolutePaths?: boolean): string[] {
return flatMap(arrayify(files), (file) =>
glob.sync(trimSingleQuotes(file), { ignore, nodir: true }))
.map((file) => outputAbsolutePaths ? path.resolve(file) : path.relative(process.cwd(), file));
function filterFiles(files: string[], patterns: string[], include: boolean): string[] {
if (patterns.length === 0) {
return include ? [] : files;
}
const matcher = patterns.map((pattern) => new Minimatch(pattern, {dot: !include})); // `glob` always enables `dot` for ignore patterns
return files.filter((file) => include === matcher.some((pattern) => pattern.match(file)));
}

function resolveGlobs(files: string[], ignore: string[], outputAbsolutePaths?: boolean): string[] {
const results = flatMap(
files,
(file) => glob.sync(trimSingleQuotes(file), { ignore, nodir: true }),
);
// warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns
for (const file of filterFiles(files, ignore, false)) {
if (!glob.hasMagic(file)) {
console.warn(`'${file}' does not exist. This will be an error in TSLint 6.`); // TODO make this an error in v6.0.0
}
}
const cwd = process.cwd();
return results.map((file) => outputAbsolutePaths ? path.resolve(cwd, file) : path.relative(cwd, file));
}

async function doLinting(
Expand All @@ -220,37 +246,39 @@ async function doLinting(

let lastFolder: string | undefined;
let configFile: IConfigurationFile | undefined;
const isFileExcluded = (filepath: string) => {
if (configFile === undefined || configFile.linterOptions == undefined || configFile.linterOptions.exclude == undefined) {
return false;
}
const fullPath = path.resolve(filepath);
return configFile.linterOptions.exclude.some((pattern) => new Minimatch(pattern).match(fullPath));
};

for (const file of files) {
if (!fs.existsSync(file)) {
throw new FatalError(`Unable to open file: ${file}`);
if (isFileExcluded(file)) {
continue;
}

const contents = await tryReadFile(file, logger);
const contents = program !== undefined ? program.getSourceFile(file).text : await tryReadFile(file, logger);
if (contents !== undefined) {
const folder = path.dirname(file);
if (lastFolder !== folder) {
configFile = findConfiguration(possibleConfigAbsolutePath, folder).results;
lastFolder = folder;
}
if (!isFileExcluded(file)) {
linter.lint(file, contents, configFile);
}
linter.lint(file, contents, configFile);
}
}

return linter.getResult();

function isFileExcluded(filepath: string) {
if (configFile === undefined || configFile.linterOptions == undefined || configFile.linterOptions.exclude == undefined) {
return false;
}
const fullPath = path.resolve(filepath);
return configFile.linterOptions.exclude.some((pattern) => new Minimatch(pattern).match(fullPath));
}
}

/** Read a file, but return undefined if it is an MPEG '.ts' file. */
async function tryReadFile(filename: string, logger: Logger): Promise<string | undefined> {
if (!fs.existsSync(filename)) {
throw new FatalError(`Unable to open file: ${filename}`);
}
const buffer = new Buffer(256);
const fd = fs.openSync(filename, "r");
try {
Expand Down
4 changes: 2 additions & 2 deletions src/tslint-cli.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as fs from "fs";

import { VERSION } from "./linter";
import { run } from "./runner";
import { dedent } from "./utils";
import { arrayify, dedent } from "./utils";

interface Argv {
config?: string;
Expand Down Expand Up @@ -259,7 +259,7 @@ run(
{
config: argv.config,
exclude: argv.exclude,
files: commander.args,
files: arrayify(commander.args),
fix: argv.fix,
force: argv.force,
format: argv.format === undefined ? "prose" : argv.format,
Expand Down
73 changes: 72 additions & 1 deletion test/executable/executableTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,33 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
done();
});
});

it("warns if file does not exist", (done) => {
execCli(["foo/bar.ts"], (err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");

assert.include(stderr, "'foo/bar.ts' does not exist");
done();
});
});

it("doesn't warn if non-existent file is excluded by --exclude", (done) => {
execCli(["foo/bar.js", "--exclude", "**/*.js"], (err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");

assert.notInclude(stderr, "does not exist");
done();
});
});

it("doesn't warn if glob pattern doesn't match any file", (done) => {
execCli(["foobar/*.js"], (err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");

assert.notInclude(stderr, "does not exist");
done();
});
});
});

describe("Configuration file", () => {
Expand Down Expand Up @@ -371,7 +398,7 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
});

it("exits with error if passed directory does not exist", (done) => {
execCli(["-c", "test/files/tsconfig-test/tslint.json", "--project", "test/files/non-existant"], (err) => {
execCli(["-c", "test/files/tsconfig-test/tslint.json", "--project", "test/files/non-existent"], (err) => {
assert.isNotNull(err, "process should exit with an error");
assert.strictEqual(err.code, 1, "error code should be 1");
done();
Expand All @@ -394,6 +421,50 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
});
});

it("warns if file-to-lint does not exist", (done) => {
execCli(
[
"--project",
"test/files/tsconfig-test/tsconfig.json",
"test/files/tsconfig-test/non-existent.test.ts",
],
(err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");
assert.include(stderr, "test/files/tsconfig-test/non-existent.test.ts' does not exist");
done();
});
});

it("doesn't warn for non-existent file-to-lint if excluded by --exclude", (done) => {
execCli(
[
"--project",
"test/files/tsconfig-test/tsconfig.json",
"test/files/tsconfig-test/non-existent.test.ts",
"--exclude",
"**/*",
],
(err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");
assert.notInclude(stderr, "does not exist");
done();
});
});

it("doesn't warn ig glob pattern doesn't match any file", (done) => {
execCli(
[
"--project",
"test/files/tsconfig-test/tsconfig.json",
"*.js",
],
(err, _stdout, stderr) => {
assert.isNull(err, "process should exit without error");
assert.notInclude(stderr, "does not exist");
done();
});
});

it("exits with code 0 if `tsconfig.json` is passed but it includes no ts files", (done) => {
execCli(
["-c", "test/files/tsconfig-no-ts-files/tslint.json", "-p", "test/files/tsconfig-no-ts-files/tsconfig.json"],
Expand Down

0 comments on commit 048be95

Please sign in to comment.