Inherit defaultSeverity and apply it to preceding base configs

docs/usage/configuration/index.md

@@ -30,7 +30,7 @@ A path to a directory or an array of paths to directories of [custom rules][2].
   - Any rules specified in this block will override those configured in any base configuration being extended.
   - [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.
+* `defaultSeverity?: "error" | "warning" | "off"`: The severity level that is applied to rules in this config file as well as rules in any inherited config files which have their severity set to "default". If undefined, "error" is used as the defaultSeverity.
 * `linterOptions?: { exclude?: string[] }`:
   - `exclude: string[]`: An array of globs. Any file matching these globs will not be linted. All exclude patterns are relative to the configuration file they were specified in.
 

src/configuration.ts

@@ -27,7 +27,10 @@ import { arrayify, hasOwnProperty, stripComments } from "./utils";
 
 export interface IConfigurationFile {
     /**
-     * The severity that is applied to rules in _this_ config with `severity === "default"`.
+     * @deprecated property is never set
+     *
+     * The severity that is applied to rules in this config file as well as rules
+     * in any inherited config files which have their severity set to "default".
      * Not inherited.
      */
     defaultSeverity?: RuleSeverity;
@@ -210,52 +213,46 @@ function findup(filenames: string[], directory: string): string | undefined {
  * './path/to/config' will be treated as a relative path
  * 'path/to/config' will attempt to load a to/config file inside a node module named path
  * @param configFilePath The configuration to load
- * @param originalFilePath The entry point configuration file
+ * @param originalFilePath (deprecated) The entry point configuration file
  * @returns a configuration object for TSLint loaded from the file at configFilePath
  */
-export function loadConfigurationFromPath(configFilePath?: string, originalFilePath = configFilePath) {
+export function loadConfigurationFromPath(configFilePath?: string, _originalFilePath?: string) {
     if (configFilePath == undefined) {
         return DEFAULT_CONFIG;
     } else {
         const resolvedConfigFilePath = resolveConfigurationPath(configFilePath);
-        const resolvedConfigFileExt = path.extname(resolvedConfigFilePath);
-        let rawConfigFile: RawConfigFile;
-        if (/\.(json|ya?ml)/.test(resolvedConfigFileExt)) {
-            const fileContent = fs.readFileSync(resolvedConfigFilePath, "utf8").replace(/^\uFEFF/, "");
-            try {
-                if (resolvedConfigFileExt === ".json") {
-                    rawConfigFile = JSON.parse(stripComments(fileContent)) as RawConfigFile;
-                } else {
-                    // choose this branch only if /\.ya?ml/.test(resolvedConfigFileExt) === true
-                    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;
-                }
-            } 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
-                throw configFilePath === originalFilePath ? error : new Error(`${error.message} in ${configFilePath}`);
-            }
-        } else {
-            rawConfigFile = require(resolvedConfigFilePath) as RawConfigFile;
-            delete (require.cache as { [key: string]: any })[resolvedConfigFilePath];
-        }
-
-        const configFileDir = path.dirname(resolvedConfigFilePath);
-        const configFile = parseConfigFile(rawConfigFile, configFileDir);
+        const rawConfigFile = readConfigurationFile(resolvedConfigFilePath);
 
-        // load configurations, in order, using their identifiers or relative paths
-        // apply the current configuration last by placing it last in this array
-        const configs: IConfigurationFile[] = configFile.extends.map((name) => {
-            const nextConfigFilePath = resolveConfigurationPath(name, configFileDir);
-            return loadConfigurationFromPath(nextConfigFilePath, originalFilePath);
-        }).concat([configFile]);
+        return parseConfigFile(rawConfigFile, path.dirname(resolvedConfigFilePath), readConfigurationFile);
+    }
+}
 
-        return configs.reduce(extendConfigurationFile, EMPTY_CONFIG);
+/** Reads the configuration file from disk and parses it as raw JSON, YAML or JS depending on the extension. */
+export function readConfigurationFile(filepath: string): RawConfigFile {
+    const resolvedConfigFileExt = path.extname(filepath);
+    if (/\.(json|ya?ml)/.test(resolvedConfigFileExt)) {
+        const fileContent = fs.readFileSync(filepath, "utf8").replace(/^\uFEFF/, "");
+        try {
+            if (resolvedConfigFileExt === ".json") {
+                return JSON.parse(stripComments(fileContent)) as RawConfigFile;
+            } else {
+                return 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;
+            }
+        } 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
+            throw new Error(`${error.message} in ${filepath}`);
+        }
+    } else {
+        const rawConfigFile = require(filepath) as RawConfigFile;
+        delete (require.cache as { [key: string]: any })[filepath];
+        return rawConfigFile;
     }
 }
 
@@ -476,34 +473,64 @@ export type RawRuleConfig = null | undefined | boolean | any[] | {
  * @param configFile The raw object read from the JSON of a config file
  * @param configFileDir The directory of the config file
  */
-export function parseConfigFile(configFile: RawConfigFile, configFileDir?: string): IConfigurationFile {
-    return {
-        extends: arrayify(configFile.extends),
-        jsRules: parseRules(configFile.jsRules),
-        linterOptions: parseLinterOptions(configFile.linterOptions),
-        rules: parseRules(configFile.rules),
-        rulesDirectory: getRulesDirectories(configFile.rulesDirectory, configFileDir),
-    };
+export function parseConfigFile(
+    configFile: RawConfigFile,
+    configFileDir?: string,
+    readConfig?: (path: string) => RawConfigFile,
+): IConfigurationFile {
+    let defaultSeverity = configFile.defaultSeverity;
+    if (readConfig === undefined) {
+        return parse(configFile, configFileDir);
+    }
+
+    return loadExtendsRecursive(configFile, configFileDir!)
+        .map(({dir, config}) => parse(config, dir))
+        .reduce(extendConfigurationFile, EMPTY_CONFIG);
+
+    /** Read files in order, depth first, and assign `defaultSeverity` (last config in extends wins). */
+    function loadExtendsRecursive(raw: RawConfigFile, dir: string) {
+        const configs: Array<{dir: string; config: RawConfigFile}> = [];
+        for (const relativePath of arrayify(raw.extends)) {
+            const resolvedPath = resolveConfigurationPath(relativePath, dir);
+            const extendedRaw = readConfig!(resolvedPath);
+            configs.push(...loadExtendsRecursive(extendedRaw, path.dirname(resolvedPath)));
+        }
+        if (raw.defaultSeverity !== undefined) {
+            defaultSeverity = raw.defaultSeverity;
+        }
+        configs.push({dir, config: raw});
+        return configs;
+    }
+
+    function parse(config: RawConfigFile, dir?: string): IConfigurationFile {
+        return {
+            extends: arrayify(config.extends),
+            jsRules: parseRules(config.jsRules),
+            linterOptions: parseLinterOptions(config.linterOptions, dir),
+            rules: parseRules(config.rules),
+            rulesDirectory: getRulesDirectories(config.rulesDirectory, dir),
+        };
+    }
 
     function parseRules(config: RawRulesConfig | undefined): Map<string, Partial<IOptions>> {
         const map = new Map<string, Partial<IOptions>>();
         if (config !== undefined) {
             for (const ruleName in config) {
                 if (hasOwnProperty(config, ruleName)) {
-                    map.set(ruleName, parseRuleOptions(config[ruleName], configFile.defaultSeverity));
+                    map.set(ruleName, parseRuleOptions(config[ruleName], defaultSeverity));
                 }
             }
         }
         return map;
     }
 
-    function parseLinterOptions(raw: RawConfigFile["linterOptions"]): IConfigurationFile["linterOptions"] {
+    function parseLinterOptions(raw: RawConfigFile["linterOptions"], dir?: string): IConfigurationFile["linterOptions"] {
         if (raw === undefined || raw.exclude === undefined) {
             return {};
         }
         return {
             exclude: arrayify(raw.exclude).map(
-                (pattern) => configFileDir === undefined ? path.resolve(pattern) : path.resolve(configFileDir, pattern),
+                (pattern) => dir === undefined ? path.resolve(pattern) : path.resolve(dir, pattern),
             ),
         };
     }

test/config/tslint-default-severity-error.json

@@ -0,0 +1,6 @@
+{
+  "defaultSeverity": "error",
+  "rules": {
+    "default-severity-error": { "severity": "default" }
+  }
+}

test/config/tslint-default-severity-off.json

@@ -0,0 +1,6 @@
+{
+  "defaultSeverity": "off",
+  "rules": {
+    "default-severity-off": { "severity": "default" }
+  }
+}

test/config/tslint-default-severity-unspecified.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "default-severity-unspecified": { "severity": "default" }
+  }
+}

test/config/tslint-default-severity-warning.json

@@ -0,0 +1,6 @@
+{
+  "defaultSeverity": "warning",
+  "rules": {
+    "default-severity-warning": { "severity": "default" }
+  }
+}

test/config/tslint-extends-default-severity-only-in-extended.json

@@ -0,0 +1,6 @@
+{
+  "extends": "./tslint-extends-default-severity.json",
+  "rules": {
+    "default-severity-only-in-extended": { "severity": "default" }
+  }
+}

test/config/tslint-extends-default-severity-precendence.json

@@ -0,0 +1,8 @@
+{
+  "extends": [
+    "./tslint-default-severity-error.json",
+    "./tslint-default-severity-warning.json",
+    "./tslint-default-severity-off.json",
+    "./tslint-default-severity-unspecified.json"
+  ]
+}

test/config/tslint-extends-default-severity.json

@@ -0,0 +1,7 @@
+{
+  "extends": [ "./tslint-default-severity-unspecified.json", "./tslint-default-severity-error.json" ],
+  "defaultSeverity": "warning",
+  "rules": {
+    "default-severity-warning": { "severity": "default" }
+  }
+}

test/configurationTests.ts

@@ -251,6 +251,62 @@ describe("Configuration", () => {
             const actualConfig = extendConfigurationFile(baseConfig, extendingConfig);
             assertConfigEquals(actualConfig, expectedConfig);
         });
+
+        it("overrides defaultSeverity of base configs", () => {
+            const config = loadConfigurationFromPath("./test/config/tslint-extends-default-severity.json");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-unspecified")!.ruleSeverity,
+                "warning",
+                "should apply defaultSeverity to base config with no defaultSeverity");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-error")!.ruleSeverity,
+                "warning",
+                "should override defaultSeverity defined in base config");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-warning")!.ruleSeverity,
+                "warning",
+                "should apply defaultSeverity to extending config");
+        });
+
+        it("inherits defaultSeverity from base config if not specified", () => {
+            const config = loadConfigurationFromPath("./test/config/tslint-extends-default-severity-only-in-extended.json");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-unspecified")!.ruleSeverity,
+                "warning",
+                "should apply defaultSeverity to base config with no defaultSeverity");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-error")!.ruleSeverity,
+                "warning",
+                "should override defaultSeverity defined in base config");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-warning")!.ruleSeverity,
+                "warning",
+                "should apply defaultSeverity to extending config");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-only-in-extended")!.ruleSeverity,
+                "warning",
+                "should inherit defaultSeverity from base configs");
+        });
+
+        it("applies defaultSeverity to preceding base configs", () => {
+            const config = loadConfigurationFromPath("./test/config/tslint-extends-default-severity-precendence.json");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-unspecified")!.ruleSeverity,
+                "off",
+                "should apply defaultSeverity to base config with no defaultSeverity");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-error")!.ruleSeverity,
+                "off",
+                "should override defaultSeverity defined in preceding base config");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-warning")!.ruleSeverity,
+                "off",
+                "should override defaultSeverity defined in preceding base config");
+            assert.equal<RuleSeverity | undefined>(
+                config.rules.get("default-severity-off")!.ruleSeverity,
+                "off",
+                "should not override last declared defaultSeverity");
+        });
     });
 
     describe("findConfigurationPath", () => {
@@ -293,11 +349,11 @@ describe("Configuration", () => {
             assert.equal<RuleSeverity | undefined>(
                 "error",
                 config.rules.get("no-fail")!.ruleSeverity,
-                "did not pick up 'no-fail' in base config");
+                "should pick up 'no-fail' in base config");
             assert.equal<RuleSeverity | undefined>(
                 "off",
                 config.rules.get("always-fail")!.ruleSeverity,
-                "did not set 'always-fail' in top config");
+                "should set 'always-fail' in top config");
             assert.equal<RuleSeverity | undefined>("error", config.jsRules.get("no-fail")!.ruleSeverity);
             assert.equal<RuleSeverity | undefined>("off", config.jsRules.get("always-fail")!.ruleSeverity);
         });