From 1b5c8157718a417e775d078836bb16db1465fd56 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 9 Feb 2020 18:49:33 +1300 Subject: [PATCH 01/17] feat(eslint-plugin): create `class-literals-style` rule --- packages/eslint-plugin/README.md | 1 + .../docs/rules/class-literals-style.md | 96 +++++++ packages/eslint-plugin/src/configs/all.json | 1 + .../src/rules/class-literals-style.ts | 142 ++++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../tests/rules/class-literals-styles.test.ts | 253 ++++++++++++++++++ 6 files changed, 495 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/class-literals-style.md create mode 100644 packages/eslint-plugin/src/rules/class-literals-style.ts create mode 100644 packages/eslint-plugin/tests/rules/class-literals-styles.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index b7927757924..1cf5ed66032 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -99,6 +99,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used | | | | | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/class-literals-style`](./docs/rules/class-literals-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literals-style.md new file mode 100644 index 00000000000..6725d15df22 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/class-literals-style.md @@ -0,0 +1,96 @@ +# Ensures that literals on classes are exposed in a consistent style (`class-literals-style`) + +When writing TypeScript applications, it's typically safe to store literal values on classes using fields with the `readonly` modifier to prevent them from being reassigned. +When writing TypeScript libraries that could be used by Javascript users however, it's typically safer to expose these literals using `getter`s, since the `readonly` modifier is enforced at compile type. + +## Rule Details + +This rule aims to ensure that literals exposed by classes are done so consistently, in one of the two style described above. +Since both styles have their place, this rule by default does nothing - you must provide it with your desired style: either `fields` or `getters`. + +Note that this rule only checks for _literals_ values, and so not objects, arrays, or functions. +This is because these types can be mutated and carry with them more complex implications about their usage. + +#### The `fields` style + +This style checks for any getters methods that return literal values, and requires them to be defined using fields with the `readonly` modifier instead. + +Examples of **correct** code with the `fields` style: + +```ts +/* eslint @typescript-eslint/class-literals-style: ["error", "fields"] */ + +class Mx { + public readonly myField1 = 1; + + // not a literal + public readonly myField2 = [1, 2, 3]; + + private readonly ['myField3'] = 'hello world'; + + public get myField4() { + return `hello from ${window.location.href}`; + } +} +``` + +Examples of **incorrect** code with the `fields` style: + +```ts +/* eslint @typescript-eslint/class-literals-style: ["error", "fields"] */ + +class Mx { + public static get myField1() { + return 1; + } + + private get ['myField2']() { + return 'hello world'; + } +} +``` + +#### The `getters` style + +This style checks for any `readonly` fields that are assigned literal values, and requires them to be defined as getters instead. +This style pairs well with the [`@typescript-eslint/prefer-readonly`](member-delimiter-style.md) rule, +as it will identify fields that can be `readonly`, and thus should be made into getters. + +Examples of **correct** code with the `getters` style: + +```ts +/* eslint @typescript-eslint/class-literals-style: ["error", "getters"] */ + +class Mx { + // no readonly modifier + public myField1 = 'hello'; + + // not a literal + public readonly myField2 = [1, 2, 3]; + + public static get myField3() { + return 1; + } + + private get ['myField4']() { + return 'hello world'; + } +} +``` + +Examples of **incorrect** code with the `getters` style: + +```ts +/* eslint @typescript-eslint/class-literals-style: ["error", "getters"] */ + +class Mx { + readonly myField1 = 1; + readonly myField2 = `hello world`; + private readonly myField3 = 'hello world'; +} +``` + +## When Not To Use It + +When you have no strong preference, or do not wish to enforce a particular style +for how literal values are exposed by your classes. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 2626767b84c..903e1b946ef 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -8,6 +8,7 @@ "@typescript-eslint/ban-types": "error", "brace-style": "off", "@typescript-eslint/brace-style": "error", + "@typescript-eslint/class-literals-style": "error", "comma-spacing": "off", "@typescript-eslint/comma-spacing": "error", "@typescript-eslint/consistent-type-assertions": "error", diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts new file mode 100644 index 00000000000..ce5b09bfadc --- /dev/null +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -0,0 +1,142 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type Options = ['any' | 'fields' | 'getters']; +type MessageIds = 'preferFieldStyle' | 'preferGetterStyle'; + +interface NodeWithModifiers { + accessibility?: TSESTree.Accessibility; + static: boolean; +} + +const printNodeModifiers = ( + node: NodeWithModifiers, + final: 'readonly' | 'get', +): string => + `${node.accessibility ?? ''}${ + node.static ? ' static' : '' + } ${final} `.trimLeft(); + +const isSupportedLiteral = ( + node: TSESTree.Node, +): node is TSESTree.LiteralExpression => + node.type === AST_NODE_TYPES.Literal || + node.type === AST_NODE_TYPES.BigIntLiteral || + (node.type === AST_NODE_TYPES.TemplateLiteral && node.quasis.length === 1); + +export default util.createRule({ + name: 'class-literals-style', + meta: { + type: 'problem', + docs: { + description: + 'Ensures that literals on classes are exposed in a consistent style', + category: 'Best Practices', + recommended: false, + }, + fixable: 'code', + messages: { + preferFieldStyle: 'Literals should be exposed using readonly fields.', + preferGetterStyle: 'Literals should be exposed using getters.', + }, + schema: [{ enum: ['fields', 'getters'] }], + }, + defaultOptions: ['any'], + create(context) { + const [style] = context.options; + + switch (style) { + case 'fields': + return { + MethodDefinition(node: TSESTree.MethodDefinition): void { + if ( + node.kind !== 'get' || + !node.value.body || + !node.value.body.body.length + ) { + return; + } + + const [statement] = node.value.body.body; + + if (statement.type !== AST_NODE_TYPES.ReturnStatement) { + return; + } + + const { argument } = statement; + + if (!argument || !isSupportedLiteral(argument)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferFieldStyle', + fix(fixer) { + const keyOffset = node.computed ? 1 : 0; + + return [ + // replace the start bits with the field modifiers + fixer.replaceTextRange( + [node.range[0], node.key.range[0] - keyOffset], + printNodeModifiers(node, 'readonly'), + ), + // replace the middle bits with the assignment + fixer.replaceTextRange( + [node.value.range[0], argument.range[0]], + '=', + ), + // remove the end bits + fixer.removeRange([argument.range[1], node.range[1]]), + ]; + }, + }); + }, + }; + case 'getters': + return { + ClassProperty(node: TSESTree.ClassProperty): void { + if (!node.readonly) { + return; + } + + const { value } = node; + + if (!value || !isSupportedLiteral(value)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferGetterStyle', + fix(fixer) { + const keyOffset = node.computed ? 1 : 0; + + return [ + // replace the start bits with the getter modifiers + fixer.replaceTextRange( + [node.range[0], node.key.range[0] - keyOffset], + printNodeModifiers(node, 'get'), + ), + // replace the middle bits with the start of the getter + fixer.replaceTextRange( + [node.key.range[1] + keyOffset, value.range[0]], + '(){return ', + ), + // replace the end bits with the end of the getter + fixer.replaceTextRange([value.range[1], node.range[1]], '}'), + ]; + }, + }); + }, + }; + case 'any': + default: + /* istanbul ignore next */ + return {}; + } + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 486b8a97945..0e120d419e4 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -7,6 +7,7 @@ import banTypes from './ban-types'; import braceStyle from './brace-style'; import camelcase from './camelcase'; import classNameCasing from './class-name-casing'; +import classLiteralsStyle from './class-literals-style'; import commaSpacing from './comma-spacing'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; @@ -102,6 +103,7 @@ export default { 'brace-style': braceStyle, camelcase: camelcase, 'class-name-casing': classNameCasing, + 'class-literals-style': classLiteralsStyle, 'comma-spacing': commaSpacing, 'consistent-type-assertions': consistentTypeAssertions, 'consistent-type-definitions': consistentTypeDefinitions, diff --git a/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts b/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts new file mode 100644 index 00000000000..8fc0cd64a59 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts @@ -0,0 +1,253 @@ +import rule from '../../src/rules/class-literals-style'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('class-literals-style', rule, { + valid: [ + { + code: 'class Mx { p1 = "hello world"; }', + options: ['fields'], + }, + { + code: 'class Mx { static p1 = "hello world"; }', + options: ['fields'], + }, + { + code: 'class Mx { readonly p1 = "hello world"; }', + options: ['fields'], + }, + { + code: 'class Mx { p1: string; }', + options: ['fields'], + }, + { + code: 'abstract class Mx { abstract get p1(): string }', + options: ['fields'], + }, + { + code: ` + class Mx { + get mySetting() { + if(this._aValue) { + return 'on'; + } + + return 'off'; + } + } + `, + options: ['fields'], + }, + { + code: ` + class Mx { + get mySetting() { + return \`build-\${process.env.build}\` + } + } + `, + options: ['fields'], + }, + { + code: ` + class Mx { + getMySetting() { + if(this._aValue) { + return 'on'; + } + + return 'off'; + } + } + `, + options: ['fields'], + }, + { + code: 'class Mx { p1 = "hello world"; }', + options: ['getters'], + }, + { + code: 'class Mx { p1: string; }', + options: ['getters'], + }, + { + code: 'class Mx { readonly p1 = [1, 2, 3]; }', + options: ['getters'], + }, + { + code: 'class Mx { static p1: string; }', + options: ['getters'], + }, + { + code: 'class Mx { get p1() { return "hello world"; } }', + options: ['getters'], + }, + { + code: 'class Mx { static get p1() { return "hello world"; } }', + options: ['getters'], + }, + ], + invalid: [ + { + code: 'class Mx { get p1() { return "hello world"; } }', + output: 'class Mx { readonly p1="hello world" }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: 'class Mx { get p1() { return `hello world`; } }', + output: 'class Mx { readonly p1=`hello world` }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: 'class Mx { static get p1() { return "hello world"; } }', + output: 'class Mx { static readonly p1="hello world" }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: ` + class Mx { + public get [myValue]() { + return 'a literal value'; + } + } + `, + output: ` + class Mx { + public readonly [myValue]='a literal value' + } + `, + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: ` + class Mx { + public get [myValue]() { + return 12345n; + } + } + `, + output: ` + class Mx { + public readonly [myValue]=12345n + } + `, + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: ` + class Mx { + public readonly [myValue] = 'a literal value'; + } + `, + output: ` + class Mx { + public get [myValue](){return 'a literal value'} + } + `, + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + { + code: 'class Mx { readonly p1 = "hello world"; }', + output: 'class Mx { get p1(){return "hello world"} }', + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + { + code: 'class Mx { readonly p1 = `hello world`; }', + output: 'class Mx { get p1(){return `hello world`} }', + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + { + code: 'class Mx { static readonly p1 = "hello world"; }', + output: 'class Mx { static get p1(){return "hello world"} }', + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + { + code: 'class Mx { protected get p1() { return "hello world"; } }', + output: 'class Mx { protected readonly p1="hello world" }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: 'class Mx { protected readonly p1 = "hello world"; }', + output: 'class Mx { protected get p1(){return "hello world"} }', + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + { + code: 'class Mx { public static get p1() { return "hello world"; } }', + output: 'class Mx { public static readonly p1="hello world" }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + options: ['fields'], + }, + { + code: 'class Mx { public static readonly p1 = "hello world"; }', + output: 'class Mx { public static get p1(){return "hello world"} }', + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, + ], +}); From 24c4c2e0ecb4723e872fc8c2095ea47537535946 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 13 Feb 2020 07:50:35 +1300 Subject: [PATCH 02/17] docs(eslint-plugin): fix typos for `class-literals-style` --- packages/eslint-plugin/docs/rules/class-literals-style.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literals-style.md index 6725d15df22..180d6529f0c 100644 --- a/packages/eslint-plugin/docs/rules/class-literals-style.md +++ b/packages/eslint-plugin/docs/rules/class-literals-style.md @@ -8,12 +8,12 @@ When writing TypeScript libraries that could be used by Javascript users however This rule aims to ensure that literals exposed by classes are done so consistently, in one of the two style described above. Since both styles have their place, this rule by default does nothing - you must provide it with your desired style: either `fields` or `getters`. -Note that this rule only checks for _literals_ values, and so not objects, arrays, or functions. +Note that this rule only checks for _literal_ values, and so not objects, arrays, or functions. This is because these types can be mutated and carry with them more complex implications about their usage. #### The `fields` style -This style checks for any getters methods that return literal values, and requires them to be defined using fields with the `readonly` modifier instead. +This style checks for any getter methods that return literal values, and requires them to be defined using fields with the `readonly` modifier instead. Examples of **correct** code with the `fields` style: From 2c5f0302eb6209abeb7c1e91b62793622e5d9a77 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 13 Feb 2020 08:12:00 +1300 Subject: [PATCH 03/17] fix(eslint-plugin): use optional tuple for option --- .../src/rules/class-literals-style.ts | 181 +++++++++--------- .../tests/rules/class-literals-styles.test.ts | 8 + 2 files changed, 98 insertions(+), 91 deletions(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index ce5b09bfadc..cccf6d3b5f9 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -4,7 +4,7 @@ import { } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; -type Options = ['any' | 'fields' | 'getters']; +type Options = [('fields' | 'getters')?]; type MessageIds = 'preferFieldStyle' | 'preferGetterStyle'; interface NodeWithModifiers { @@ -44,99 +44,98 @@ export default util.createRule({ }, schema: [{ enum: ['fields', 'getters'] }], }, - defaultOptions: ['any'], + defaultOptions: [], create(context) { const [style] = context.options; - switch (style) { - case 'fields': - return { - MethodDefinition(node: TSESTree.MethodDefinition): void { - if ( - node.kind !== 'get' || - !node.value.body || - !node.value.body.body.length - ) { - return; - } - - const [statement] = node.value.body.body; - - if (statement.type !== AST_NODE_TYPES.ReturnStatement) { - return; - } - - const { argument } = statement; - - if (!argument || !isSupportedLiteral(argument)) { - return; - } - - context.report({ - node: node.key, - messageId: 'preferFieldStyle', - fix(fixer) { - const keyOffset = node.computed ? 1 : 0; - - return [ - // replace the start bits with the field modifiers - fixer.replaceTextRange( - [node.range[0], node.key.range[0] - keyOffset], - printNodeModifiers(node, 'readonly'), - ), - // replace the middle bits with the assignment - fixer.replaceTextRange( - [node.value.range[0], argument.range[0]], - '=', - ), - // remove the end bits - fixer.removeRange([argument.range[1], node.range[1]]), - ]; - }, - }); - }, - }; - case 'getters': - return { - ClassProperty(node: TSESTree.ClassProperty): void { - if (!node.readonly) { - return; - } - - const { value } = node; - - if (!value || !isSupportedLiteral(value)) { - return; - } - - context.report({ - node: node.key, - messageId: 'preferGetterStyle', - fix(fixer) { - const keyOffset = node.computed ? 1 : 0; - - return [ - // replace the start bits with the getter modifiers - fixer.replaceTextRange( - [node.range[0], node.key.range[0] - keyOffset], - printNodeModifiers(node, 'get'), - ), - // replace the middle bits with the start of the getter - fixer.replaceTextRange( - [node.key.range[1] + keyOffset, value.range[0]], - '(){return ', - ), - // replace the end bits with the end of the getter - fixer.replaceTextRange([value.range[1], node.range[1]], '}'), - ]; - }, - }); - }, - }; - case 'any': - default: - /* istanbul ignore next */ - return {}; + if (style === 'fields') { + return { + MethodDefinition(node: TSESTree.MethodDefinition): void { + if ( + node.kind !== 'get' || + !node.value.body || + !node.value.body.body.length + ) { + return; + } + + const [statement] = node.value.body.body; + + if (statement.type !== AST_NODE_TYPES.ReturnStatement) { + return; + } + + const { argument } = statement; + + if (!argument || !isSupportedLiteral(argument)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferFieldStyle', + fix(fixer) { + const keyOffset = node.computed ? 1 : 0; + + return [ + // replace the start bits with the field modifiers + fixer.replaceTextRange( + [node.range[0], node.key.range[0] - keyOffset], + printNodeModifiers(node, 'readonly'), + ), + // replace the middle bits with the assignment + fixer.replaceTextRange( + [node.value.range[0], argument.range[0]], + '=', + ), + // remove the end bits + fixer.removeRange([argument.range[1], node.range[1]]), + ]; + }, + }); + }, + }; } + + if (style === 'getters') { + return { + ClassProperty(node: TSESTree.ClassProperty): void { + if (!node.readonly) { + return; + } + + const { value } = node; + + if (!value || !isSupportedLiteral(value)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferGetterStyle', + fix(fixer) { + const keyOffset = node.computed ? 1 : 0; + + return [ + // replace the start bits with the getter modifiers + fixer.replaceTextRange( + [node.range[0], node.key.range[0] - keyOffset], + printNodeModifiers(node, 'get'), + ), + // replace the middle bits with the start of the getter + fixer.replaceTextRange( + [node.key.range[1] + keyOffset, value.range[0]], + '(){return ', + ), + // replace the end bits with the end of the getter + fixer.replaceTextRange([value.range[1], node.range[1]], '}'), + ]; + }, + }); + }, + }; + } + + return {}; }, }); diff --git a/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts b/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts index 8fc0cd64a59..6f07da9241e 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts @@ -7,6 +7,14 @@ const ruleTester = new RuleTester({ ruleTester.run('class-literals-style', rule, { valid: [ + { + code: 'class Mx { readonly p1 = "hello world"; }', + options: [], + }, + { + code: 'class Mx { get p1() { return "hello world"; } }', + options: [], + }, { code: 'class Mx { p1 = "hello world"; }', options: ['fields'], From 79349d5d57d0dfb83c77104d33d6cbf47479f6d5 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 13 Feb 2020 08:12:52 +1300 Subject: [PATCH 04/17] chore(eslint-plugin): fix test file name --- ...class-literals-styles.test.ts => class-literals-style.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/eslint-plugin/tests/rules/{class-literals-styles.test.ts => class-literals-style.test.ts} (100%) diff --git a/packages/eslint-plugin/tests/rules/class-literals-styles.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts similarity index 100% rename from packages/eslint-plugin/tests/rules/class-literals-styles.test.ts rename to packages/eslint-plugin/tests/rules/class-literals-style.test.ts From f8573bef8979a06b28d259234f1309bef4b35e57 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 13 Feb 2020 08:14:43 +1300 Subject: [PATCH 05/17] docs(eslint-plugin): link to intended rule --- packages/eslint-plugin/docs/rules/class-literals-style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literals-style.md index 180d6529f0c..11b8c33d17c 100644 --- a/packages/eslint-plugin/docs/rules/class-literals-style.md +++ b/packages/eslint-plugin/docs/rules/class-literals-style.md @@ -53,7 +53,7 @@ class Mx { #### The `getters` style This style checks for any `readonly` fields that are assigned literal values, and requires them to be defined as getters instead. -This style pairs well with the [`@typescript-eslint/prefer-readonly`](member-delimiter-style.md) rule, +This style pairs well with the [`@typescript-eslint/prefer-readonly`](prefer-readonly.md) rule, as it will identify fields that can be `readonly`, and thus should be made into getters. Examples of **correct** code with the `getters` style: From 40b0a45d8a407015b9ba714adfecbdc1b18ca943 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 07:50:57 +1300 Subject: [PATCH 06/17] docs(eslint-plugin): add clarity to `class-literals-style` rule docs Co-Authored-By: Brad Zacher --- packages/eslint-plugin/docs/rules/class-literals-style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literals-style.md index 11b8c33d17c..d83ce714caf 100644 --- a/packages/eslint-plugin/docs/rules/class-literals-style.md +++ b/packages/eslint-plugin/docs/rules/class-literals-style.md @@ -8,7 +8,7 @@ When writing TypeScript libraries that could be used by Javascript users however This rule aims to ensure that literals exposed by classes are done so consistently, in one of the two style described above. Since both styles have their place, this rule by default does nothing - you must provide it with your desired style: either `fields` or `getters`. -Note that this rule only checks for _literal_ values, and so not objects, arrays, or functions. +Note that this rule only checks for constant _literal_ values (string, template string, number, bigint, boolean, regexp, null). It does not check objects or arrays, because a readonly field behaves differently to a getter in those cases. It also does not check functions, as it is a common pattern to use readonly fields with arrow function values as auto-bound methods. This is because these types can be mutated and carry with them more complex implications about their usage. #### The `fields` style From 3761809071823c9f4b819ff2961ad781c931c878 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 08:13:07 +1300 Subject: [PATCH 07/17] fix(eslint-plugin): make `fields` default for `class-literals-style` --- .../docs/rules/class-literals-style.md | 2 +- .../src/rules/class-literals-style.ts | 8 +- .../tests/rules/class-literals-style.test.ts | 99 +++++++------------ 3 files changed, 37 insertions(+), 72 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literals-style.md index d83ce714caf..921de8eb135 100644 --- a/packages/eslint-plugin/docs/rules/class-literals-style.md +++ b/packages/eslint-plugin/docs/rules/class-literals-style.md @@ -6,7 +6,7 @@ When writing TypeScript libraries that could be used by Javascript users however ## Rule Details This rule aims to ensure that literals exposed by classes are done so consistently, in one of the two style described above. -Since both styles have their place, this rule by default does nothing - you must provide it with your desired style: either `fields` or `getters`. +By default this rule prefers the `fields` style as it means JS doesn't have to setup & teardown a function closure. Note that this rule only checks for constant _literal_ values (string, template string, number, bigint, boolean, regexp, null). It does not check objects or arrays, because a readonly field behaves differently to a getter in those cases. It also does not check functions, as it is a common pattern to use readonly fields with arrow function values as auto-bound methods. This is because these types can be mutated and carry with them more complex implications about their usage. diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index cccf6d3b5f9..3351498e85d 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -4,7 +4,7 @@ import { } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; -type Options = [('fields' | 'getters')?]; +type Options = ['fields' | 'getters']; type MessageIds = 'preferFieldStyle' | 'preferGetterStyle'; interface NodeWithModifiers { @@ -44,10 +44,8 @@ export default util.createRule({ }, schema: [{ enum: ['fields', 'getters'] }], }, - defaultOptions: [], - create(context) { - const [style] = context.options; - + defaultOptions: ['fields'], + create(context, [style]) { if (style === 'fields') { return { MethodDefinition(node: TSESTree.MethodDefinition): void { diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 6f07da9241e..7f3031e4b82 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -7,71 +7,44 @@ const ruleTester = new RuleTester({ ruleTester.run('class-literals-style', rule, { valid: [ - { - code: 'class Mx { readonly p1 = "hello world"; }', - options: [], - }, - { - code: 'class Mx { get p1() { return "hello world"; } }', - options: [], - }, - { - code: 'class Mx { p1 = "hello world"; }', - options: ['fields'], - }, - { - code: 'class Mx { static p1 = "hello world"; }', - options: ['fields'], - }, - { - code: 'class Mx { readonly p1 = "hello world"; }', - options: ['fields'], - }, - { - code: 'class Mx { p1: string; }', - options: ['fields'], - }, - { - code: 'abstract class Mx { abstract get p1(): string }', - options: ['fields'], - }, - { - code: ` - class Mx { - get mySetting() { - if(this._aValue) { - return 'on'; - } - - return 'off'; + 'class Mx { readonly p1 = "hello world"; }', + 'class Mx { p1 = "hello world"; }', + 'class Mx { static p1 = "hello world"; }', + 'class Mx { readonly p1 = "hello world"; }', + 'class Mx { p1: string; }', + 'abstract class Mx { abstract get p1(): string }', + ` + class Mx { + get mySetting() { + if(this._aValue) { + return 'on'; } + + return 'off'; } - `, - options: ['fields'], - }, - { - code: ` - class Mx { - get mySetting() { - return \`build-\${process.env.build}\` - } + } + `, + ` + class Mx { + get mySetting() { + return \`build-\${process.env.build}\` } - `, - options: ['fields'], - }, - { - code: ` - class Mx { - getMySetting() { - if(this._aValue) { - return 'on'; - } - - return 'off'; + } + `, + ` + class Mx { + getMySetting() { + if(this._aValue) { + return 'on'; } + + return 'off'; } - `, - options: ['fields'], + } + `, + { + code: 'class Mx { get p1() { return "hello world"; } }', + options: ['getters'], }, { code: 'class Mx { p1 = "hello world"; }', @@ -107,7 +80,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: 'class Mx { get p1() { return `hello world`; } }', @@ -117,7 +89,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: 'class Mx { static get p1() { return "hello world"; } }', @@ -127,7 +98,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: ` @@ -147,7 +117,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: ` @@ -167,7 +136,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: ` @@ -245,7 +213,6 @@ ruleTester.run('class-literals-style', rule, { messageId: 'preferFieldStyle', }, ], - options: ['fields'], }, { code: 'class Mx { public static readonly p1 = "hello world"; }', From 85d6b049905fe62af4c0acceb8cfcdaf2549b1d9 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 08:17:11 +1300 Subject: [PATCH 08/17] fix(eslint-plugin): check if class field is `declare`d --- packages/eslint-plugin/src/rules/class-literals-style.ts | 2 +- .../eslint-plugin/tests/rules/class-literals-style.test.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index 3351498e85d..f1cde4922d3 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -98,7 +98,7 @@ export default util.createRule({ if (style === 'getters') { return { ClassProperty(node: TSESTree.ClassProperty): void { - if (!node.readonly) { + if (!node.readonly || node.declare) { return; } diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 7f3031e4b82..4d5062acfda 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -7,6 +7,7 @@ const ruleTester = new RuleTester({ ruleTester.run('class-literals-style', rule, { valid: [ + 'class Mx { declare readonly p1 = 1; }', 'class Mx { readonly p1 = "hello world"; }', 'class Mx { p1 = "hello world"; }', 'class Mx { static p1 = "hello world"; }', @@ -42,6 +43,10 @@ ruleTester.run('class-literals-style', rule, { } } `, + { + code: 'class Mx { declare public readonly foo = 1; }', + options: ['getters'], + }, { code: 'class Mx { get p1() { return "hello world"; } }', options: ['getters'], From 705b85fffeec6613966d3524354f3ccba669ef20 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 08:20:45 +1300 Subject: [PATCH 09/17] test(eslint-plugin): add cases for parasble semantically incorrect code --- packages/eslint-plugin/tests/rules/class-literals-style.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 4d5062acfda..fd53d00ac6b 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -13,6 +13,8 @@ ruleTester.run('class-literals-style', rule, { 'class Mx { static p1 = "hello world"; }', 'class Mx { readonly p1 = "hello world"; }', 'class Mx { p1: string; }', + 'class Mx { get p1(); }', + 'class Mx { get p1() {} }', 'abstract class Mx { abstract get p1(): string }', ` class Mx { From 1a8859f4a05f7ea84593f93848271fcbd81ab3ff Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 08:58:17 +1300 Subject: [PATCH 10/17] test(eslint-plugin): add case for silly absurd parsable code --- .../tests/rules/class-literals-style.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index fd53d00ac6b..7043c9d9908 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -106,6 +106,16 @@ ruleTester.run('class-literals-style', rule, { }, ], }, + { + code: + 'class Mx { public static readonly static private public protected get foo() { return 1; } }', + output: 'class Mx { public static readonly foo=1 }', + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + }, { code: ` class Mx { From 027902a2e0d1a6dbbc5822dbb614d2b8af66b4e5 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 09:02:17 +1300 Subject: [PATCH 11/17] fix(eslint-plugin): add space around equals in fixer --- .../src/rules/class-literals-style.ts | 2 +- .../tests/rules/class-literals-style.test.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index f1cde4922d3..68633e22ae1 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -84,7 +84,7 @@ export default util.createRule({ // replace the middle bits with the assignment fixer.replaceTextRange( [node.value.range[0], argument.range[0]], - '=', + ' = ', ), // remove the end bits fixer.removeRange([argument.range[1], node.range[1]]), diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 7043c9d9908..5b45faae75e 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -81,7 +81,7 @@ ruleTester.run('class-literals-style', rule, { invalid: [ { code: 'class Mx { get p1() { return "hello world"; } }', - output: 'class Mx { readonly p1="hello world" }', + output: 'class Mx { readonly p1 = "hello world" }', errors: [ { messageId: 'preferFieldStyle', @@ -90,7 +90,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { get p1() { return `hello world`; } }', - output: 'class Mx { readonly p1=`hello world` }', + output: 'class Mx { readonly p1 = `hello world` }', errors: [ { messageId: 'preferFieldStyle', @@ -99,7 +99,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { static get p1() { return "hello world"; } }', - output: 'class Mx { static readonly p1="hello world" }', + output: 'class Mx { static readonly p1 = "hello world" }', errors: [ { messageId: 'preferFieldStyle', @@ -109,7 +109,7 @@ ruleTester.run('class-literals-style', rule, { { code: 'class Mx { public static readonly static private public protected get foo() { return 1; } }', - output: 'class Mx { public static readonly foo=1 }', + output: 'class Mx { public static readonly foo = 1 }', errors: [ { messageId: 'preferFieldStyle', @@ -126,7 +126,7 @@ ruleTester.run('class-literals-style', rule, { `, output: ` class Mx { - public readonly [myValue]='a literal value' + public readonly [myValue] = 'a literal value' } `, errors: [ @@ -145,7 +145,7 @@ ruleTester.run('class-literals-style', rule, { `, output: ` class Mx { - public readonly [myValue]=12345n + public readonly [myValue] = 12345n } `, errors: [ @@ -204,7 +204,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { protected get p1() { return "hello world"; } }', - output: 'class Mx { protected readonly p1="hello world" }', + output: 'class Mx { protected readonly p1 = "hello world" }', errors: [ { messageId: 'preferFieldStyle', @@ -224,7 +224,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { public static get p1() { return "hello world"; } }', - output: 'class Mx { public static readonly p1="hello world" }', + output: 'class Mx { public static readonly p1 = "hello world" }', errors: [ { messageId: 'preferFieldStyle', From e0934833a9c87a02fc8fd3e2db7ea1c9006c3977 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 11 Mar 2020 13:45:29 +1300 Subject: [PATCH 12/17] test(eslint-plugin): remove duplicated cases --- .../eslint-plugin/tests/rules/class-literals-style.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 5b45faae75e..196965591bb 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -11,7 +11,6 @@ ruleTester.run('class-literals-style', rule, { 'class Mx { readonly p1 = "hello world"; }', 'class Mx { p1 = "hello world"; }', 'class Mx { static p1 = "hello world"; }', - 'class Mx { readonly p1 = "hello world"; }', 'class Mx { p1: string; }', 'class Mx { get p1(); }', 'class Mx { get p1() {} }', @@ -69,10 +68,6 @@ ruleTester.run('class-literals-style', rule, { code: 'class Mx { static p1: string; }', options: ['getters'], }, - { - code: 'class Mx { get p1() { return "hello world"; } }', - options: ['getters'], - }, { code: 'class Mx { static get p1() { return "hello world"; } }', options: ['getters'], From 11bba6355851203fe1954c63522d64164e19c734 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 07:57:37 +1300 Subject: [PATCH 13/17] fix(eslint-plugin): replace whole node on fixing `class-literal-styles` --- .../src/rules/class-literals-style.ts | 52 +++++++------------ .../tests/rules/class-literals-style.test.ts | 28 +++++----- 2 files changed, 34 insertions(+), 46 deletions(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index 68633e22ae1..b81f0b7c15a 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -73,22 +73,16 @@ export default util.createRule({ node: node.key, messageId: 'preferFieldStyle', fix(fixer) { - const keyOffset = node.computed ? 1 : 0; - - return [ - // replace the start bits with the field modifiers - fixer.replaceTextRange( - [node.range[0], node.key.range[0] - keyOffset], - printNodeModifiers(node, 'readonly'), - ), - // replace the middle bits with the assignment - fixer.replaceTextRange( - [node.value.range[0], argument.range[0]], - ' = ', - ), - // remove the end bits - fixer.removeRange([argument.range[1], node.range[1]]), - ]; + const sourceCode = context.getSourceCode(); + const name = sourceCode.getText(node.key); + + let text = ''; + + text += printNodeModifiers(node, 'readonly'); + text += node.computed ? `[${name}]` : name; + text += ` = ${sourceCode.getText(argument)};`; + + return fixer.replaceText(node, text); }, }); }, @@ -112,22 +106,16 @@ export default util.createRule({ node: node.key, messageId: 'preferGetterStyle', fix(fixer) { - const keyOffset = node.computed ? 1 : 0; - - return [ - // replace the start bits with the getter modifiers - fixer.replaceTextRange( - [node.range[0], node.key.range[0] - keyOffset], - printNodeModifiers(node, 'get'), - ), - // replace the middle bits with the start of the getter - fixer.replaceTextRange( - [node.key.range[1] + keyOffset, value.range[0]], - '(){return ', - ), - // replace the end bits with the end of the getter - fixer.replaceTextRange([value.range[1], node.range[1]], '}'), - ]; + const sourceCode = context.getSourceCode(); + const name = sourceCode.getText(node.key); + + let text = ''; + + text += printNodeModifiers(node, 'get'); + text += node.computed ? `[${name}]` : name; + text += `() { return ${sourceCode.getText(value)}; }`; + + return fixer.replaceText(node, text); }, }); }, diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 196965591bb..4df6d389529 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -76,7 +76,7 @@ ruleTester.run('class-literals-style', rule, { invalid: [ { code: 'class Mx { get p1() { return "hello world"; } }', - output: 'class Mx { readonly p1 = "hello world" }', + output: 'class Mx { readonly p1 = "hello world"; }', errors: [ { messageId: 'preferFieldStyle', @@ -85,7 +85,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { get p1() { return `hello world`; } }', - output: 'class Mx { readonly p1 = `hello world` }', + output: 'class Mx { readonly p1 = `hello world`; }', errors: [ { messageId: 'preferFieldStyle', @@ -94,7 +94,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { static get p1() { return "hello world"; } }', - output: 'class Mx { static readonly p1 = "hello world" }', + output: 'class Mx { static readonly p1 = "hello world"; }', errors: [ { messageId: 'preferFieldStyle', @@ -104,7 +104,7 @@ ruleTester.run('class-literals-style', rule, { { code: 'class Mx { public static readonly static private public protected get foo() { return 1; } }', - output: 'class Mx { public static readonly foo = 1 }', + output: 'class Mx { public static readonly foo = 1; }', errors: [ { messageId: 'preferFieldStyle', @@ -121,7 +121,7 @@ ruleTester.run('class-literals-style', rule, { `, output: ` class Mx { - public readonly [myValue] = 'a literal value' + public readonly [myValue] = 'a literal value'; } `, errors: [ @@ -140,7 +140,7 @@ ruleTester.run('class-literals-style', rule, { `, output: ` class Mx { - public readonly [myValue] = 12345n + public readonly [myValue] = 12345n; } `, errors: [ @@ -157,7 +157,7 @@ ruleTester.run('class-literals-style', rule, { `, output: ` class Mx { - public get [myValue](){return 'a literal value'} + public get [myValue]() { return 'a literal value'; } } `, errors: [ @@ -169,7 +169,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { readonly p1 = "hello world"; }', - output: 'class Mx { get p1(){return "hello world"} }', + output: 'class Mx { get p1() { return "hello world"; } }', errors: [ { messageId: 'preferGetterStyle', @@ -179,7 +179,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { readonly p1 = `hello world`; }', - output: 'class Mx { get p1(){return `hello world`} }', + output: 'class Mx { get p1() { return `hello world`; } }', errors: [ { messageId: 'preferGetterStyle', @@ -189,7 +189,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { static readonly p1 = "hello world"; }', - output: 'class Mx { static get p1(){return "hello world"} }', + output: 'class Mx { static get p1() { return "hello world"; } }', errors: [ { messageId: 'preferGetterStyle', @@ -199,7 +199,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { protected get p1() { return "hello world"; } }', - output: 'class Mx { protected readonly p1 = "hello world" }', + output: 'class Mx { protected readonly p1 = "hello world"; }', errors: [ { messageId: 'preferFieldStyle', @@ -209,7 +209,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { protected readonly p1 = "hello world"; }', - output: 'class Mx { protected get p1(){return "hello world"} }', + output: 'class Mx { protected get p1() { return "hello world"; } }', errors: [ { messageId: 'preferGetterStyle', @@ -219,7 +219,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { public static get p1() { return "hello world"; } }', - output: 'class Mx { public static readonly p1 = "hello world" }', + output: 'class Mx { public static readonly p1 = "hello world"; }', errors: [ { messageId: 'preferFieldStyle', @@ -228,7 +228,7 @@ ruleTester.run('class-literals-style', rule, { }, { code: 'class Mx { public static readonly p1 = "hello world"; }', - output: 'class Mx { public static get p1(){return "hello world"} }', + output: 'class Mx { public static get p1() { return "hello world"; } }', errors: [ { messageId: 'preferGetterStyle', From d7257d6d7bb59cba32479897d725e7d829ec9dd7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 08:37:41 +1300 Subject: [PATCH 14/17] feat(eslint-plugin): support `TaggedTemplateExpression` --- .../src/rules/class-literals-style.ts | 21 +++- .../tests/rules/class-literals-style.test.ts | 106 ++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index b81f0b7c15a..89fe1e2e658 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -22,10 +22,23 @@ const printNodeModifiers = ( const isSupportedLiteral = ( node: TSESTree.Node, -): node is TSESTree.LiteralExpression => - node.type === AST_NODE_TYPES.Literal || - node.type === AST_NODE_TYPES.BigIntLiteral || - (node.type === AST_NODE_TYPES.TemplateLiteral && node.quasis.length === 1); +): node is TSESTree.LiteralExpression => { + if ( + node.type === AST_NODE_TYPES.Literal || + node.type === AST_NODE_TYPES.BigIntLiteral + ) { + return true; + } + + if ( + node.type === AST_NODE_TYPES.TaggedTemplateExpression || + node.type === AST_NODE_TYPES.TemplateLiteral + ) { + return ('quasi' in node ? node.quasi.quasis : node.quasis).length === 1; + } + + return false; +}; export default util.createRule({ name: 'class-literals-style', diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 4df6d389529..9f10438d8e7 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -44,6 +44,25 @@ ruleTester.run('class-literals-style', rule, { } } `, + ` + class Mx { + public readonly myButton = styled.button\` + color: \${props => (props.primary ? 'hotpink' : 'turquoise')}; + \`; + } + `, + { + code: ` + class Mx { + public get myButton() { + return styled.button\` + color: \${props => (props.primary ? 'hotpink' : 'turquoise')}; + \`; + } + } + `, + options: ['fields'], + }, { code: 'class Mx { declare public readonly foo = 1; }', options: ['getters'], @@ -72,6 +91,28 @@ ruleTester.run('class-literals-style', rule, { code: 'class Mx { static get p1() { return "hello world"; } }', options: ['getters'], }, + { + code: ` + class Mx { + public readonly myButton = styled.button\` + color: \${props => (props.primary ? 'hotpink' : 'turquoise')}; + \`; + } + `, + options: ['getters'], + }, + { + code: ` + class Mx { + public get myButton() { + return styled.button\` + color: \${props => (props.primary ? 'hotpink' : 'turquoise')}; + \`; + } + } + `, + options: ['getters'], + }, ], invalid: [ { @@ -236,5 +277,70 @@ ruleTester.run('class-literals-style', rule, { ], options: ['getters'], }, + { + code: ` + class Mx { + public get myValue() { + return gql\` + { + user(id: 5) { + firstName + lastName + } + } + \`; + } + } + `, + output: ` + class Mx { + public readonly myValue = gql\` + { + user(id: 5) { + firstName + lastName + } + } + \`; + } + `, + errors: [ + { + messageId: 'preferFieldStyle', + }, + ], + }, + { + code: ` + class Mx { + public readonly myValue = gql\` + { + user(id: 5) { + firstName + lastName + } + } + \`; + } + `, + output: ` + class Mx { + public get myValue() { return gql\` + { + user(id: 5) { + firstName + lastName + } + } + \`; } + } + `, + errors: [ + { + messageId: 'preferGetterStyle', + }, + ], + options: ['getters'], + }, ], }); From 30e0c02ffc9545a4ecaa089b6a53df4a5bfba4be Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 08:42:08 +1300 Subject: [PATCH 15/17] test(eslint-plugin): add `line` & `column` expectations --- .../tests/rules/class-literals-style.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts index 9f10438d8e7..f4c47bf4f43 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literals-style.test.ts @@ -121,6 +121,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 16, + line: 1, }, ], }, @@ -130,6 +132,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 16, + line: 1, }, ], }, @@ -139,6 +143,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 23, + line: 1, }, ], }, @@ -149,6 +155,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 71, + line: 1, }, ], }, @@ -168,6 +176,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 23, + line: 3, }, ], }, @@ -187,6 +197,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 23, + line: 3, }, ], }, @@ -204,6 +216,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 28, + line: 3, }, ], options: ['getters'], @@ -214,6 +228,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 21, + line: 1, }, ], options: ['getters'], @@ -224,6 +240,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 21, + line: 1, }, ], options: ['getters'], @@ -234,6 +252,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 28, + line: 1, }, ], options: ['getters'], @@ -244,6 +264,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 26, + line: 1, }, ], options: ['fields'], @@ -254,6 +276,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 31, + line: 1, }, ], options: ['getters'], @@ -264,6 +288,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 30, + line: 1, }, ], }, @@ -273,6 +299,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 35, + line: 1, }, ], options: ['getters'], @@ -307,6 +335,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferFieldStyle', + column: 22, + line: 3, }, ], }, @@ -338,6 +368,8 @@ ruleTester.run('class-literals-style', rule, { errors: [ { messageId: 'preferGetterStyle', + column: 27, + line: 3, }, ], options: ['getters'], From 41b637be972f5ed26680899c13291d319d889fd1 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Mar 2020 08:45:49 +1300 Subject: [PATCH 16/17] fix(eslint-plugin): remove unneeded conditional check --- .../src/rules/class-literals-style.ts | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literals-style.ts index 89fe1e2e658..4b506e36443 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literals-style.ts @@ -102,39 +102,35 @@ export default util.createRule({ }; } - if (style === 'getters') { - return { - ClassProperty(node: TSESTree.ClassProperty): void { - if (!node.readonly || node.declare) { - return; - } - - const { value } = node; - - if (!value || !isSupportedLiteral(value)) { - return; - } - - context.report({ - node: node.key, - messageId: 'preferGetterStyle', - fix(fixer) { - const sourceCode = context.getSourceCode(); - const name = sourceCode.getText(node.key); - - let text = ''; - - text += printNodeModifiers(node, 'get'); - text += node.computed ? `[${name}]` : name; - text += `() { return ${sourceCode.getText(value)}; }`; - - return fixer.replaceText(node, text); - }, - }); - }, - }; - } - - return {}; + return { + ClassProperty(node: TSESTree.ClassProperty): void { + if (!node.readonly || node.declare) { + return; + } + + const { value } = node; + + if (!value || !isSupportedLiteral(value)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferGetterStyle', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const name = sourceCode.getText(node.key); + + let text = ''; + + text += printNodeModifiers(node, 'get'); + text += node.computed ? `[${name}]` : name; + text += `() { return ${sourceCode.getText(value)}; }`; + + return fixer.replaceText(node, text); + }, + }); + }, + }; }, }); From 9647f56e1bd82933d9fd33ddef4061ad6e62dff2 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 23 Mar 2020 17:37:38 +1300 Subject: [PATCH 17/17] chore(eslint-plugin): rename rule to `class-literal-property-style` --- packages/eslint-plugin/README.md | 2 +- ...terals-style.md => class-literal-property-style.md} | 10 +++++----- packages/eslint-plugin/src/configs/all.json | 2 +- ...terals-style.ts => class-literal-property-style.ts} | 2 +- packages/eslint-plugin/src/rules/index.ts | 4 ++-- ...le.test.ts => class-literal-property-style.test.ts} | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) rename packages/eslint-plugin/docs/rules/{class-literals-style.md => class-literal-property-style.md} (88%) rename packages/eslint-plugin/src/rules/{class-literals-style.ts => class-literal-property-style.ts} (98%) rename packages/eslint-plugin/tests/rules/{class-literals-style.test.ts => class-literal-property-style.test.ts} (98%) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 1cf5ed66032..fa049b2cfb2 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -99,7 +99,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used | | | | | [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/class-literals-style`](./docs/rules/class-literals-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | | +| [`@typescript-eslint/class-literal-property-style`](./docs/rules/class-literal-property-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/class-literals-style.md b/packages/eslint-plugin/docs/rules/class-literal-property-style.md similarity index 88% rename from packages/eslint-plugin/docs/rules/class-literals-style.md rename to packages/eslint-plugin/docs/rules/class-literal-property-style.md index 921de8eb135..903a695dd76 100644 --- a/packages/eslint-plugin/docs/rules/class-literals-style.md +++ b/packages/eslint-plugin/docs/rules/class-literal-property-style.md @@ -1,4 +1,4 @@ -# Ensures that literals on classes are exposed in a consistent style (`class-literals-style`) +# Ensures that literals on classes are exposed in a consistent style (`class-literal-property-style`) When writing TypeScript applications, it's typically safe to store literal values on classes using fields with the `readonly` modifier to prevent them from being reassigned. When writing TypeScript libraries that could be used by Javascript users however, it's typically safer to expose these literals using `getter`s, since the `readonly` modifier is enforced at compile type. @@ -18,7 +18,7 @@ This style checks for any getter methods that return literal values, and require Examples of **correct** code with the `fields` style: ```ts -/* eslint @typescript-eslint/class-literals-style: ["error", "fields"] */ +/* eslint @typescript-eslint/class-literal-property-style: ["error", "fields"] */ class Mx { public readonly myField1 = 1; @@ -37,7 +37,7 @@ class Mx { Examples of **incorrect** code with the `fields` style: ```ts -/* eslint @typescript-eslint/class-literals-style: ["error", "fields"] */ +/* eslint @typescript-eslint/class-literal-property-style: ["error", "fields"] */ class Mx { public static get myField1() { @@ -59,7 +59,7 @@ as it will identify fields that can be `readonly`, and thus should be made into Examples of **correct** code with the `getters` style: ```ts -/* eslint @typescript-eslint/class-literals-style: ["error", "getters"] */ +/* eslint @typescript-eslint/class-literal-property-style: ["error", "getters"] */ class Mx { // no readonly modifier @@ -81,7 +81,7 @@ class Mx { Examples of **incorrect** code with the `getters` style: ```ts -/* eslint @typescript-eslint/class-literals-style: ["error", "getters"] */ +/* eslint @typescript-eslint/class-literal-property-style: ["error", "getters"] */ class Mx { readonly myField1 = 1; diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 903e1b946ef..c5575d4970d 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -8,7 +8,7 @@ "@typescript-eslint/ban-types": "error", "brace-style": "off", "@typescript-eslint/brace-style": "error", - "@typescript-eslint/class-literals-style": "error", + "@typescript-eslint/class-literal-property-style": "error", "comma-spacing": "off", "@typescript-eslint/comma-spacing": "error", "@typescript-eslint/consistent-type-assertions": "error", diff --git a/packages/eslint-plugin/src/rules/class-literals-style.ts b/packages/eslint-plugin/src/rules/class-literal-property-style.ts similarity index 98% rename from packages/eslint-plugin/src/rules/class-literals-style.ts rename to packages/eslint-plugin/src/rules/class-literal-property-style.ts index 4b506e36443..a4500cdfdbf 100644 --- a/packages/eslint-plugin/src/rules/class-literals-style.ts +++ b/packages/eslint-plugin/src/rules/class-literal-property-style.ts @@ -41,7 +41,7 @@ const isSupportedLiteral = ( }; export default util.createRule({ - name: 'class-literals-style', + name: 'class-literal-property-style', meta: { type: 'problem', docs: { diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 0e120d419e4..29db58b681a 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -7,7 +7,7 @@ import banTypes from './ban-types'; import braceStyle from './brace-style'; import camelcase from './camelcase'; import classNameCasing from './class-name-casing'; -import classLiteralsStyle from './class-literals-style'; +import classLiteralPropertyStyle from './class-literal-property-style'; import commaSpacing from './comma-spacing'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; @@ -103,7 +103,7 @@ export default { 'brace-style': braceStyle, camelcase: camelcase, 'class-name-casing': classNameCasing, - 'class-literals-style': classLiteralsStyle, + 'class-literal-property-style': classLiteralPropertyStyle, 'comma-spacing': commaSpacing, 'consistent-type-assertions': consistentTypeAssertions, 'consistent-type-definitions': consistentTypeDefinitions, diff --git a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts b/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts similarity index 98% rename from packages/eslint-plugin/tests/rules/class-literals-style.test.ts rename to packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts index f4c47bf4f43..f0c5f2afc05 100644 --- a/packages/eslint-plugin/tests/rules/class-literals-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts @@ -1,11 +1,11 @@ -import rule from '../../src/rules/class-literals-style'; +import rule from '../../src/rules/class-literal-property-style'; import { RuleTester } from '../RuleTester'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', }); -ruleTester.run('class-literals-style', rule, { +ruleTester.run('class-literal-property-style', rule, { valid: [ 'class Mx { declare readonly p1 = 1; }', 'class Mx { readonly p1 = "hello world"; }',