Skip to content

Commit

Permalink
refactor(lint/useCollapsedElseIf): remove unsafe code fixes (#244)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Conaclos
Conaclos authored Sep 11, 2023
1 parent 239d1e2 commit 319bb9f
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 148 deletions.
9 changes: 5 additions & 4 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
export * as MY_NAMESPACE from "./lib.js";
```

- [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) now ignores decorated classes and decorated parameters.
The rule now gives suggestions instead of safe fixes when parameters are annotated with types.

- [useCollapsedElseIf](https://biomejs.dev/linter/rules/use-collapsed-else-if/) now only provides safe code fixes.

## 1.1.1 (2023-09-07)

### Analyzer
Expand Down Expand Up @@ -154,10 +159,6 @@ multiple servers in the `rage` output.
- You can use `// biome-ignore` as suppression comment.
- The `// rome-ignore` suppression is deprecated.


- [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) now ignores decorated classes and decorated parameters.
The rule now gives suggestions instead of safe fixes when parameters are annotated with types.

#### Bug fixes

- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) case-insensitive.
Expand Down
25 changes: 16 additions & 9 deletions crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ declare_rule! {
/// }
/// ```
///
/// ```js,expect_diagnostic
/// if (condition) {
/// // ...
/// } else {
/// // Comment
/// if (anotherCondition) {
/// // ...
/// }
/// }
/// ```
///
/// ### Valid
///
/// ```js
Expand Down Expand Up @@ -129,15 +140,11 @@ impl Rule for UseCollapsedElseIf {
.l_curly_token()
.ok()?
.has_trailing_comments()
|| if_statement.syntax().has_leading_comments()
|| if_statement.syntax().has_trailing_comments()
|| if_statement.syntax().has_comments_direct()
|| block_statement.r_curly_token().ok()?.has_leading_comments();

let applicability = if has_comments {
Applicability::MaybeIncorrect
} else {
Applicability::Always
};
if has_comments {
return None;
}

let mut mutation = ctx.root().begin();
mutation.replace_node(
Expand All @@ -147,7 +154,7 @@ impl Rule for UseCollapsedElseIf {

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability,
applicability: Applicability::Always,
message: markup! { "Use collapsed "<Emphasis>"else if"</Emphasis>" instead." }
.to_owned(),
mutation,
Expand Down
139 changes: 8 additions & 131 deletions crates/rome_js_analyze/tests/specs/nursery/useCollapsedElseIf/invalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ invalid.js:15:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
```

```
invalid.js:29:20 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:29:20 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -195,25 +195,11 @@ invalid.js:29:20 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
33 │ }
34 │
i Suggested fix: Use collapsed else if instead.
27 27 │ if (condition) {
28 28 │ // ...
29 │ - }·else·{·//·Comment
30 │ - → if·(anotherCondition)·{
29 │ + }·else·if·(anotherCondition)·{
31 30 │ // ...
32 │ - → }
33 │ - }
31 │ + → }
34 32 │
35 33 │ if (condition) {
```

```
invalid.js:37:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:37:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -229,26 +215,11 @@ invalid.js:37:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
42 │ }
43 │
i Suggested fix: Use collapsed else if instead.
35 35 │ if (condition) {
36 36 │ // ...
37 │ - }·else·{
38 │ - → //·Comment
39 │ - → if·(anotherCondition)·{
37 │ + }·else·if·(anotherCondition)·{
40 38 │ // ...
41 │ - → }
42 │ - }
39 │ + → }
43 40 │
44 41 │ if (condition) {
```

```
invalid.js:46:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:46:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -263,25 +234,11 @@ invalid.js:46:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
50 │ }
51 │
i Suggested fix: Use collapsed else if instead.
44 44 │ if (condition) {
45 45 │ // ...
46 │ - }·else·{
47 │ - → if·(anotherCondition)·{
46 │ + }·else·if·(anotherCondition)·{
48 47 │ // ...
49 │ - → }·//·Comment
50 │ - }
48 │ + → }
51 49 │
52 50 │ if (condition) {
```

```
invalid.js:54:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:54:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -296,26 +253,11 @@ invalid.js:54:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
58 │ // Comment
59 │ }
i Suggested fix: Use collapsed else if instead.
52 52 │ if (condition) {
53 53 │ // ...
54 │ - }·else·{
55 │ - → if·(anotherCondition)·{
54 │ + }·else·if·(anotherCondition)·{
56 55 │ // ...
57 │ - → }
58 │ - → //·Comment
59 │ - }
56 │ + → }
60 57 │
61 58 │ if (condition) {
```

```
invalid.js:63:20 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:63:20 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -332,27 +274,11 @@ invalid.js:63:20 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
69 │ }
70 │
i Suggested fix: Use collapsed else if instead.
61 61 │ if (condition) {
62 62 │ // ...
63 │ - }·else·{·//·Comment
64 │ - → if·(anotherCondition)·{
63 │ + }·else·if·(anotherCondition)·{
65 64 │ // ...
66 65 │ } else {
67 66 │ // ...
68 │ - → }
69 │ - }
67 │ + → }
70 68 │
71 69 │ if (condition) {
```

```
invalid.js:73:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:73:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -368,28 +294,11 @@ invalid.js:73:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
80 │ }
81 │
i Suggested fix: Use collapsed else if instead.
71 71 │ if (condition) {
72 72 │ // ...
73 │ - }·else·{
74 │ - → //·Comment
75 │ - → if·(anotherCondition)·{
73 │ + }·else·if·(anotherCondition)·{
76 74 │ // ...
77 75 │ } else {
78 76 │ // ...
79 │ - → }
80 │ - }
77 │ + → }
81 78 │
82 79 │ if (condition) {
```

```
invalid.js:84:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:84:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -406,27 +315,11 @@ invalid.js:84:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
90 │ }
91 │
i Suggested fix: Use collapsed else if instead.
82 82 │ if (condition) {
83 83 │ // ...
84 │ - }·else·{
85 │ - → if·(anotherCondition)·{
84 │ + }·else·if·(anotherCondition)·{
86 85 │ // ...
87 86 │ } else {
88 87 │ // ...
89 │ - → }·//·Comment
90 │ - }
88 │ + → }
91 89 │
92 90 │ if (condition) {
```

```
invalid.js:94:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:94:9 lint/nursery/useCollapsedElseIf ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This if statement can be collapsed into an else if statement.
Expand All @@ -443,22 +336,6 @@ invalid.js:94:9 lint/nursery/useCollapsedElseIf FIXABLE ━━━━━━━
100 │ // Comment
101 │ }
i Suggested fix: Use collapsed else if instead.
92 92 │ if (condition) {
93 93 │ // ...
94 │ - }·else·{
95 │ - → if·(anotherCondition)·{
94 │ + }·else·if·(anotherCondition)·{
96 95 │ // ...
97 96 │ } else {
98 97 │ // ...
99 │ - → }
100 │ - → //·Comment
101 │ - }
98 │ + → }
102 99 │
```

Expand Down
9 changes: 5 additions & 4 deletions website/src/content/docs/internals/changelog.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
export * as MY_NAMESPACE from "./lib.js";
```

