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

Add exclude section to configuration files. #2409

Merged
merged 17 commits into from
Sep 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/usage/configuration/index.md
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
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
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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The excluded paths should probably be resolved relative to the location of tslint.json.
If /foo/tslint.json excludes baz.ts we should only exclude /foo/baz.ts and still lint /foo/bar/baz.ts.

That will only work when filepath is an absolute path.

};

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
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
@@ -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
@@ -0,0 +1,13 @@
{
"rules": {
"semicolon": [
true,
"always"
]
},
"linterOptions": {
"exclude": [
"**/*excluded.ts"
]
}
}
46 changes: 45 additions & 1 deletion test/configurationTests.ts
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity: what happens if extendingConfig contains an empty object as "linterOptions"? Would that override the whole object including "exclude"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, only if exclude is specified. I added a test for this below.

});

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
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
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