8000 feat(eslint-plugin): [prefer-readonly-parameter-types] add option tre… · mvximvs/typescript-eslint@a46e318 · GitHub
[go: up one dir, main page]
Skip to content

Commit a46e318

Browse files
RebeccaStevensRebeccaStevens
authored
feat(eslint-plugin): [prefer-readonly-parameter-types] add option treatMethodsAsReadonly (typescript-eslint#3733)
1 parent 363b3dc commit a46e318

File tree

4 files changed

+196
-11
lines changed

4 files changed

+196
-11
lines changed

‎packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ interface Options {
130130
const defaultOptions: Options = {
131131
checkParameterProperties: true,
132132
ignoreInferredTypes: false,
133+
treatMethodsAsReadonly: false,
133134
};
134135
```
135136

@@ -214,3 +215,43 @@ export const acceptsCallback: AcceptsCallback;
214215
```
215216

216217
</details>
218+
219+
### `treatMethodsAsReadonly`
220+
221+
This option allows you to treat all mutable methods as though they were readonly. This may be desirable in when you are never reassigning methods.
222+
223+
Examples of **incorrect** code for this rule with `{treatMethodsAsReadonly: false}`:
224+
225+
```ts
226+
type MyType = {
227+
readonly prop: string;
228+
method(): string; // note: this method is mutable
229+
};
230+
function foo(arg: MyType) {}
231+
```
232+
233+
Examples of **correct** code for this rule with `{treatMethodsAsReadonly: false}`:
234+
235+
```ts
236+
type MyType = Readonly<{
237+
prop: string;
238+
method(): string;
239+
}>;
240+
function foo(arg: MyType) {}
241+
242+
type MyOtherType = {
243+
readonly prop: string;
244+
readonly method: () => string;
245+
};
246+
function bar(arg: MyOtherType) {}
247+
```
248+
249+
Examples of **correct** code for this rule with `{treatMethodsAsReadonly: true}`:
250+
251+
```ts
252+
type MyType = {
253+
readonly prop: string;
254+
method(): string; // note: this method is mutable
255+
};
256+
function foo(arg: MyType) {}
257+
```

‎packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ type Options = [
88
{
99
checkParameterProperties?: boolean;
1010
ignoreInferredTypes?: boolean;
11-
},
11+
} & util.ReadonlynessOptions,
1212
];
1313
type MessageIds = 'shouldBeReadonly';
1414

@@ -34,6 +34,7 @@ export default util.createRule<Options, MessageIds>({
3434
ignoreInferredTypes: {
3535
type: 'boolean',
3636
},
37+
...util.readonlynessOptionsSchema.properties,
3738
},
3839
},
3940
],
@@ -45,10 +46,13 @@ export default util.createRule<Options, MessageIds>({
4546
{
4647
checkParameterProperties: true,
4748
ignoreInferredTypes: false,
49+
...util.readonlynessOptionsDefaults,
4850
},
4951
],
5052
create(context, options) {
51-
const [{ checkParameterProperties, ignoreInferredTypes }] = options;
53+
const [
54+
{ checkParameterProperties, ignoreInferredTypes, treatMethodsAsReadonly },
55+
] = options;
5256
const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context);
5357
const checker = program.getTypeChecker();
5458

@@ -94,7 +98,9 @@ export default util.createRule<Options, MessageIds>({
9498

9599
const tsNode = esTreeNodeToTSNodeMap.get(actualParam);
96100
const type = checker.getTypeAtLocation(tsNode);
97-
const isReadOnly = util.isTypeReadonly(checker, type);
101+
const isReadOnly = util.isTypeReadonly(checker, type, {
102+
treatMethodsAsReadonly: treatMethodsAsReadonly!,
103+
});
98104

99105
if (!isReadOnly) {
100106
context.report({

‎packages/eslint-plugin/src/util/isTypeReadonly.ts

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
isUnionOrIntersectionType,
55
unionTypeParts,
66
isPropertyReadonlyInType,
7+
isSymbolFlagSet,
78
} from 'tsutils';
89
import * as ts from 'typescript';
910
import { getTypeOfPropertyOfType, nullThrows, NullThrowsReasons } from '.';
@@ -17,9 +18,32 @@ const enum Readonlyness {
1718
Readonly = 3,
1819
}
1920

21+
export interface ReadonlynessOptions {
22+
readonly treatMethodsAsReadonly?: boolean;
23+
}
24+
25+
export const readonlynessOptionsSchema = {
26+
type: 'object',
27+
additionalProperties: false,
28+
properties: {
29+
treatMethodsAsReadonly: {
30+
type: 'boolean',
31+
},
32+
},
33+
};
34+
35+
export const readonlynessOptionsDefaults: ReadonlynessOptions = {
36+
treatMethodsAsReadonly: false,
37+
};
38+
39+
function hasSymbol(node: ts.Node): node is ts.Node & { symbol: ts.Symbol } {
40+
return Object.prototype.hasOwnProperty.call(node, 'symbol');
41+
}
42+
2043
function isTypeReadonlyArrayOrTuple(
2144
checker: ts.TypeChecker,
2245
type: ts.Type,
46+
options: ReadonlynessOptions,
2347
seenTypes: Set<ts.Type>,
2448
): Readonlyness {
2549
function checkTypeArguments(arrayType: ts.TypeReference): Readonlyness {
@@ -40,7 +64,7 @@ function isTypeReadonlyArrayOrTuple(
4064
if (
4165
typeArguments.some(
4266
typeArg =>
43-
isTypeReadonlyRecurser(checker, typeArg, seenTypes) ===
67+
isTypeReadonlyRecurser(checker, typeArg, options, seenTypes) ===
4468
Readonlyness.Mutable,
4569
)
4670
) {
@@ -76,6 +100,7 @@ function isTypeReadonlyArrayOrTuple(
76100
function isTypeReadonlyObject(
77101
checker: ts.TypeChecker,
78102
type: ts.Type,
103+
options: ReadonlynessOptions,
79104
seenTypes: Set<ts.Type>,
80105
): Readonlyness {
81106
function checkIndexSignature(kind: ts.IndexKind): Readonlyness {
@@ -93,7 +118,18 @@ function isTypeReadonlyObject(
93118
if (properties.length) {
94119
// ensure the properties are marked as readonly
95120
for (const property of properties) {
96-
if (!isPropertyReadonlyInType(type, property.getEscapedName(), checker)) {
121+
if (
122+
!(
123+
isPropertyReadonlyInType(type, property.getEscapedName(), checker) ||
124+
(options.treatMethodsAsReadonly &&
125+
property.valueDeclaration !== undefined &&
126+
hasSymbol(property.valueDeclaration) &&
127+
isSymbolFlagSet(
128+
property.valueDeclaration.symbol,
129+
ts.SymbolFlags.Method,
130+
))
131+
)
132+
) {
97133
return Readonlyness.Mutable;
98134
}
99135
}
@@ -117,7 +153,7 @@ function isTypeReadonlyObject(
117153
}
118154

119155
if (
120-
isTypeReadonlyRecurser(checker, propertyType, seenTypes) ===
156+
isTypeReadonlyRecurser(checker, propertyType, options, seenTypes) ===
121157
Readonlyness.Mutable
122158
) {
123159
return Readonlyness.Mutable;
@@ -142,14 +178,15 @@ function isTypeReadonlyObject(
142178
function isTypeReadonlyRecurser(
143179
checker: ts.TypeChecker,
144180
type: ts.Type,
181+
options: ReadonlynessOptions,
145182
seenTypes: Set<ts.Type>,
146183
): Readonlyness.Readonly | Readonlyness.Mutable {
147184
seenTypes.add(type);
148185

149186
if (isUnionType(type)) {
150187
// all types in the union must be readonly
151188
const result = unionTypeParts(type).every(t =>
152-
isTypeReadonlyRecurser(checker, t, seenTypes),
189+
isTypeReadonlyRecurser(checker, t, options, seenTypes),
153190
);
154191
const readonlyness = result ? Readonlyness.Readonly : Readonlyness.Mutable;
155192
return readonlyness;
@@ -169,12 +206,22 @@ function isTypeReadonlyRecurser(
169206
return Readonlyness.Readonly;
170207
}
171208

172-
const isReadonlyArray = isTypeReadonlyArrayOrTuple(checker, type, seenTypes);
209+
const isReadonlyArray = isTypeReadonlyArrayOrTuple(
210+
checker,
211+
type,
212+
options,
213+
seenTypes,
214+
);
173215
if (isReadonlyArray !== Readonlyness.UnknownType) {
174216
return isReadonlyArray;
175217
}
176218

177-
const isReadonlyObject = isTypeReadonlyObject(checker, type, seenTypes);
219+
const isReadonlyObject = isTypeReadonlyObject(
220+
checker,
221+
type,
222+
options,
223+
seenTypes,
224+
);
178225
/* istanbul ignore else */ if (
179226
isReadonlyObject !== Readonlyness.UnknownType
180227
) {
@@ -187,9 +234,14 @@ function isTypeReadonlyRecurser(
187234
/**
188235
* Checks if the given type is readonly
189236
*/
190-
function isTypeReadonly(checker: ts.TypeChecker, type: ts.Type): boolean {
237+
function isTypeReadonly(
238+
checker: ts.TypeChecker,
239+
type: ts.Type,
240+
options: ReadonlynessOptions,
241+
): boolean {
191242
return (
192-
isTypeReadonlyRecurser(checker, type, new Set()) === Readonlyness.Readonly
243+
isTypeReadonlyRecurser(checker, type, options, new Set()) ===
244+
Readonlyness.Readonly
193245
);
194246
}
195247

‎packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,74 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
154154
}
155155
function foo(arg: Readonly<Foo>) {}
156156
`,
157+
// immutable methods
158+
`
159+
type MyType = Readonly<{
160+
prop: string;
161+
method(): string;
162+
}>;
163+
function foo(arg: MyType) {}
164+
`,
165+
`
166+
type MyType = {
167+
readonly prop: string;
168+
readonly method: () => string;
169+
};
170+
function bar(arg: MyType) {}
171+
`,
172+
// methods treated as readonly
173+
{
174+
code: `
175+
type MyType = {
176+
readonly prop: string;
177+
method(): string;
178+
};
179+
function foo(arg: MyType) {}
180+
`,
181+
options: [
182+
{
183+
treatMethodsAsReadonly: true,
184+
},
185+
],
186+
},
187+
{
188+
code: `
189+
class Foo {
190+
method() {}
191+
}
192+
function foo(arg: Foo) {}
193+
`,
194+
options: [
195+
{
196+
treatMethodsAsReadonly: true,
197+
},
198+
],
199+
},
200+
{
201+
code: `
202+
interface Foo {
203+
method(): void;
204+
}
205+
function foo(arg: Foo) {}
206+
`,
207+
options: [
208+
{
209+
treatMethodsAsReadonly: true,
210+
},
211+
],
212+
},
213+
// ReadonlySet and ReadonlyMap are seen as readonly when methods are treated as readonly
214+
{
215+
code: `
216+
function foo(arg: ReadonlySet<string>) {}
217+
function bar(arg: ReadonlyMap<string, string>) {}
218+
`,
219+
options: [
220+
{
221+
treatMethodsAsReadonly: true,
222+
},
223+
],
224+
},
157225

158226
// parameter properties should work fine
159227
{
@@ -715,5 +783,23 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
715783
},
716784
],
717785
},
786+
// Mutable methods.
787+
{
788+
code: `
789+
type MyType = {
790+
readonly prop: string;
791+
method(): string;
792+
};
793+
function foo(arg: MyType) {}
794+
`,
795+
errors: [
796+
{
797+
messageId: 'shouldBeReadonly',
798+
line: 6,
799+
column: 22,
800+
endColumn: 33,
801+
},
802+
],
803+
},
718804
],
719805
});

0 commit comments

Comments
 (0)
0