From 833ccc759d361ab6eff1b0b1ce435e7b9e7986c4 Mon Sep 17 00:00:00 2001 From: Johannes Choo Date: Wed, 1 Nov 2017 11:30:53 +0800 Subject: [PATCH] Support yaml configuration (#1598) --- package.json | 4 +- src/configuration.ts | 69 ++++++++++++------- src/runner.ts | 8 +-- test/config/tslint-invalid.yaml | 1 + test/config/tslint-with-comments.yaml | 17 +++++ test/configurationTests.ts | 21 ++++++ test/executable/executableTests.ts | 13 +++- .../config-findup/yaml-config/tslint.json | 1 + .../config-findup/yaml-config/tslint.yaml | 2 + .../config-findup/yaml-config/tslint.yml | 2 + .../config-findup/yml-config/tslint.json | 1 + .../files/config-findup/yml-config/tslint.yml | 2 + 12 files changed, 110 insertions(+), 31 deletions(-) create mode 100644 test/config/tslint-invalid.yaml create mode 100644 test/config/tslint-with-comments.yaml create mode 100644 test/files/config-findup/yaml-config/tslint.json create mode 100644 test/files/config-findup/yaml-config/tslint.yaml create mode 100644 test/files/config-findup/yaml-config/tslint.yml create mode 100644 test/files/config-findup/yml-config/tslint.json create mode 100644 test/files/config-findup/yml-config/tslint.yml diff --git a/package.json b/package.json index a91be371bc6..2e321e7dbd1 100644 --- a/package.json +++ b/package.json @@ -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", @@ -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", diff --git a/src/configuration.ts b/src/configuration.ts index 4284210c6dc..ceeee20aa36 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -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"; @@ -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", @@ -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; @@ -140,7 +144,7 @@ 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; } @@ -148,9 +152,11 @@ export function findConfigurationPath(suppliedConfigFilePath: string | null, inp // 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 @@ -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) { @@ -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; } } @@ -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 @@ -286,7 +308,6 @@ export function extendConfigurationFile(targetConfig: IConfigurationFile, } } } - } } diff --git a/src/runner.ts b/src/runner.ts index c611a645385..d29aa8c3511 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -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"; @@ -131,11 +131,11 @@ export async function run(options: Options, logger: Logger): Promise { async function runWorker(options: Options, logger: Logger): Promise { 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; } diff --git a/test/config/tslint-invalid.yaml b/test/config/tslint-invalid.yaml new file mode 100644 index 00000000000..eb03ead12b0 --- /dev/null +++ b/test/config/tslint-invalid.yaml @@ -0,0 +1 @@ +{{ hello I am invalid } diff --git a/test/config/tslint-with-comments.yaml b/test/config/tslint-with-comments.yaml new file mode 100644 index 00000000000..3ebcb219524 --- /dev/null +++ b/test/config/tslint-with-comments.yaml @@ -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" +... diff --git a/test/configurationTests.ts b/test/configurationTests.ts index 522c74a8565..3e9053e37b1 100644 --- a/test/configurationTests.ts +++ b/test/configurationTests.ts @@ -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", () => { @@ -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("error", config.jsRules.get("no-eval")!.ruleSeverity); diff --git a/test/executable/executableTests.ts b/test/executable/executableTests.ts index cb3076de004..a49a6c80659 100644 --- a/test/executable/executableTests.ts +++ b/test/executable/executableTests.ts @@ -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"); @@ -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"); diff --git a/test/files/config-findup/yaml-config/tslint.json b/test/files/config-findup/yaml-config/tslint.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/test/files/config-findup/yaml-config/tslint.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/test/files/config-findup/yaml-config/tslint.yaml b/test/files/config-findup/yaml-config/tslint.yaml new file mode 100644 index 00000000000..91da2a75b43 --- /dev/null +++ b/test/files/config-findup/yaml-config/tslint.yaml @@ -0,0 +1,2 @@ +--- +... diff --git a/test/files/config-findup/yaml-config/tslint.yml b/test/files/config-findup/yaml-config/tslint.yml new file mode 100644 index 00000000000..91da2a75b43 --- /dev/null +++ b/test/files/config-findup/yaml-config/tslint.yml @@ -0,0 +1,2 @@ +--- +... diff --git a/test/files/config-findup/yml-config/tslint.json b/test/files/config-findup/yml-config/tslint.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/test/files/config-findup/yml-config/tslint.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/test/files/config-findup/yml-config/tslint.yml b/test/files/config-findup/yml-config/tslint.yml new file mode 100644 index 00000000000..91da2a75b43 --- /dev/null +++ b/test/files/config-findup/yml-config/tslint.yml @@ -0,0 +1,2 @@ +--- +...