Skip to content

Commit

Permalink
Support yaml configuration (palantir#1598)
Browse files Browse the repository at this point in the history
  • Loading branch information
jhanschoo committed Nov 1, 2017
1 parent d1caf11 commit 833ccc7
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 31 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@
"dependencies": {
"babel-code-frame": "^6.22.0",
"builtin-modules": "^1.1.1",
"chalk": "^2.1.0",
"chalk": "~2.1.0",
"commander": "^2.9.0",
"diff": "^3.2.0",
"glob": "^7.1.1",
"js-yaml": "^3.7.0",
"minimatch": "^3.0.4",
"resolve": "^1.3.2",
"semver": "^5.3.0",
Expand All @@ -68,7 +69,6 @@
"@types/semver": "^5.3.30",
"chai": "^3.5.0",
"github": "^8.2.1",
"js-yaml": "^3.7.0",
"json-stringify-pretty-compact": "^1.0.3",
"mocha": "^3.2.0",
"npm-run-all": "^4.0.2",
Expand Down
69 changes: 45 additions & 24 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import * as fs from "fs";
import * as yaml from "js-yaml";
import * as path from "path";
import * as resolve from "resolve";
import { FatalError, showWarningOnce } from "./error";
Expand Down Expand Up @@ -63,7 +64,10 @@ export interface IConfigurationLoadResult {
results?: IConfigurationFile;
}

export const CONFIG_FILENAME = "tslint.json";
// As per eslint convention, yaml config is used if present
// over json config
export const JSON_CONFIG_FILENAME = "tslint.json";
export const CONFIG_FILENAMES = ["tslint.yaml", "tslint.yml", JSON_CONFIG_FILENAME];

export const DEFAULT_CONFIG: IConfigurationFile = {
defaultSeverity: "error",
Expand Down Expand Up @@ -111,7 +115,7 @@ export function findConfiguration(configFile: string | null, inputFilePath?: str
* the location of the config file is not known and you want to search for one.
* @param inputFilePath A path to the current file being linted. This is the starting location
* of the search for a configuration.
* @returns An absolute path to a tslint.json file
* @returns An absolute path to a tslint.json or tslint.yml or tslint.yaml file
* or undefined if neither can be found.
*/
export function findConfigurationPath(suppliedConfigFilePath: string | null, inputFilePath: string): string | undefined;
Expand Down Expand Up @@ -140,17 +144,19 @@ export function findConfigurationPath(suppliedConfigFilePath: string | null, inp
}

// search for tslint.json from input file location
let configFilePath = findup(CONFIG_FILENAME, path.resolve(inputFilePath!));
let configFilePath = findup(CONFIG_FILENAMES, path.resolve(inputFilePath!));
if (configFilePath !== undefined) {
return configFilePath;
}

// search for tslint.json in home directory
const homeDir = getHomeDir();
if (homeDir != undefined) {
configFilePath = path.join(homeDir, CONFIG_FILENAME);
if (fs.existsSync(configFilePath)) {
return path.resolve(configFilePath);
for (const configFilename of CONFIG_FILENAMES) {
configFilePath = path.join(homeDir, configFilename);
if (fs.existsSync(configFilePath)) {
return path.resolve(configFilePath);
}
}
}
// no path could be found
Expand All @@ -159,10 +165,11 @@ export function findConfigurationPath(suppliedConfigFilePath: string | null, inp
}

/**
* Find a file by name in a directory or any ancestory directory.
* Find a file by names in a directory or any ancestor directory.
* Will try each filename in filenames before recursing to a parent directory.
* This is case-insensitive, so it can find 'TsLiNt.JsOn' when searching for 'tslint.json'.
*/
function findup(filename: string, directory: string): string | undefined {
function findup(filenames: string[], directory: string): string | undefined {
while (true) {
const res = findFile(directory);
if (res !== undefined) {
Expand All @@ -177,18 +184,21 @@ function findup(filename: string, directory: string): string | undefined {
}

function findFile(cwd: string): string | undefined {
if (fs.existsSync(path.join(cwd, filename))) {
return filename;
}
for (const filename of filenames) {
if (fs.existsSync(path.join(cwd, filename))) {
return filename;
}

// TODO: remove in v6.0.0
// Try reading in the entire directory and looking for a file with different casing.
const filenameLower = filename.toLowerCase();
const result = fs.readdirSync(cwd).find((entry) => entry.toLowerCase() === filenameLower);
if (result !== undefined) {
showWarningOnce(`Using mixed case tslint.json is deprecated. Found: ${path.join(cwd, result)}`);
// TODO: remove in v6.0.0
// Try reading in the entire directory and looking for a file with different casing.
const filenameLower = filename.toLowerCase();
const result = fs.readdirSync(cwd).find((entry) => entry.toLowerCase() === filenameLower);
if (result !== undefined) {
showWarningOnce(`Using mixed case ${filenameLower} is deprecated. Found: ${path.join(cwd, result)}`);
return result;
}
}
return result;
return undefined;
}
}

Expand All @@ -207,13 +217,25 @@ export function loadConfigurationFromPath(configFilePath?: string, originalFileP
return DEFAULT_CONFIG;
} else {
const resolvedConfigFilePath = resolveConfigurationPath(configFilePath);
const resolvedConfigFileExt = path.extname(resolvedConfigFilePath);
let rawConfigFile: RawConfigFile;
if (path.extname(resolvedConfigFilePath) === ".json") {
const fileContent = stripComments(fs.readFileSync(resolvedConfigFilePath)
.toString()
.replace(/^\uFEFF/, ""));
if (resolvedConfigFileExt.match(/\.(json|ya?ml)/) !== null) {
const fileContent = fs.readFileSync(resolvedConfigFilePath).toString().replace(/^\uFEFF/, "");
try {
rawConfigFile = JSON.parse(fileContent) as RawConfigFile;
if (resolvedConfigFileExt === ".json") {
rawConfigFile = JSON.parse(stripComments(fileContent)) as RawConfigFile;
} else if (resolvedConfigFileExt.match(/ya?ml/) !== null) {
rawConfigFile = yaml.safeLoad(fileContent, {
// Note: yaml.LoadOptions expects a schema value of type "any",
// but this trips up the no-unsafe-any rule.
// tslint:disable-next-line:no-unsafe-any
schema: yaml.JSON_SCHEMA,
strict: true,
}) as RawConfigFile;
} else {
// throw error for static analysis purpose; should not happen
throw new Error("File format not supported yet.");
}
} catch (e) {
const error = e as Error;
// include the configuration file being parsed in the error since it may differ from the directly referenced config
Expand Down Expand Up @@ -286,7 +308,6 @@ export function extendConfigurationFile(targetConfig: IConfigurationFile,
}
}
}

}
}

Expand Down
8 changes: 4 additions & 4 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import * as path from "path";
import * as ts from "typescript";

import {
CONFIG_FILENAME,
DEFAULT_CONFIG,
findConfiguration,
IConfigurationFile,
JSON_CONFIG_FILENAME,
} from "./configuration";
import { FatalError } from "./error";
import { LintResult } from "./index";
Expand Down Expand Up @@ -131,11 +131,11 @@ export async function run(options: Options, logger: Logger): Promise<Status> {

async function runWorker(options: Options, logger: Logger): Promise<Status> {
if (options.init) {
if (fs.existsSync(CONFIG_FILENAME)) {
throw new FatalError(`Cannot generate ${CONFIG_FILENAME}: file already exists`);
if (fs.existsSync(JSON_CONFIG_FILENAME)) {
throw new FatalError(`Cannot generate ${JSON_CONFIG_FILENAME}: file already exists`);
}

fs.writeFileSync(CONFIG_FILENAME, JSON.stringify(DEFAULT_CONFIG, undefined, " "));
fs.writeFileSync(JSON_CONFIG_FILENAME, JSON.stringify(DEFAULT_CONFIG, undefined, " "));
return Status.Ok;
}

Expand Down
1 change: 1 addition & 0 deletions test/config/tslint-invalid.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ hello I am invalid }
17 changes: 17 additions & 0 deletions test/config/tslint-with-comments.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
jsRules:
# rule-one: true
rule-two:
severity: error # after comment
rule-three:
- true
- "#not a comment"
# a noice comment
rules:
# rule-one: true
rule-two:
severity: error # after comment
rule-three:
- true
- "#not a comment"
...
21 changes: 21 additions & 0 deletions test/configurationTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,16 @@ describe("Configuration", () => {
path.resolve("./test/files/config-findup/tslint.json"),
);
});
it("prefers yaml configurations to yml and json configurations", () => {
assert.strictEqual(
findConfigurationPath(null, "./test/files/config-findup/yaml-config"),
path.resolve("test/files/config-findup/yaml-config/tslint.yaml"),
);
assert.strictEqual(
findConfigurationPath(null, "./test/files/config-findup/yml-config"),
path.resolve("test/files/config-findup/yml-config/tslint.yml"),
);
});
});

describe("loadConfigurationFromPath", () => {
Expand Down Expand Up @@ -421,6 +431,17 @@ describe("Configuration", () => {
assert.doesNotThrow(() => loadConfigurationFromPath("./test/config/tslint-with-bom.json"));
});

it("can load .yaml files with comments", () => {
const config = loadConfigurationFromPath("./test/config/tslint-with-comments.yaml");

const expectedConfig = getEmptyConfig();
expectedConfig.rules.set("rule-two", { ruleSeverity: "error" });
expectedConfig.rules.set("rule-three", { ruleSeverity: "error", ruleArguments: ["#not a comment"] });

assertConfigEquals(config.rules, expectedConfig.rules);
assertConfigEquals(config.jsRules, expectedConfig.rules);
});

it("can load a built-in configuration", () => {
const config = loadConfigurationFromPath("tslint:recommended");
assert.strictEqual<RuleSeverity | undefined>("error", config.jsRules.get("no-eval")!.ruleSeverity);
Expand Down
13 changes: 12 additions & 1 deletion test/executable/executableTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
});
});

it("exits with code 1 if config file is invalid", (done) => {
it("exits with code 1 if json config file is invalid", (done) => {
execCli(["-c", "test/config/tslint-invalid.json", "src/test.ts"], (err, stdout, stderr) => {
assert.isNotNull(err, "process should exit with error");
assert.strictEqual(err.code, 1, "error code should be 1");
Expand All @@ -98,6 +98,17 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) {
});
});

it("exits with code 1 if yaml config file is invalid", (done) => {
execCli(["-c", "test/config/tslint-invalid.yaml", "src/test.ts"], (err, stdout, stderr) => {
assert.isNotNull(err, "process should exit with error");
assert.strictEqual(err.code, 1, "error code should be 1");

assert.include(stderr, "Failed to load", "stderr should contain notification about failing to load json");
assert.strictEqual(stdout, "", "shouldn't contain any output in stdout");
done();
});
});

it("mentions the root cause if a config file extends from an invalid file", (done) => {
execCli(["-c", "test/config/tslint-extends-invalid.json", "src/test.ts"], (err, stdout, stderr) => {
assert.isNotNull(err, "process should exit with error");
Expand Down
1 change: 1 addition & 0 deletions test/files/config-findup/yaml-config/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
2 changes: 2 additions & 0 deletions test/files/config-findup/yaml-config/tslint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
...
2 changes: 2 additions & 0 deletions test/files/config-findup/yaml-config/tslint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
...
1 change: 1 addition & 0 deletions test/files/config-findup/yml-config/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
2 changes: 2 additions & 0 deletions test/files/config-findup/yml-config/tslint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
...

0 comments on commit 833ccc7

Please sign in to comment.