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

Commit 0deb4d7

Browse files
kaioduartekaioduarte
committed
chore: respond PR feedback
1 parent 76110b6 commit 0deb4d7

File tree

3 files changed

+171
-39
lines changed

3 files changed

+171
-39
lines changed

‎crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use rome_analyze::context::RuleContext;
22
use rome_analyze::{declare_rule, Rule, RuleDiagnostic};
3-
use rome_js_semantic::ReferencesExtensions;
4-
use rome_js_syntax::{AnyJsClass, JsIdentifierBinding};
5-
use rome_rowan::AstNode;
3+
use rome_console::markup;
4+
use rome_js_semantic::{Reference, ReferencesExtensions};
5+
use rome_js_syntax::AnyJsClass;
66

77
use crate::semantic_services::Semantic;
88

@@ -74,28 +74,43 @@ declare_rule! {
7474

7575
impl Rule for NoClassAssign {
7676
type Query = Semantic<AnyJsClass>;
77-
type State = JsIdentifierBinding;
78-
type Signals = Option<Self::State>;
77+
type State = Reference;
78+
type Signals = Vec<Self::State>;
7979
type Options = ();
8080

8181
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
8282
let node = ctx.query();
8383
let model = ctx.model();
84-
let binding = node.id().ok()??;
85-
let binding = binding.as_js_identifier_binding()?;
8684

87-
if binding.all_writes(model).count() > 0 {
88-
Some(binding.clone())
89-
} else {
90-
None
85+
if let Ok(Some(id)) = node.id() {
86+
if let Some(id_binding) = id.as_js_identifier_binding() {
87+
return id_binding.all_writes(model).collect();
88+
}
9189
}
90+
91+
Vec::new()
9292
}
9393

94-
fn diagnostic(_: &RuleContext<Self>, class_id: &Self::State) -> Option<RuleDiagnostic> {
95-
Some(RuleDiagnostic::new(
96-
rule_category!(),
97-
class_id.range(),
98-
"Don't reassign classes.",
99-
))
94+
fn diagnostic(ctx: &RuleContext<Self>, reference: &Self::State) -> Option<RuleDiagnostic> {
95+
let binding = ctx
96+
.query()
97+
.id()
98+
.ok()??
99+
.as_js_identifier_binding()?
100+
.name_token()
101+
.ok()?;
102+
let class_name = binding.text_trimmed();
103+
104+
Some(
105+
RuleDiagnostic::new(
106+
rule_category!(),
107+
reference.syntax().text_trimmed_range(),
108+
markup! {"'"{class_name}"' is a class."},
109+
)
110+
.detail(
111+
binding.text_trimmed_range(),
112+
markup! {"'"{class_name}"' is defined here."},
113+
),
114+
)
100115
}
101116
}

‎crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap‎

Lines changed: 99 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,18 @@ function case18() {
120120

121121
# Diagnostics
122122
```
123-
noClassAssign.js:71:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
123+
noClassAssign.js:72:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
124124
125-
! Don't reassign classes.
125+
! 'A' is a class.
126+
127+
70 │ function case12() {
128+
71 │ class A { }
129+
> 72 │ A = 0;
130+
│ ^
131+
73 │ }
132+
74 │
133+
134+
i 'A' is defined here.
126135
127136
69 │ // /* Invalid */
128137
70 │ function case12() {
@@ -135,9 +144,18 @@ noClassAssign.js:71:8 lint/nursery/noClassAssign ━━━━━━━━━━
135144
```
136145

137146
```
138-
noClassAssign.js:76:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
147+
noClassAssign.js:77:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
139148
140-
! Don't reassign classes.
149+
! 'B' is a class.
150+
151+
75 │ function case13() {
152+
76 │ class B { }
153+
> 77 │ ({B} = 0);
154+
│ ^
155+
78 │ }
156+
79 │
157+
158+
i 'B' is defined here.
141159
142160
75 │ function case13() {
143161
> 76 │ class B { }
@@ -149,9 +167,18 @@ noClassAssign.js:76:8 lint/nursery/noClassAssign ━━━━━━━━━━
149167
```
150168

151169
```
152-
noClassAssign.js:81:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
170+
noClassAssign.js:82:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
153171
154-
! Don't reassign classes.
172+
! 'C' is a class.
173+
174+
80 │ function case14() {
175+
81 │ class C { }
176+
> 82 │ ({b: C = 0} = {});
177+
│ ^
178+
83 │ }
179+
84 │
180+
181+
i 'C' is defined here.
155182
156183
80 │ function case14() {
157184
> 81 │ class C { }
@@ -163,9 +190,17 @@ noClassAssign.js:81:8 lint/nursery/noClassAssign ━━━━━━━━━━
163190
```
164191

165192
```
166-
noClassAssign.js:87:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
193+
noClassAssign.js:86:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
167194
168-
! Don't reassign classes.
195+
! 'D' is a class.
196+
197+
85 │ function case15() {
198+
> 86 │ D = 0;
199+
│ ^
200+
87 │ class D { }
201+
88 │ }
202+
203+
i 'D' is defined here.
169204
170205
85 │ function case15() {
171206
86 │ D = 0;
@@ -178,9 +213,18 @@ noClassAssign.js:87:8 lint/nursery/noClassAssign ━━━━━━━━━━
178213
```
179214

180215
```
181-
noClassAssign.js:91:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
216+
noClassAssign.js:93:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
182217
183-
! Don't reassign classes.
218+
! 'E' is a class.
219+
220+
91 │ class E {
221+
92 │ b() {
222+
> 93 │ E = 0;
223+
│ ^
224+
94 │ }
225+
95 │ }
226+
227+
i 'E' is defined here.
184228
185229
90 │ function case16() {
186230
> 91 │ class E {
@@ -192,9 +236,18 @@ noClassAssign.js:91:8 lint/nursery/noClassAssign ━━━━━━━━━━
192236
```
193237
194238
```
195-
noClassAssign.js:99:16 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
239+
noClassAssign.js:101:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
196240
197-
! Don't reassign classes.
241+
! 'F' is a class.
242+
243+
99 │ let F = class F {
244+
100 │ b() {
245+
> 101 │ F = 0;
246+
│ ^
247+
102 │ }
248+
103 │ }
249+
250+
i 'F' is defined here.
198251
199252
98 │ function case17() {
200253
> 99 │ let F = class F {
@@ -206,9 +259,41 @@ noClassAssign.js:99:16 lint/nursery/noClassAssign ━━━━━━━━━━
206259
```
207260
208261
```
209-
noClassAssign.js:107:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
262+
noClassAssign.js:108:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
263+
264+
! 'G' is a class.
265+
266+
106 │ function case18() {
267+
107 │ class G { }
268+
> 108 │ G = 0;
269+
│ ^
270+
109 │ G = 1;
271+
110 │ }
272+
273+
i 'G' is defined here.
274+
275+
106 │ function case18() {
276+
> 107 │ class G { }
277+
│ ^
278+
108 │ G = 0;
279+
109 │ G = 1;
280+
281+
282+
```
283+
284+
```
285+
noClassAssign.js:109:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
210286
211-
! Don't reassign classes.
287+
! 'G' is a class.
288+
289+
107 │ class G { }
290+
108 │ G = 0;
291+
> 109 │ G = 1;
292+
│ ^
293+
110 │ }
294+
111 │
295+
296+
i 'G' is defined here.
212297
213298
106 │ function case18() {
214299
> 107 │ class G { }

‎website/src/pages/lint/rules/noClassAssign.md‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,16 @@ class A {}
1818
A = 0;
1919
```
2020

21-
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:1:7 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
21+
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:2:1 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
2222

23-
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">Don't reassign classes.</span>
23+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">'A' is a class.</span>
24+
25+
<strong>1 │ </strong>class A {}
26+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>2 │ </strong>A = 0;
27+
<strong> │ </strong><strong><span style="color: Tomato;">^</span></strong>
28+
<strong>3 │ </strong>
29+
30+
<strong><span style="color: rgb(38, 148, 255);"> </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">'A' is defined here.</span>
2431

2532
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>1 │ </strong>class A {}
2633
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>
@@ -34,9 +41,16 @@ A = 0;
3441
class A {}
3542
```
3643

37-
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:2:7 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
44+
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:1:1 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
3845

39-
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">Don't reassign classes.</span>
46+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">'A' is a class.</span>
47+
48+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>1 │ </strong>A = 0;
49+
<strong> │ </strong><strong><span style="color: Tomato;">^</span></strong>
50+
<strong>2 │ </strong>class A {}
51+
<strong>3 │ </strong>
52+
53+
<strong><span style="color: rgb(38, 148, 255);"> </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">'A' is defined here.</span>
4054

4155
<strong>1 │ </strong>A = 0;
4256
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>2 │ </strong>class A {}
@@ -53,9 +67,18 @@ class A {
5367
}
5468
```
5569

56-
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:1:7 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
70+
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:3:3 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
5771

58-
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">Don't reassign classes.</span>
72+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">'A' is a class.</span>
73+
74+
<strong>1 │ </strong>class A {
75+
<strong>2 │ </strong> b() {
76+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>3 │ </strong> A = 0;
77+
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>
78+
<strong>4 │ </strong> }
79+
<strong>5 │ </strong>}
80+
81+
<strong><span style="color: rgb(38, 148, 255);"> </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">'A' is defined here.</span>
5982

6083
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>1 │ </strong>class A {
6184
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>
@@ -73,9 +96,18 @@ let A = class A {
7396
}
7497
```
7598

76-
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:1:15 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
99+
<pre class="language-text"><code class="language-text">nursery/noClassAssign.js:3:3 <a href="https://docs.rome.tools/lint/rules/noClassAssign">lint/nursery/noClassAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
77100

78-
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">Don't reassign classes.</span>
101+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">'A' is a class.</span>
102+
103+
<strong>1 │ </strong>let A = class A {
104+
<strong>2 │ </strong> b() {
105+
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>3 │ </strong> A = 0;
106+
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>
107+
<strong>4 │ </strong> // `let A` is shadowed by the class name.
108+
<strong>5 │ </strong> }
109+
110+
<strong><span style="color: rgb(38, 148, 255);"> </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">'A' is defined here.</span>
79111

80112
<strong><span style="color: Tomato;"> </span></strong><strong><span style="color: Tomato;">&gt;</span></strong> <strong>1 │ </strong>let A = class A {
81113
<strong> │ </strong> <strong><span style="color: Tomato;">^</span></strong>

0 commit comments

Comments
 (0)