From 19e9056ea34ff622528fd0ab94722fe06ef2d1c6 Mon Sep 17 00:00:00 2001 From: threehams Date: Tue, 24 Dec 2019 10:16:09 -0800 Subject: [PATCH 1/2] fix(eslint-plugin): replace default options on ban-types --- packages/eslint-plugin/src/rules/ban-types.ts | 60 ++++++++++--------- .../tests/rules/ban-types.test.ts | 24 ++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-types.ts b/packages/eslint-plugin/src/rules/ban-types.ts index d15d822de06..c1fdca8b85b 100644 --- a/packages/eslint-plugin/src/rules/ban-types.ts +++ b/packages/eslint-plugin/src/rules/ban-types.ts @@ -13,7 +13,7 @@ type Types = Record< type Options = [ { - types: Types; + types?: Types; }, ]; type MessageIds = 'bannedTypeMessage'; @@ -47,6 +47,35 @@ function getCustomMessage( return ''; } +/* + Defaults for this rule should be treated as an "all or nothing" + merge, so we need special handling here. + + See: https://github.com/typescript-eslint/typescript-eslint/issues/686 + */ +const defaultTypes = { + String: { + message: 'Use string instead', + fixWith: 'string', + }, + Boolean: { + message: 'Use boolean instead', + fixWith: 'boolean', + }, + Number: { + message: 'Use number instead', + fixWith: 'number', + }, + Object: { + message: 'Use Record instead', + fixWith: 'Record', + }, + Symbol: { + message: 'Use symbol instead', + fixWith: 'symbol', + }, +}; + export default util.createRule({ name: 'ban-types', meta: { @@ -86,33 +115,8 @@ export default util.createRule({ }, ], }, - defaultOptions: [ - { - types: { - String: { - message: 'Use string instead', - fixWith: 'string', - }, - Boolean: { - message: 'Use boolean instead', - fixWith: 'boolean', - }, - Number: { - message: 'Use number instead', - fixWith: 'number', - }, - Object: { - message: 'Use Record instead', - fixWith: 'Record', - }, - Symbol: { - message: 'Use symbol instead', - fixWith: 'symbol', - }, - }, - }, - ], - create(context, [{ types }]) { + defaultOptions: [{}], + create(context, [{ types = defaultTypes }]) { const bannedTypes: Types = Object.keys(types).reduce( (res, type) => ({ ...res, [removeSpaces(type)]: types[type] }), {}, diff --git a/packages/eslint-plugin/tests/rules/ban-types.test.ts b/packages/eslint-plugin/tests/rules/ban-types.test.ts index 416c71a1e7a..0fb25fcc780 100644 --- a/packages/eslint-plugin/tests/rules/ban-types.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-types.test.ts @@ -53,8 +53,32 @@ ruleTester.run('ban-types', rule, { code: 'let a: NS.Bad._', options, }, + // Replace default options instead of merging + { + code: 'let a: String;', + options: [ + { + types: { + Number: { + message: 'Use number instead.', + fixWith: 'number', + }, + }, + }, + ], + }, ], invalid: [ + { + code: 'let a: String;', + errors: [ + { + messageId: 'bannedTypeMessage', + line: 1, + column: 8, + }, + ], + }, { code: 'let a: Object;', errors: [ From 330568863476503f8667c5a00ad8a622a442cdf8 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Sun, 23 Feb 2020 08:57:38 -0800 Subject: [PATCH 2/2] fix(eslint-plugin): change to extendDefaults --- packages/eslint-plugin/docs/rules/ban-types.md | 17 +++++++++++++++++ packages/eslint-plugin/src/rules/ban-types.ts | 12 +++++++++++- .../eslint-plugin/tests/rules/ban-types.test.ts | 3 ++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/ban-types.md b/packages/eslint-plugin/docs/rules/ban-types.md index d4764328448..e92e57b521f 100644 --- a/packages/eslint-plugin/docs/rules/ban-types.md +++ b/packages/eslint-plugin/docs/rules/ban-types.md @@ -58,6 +58,23 @@ The banned type can either be a type name literal (`Foo`), a type name with gene } ``` +By default, this rule includes types which are likely to be mistakes, such as `String` and `Number`. If you don't want these enabled, set the `extendDefaults` option to `false`: + +```CJSON +{ + "@typescript-eslint/ban-types": ["error", { + "types": { + // add a custom message, AND tell the plugin how to fix it + "String": { + "message": "Use string instead", + "fixWith": "string" + } + }, + "extendDefaults": false + }] +} +``` + ### Example ```json diff --git a/packages/eslint-plugin/src/rules/ban-types.ts b/packages/eslint-plugin/src/rules/ban-types.ts index ca240831336..7e056959a3f 100644 --- a/packages/eslint-plugin/src/rules/ban-types.ts +++ b/packages/eslint-plugin/src/rules/ban-types.ts @@ -14,6 +14,7 @@ type Types = Record< type Options = [ { types?: Types; + extendDefaults?: boolean; }, ]; type MessageIds = 'bannedTypeMessage'; @@ -114,13 +115,22 @@ export default util.createRule({ ], }, }, + extendDefaults: { + type: 'boolean', + }, }, additionalProperties: false, }, ], }, defaultOptions: [{}], - create(context, [{ types = defaultTypes }]) { + create(context, [options]) { + const extendDefaults = options.extendDefaults ?? true; + const customTypes = options.types ?? {}; + const types: Types = { + ...(extendDefaults ? defaultTypes : {}), + ...customTypes, + }; const bannedTypes = new Map( Object.entries(types).map(([type, data]) => [removeSpaces(type), data]), ); diff --git a/packages/eslint-plugin/tests/rules/ban-types.test.ts b/packages/eslint-plugin/tests/rules/ban-types.test.ts index 5dc9f4bc4b1..0edeb74f8d1 100644 --- a/packages/eslint-plugin/tests/rules/ban-types.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-types.test.ts @@ -72,7 +72,7 @@ ruleTester.run('ban-types', rule, { code: 'let a: NS.Bad._', options, }, - // Replace default options instead of merging + // Replace default options instead of merging with extendDefaults: false { code: 'let a: String;', options: [ @@ -83,6 +83,7 @@ ruleTester.run('ban-types', rule, { fixWith: 'number', }, }, + extendDefaults: false, }, ], },