Skip to content

Commit 63d1d81

Browse files
jsone-studiosjsone-studios
authored
fix(eslint-plugin): [array-type] fix issues with readonly option (typescript-eslint#2667)
1 parent f1329f6 commit 63d1d81

File tree

3 files changed

+879
-277
lines changed

3 files changed

+879
-277
lines changed

‎packages/eslint-plugin/docs/rules/array-type.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,25 @@ const e: string[] = ['a', 'b'];
9292
const f: readonly string[] = ['a', 'b'];
9393
```
9494

95+
## Combination matrix
96+
97+
This matrix lists all possible option combinations and their expected results for different types of Arrays.
98+
99+
| defaultOption | readonlyOption | Array with simple type | Array with non simple type | Readonly array with simple type | Readonly array with non simple type |
100+
| -------------- | -------------- | ---------------------- | -------------------------- | ------------------------------- | ----------------------------------- |
101+
| `array` | | `number[]` | `(Foo & Bar)[]` | `readonly number[]` | `readonly (Foo & Bar)[]` |
102+
| `array` | `array` | `number[]` | `(Foo & Bar)[]` | `readonly number[]` | `readonly (Foo & Bar)[]` |
103+
| `array` | `array-simple` | `number[]` | `(Foo & Bar)[]` | `readonly number[]` | `ReadonlyArray<Foo & Bar>` |
104+
| `array` | `generic` | `number[]` | `(Foo & Bar)[]` | `ReadonlyArray<number>` | `ReadonlyArray<Foo & Bar>` |
105+
| `array-simple` | | `number[]` | `Array<Foo & Bar>` | `readonly number[]` | `ReadonlyArray<Foo & Bar>` |
106+
| `array-simple` | `array` | `number[]` | `Array<Foo & Bar>` | `readonly number[]` | `readonly (Foo & Bar)[]` |
107+
| `array-simple` | `array-simple` | `number[]` | `Array<Foo & Bar>` | `readonly number[]` | `ReadonlyArray<Foo & Bar>` |
108+
| `array-simple` | `generic` | `number[]` | `Array<Foo & Bar>` | `ReadonlyArray<number>` | `ReadonlyArray<Foo & Bar>` |
109+
| `generic` | | `Array<number>` | `Array<Foo & Bar>` | `ReadonlyArray<number>` | `ReadonlyArray<Foo & Bar>` |
110+
| `generic` | `array` | `Array<number>` | `Array<Foo & Bar>` | `readonly number[]` | `readonly (Foo & Bar)[]` |
111+
| `generic` | `array-simple` | `Array<number>` | `Array<Foo & Bar>` | `readonly number[]` | `ReadonlyArray<Foo & Bar>` |
112+
| `generic` | `generic` | `Array<number>` | `Array<Foo & Bar>` | `ReadonlyArray<number>` | `ReadonlyArray<Foo & Bar>` |
113+
95114
## Related to
96115

97116
- TSLint: [array-type](https://palantir.github.io/tslint/rules/array-type/)

‎packages/eslint-plugin/src/rules/array-type.ts

Lines changed: 31 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {
22
AST_NODE_TYPES,
3-
AST_TOKEN_TYPES,
43
TSESTree,
54
} from '@typescript-eslint/experimental-utils';
65
import * as util from '../util';
@@ -129,31 +128,6 @@ export default util.createRule<Options, MessageIds>({
129128
const defaultOption = options.default;
130129
const readonlyOption = options.readonly ?? defaultOption;
131130

132-
const isArraySimpleOption =
133-
defaultOption === 'array-simple' && readonlyOption === 'array-simple';
134-
const isArrayOption =
135-
defaultOption === 'array' && readonlyOption === 'array';
136-
const isGenericOption =
137-
defaultOption === 'generic' && readonlyOption === 'generic';
138-
139-
/**
140-
* Check if whitespace is needed before this node
141-
* @param node the node to be evaluated.
142-
*/
143-
function requireWhitespaceBefore(node: TSESTree.Node): boolean {
144-
const prevToken = sourceCode.getTokenBefore(node);
145-
if (!prevToken) {
146-
return false;
147-
}
148-
149-
const nextToken = sourceCode.getTokenAfter(prevToken);
150-
if (nextToken && sourceCode.isSpaceBetweenTokens(prevToken, nextToken)) {
151-
return false;
152-
}
153-
154-
return prevToken.type === AST_TOKEN_TYPES.Identifier;
155-
}
156-
157131
/**
158132
* @param node the node to be evaluated.
159133
*/
@@ -169,121 +143,80 @@ export default util.createRule<Options, MessageIds>({
169143
return 'T';
170144
}
171145

172-
/**
173-
* @param node the node to be evaluated
174-
*/
175-
function getTypeOpNodeRange(
176-
node: TSESTree.Node | null,
177-
): [number, number] | undefined {
178-
if (!node) {
179-
return undefined;
180-
}
181-
182-
const firstToken = sourceCode.getFirstToken(node)!;
183-
const nextToken = sourceCode.getTokenAfter(firstToken)!;
184-
return [firstToken.range[0], nextToken.range[0]];
185-
}
186-
187146
return {
188147
TSArrayType(node): void {
189-
if (
190-
isArrayOption ||
191-
(isArraySimpleOption && isSimpleType(node.elementType))
192-
) {
193-
return;
194-
}
195-
196148
const isReadonly =
197149
node.parent &&
198150
node.parent.type === AST_NODE_TYPES.TSTypeOperator &&
199151
node.parent.operator === 'readonly';
200152

201-
const isReadonlyGeneric =
202-
readonlyOption === 'generic' && defaultOption !== 'generic';
203-
204-
const isReadonlyArray =
205-
readonlyOption !== 'generic' && defaultOption === 'generic';
153+
const currentOption = isReadonly ? readonlyOption : defaultOption;
206154

207155
if (
208-
(isReadonlyGeneric && !isReadonly) ||
209-
(isReadonlyArray && isReadonly)
156+
currentOption === 'array' ||
157+
(currentOption === 'array-simple' && isSimpleType(node.elementType))
210158
) {
211159
return;
212160
}
213161

214162
const messageId =
215-
defaultOption === 'generic'
163+
currentOption === 'generic'
216164
? 'errorStringGeneric'
217165
: 'errorStringGenericSimple';
218-
const typeOpNode = isReadonly ? node.parent! : null;
166+
const errorNode = isReadonly ? node.parent! : node;
219167

220168
context.report({
221-
node: isReadonly ? node.parent! : node,
169+
node: errorNode,
222170
messageId,
223171
data: {
224172
type: getMessageType(node.elementType),
225173
},
226174
fix(fixer) {
227-
const toFix = [
228-
fixer.replaceTextRange([node.range[1] - 2, node.range[1]], '>'),
229-
];
230-
const startText = requireWhitespaceBefore(node);
231-
const typeOpNodeRange = getTypeOpNodeRange(typeOpNode);
175+
const typeNode =
176+
node.elementType.type === AST_NODE_TYPES.TSParenthesizedType
177+
? node.elementType.typeAnnotation
178+
: node.elementType;
232179

233-
if (typeOpNodeRange) {
234-
toFix.unshift(fixer.removeRange(typeOpNodeRange));
235-
} else {
236-
toFix.push(
237-
fixer.insertTextBefore(node, `${startText ? ' ' : ''}`),
238-
);
239-
}
180+
const arrayType = isReadonly ? 'ReadonlyArray' : 'Array';
240181

241-
toFix.push(
242-
fixer.insertTextBefore(
243-
node,
244-
`${isReadonly ? 'Readonly' : ''}Array<`,
182+
return [
183+
fixer.replaceTextRange(
184+
[errorNode.range[0], typeNode.range[0]],
185+
`${arrayType}<`,
245186
),
246-
);
247-
248-
if (node.elementType.type === AST_NODE_TYPES.TSParenthesizedType) {
249-
const first = sourceCode.getFirstToken(node.elementType);
250-
const last = sourceCode.getLastToken(node.elementType);
251-
if (!first || !last) {
252-
return null;
253-
}
254-
255-
toFix.push(fixer.remove(first));
256-
toFix.push(fixer.remove(last));
257-
}
258-
259-
return toFix;
187+
fixer.replaceTextRange(
188+
[typeNode.range[1], errorNode.range[1]],
189+
'>',
190+
),
191+
];
260192
},
261193
});
262194
},
263195

264196
TSTypeReference(node): void {
265197
if (
266-
isGenericOption ||
267-
node.typeName.type !== AST_NODE_TYPES.Identifier
198+
node.typeName.type !== AST_NODE_TYPES.Identifier ||
199+
!(
200+
node.typeName.name === 'Array' ||
201+
node.typeName.name === 'ReadonlyArray'
202+
)
268203
) {
269204
return;
270205
}
271206

272207
const isReadonlyArrayType = node.typeName.name === 'ReadonlyArray';
273-
const isArrayType = node.typeName.name === 'Array';
208+
const currentOption = isReadonlyArrayType
209+
? readonlyOption
210+
: defaultOption;
274211

275-
if (
276-
!(isArrayType || isReadonlyArrayType) ||
277-
(readonlyOption === 'generic' && isReadonlyArrayType) ||
278-
(defaultOption === 'generic' && !isReadonlyArrayType)
279-
) {
212+
if (currentOption === 'generic') {
280213
return;
281214
}
282215

283216
const readonlyPrefix = isReadonlyArrayType ? 'readonly ' : '';
284217
const typeParams = node.typeParameters?.params;
285218
const messageId =
286-
defaultOption === 'array'
219+
currentOption === 'array'
287220
? 'errorStringArray'
288221
: 'errorStringArraySimple';
289222

@@ -305,7 +238,7 @@ export default util.createRule<Options, MessageIds>({
305238

306239
if (
307240
typeParams.length !== 1 ||
308-
(defaultOption === 'array-simple' && !isSimpleType(typeParams[0]))
241+
(currentOption === 'array-simple' && !isSimpleType(typeParams[0]))
309242
) {
310243
return;
311244
}

0 commit comments

Comments
 (0)