From e03316d62ce63c3e8947057599bc5980435e1384 Mon Sep 17 00:00:00 2001 From: Noah Chen Date: Mon, 27 Mar 2017 17:07:54 -0400 Subject: [PATCH] Adds default severity --- docs/usage/tslint-json/index.md | 83 +++++++------------ src/configuration.ts | 73 +++++++++++++--- .../tslint-extends-package-warning.json | 13 +++ test/configurationTests.ts | 33 ++++++-- test/executable/executableTests.ts | 9 ++ 5 files changed, 142 insertions(+), 69 deletions(-) create mode 100644 test/config/tslint-extends-package-warning.json diff --git a/docs/usage/tslint-json/index.md b/docs/usage/tslint-json/index.md index 0b3e3135ee0..ab9f6a47091 100644 --- a/docs/usage/tslint-json/index.md +++ b/docs/usage/tslint-json/index.md @@ -9,73 +9,54 @@ configure which rules get run. `tslint.json` files can have the following fields specified: -* `extends?: string | string[]`: -A path(s) to another configuration file which to extend. +* `extends?: string | string[]`: +A path or array of paths to other configuration files which are extended by this configuration. This value is handled using node module resolution semantics. For example a value of "tslint-config" would cause TSLint to try and load the main file of a module named "tslint-config" as a configuration file. A value of "./tslint-config", on the other hand, would be treated as a relative path to file. * `rulesDirectory?: string | string[]`: -A path(s) to a directory of [custom rules][2]. This will always be treated as a relative or absolute path. -* `rules?: any`: Pairs of keys and values where each key is a rule name and each value is the configuration for that rule. -If a rule takes no options, you can simply set its value to a boolean, either `true` or `false`, to enable or disable it. -If a rule takes options, you set its value to an array where the first value is a boolean indicating if the rule is enabled and the next values are options handled by the rule. +A path or array of paths to a directories of [custom rules][2]. This will always be treated as a relative or absolute path. +* `rules?: { [name: string]: RuleSetting }`: A map of rule names to their configuration settings. + - Each rule is associated with an object containing: + - `options?: any`: An array of values that are specific to a rule. + - `severity?: "default" | "error" | "warning" | "off"`: Severity level. Level "error" will cause exit code 2. + - A boolean value may be specified instead of the above object, and is equivalent to setting no options with default severity. Not all possible rules are listed here, be sure to [check out the full list][3]. These rules are applied to `.ts` and `.tsx` files. -* `jsRules?: any`: Same format as `rules`. These rules are applied to `.js` and `.jsx` files. +* `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. `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. An example `tslint.json` file might look like this: -```ts +```json { "rulesDirectory": ["path/to/custom/rules/directory/", "another/path/"], "rules": { - "class-name": true, - "comment-format": [true, "check-space"], - "indent": [true, "spaces"], - "no-duplicate-variable": true, - "no-eval": true, - "no-internal-module": true, - "no-trailing-whitespace": true, - "no-var-keyword": true, - "one-line": [true, "check-open-brace", "check-whitespace"], - "quotemark": [true, "double"], - "semicolon": false, - "triple-equals": [true, "allow-null-check"], - "typedef-whitespace": [true, { - "call-signature": "nospace", - "index-signature": "nospace", - "parameter": "nospace", - "property-declaration": "nospace", - "variable-declaration": "nospace" - }], - "variable-name": [true, "ban-keywords"], - "whitespace": [true, - "check-branch", - "check-decl", - "check-operator", - "check-separator", - "check-type" - ] + "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": { + "options": [ + "debug", + "info", + "log", + "time", + "timeEnd", + "trace", + ], + } }, "jsRules": { - "indent": [true, "spaces"], - "no-duplicate-variable": true, - "no-eval": true, - "no-trailing-whitespace": true, - "one-line": [true, "check-open-brace", "check-whitespace"], - "quotemark": [true, "double"], - "semicolon": false, - "triple-equals": [true, "allow-null-check"], - "variable-name": [true, "ban-keywords"], - "whitespace": [true, - "check-branch", - "check-decl", - "check-operator", - "check-separator", - "check-type" - ] + "max-line-length": { + "options": [120], + } } } ``` diff --git a/src/configuration.ts b/src/configuration.ts index 7b3e9bc0ac8..45eb4eece3a 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -25,12 +25,37 @@ import { IOptions, RuleSeverity } from "./language/rule/rule"; import { arrayify, objectify, stripComments } from "./utils"; export interface IConfigurationFile { + /** + * The severity that is applied to rules in _this_ config with `severity === "default"`. + * Not inherited. + */ + defaultSeverity?: RuleSeverity; + + /** + * An array of config files whose rules are inherited by this config file. + */ extends: string[]; + + /** + * Rules that are used to lint to JavaScript files. + */ jsRules: Map>; + + /** + * Other linter options, currently for testing. Not publicly supported. + */ linterOptions?: { typeCheck?: boolean, }; + + /** + * Directories containing custom rules. Resolved using node module semantics. + */ rulesDirectory: string[]; + + /** + * Rules that are used to lint TypeScript files. + */ rules: Map>; } @@ -42,6 +67,7 @@ export interface IConfigurationLoadResult { export const CONFIG_FILENAME = "tslint.json"; export const DEFAULT_CONFIG: IConfigurationFile = { + defaultSeverity: "error", extends: ["tslint:recommended"], jsRules: new Map>(), rules: new Map>(), @@ -49,6 +75,7 @@ export const DEFAULT_CONFIG: IConfigurationFile = { }; export const EMPTY_CONFIG: IConfigurationFile = { + defaultSeverity: "error", extends: [], jsRules: new Map>(), rules: new Map>(), @@ -276,9 +303,25 @@ export function getRulesDirectories(directories?: string | string[], relativeTo? * * @param ruleConfigValue The raw option setting of a rule */ -function parseRuleOptions(ruleConfigValue: any): Partial { +function parseRuleOptions(ruleConfigValue: any, rawDefaultRuleSeverity: string): Partial { let ruleArguments: any[] | undefined; - let ruleSeverity: RuleSeverity | undefined; + let ruleSeverity: RuleSeverity; + let defaultRuleSeverity: RuleSeverity = "error"; + + if (rawDefaultRuleSeverity) { + switch (rawDefaultRuleSeverity.toLowerCase()) { + case "warn": + case "warning": + defaultRuleSeverity = "warning"; + break; + case "off": + case "none": + defaultRuleSeverity = "off"; + break; + default: + defaultRuleSeverity = "error"; + } + } if (ruleConfigValue == null) { ruleArguments = []; @@ -286,27 +329,33 @@ function parseRuleOptions(ruleConfigValue: any): Partial { } else if (Array.isArray(ruleConfigValue) && ruleConfigValue.length > 0) { // old style: array ruleArguments = ruleConfigValue.slice(1); - ruleSeverity = ruleConfigValue[0] === true ? "error" : "off"; + ruleSeverity = ruleConfigValue[0] === true ? defaultRuleSeverity : "off"; } else if (typeof ruleConfigValue === "boolean") { // old style: boolean ruleArguments = []; - ruleSeverity = ruleConfigValue === true ? "error" : "off"; + ruleSeverity = ruleConfigValue === true ? defaultRuleSeverity : "off"; } else if (ruleConfigValue.severity) { switch (ruleConfigValue.severity.toLowerCase()) { + case "default": + ruleSeverity = defaultRuleSeverity; + break; + case "error": + ruleSeverity = "error"; + break; case "warn": case "warning": ruleSeverity = "warning"; break; - case "error": - ruleSeverity = "error"; + case "off": + case "none": + ruleSeverity = "off"; break; default: - ruleSeverity = "off"; + console.warn(`Invalid severity level: ${ruleConfigValue.severity}`); + ruleSeverity = defaultRuleSeverity; } - } else if (typeof ruleConfigValue === "object") { - ruleSeverity = undefined; } else { - ruleSeverity = "off"; + ruleSeverity = defaultRuleSeverity; } if (ruleConfigValue && ruleConfigValue.options) { @@ -332,7 +381,7 @@ export function parseConfigFile(configFile: any, configFileDir?: string): IConfi if (configFile.rules) { for (const ruleName in configFile.rules) { if (configFile.rules.hasOwnProperty(ruleName)) { - rules.set(ruleName, parseRuleOptions(configFile.rules[ruleName])); + rules.set(ruleName, parseRuleOptions(configFile.rules[ruleName], configFile.defaultSeverity)); } } } @@ -340,7 +389,7 @@ export function parseConfigFile(configFile: any, configFileDir?: string): IConfi if (configFile.jsRules) { for (const ruleName in configFile.jsRules) { if (configFile.jsRules.hasOwnProperty(ruleName)) { - jsRules.set(ruleName, parseRuleOptions(configFile.jsRules[ruleName])); + jsRules.set(ruleName, parseRuleOptions(configFile.jsRules[ruleName], configFile.defaultSeverity)); } } } diff --git a/test/config/tslint-extends-package-warning.json b/test/config/tslint-extends-package-warning.json new file mode 100644 index 00000000000..7edf9cc50ad --- /dev/null +++ b/test/config/tslint-extends-package-warning.json @@ -0,0 +1,13 @@ +{ + "extends": "tslint-test-custom-rules", + "jsRules": { + "always-fail": { + "severity": "warning" + } + }, + "rules": { + "always-fail": { + "severity": "warning" + } + } +} diff --git a/test/configurationTests.ts b/test/configurationTests.ts index b3dac1c78f2..805be07f7b1 100644 --- a/test/configurationTests.ts +++ b/test/configurationTests.ts @@ -80,14 +80,14 @@ describe("Configuration", () => { expected.rules.set("i", { ruleArguments: undefined, ruleSeverity: "warning" }); expected.rules.set("j", { ruleArguments: undefined, ruleSeverity: "error" }); expected.rules.set("k", { ruleArguments: undefined, ruleSeverity: "off" }); - expected.rules.set("l", { ruleArguments: [1], ruleSeverity: undefined }); - expected.rules.set("m", { ruleArguments: [2], ruleSeverity: undefined }); - expected.rules.set("n", { ruleArguments: [{ no: false }], ruleSeverity: undefined }); + expected.rules.set("l", { ruleArguments: [1], ruleSeverity: "error" }); + expected.rules.set("m", { ruleArguments: [2], ruleSeverity: "error" }); + expected.rules.set("n", { ruleArguments: [{ no: false }], ruleSeverity: "error" }); expected.rules.set("o", { ruleArguments: [1], ruleSeverity: "warning" }); expected.rules.set("p", { ruleArguments: [], ruleSeverity: "off" }); - expected.rules.set("q", { ruleArguments: undefined, ruleSeverity: undefined }); - expected.rules.set("r", { ruleArguments: undefined, ruleSeverity: "off" }); - expected.rules.set("s", { ruleArguments: undefined, ruleSeverity: undefined }); + expected.rules.set("q", { ruleArguments: undefined, ruleSeverity: "error" }); + expected.rules.set("r", { ruleArguments: undefined, ruleSeverity: "error" }); + expected.rules.set("s", { ruleArguments: undefined, ruleSeverity: "error" }); assertConfigEquals(parseConfigFile(rawConfig), expected); }); @@ -103,6 +103,27 @@ describe("Configuration", () => { }); }); + describe("defaultSeverity", () => { + it("uses defaultSeverity if severity is default", () => { + const rawConfig = { + defaultSeverity: "warning", + rules: { + a: { severity: "error" }, + b: { severity: "warning" }, + c: { severity: "off" }, + d: { severity: "default" }, + }, + }; + + const expected = getEmptyConfig(); + expected.rules.set("a", { ruleArguments: undefined, ruleSeverity: "error" }); + expected.rules.set("b", { ruleArguments: undefined, ruleSeverity: "warning" }); + expected.rules.set("c", { ruleArguments: undefined, ruleSeverity: "off" }); + expected.rules.set("d", { ruleArguments: undefined, ruleSeverity: "warning" }); + assertConfigEquals(parseConfigFile(rawConfig), expected); + }); + }); + describe("extendConfigurationFile", () => { const EMPTY_CONFIG = getEmptyConfig(); diff --git a/test/executable/executableTests.ts b/test/executable/executableTests.ts index 3ae71dcac52..c0a27503558 100644 --- a/test/executable/executableTests.ts +++ b/test/executable/executableTests.ts @@ -113,6 +113,15 @@ describe("Executable", function(this: Mocha.ISuiteCallbackContext) { }); }); + it("exits with code 0 if custom rules directory is passed and file contains lint warnings", (done) => { + execCli(["-c", "./test/config/tslint-extends-package-warning.json", "-r", "./test/files/custom-rules", "src/test.ts"], + (err) => { + assert.isNull(err, "process should exit without an error"); + done(); + }, + ); + }); + it("exits with code 2 if custom rules directory is specified in config file and file contains lint errors", (done) => { execCli(["-c", "./test/config/tslint-custom-rules-with-dir.json", "src/test.ts"], (err) => { assert.isNotNull(err, "process should exit with error");