- [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) now ignores decorated classes and decorated parameters.
The rule now gives suggestions instead of safe fixes when parameters are annotated with types.

- [useCollapsedElseIf](https://biomejs.dev/linter/rules/use-collapsed-else-if/) now only provides safe code fixes.

## 1.1.1 (2023-09-07)

### Analyzer
Expand Down Expand Up @@ -160,10 +165,6 @@ multiple servers in the `rage` output.
- You can use `// biome-ignore` as suppression comment.
- The `// rome-ignore` suppression is deprecated.


- [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) now ignores decorated classes and decorated parameters.
The rule now gives suggestions instead of safe fixes when parameters are annotated with types.

#### Bug fixes

- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) case-insensitive.
Expand Down
29 changes: 29 additions & 0 deletions website/src/content/docs/linter/rules/use-collapsed-else-if.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,35 @@ if (condition) {

</code></pre>

```jsx
if (condition) {
// ...
} else {
// Comment
if (anotherCondition) {
// ...
}
}
```

<pre class="language-text"><code class="language-text">nursery/useCollapsedElseIf.js:3:9 <a href="https://biomejs.dev/lint/rules/use-collapsed-else-if">lint/nursery/useCollapsedElseIf</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

<strong><span style="color: Orange;"> </span></strong><strong><span style="color: Orange;">⚠</span></strong> <span style="color: Orange;">This </span><span style="color: Orange;"><strong>if</strong></span><span style="color: Orange;"> statement can be collapsed into an </span><span style="color: Orange;"><strong>else if</strong></span><span style="color: Orange;"> statement.</span>

<strong>1 │ </strong>if (condition) {
<strong>2 │ </strong> // ...
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>3 │ </strong>} else {
<strong> │ </strong>
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>4 │ </strong> // Comment
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>5 │ </strong> if (anotherCondition) {
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>6 │ </strong> // ...
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>7 │ </strong> }
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>
<strong>8 │ </strong>}
<strong>9 │ </strong>

</code></pre>

### Valid

```jsx
Expand Down

0 comments on commit 319bb9f

Please sign in to comment.