Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_analyze): useShorthandArrayType should handle nested ReadonlyArray and not report to TsObjectType #4354

Merged
merged 3 commits into from
Apr 11, 2023
Merged

fix(rome_js_analyze): useShorthandArrayType should handle nested ReadonlyArray and not report to TsObjectType #4354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80e2d1e
fix(rome_js_analyze): useShorthandArrayType to handle nested Readonly…
unvalley Apr 6, 2023
1dedbff
fix: to ignore object type
unvalley Apr 8, 2023
fff746c
chore: update the CHANGELOG.md
unvalley Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

- Code actions are formatted using Rome's formatter. If the formatter is disabled,
the code action is not formatted.
- Fixed an issue that [`useShorthandArrayType`](https://docs.rome.tools/lint/rules/useShorthandArrayType) rule did not handle nested ReadonlyArray types correctly and erroneously reported TsObjectType [#4354](https://github.com/rome/tools/issues/4353)

#### New rules
- [`noConfusingArrow`](https://docs.rome.tools/lint/rules/noConfusingArrow/)
Expand Down
62 changes: 46 additions & 16 deletions crates/rome_js_analyze/src/analyzers/style/use_shorthand_array_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use crate::JsRuleAction;
declare_rule! {
/// When expressing array types, this rule promotes the usage of `T[]` shorthand instead of `Array<T>`.
///
/// ESLint (typescript-eslint) equivalent: [array-type/array-simple](https://typescript-eslint.io/rules/array-type/#array-simple)
///
/// ## Examples
///
/// ### Invalid
Expand Down Expand Up @@ -155,6 +157,7 @@ fn convert_to_array_type(
| AnyTsType::TsConditionalType(_)
| AnyTsType::TsTypeOperatorType(_)
| AnyTsType::TsInferType(_)
| AnyTsType::TsObjectType(_)
| AnyTsType::TsMappedType(_) => None,

AnyTsType::TsReferenceType(ty) => match get_array_kind_by_reference(ty) {
Expand All @@ -169,26 +172,53 @@ fn convert_to_array_type(
},
_ => Some(param),
};
element_type.map(|element_type| {
let array_type = make::ts_array_type(
element_type.map(|element_type| match array_kind {
TsArrayKind::Simple => AnyTsType::TsArrayType(make::ts_array_type(
element_type,
make::token(T!['[']),
make::token(T![']']),
);
match array_kind {
TsArrayKind::Simple => AnyTsType::TsArrayType(array_type),
TsArrayKind::Readonly => {
let readonly_token = JsSyntaxToken::new_detached(
JsSyntaxKind::TS_READONLY_MODIFIER,
"readonly ",
[],
[TriviaPiece::new(TriviaPieceKind::Whitespace, 1)],
);
AnyTsType::TsTypeOperatorType(make::ts_type_operator_type(
readonly_token,
AnyTsType::TsArrayType(array_type),
))
)),
TsArrayKind::Readonly => {
let readonly_token = JsSyntaxToken::new_detached(
JsSyntaxKind::TS_READONLY_MODIFIER,
"readonly ",
[],
[TriviaPiece::new(TriviaPieceKind::Whitespace, 1)],
);

// Modify `ReadonlyArray<ReadonlyArray<T>>` to `readonly (readonly T[])[]`
if let AnyTsType::TsTypeOperatorType(op) = &element_type {
if let Ok(op) = op.operator_token() {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
if op.text_trimmed() == "readonly" {
return AnyTsType::TsTypeOperatorType(
make::ts_type_operator_type(
readonly_token,
// wrap ArrayType
AnyTsType::TsArrayType(make::ts_array_type(
AnyTsType::TsParenthesizedType(
make::ts_parenthesized_type(
make::token(T!['(']),
element_type,
make::token(T![')']),
),
),
make::token(T!['[']),
make::token(T![']']),
)),
),
);
}
}
}

AnyTsType::TsTypeOperatorType(make::ts_type_operator_type(
readonly_token,
AnyTsType::TsArrayType(make::ts_array_type(
element_type,
make::token(T!['[']),
make::token(T![']']),
)),
))
}
})
})
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/style/useShorthandArrayType/invalid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ let readonlyInvalid1: ReadonlyArray<foo>;
let readonlyInvalid2: Promise<ReadonlyArray<string>>;
let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
let readonlyInvalid4: ReadonlyArray<[number, number]>;
let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
126 changes: 122 additions & 4 deletions crates/rome_js_analyze/tests/specs/style/useShorthandArrayType/invalid.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ let readonlyInvalid1: ReadonlyArray<foo>;
let readonlyInvalid2: Promise<ReadonlyArray<string>>;
let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
let readonlyInvalid4: ReadonlyArray<[number, number]>;
let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;

```

Expand Down Expand Up @@ -211,7 +213,7 @@ invalid.ts:9:23 lint/style/useShorthandArrayType FIXABLE ━━━━━━━
> 9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
│ ^^^^^^^^^^^^^^^^^^^^^^^
10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │
11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;

i Suggested fix: Use shorthand readonly T[] syntax to replace

Expand All @@ -220,7 +222,7 @@ invalid.ts:9:23 lint/style/useShorthandArrayType FIXABLE ━━━━━━━
9 │ - let·readonlyInvalid3:·ReadonlyArray<Foo<Bar>>;
9 │ + let·readonlyInvalid3:·readonly·Foo<Bar>[];
10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 11 │
11 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;


```
Expand All @@ -234,15 +236,131 @@ invalid.ts:10:23 lint/style/useShorthandArrayType FIXABLE ━━━━━━
9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
> 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11 │
11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;

i Suggested fix: Use shorthand readonly T[] syntax to replace

8 8 │ let readonlyInvalid2: Promise<ReadonlyArray<string>>;
9 9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
10 │ - let·readonlyInvalid4:·ReadonlyArray<[number,·number]>;
10 │ + let·readonlyInvalid4:·readonly·[number,·number][];
11 11 │
11 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
12 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;


```

```
invalid.ts:11:23 lint/style/useShorthandArrayType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Use shorthand readonly T[] syntax instead of ReadonlyArray<T> syntax.

9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
> 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
13 │

i Suggested fix: Use shorthand readonly T[] syntax to replace

9 9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │ - let·readonlyInvalid5:·ReadonlyArray<ReadonlyArray<number>>;
11 │ + let·readonlyInvalid5:·readonly·(readonly·number[])[];
12 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
13 13 │


```

```
invalid.ts:11:37 lint/style/useShorthandArrayType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Use shorthand readonly T[] syntax instead of ReadonlyArray<T> syntax.

9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
> 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
│ ^^^^^^^^^^^^^^^^^^^^^
12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
13 │

i Suggested fix: Use shorthand readonly T[] syntax to replace

9 9 │ let readonlyInvalid3: ReadonlyArray<Foo<Bar>>;
10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │ - let·readonlyInvalid5:·ReadonlyArray<ReadonlyArray<number>>;
11 │ + let·readonlyInvalid5:·ReadonlyArray<readonly·number[]>;
12 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
13 13 │


```

```
invalid.ts:12:23 lint/style/useShorthandArrayType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Use shorthand readonly T[] syntax instead of ReadonlyArray<T> syntax.

10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
> 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 │

i Suggested fix: Use shorthand readonly T[] syntax to replace

10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
12 │ - let·readonlyInvalid6:·ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
12 │ + let·readonlyInvalid6:·readonly·(readonly·(readonly·number[])[])[];
13 13 │


```

```
invalid.ts:12:37 lint/style/useShorthandArrayType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Use shorthand readonly T[] syntax instead of ReadonlyArray<T> syntax.

10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
> 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 │

i Suggested fix: Use shorthand readonly T[] syntax to replace

10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
12 │ - let·readonlyInvalid6:·ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
12 │ + let·readonlyInvalid6:·ReadonlyArray<readonly·(readonly·number[])[]>;
13 13 │


```

```
invalid.ts:12:51 lint/style/useShorthandArrayType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Use shorthand readonly T[] syntax instead of ReadonlyArray<T> syntax.

10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
> 12 │ let readonlyInvalid6: ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
│ ^^^^^^^^^^^^^^^^^^^^^
13 │

i Suggested fix: Use shorthand readonly T[] syntax to replace

10 10 │ let readonlyInvalid4: ReadonlyArray<[number, number]>;
11 11 │ let readonlyInvalid5: ReadonlyArray<ReadonlyArray<number>>;
12 │ - let·readonlyInvalid6:·ReadonlyArray<ReadonlyArray<ReadonlyArray<number>>>;
12 │ + let·readonlyInvalid6:·ReadonlyArray<ReadonlyArray<readonly·number[]>>;
13 13 │


```
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/style/useShorthandArrayType/valid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ let valid8: Array<string & number>;
type valid9<T> = T extends Array<infer R> ? R : any;
// mapped type
type valid10<T> = { [K in keyof T]: T[K] };
// object type
type valid11 = Array<{ value: string; label: string }>;

let readonlyValid1: ReadonlyArray<Foo | Bar>;
let readonlyValid2: ReadonlyArray<keyof Bar>;
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/style/useShorthandArrayType/valid.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ let valid8: Array<string & number>;
type valid9<T> = T extends Array<infer R> ? R : any;
// mapped type
type valid10<T> = { [K in keyof T]: T[K] };
// object type
type valid11 = Array<{ value: string; label: string }>;

let readonlyValid1: ReadonlyArray<Foo | Bar>;
let readonlyValid2: ReadonlyArray<keyof Bar>;
Expand Down
2 changes: 2 additions & 0 deletions website/src/pages/lint/rules/useShorthandArrayType.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ parent: lint/rules/index

When expressing array types, this rule promotes the usage of `T[]` shorthand instead of `Array<T>`.

ESLint (typescript-eslint) equivalent: [array-type/array-simple](https://typescript-eslint.io/rules/array-type/#array-simple)

## Examples

### Invalid
Expand Down