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

Commit

Permalink
fix(rome_js_analyze): Files with fn(() => (aborted = true)); cause - …
Browse files Browse the repository at this point in the history
…entered unreachable code - when --apply-unsafe #4464
  • Loading branch information
denbezrukov committed May 14, 2023
1 parent 66a6bf8 commit 77d9e64
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rome_js_syntax::{
TsSatisfiesExpression, TsTypeAssertionAssignment, TsTypeAssertionAssignmentFields,
TsTypeAssertionExpression,
};
use rome_rowan::{AstNode, BatchMutationExt, SyntaxError, SyntaxResult};
use rome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, SyntaxError, SyntaxResult};

use crate::JsRuleAction;

Expand Down Expand Up @@ -204,9 +204,14 @@ impl TryFromAssignment<AnyJsAssignment> for AnyJsExpression {
impl TryFromAssignment<JsArrayAssignmentPattern> for JsArrayExpression {
fn try_from_assignment(value: JsArrayAssignmentPattern) -> SyntaxResult<Self> {
let mut elements = Vec::new();
let mut separators = Vec::new();

for element in value.elements() {
let element = match element? {
for element in value.elements().elements() {
if let Ok(Some(separator)) = element.trailing_separator {
separators.push(separator);
}

let element = match element.node? {
AnyJsArrayAssignmentPatternElement::AnyJsAssignmentPattern(assignment) => {
AnyJsArrayElement::AnyJsExpression(assignment.try_into_expression()?)
}
Expand All @@ -225,7 +230,7 @@ impl TryFromAssignment<JsArrayAssignmentPattern> for JsArrayExpression {
elements.push(element);
}

let elements = make::js_array_element_list(elements.into_iter(), None);
let elements = make::js_array_element_list(elements.into_iter(), separators.into_iter());

let expression =
make::js_array_expression(value.l_brack_token()?, elements, value.r_brack_token()?);
Expand Down Expand Up @@ -257,9 +262,14 @@ impl TryFromAssignment<JsObjectAssignmentPattern> for JsObjectExpression {
} = value.as_fields();

let mut members = Vec::new();
let mut separators = Vec::new();

for property in properties.elements() {
if let Ok(Some(separator)) = property.trailing_separator {
separators.push(separator);
}

for property in properties {
let member = match property? {
let member = match property.node? {
AnyJsObjectAssignmentPatternMember::JsBogusAssignment(assigment) => {
AnyJsObjectMember::JsBogusMember(make::js_bogus_member([Some(
JsSyntaxElement::Node(assigment.into_syntax()),
Expand Down Expand Up @@ -310,7 +320,7 @@ impl TryFromAssignment<JsObjectAssignmentPattern> for JsObjectExpression {
members.push(member);
}

let member_list = make::js_object_member_list(members.into_iter(), None);
let member_list = make::js_object_member_list(members.into_iter(), separators.into_iter());

let expression = make::js_object_expression(l_curly_token?, member_list, r_curly_token?);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,6 @@ switch (foo) {

res.onAborted(() => /*0*/(/*1*/(/*2*/a/*3*/./*4*/b/*5*/)/*6*/ /*7*/ = /*8*/ /*9*/true/*10*/));


(/*1*/[/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/]/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
(/*1*/{/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/}/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ switch (foo) {
res.onAborted(() => /*0*/(/*1*/(/*2*/a/*3*/./*4*/b/*5*/)/*6*/ /*7*/ = /*8*/ /*9*/true/*10*/));


(/*1*/[/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/]/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
(/*1*/{/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/}/*8*//*9*/=/*10*/ 2e308) ? foo : bar;

```

# Diagnostics
Expand Down Expand Up @@ -662,4 +665,44 @@ invalid.js:83:32 lint/suspicious/noAssignInExpressions FIXABLE ━━━━━
```

```
invalid.js:86:7 lint/suspicious/noAssignInExpressions FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The assignment should not be in an expression.
> 86 │ (/*1*/[/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/]/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
87 │ (/*1*/{/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/}/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
88 │
i The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
i Suggested fix: Did you mean '==='?
86 │ (/*1*/[/*2*/a/*3*/,/*4*/b/*5*/,·/*6*/c/*7*/]/*8*//*9*/===/*10*/·2e308)·?·foo·:·bar;
│ ++
```

```
invalid.js:87:7 lint/suspicious/noAssignInExpressions FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The assignment should not be in an expression.
86 │ (/*1*/[/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/]/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
> 87 │ (/*1*/{/*2*/a/*3*/,/*4*/b/*5*/, /*6*/c/*7*/}/*8*//*9*/=/*10*/ 2e308) ? foo : bar;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
88 │
i The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
i Suggested fix: Did you mean '==='?
87 │ (/*1*/{/*2*/a/*3*/,/*4*/b/*5*/,·/*6*/c/*7*/}/*8*//*9*/===/*10*/·2e308)·?·foo·:·bar;
│ ++
```


0 comments on commit 77d9e64

Please sign in to comment.