Support yaml configuration (palantir#1598)

docs/usage/configuration/index.md

@@ -4,12 +4,12 @@ title: Configuring TSLint
 permalink: /usage/configuration/
 ---
 
-### tslint.json
+### The `tslint` File
 
-When using [the CLI][0] or many [third-party tools][1], a file named `tslint.json` is used to
-configure which rules get run and each of their options. This configuration file may be comp
+When using [the CLI][0] or many [third-party tools][1], a file named `tslint.json` or `tslint.yaml` is used to
+configure which rules get run and each of their options.
 
-`tslint.json` files can have the following fields specified:
+`tslint.json` or `tslint.yaml` files can have the following fields specified:
 
 * `extends?: string | string[]`:
 The name of a built-in configuration preset (see built-in presets below), or a path or
@@ -71,6 +71,37 @@ An example `tslint.json` file might look like this:
 }
 ```
 
+The corresponding YAML file looks like this:
+
+```yaml
+---
+extends: "tslint:recommended"
+rulesDirectory:
+    - path/to/custom/rules/directory/
+    - another/path/
+rules:
+    max-line-length:
+        options: [120]
+    new-parens: true
+    no-arg: true
+    no-bitwise: true
+    no-conditional-assignment: true
+    no-consecutive-blank-lines: false
+    no-console:
+        severity: warning
+        options:
+            - debug
+            - info
+            - log
+            - time
+            - timeEnd
+            - trace
+jsRules:
+    max-line-length:
+        options: [120]
+...
+```
+
 ### Rule severity
 
 The severity level of each rule can can be configured to `default`, `error`, `warning`/`warn`, or `off`/`none`. If no severity level is specified, `default` is used. The `defaultSeverity` top-level option replaces the severity level for each rule that uses severity level `default` in the current file. Valid values for `defaultSeverity` include `error`, `warning`/`warn`, and `off`/`none`.

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

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,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
@@ -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,
                     }
                 }
             }
-
         }
     }
 

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

test/config/tslint-invalid.yaml

@@ -0,0 +1 @@
+{{ hello I am invalid }

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"
+...

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<RuleSeverity | undefined>("error", config.jsRules.get("no-eval")!.ruleSeverity);

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");

test/files/config-findup/yaml-config/tslint.json

@@ -0,0 +1 @@
+{}

test/files/config-findup/yaml-config/tslint.yaml

@@ -0,0 +1,2 @@
+---
+...

test/files/config-findup/yaml-config/tslint.yml

@@ -0,0 +1,2 @@
+---
+...

test/files/config-findup/yml-config/tslint.json

@@ -0,0 +1 @@
+{}

test/files/config-findup/yml-config/tslint.yml

@@ -0,0 +1,2 @@
+---
+...