Skip to content

Commit b2dbd89

Browse files
G-RathG-Rath
authored
feat(eslint-plugin): add class-literal-property-style rule (typescript-eslint#1582)
1 parent 3eb5d45 commit b2dbd89

File tree

6 files changed

+614
-0
lines changed

6 files changed

+614
-0
lines changed

‎packages/eslint-plugin/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
9999
| [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: |
100100
| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-<directive>` comments from being used | | | |
101101
| [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | |
102+
| [`@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: | |
102103
| [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | |
103104
| [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | |
104105
| [`@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: | | |

‎packages/eslint-plugin/docs/rules/class-literal-property-style.md

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Ensures that literals on classes are exposed in a consistent style (`class-literal-property-style`)
2+
3+
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.
4+
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.
5+
6+
## Rule Details
7+
8+
This rule aims to ensure that literals exposed by classes are done so consistently, in one of the two style described above.
9+
By default this rule prefers the `fields` style as it means JS doesn't have to setup & teardown a function closure.
10+
11+
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.
12+
This is because these types can be mutated and carry with them more complex implications about their usage.
13+
14+
#### The `fields` style
15+
16+
This style checks for any getter methods that return literal values, and requires them to be defined using fields with the `readonly` modifier instead.
17+
18+
Examples of **correct** code with the `fields` style:
19+
20+
```ts
21+
/* eslint @typescript-eslint/class-literal-property-style: ["error", "fields"] */
22+
23+
class Mx {
24+
public readonly myField1 = 1;
25+
26+
// not a literal
27+
public readonly myField2 = [1, 2, 3];
28+
29+
private readonly ['myField3'] = 'hello world';
30+
31+
public get myField4() {
32+
return `hello from ${window.location.href}`;
33+
}
34+
}
35+
```
36+
37+
Examples of **incorrect** code with the `fields` style:
38+
39+
```ts
40+
/* eslint @typescript-eslint/class-literal-property-style: ["error", "fields"] */
41+
42+
class Mx {
43+
public static get myField1() {
44+
return 1;
45+
}
46+
47+
private get ['myField2']() {
48+
return 'hello world';
49+
}
50+
}
51+
```
52+
53+
#### The `getters` style
54+
55+
This style checks for any `readonly` fields that are assigned literal values, and requires them to be defined as getters instead.
56+
This style pairs well with the [`@typescript-eslint/prefer-readonly`](prefer-readonly.md) rule,
57+
as it will identify fields that can be `readonly`, and thus should be made into getters.
58+
59+
Examples of **correct** code with the `getters` style:
60+
61+
```ts
62+
/* eslint @typescript-eslint/class-literal-property-style: ["error", "getters"] */
63+
64+
class Mx {
65+
// no readonly modifier
66+
public myField1 = 'hello';
67+
68+
// not a literal
69+
public readonly myField2 = [1, 2, 3];
70+
71+
public static get myField3() {
72+
return 1;
73+
}
74+
75+
private get ['myField4']() {
76+
return 'hello world';
77+
}
78+
}
79+
```
80+
81+
Examples of **incorrect** code with the `getters` style:
82+
83+
```ts
84+
/* eslint @typescript-eslint/class-literal-property-style: ["error", "getters"] */
85+
86+
class Mx {
87+
readonly myField1 = 1;
88+
readonly myField2 = `hello world`;
89+
private readonly myField3 = 'hello world';
90+
}
91+
```
92+
93+
## When Not To Use It
94+
95+
When you have no strong preference, or do not wish to enforce a particular style
96+
for how literal values are exposed by your classes.

‎packages/eslint-plugin/src/configs/all.json

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"@typescript-eslint/ban-types": "error",
99
"brace-style": "off",
1010
"@typescript-eslint/brace-style": "error",
11+
"@typescript-eslint/class-literal-property-style": "error",
1112
"comma-spacing": "off",
1213
"@typescript-eslint/comma-spacing": "error",
1314
"@typescript-eslint/consistent-type-assertions": "error",

‎packages/eslint-plugin/src/rules/class-literal-property-style.ts

+136
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import {
2+
AST_NODE_TYPES,
3+
TSESTree,
4+
} from '@typescript-eslint/experimental-utils';
5+
import * as util from '../util';
6+
7+
type Options = ['fields' | 'getters'];
8+
type MessageIds = 'preferFieldStyle' | 'preferGetterStyle';
9+
10+
interface NodeWithModifiers {
11+
accessibility?: TSESTree.Accessibility;
12+
static: boolean;
13+
}
14+
15+
const printNodeModifiers = (
16+
node: NodeWithModifiers,
17+
final: 'readonly' | 'get',
18+
): string =>
19+
`${node.accessibility ?? ''}${
20+
node.static ? ' static' : ''
21+
} ${final} `.trimLeft();
22+
23+
const isSupportedLiteral = (
24+
node: TSESTree.Node,
25+
): node is TSESTree.LiteralExpression => {
26+
if (
27+
node.type === AST_NODE_TYPES.Literal ||
28+
node.type === AST_NODE_TYPES.BigIntLiteral
29+
) {
30+
return true;
31+
}
32+
33+
if (
34+
node.type === AST_NODE_TYPES.TaggedTemplateExpression ||
35+
node.type === AST_NODE_TYPES.TemplateLiteral
36+
) {
37+
return ('quasi' in node ? node.quasi.quasis : node.quasis).length === 1;
38+
}
39+
40+
return false;
41+
};
42+
43+
export default util.createRule<Options, MessageIds>({
44+
name: 'class-literal-property-style',
45+
meta: {
46+
type: 'problem',
47+
docs: {
48+
description:
49+
'Ensures that literals on classes are exposed in a consistent style',
50+
category: 'Best Practices',
51+
recommended: false,
52+
},
53+
fixable: 'code',
54+
messages: {
55+
preferFieldStyle: 'Literals should be exposed using readonly fields.',
56+
preferGetterStyle: 'Literals should be exposed using getters.',
57+
},
58+
schema: [{ enum: ['fields', 'getters'] }],
59+
},
60+
defaultOptions: ['fields'],
61+
create(context, [style]) {
62+
if (style === 'fields') {
63+
return {
64+
MethodDefinition(node: TSESTree.MethodDefinition): void {
65+
if (
66+
node.kind !== 'get' ||
67+
!node.value.body ||
68+
!node.value.body.body.length
69+
) {
70+
return;
71+
}
72+
73+
const [statement] = node.value.body.body;
74+
75+
if (statement.type !== AST_NODE_TYPES.ReturnStatement) {
76+
return;
77+
}
78+
79+
const { argument } = statement;
80+
81+
if (!argument || !isSupportedLiteral(argument)) {
82+
return;
83+
}
84+
85+
context.report({
86+
node: node.key,
87+
messageId: 'preferFieldStyle',
88+
fix(fixer) {
89+
const sourceCode = context.getSourceCode();
90+
const name = sourceCode.getText(node.key);
91+
92+
let text = '';
93+
94+
text += printNodeModifiers(node, 'readonly');
95+
text += node.computed ? `[${name}]` : name;
96+
text += ` = ${sourceCode.getText(argument)};`;
97+
98+
return fixer.replaceText(node, text);
99+
},
100+
});
101+
},
102+
};
103+
}
104+
105+
return {
106+
ClassProperty(node: TSESTree.ClassProperty): void {
107+
if (!node.readonly || node.declare) {
108+
return;
109+
}
110+
111+
const { value } = node;
112+
113+
if (!value || !isSupportedLiteral(value)) {
114+
return;
115+
}
116+
117+
context.report({
118+
node: node.key,
119+
messageId: 'preferGetterStyle',
120+
fix(fixer) {
121+
const sourceCode = context.getSourceCode();
122+
const name = sourceCode.getText(node.key);
123+
124+
let text = '';
125+
126+
text += printNodeModifiers(node, 'get');
127+
text += node.computed ? `[${name}]` : name;
128+
text += `() { return ${sourceCode.getText(value)}; }`;
129+
130+
return fixer.replaceText(node, text);
131+
},
132+
});
133+
},
134+
};
135+
},
136+
});

‎packages/eslint-plugin/src/rules/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import banTypes from './ban-types';
77
import braceStyle from './brace-style';
88
import camelcase from './camelcase';
99
import classNameCasing from './class-name-casing';
10+
import classLiteralPropertyStyle from './class-literal-property-style';
1011
import commaSpacing from './comma-spacing';
1112
import consistentTypeAssertions from './consistent-type-assertions';
1213
import consistentTypeDefinitions from './consistent-type-definitions';
@@ -102,6 +103,7 @@ export default {
102103
'brace-style': braceStyle,
103104
camelcase: camelcase,
104105
'class-name-casing': classNameCasing,
106+
'class-literal-property-style': classLiteralPropertyStyle,
105107
'comma-spacing': commaSpacing,
106108
'consistent-type-assertions': consistentTypeAssertions,
107109
'consistent-type-definitions': consistentTypeDefinitions,

0 commit comments

Comments
 (0)