Skip to content

Commit

Permalink
Add support for exclude globs in configuration files (palantir#2409)
Browse files Browse the repository at this point in the history
  • Loading branch information
mmkal authored and adidahiya committed Sep 7, 2017
1 parent 7131f01 commit ec9fe9f
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 8 deletions.
2 changes: 2 additions & 0 deletions docs/usage/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ A path to a directory or an array of paths to directories of [custom rules][2].
- [Check out the full rules list here][3].
* `jsRules?: any`: Same format as `rules`. These rules are applied to `.js` and `.jsx` files.
* `defaultSeverity?: "error" | "warning" | "off"`: The severity level used when a rule specifies a default warning level. If undefined, "error" is used. This value is not inherited and is only applied to rules in this file.
* `linterOptions?: { exclude?: string[] }`:
- `exclude: string[]`: An array of globs. Any file matching these globs will not be linted.

`tslint.json` configuration files may have JavaScript-style `// single-line` and `/* multi-line */` comments in them (even though this is technically invalid JSON). If this confuses your syntax highlighter, you may want to switch it to JavaScript format.

Expand Down
8 changes: 4 additions & 4 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ export interface IConfigurationFile {
jsRules: Map<string, Partial<IOptions>>;

/**
* Other linter options, currently for testing. Not publicly supported.
* A subset of the CLI options.
*/
linterOptions?: {
typeCheck?: boolean;
};
linterOptions?: Partial<{
exclude: string[];
}>;

/**
* Directories containing custom rules. Resolved using node module semantics.
Expand Down
14 changes: 12 additions & 2 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export interface Options {
/**
* Exclude globs from path expansion.
*/
exclude?: string | string[];
exclude: string[];

/**
* File paths to lint.
Expand Down Expand Up @@ -218,6 +218,14 @@ async function doLinting(

let lastFolder: string | undefined;
let configFile: IConfigurationFile | undefined;
const isFileExcluded = (filepath: string) => {
if (configFile === undefined || configFile.linterOptions == null || configFile.linterOptions.exclude == null) {
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}`);
Expand All @@ -230,7 +238,9 @@ async function doLinting(
configFile = findConfiguration(possibleConfigAbsolutePath, folder).results;
lastFolder = folder;
}
linter.lint(file, contents, configFile);
if (!isFileExcluded(file)) {
linter.lint(file, contents, configFile);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tslint-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { dedent } from "./utils";

interface Argv {
config?: string;
exclude?: string;
exclude: string[];
fix?: boolean;
force?: boolean;
help?: boolean;
Expand Down
14 changes: 14 additions & 0 deletions test/config/tslint-exclude-many.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"rules": {
"semicolon": [
true,
"always"
]
},
"linterOptions": {
"exclude": [
"**/*excluded1.ts",
"**/*excluded2.ts"
]
}
}
13 changes: 13 additions & 0 deletions test/config/tslint-exclude-one.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"rules": {
"semicolon": [
true,
"always"
]
},
"linterOptions": {
"exclude": [
"**/*excluded.ts"
]
}
}
46 changes: 45 additions & 1 deletion test/configurationTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe("Configuration", () => {
config.jsRules.set("row", { ruleArguments: ["oar", "column"], ruleSeverity: "error" });
config.rules.set("foo", { ruleArguments: ["bar"], ruleSeverity: "off" });
config.rulesDirectory = ["foo"];
config.linterOptions = { typeCheck: true };
config.linterOptions = { exclude: ["foo"] };
assertConfigEquals(extendConfigurationFile(EMPTY_CONFIG, config), config);
});

Expand Down Expand Up @@ -193,6 +193,50 @@ describe("Configuration", () => {
const actualConfig = extendConfigurationFile(baseConfig, extendingConfig);
assertConfigEquals(actualConfig, expectedConfig);
});

it("replaces exclude option", () => {
const baseConfig = getEmptyConfig();
baseConfig.linterOptions = {
exclude: ["src"],
};

const extendingConfig = getEmptyConfig();
extendingConfig.linterOptions = {
exclude: [
"lib",
"bin",
],
};

const expectedConfig = getEmptyConfig();
expectedConfig.linterOptions = {
exclude: [
"lib",
"bin",
],
};

const actualConfig = extendConfigurationFile(baseConfig, extendingConfig);
assertConfigEquals(actualConfig, expectedConfig);
});

it("empty linter options does not replace exclude", () => {
const baseConfig = getEmptyConfig();
baseConfig.linterOptions = {
exclude: ["src"],
};

const extendingConfig = getEmptyConfig();
extendingConfig.linterOptions = {};

const expectedConfig = getEmptyConfig();
expectedConfig.linterOptions = {
exclude: ["src"],
};

const actualConfig = extendConfigurationFile(baseConfig, extendingConfig);
assertConfigEquals(actualConfig, expectedConfig);
});
});

describe("findConfigurationPath", () => {
Expand Down
30 changes: 30 additions & 0 deletions test/executable/executableTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,36 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
});
});

describe("Config with excluded files", () => {
it("exits with code 1 if linter options doesn't exclude file with lint errors", (done) => {
const tempFile = createTempFile("included.ts");
fs.writeFileSync(tempFile, "console.log(\"missing semi-colon at end of line\")", { encoding: "utf8" });
execCli(["-c", "./test/config/tslint-exclude.json", tempFile], (err) => {
assert.isNotNull(err, "process should exit with error");
assert.strictEqual(err.code, 1, "error code should be 1");
done();
});
});

it("exits with code 0 if linter options exclude one file with lint errors", (done) => {
const tempFile = createTempFile("excluded.ts");
fs.writeFileSync(tempFile, "console.log(\"missing semi-colon at end of line\")", { encoding: "utf8" });
execCli(["-c", "./test/config/tslint-exclude-one.json", tempFile], (err) => {
assert.isNull(err, "process should exit without an error");
done();
});
});

it("exits with code 0 if linter options excludes many files with lint errors", (done) => {
const tempFiles = [1, 2].map((x) => createTempFile(`excluded${x}.ts`));
tempFiles.forEach((f) => fs.writeFileSync(f, "console.log(\"missing semi-colon at end of line\")", { encoding: "utf8" }));
execCli(["-c", "./test/config/tslint-exclude-many.json", ...tempFiles], (err) => {
assert.isNull(err, "process should exit without an error");
done();
});
});
});

describe("--fix flag", () => {
it("fixes multiple rules without overwriting each other", (done) => {
const tempFile = path.relative(process.cwd(), createTempFile("ts"));
Expand Down
1 change: 1 addition & 0 deletions test/runner/runnerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { Options, run, Status } from "../../src/runner";

const customRulesOptions: Options = {
config: "./test/config/tslint-custom-rules.json",
exclude: [],
files: ["src/test.ts"],
rulesDirectory: "./test/files/custom-rules",
};
Expand Down

0 comments on commit ec9fe9f

Please sign in to comment.