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

Commit 388df4e

Browse files
xunilrjxunilrj
authored
fix(rome_js_analyzer): correctly delegate unresolved references to its parent scope (#4004)
* correctly delegate unresolved references to its parent scope
1 parent df86e5e commit 388df4e

File tree

6 files changed

+160
-19
lines changed

6 files changed

+160
-19
lines changed

‎crates/rome_js_analyze/tests/specs/style/noArguments.cjs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
function f() {
22
console.log(arguments);
3+
4+
for(let i = 0;i < arguments.length; ++i) {
5+
console.log(arguments[i]);
6+
}
37
}
48

59
function f() {

‎crates/rome_js_analyze/tests/specs/style/noArguments.cjs.snap‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ expression: noArguments.cjs
66
```js
77
function f() {
88
console.log(arguments);
9+
10+
for(let i = 0;i < arguments.length; ++i) {
11+
console.log(arguments[i]);
12+
}
913
}
1014
1115
function f() {
@@ -23,8 +27,41 @@ noArguments.cjs:2:17 lint/style/noArguments ━━━━━━━━━━━━
2327
1 │ function f() {
2428
> 2 │ console.log(arguments);
2529
│ ^^^^^^^^^
26-
3 │ }
27-
4 │
30+
3 │
31+
4 │ for(let i = 0;i < arguments.length; ++i) {
32+
33+
i arguments does not have Array.prototype methods and can be inconvenient to use.
34+
35+
36+
```
37+
38+
```
39+
noArguments.cjs:4:23 lint/style/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
40+
41+
! Use the rest parameters instead of arguments.
42+
43+
2 │ console.log(arguments);
44+
3 │
45+
> 4 │ for(let i = 0;i < arguments.length; ++i) {
46+
│ ^^^^^^^^^
47+
5 │ console.log(arguments[i]);
48+
6 │ }
49+
50+
i arguments does not have Array.prototype methods and can be inconvenient to use.
51+
52+
53+
```
54+
55+
```
56+
noArguments.cjs:5:21 lint/style/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
57+
58+
! Use the rest parameters instead of arguments.
59+
60+
4 │ for(let i = 0;i < arguments.length; ++i) {
61+
> 5 │ console.log(arguments[i]);
62+
│ ^^^^^^^^^
63+
6 │ }
64+
7 │ }
2865
2966
i arguments does not have Array.prototype methods and can be inconvenient to use.
3067

‎crates/rome_js_analyze/tests/suppression/correctness/noUndeclaredVariables.ts.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,27 @@ export type Invalid<S extends number> = `
1313
```
1414
1515
# Diagnostics
16+
```
17+
noUndeclaredVariables.ts:1:50 lint/correctness/noUndeclaredVariables FIXABLE ━━━━━━━━━━━━━━━━━━━━━
18+
19+
! The T variable is undeclared
20+
21+
> 1 │ export type Invalid<S extends number> = `Hello ${T}`
22+
│ ^
23+
2 │
24+
3 │ export type Invalid<S extends number> = `
25+
26+
i Safe fix: Suppress rule lint/correctness/noUndeclaredVariables
27+
28+
1 │ - export·type·Invalid<S·extends·number>·=·`Hello·${T}`
29+
1 │ + export·type·Invalid<S·extends·number>·=·`Hello·${//·rome-ignore·lint/correctness/noUndeclaredVariables:·<explanation>
30+
2 │ + T}`
31+
2 3 │
32+
3 4 │ export type Invalid<S extends number> = `
33+
34+
35+
```
36+
1637
```
1738
noUndeclaredVariables.ts:5:7 lint/correctness/noUndeclaredVariables FIXABLE ━━━━━━━━━━━━━━━━━━━━━━
1839

‎crates/rome_js_semantic/src/events.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ impl SemanticEventExtractor {
606606

607607
if let Some(scope) = self.scopes.pop() {
608608
// Match references and declarations
609-
for (name, references) in scope.references {
609+
for (name, mut references) in scope.references {
610610
// If we know the declaration of these reference push the correct events...
611611
if let Some(declaration_at) = self.bindings.get(&name) {
612612
for reference in references {
@@ -647,7 +647,8 @@ impl SemanticEventExtractor {
647647
}
648648
} else if let Some(parent) = self.scopes.last_mut() {
649649
// ... if not, promote these references to the parent scope ...
650-
parent.references.insert(name, references);
650+
let parent_references = parent.references.entry(name).or_default();
651+
parent_references.append(&mut references);
651652
} else {
652653
// ... or raise UnresolvedReference if this is the global scope.
653654
for reference in references {

‎crates/rome_js_semantic/src/tests/assertions.rs‎

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ struct UniqueAssertion {
238238
}
239239

240240
#[derive(Clone, Debug)]
241-
struct UnmatchedAssertion {
241+
struct UnresolvedReferenceAssertion {
242242
range: TextRange,
243243
}
244244

@@ -252,7 +252,7 @@ enum SemanticAssertion {
252252
AtScope(AtScopeAssertion),
253253
NoEvent(NoEventAssertion),
254254
Unique(UniqueAssertion),
255-
Unmatched(UnmatchedAssertion),
255+
UnresolvedReference(UnresolvedReferenceAssertion),
256256
}
257257

258258
impl SemanticAssertion {
@@ -335,9 +335,11 @@ impl SemanticAssertion {
335335
range: token.parent().unwrap().text_range(),
336336
}))
337337
} else if assertion_text.contains("/*?") {
338-
Some(SemanticAssertion::Unmatched(UnmatchedAssertion {
339-
range: token.parent().unwrap().text_range(),
340-
}))
338+
Some(SemanticAssertion::UnresolvedReference(
339+
UnresolvedReferenceAssertion {
340+
range: token.parent().unwrap().text_range(),
341+
},
342+
))
341343
} else {
342344
None
343345
}
@@ -354,7 +356,7 @@ struct SemanticAssertions {
354356
scope_end_assertions: Vec<ScopeEndAssertion>,
355357
no_events: Vec<NoEventAssertion>,
356358
uniques: Vec<UniqueAssertion>,
357-
unmatched: Vec<UnmatchedAssertion>,
359+
unresolved_references: Vec<UnresolvedReferenceAssertion>,
358360
}
359361

360362
impl SemanticAssertions {
@@ -367,7 +369,7 @@ impl SemanticAssertions {
367369
let mut scope_end_assertions = vec![];
368370
let mut no_events = vec![];
369371
let mut uniques = vec![];
370-
let mut unmatched = vec![];
372+
let mut unresolved_references = vec![];
371373

372374
for node in root
373375
.syntax()
@@ -419,8 +421,8 @@ impl SemanticAssertions {
419421
Some(SemanticAssertion::Unique(assertion)) => {
420422
uniques.push(assertion);
421423
}
422-
Some(SemanticAssertion::Unmatched(assertion)) => {
423-
unmatched.push(assertion);
424+
Some(SemanticAssertion::UnresolvedReference(assertion)) => {
425+
unresolved_references.push(assertion);
424426
}
425427
None => {}
426428
};
@@ -437,7 +439,7 @@ impl SemanticAssertions {
437439
scope_end_assertions,
438440
no_events,
439441
uniques,
440-
unmatched,
442+
unresolved_references,
441443
}
442444
}
443445

@@ -718,26 +720,94 @@ impl SemanticAssertions {
718720
}
719721
}
720722

721-
// Check every unmatched assertion
723+
// Check every unresolved_reference assertion
724+
let is_unresolved_reference =
725+
|e: &SemanticEvent| matches!(e, SemanticEvent::UnresolvedReference { .. });
722726

723-
for unmatched in self.unmatched.iter() {
724-
match events_by_pos.get(&unmatched.range.start()) {
727+
for unresolved_reference in self.unresolved_references.iter() {
728+
match events_by_pos.get(&unresolved_reference.range.start()) {
725729
Some(v) => {
726730
let ok = v
727731
.iter()
728732
.any(|e| matches!(e, SemanticEvent::UnresolvedReference { .. }));
729733
if !ok {
734+
show_all_events(test_name, code, events_by_pos, is_unresolved_reference);
735+
show_unmatched_assertion(
736+
test_name,
737+
code,
738+
unresolved_reference,
739+
unresolved_reference.range,
740+
);
730741
panic!("No UnresolvedReference event found");
731742
}
732743
}
733744
None => {
745+
show_all_events(test_name, code, events_by_pos, is_unresolved_reference);
746+
show_unmatched_assertion(
747+
test_name,
748+
code,
749+
unresolved_reference,
750+
unresolved_reference.range,
751+
);
734752
panic!("No UnresolvedReference event found");
735753
}
736754
}
737755
}
738756
}
739757
}
740758

759+
fn show_unmatched_assertion(
760+
test_name: &str,
761+
code: &str,
762+
assertion: &impl std::fmt::Debug,
763+
assertion_range: TextRange,
764+
) {
765+
let diagnostic = TestSemanticDiagnostic::new(
766+
format!("This assertion was not matched: {assertion:?}"),
767+
assertion_range,
768+
);
769+
let error = diagnostic
770+
.with_file_path((test_name.to_string(), FileId::zero()))
771+
.with_file_source_code(code);
772+
773+
let mut console = EnvConsole::default();
774+
console.log(markup! {
775+
{PrintDiagnostic::verbose(&error)}
776+
});
777+
}
778+
779+
fn show_all_events<F>(
780+
test_name: &str,
781+
code: &str,
782+
events_by_pos: HashMap<TextSize, Vec<SemanticEvent>>,
783+
f: F,
784+
) where
785+
F: Fn(&SemanticEvent) -> bool,
786+
{
787+
let mut console = EnvConsole::default();
788+
let mut all_events = vec![];
789+
for (_, events) in events_by_pos {
790+
for e in events {
791+
if f(&e) {
792+
all_events.push(e);
793+
}
794+
}
795+
}
796+
797+
all_events.sort_by_key(|l| l.range().start());
798+
799+
for e in all_events {
800+
let diagnostic = TestSemanticDiagnostic::new(format!("{e:?}"), e.range());
801+
let error = diagnostic
802+
.with_file_path((test_name.to_string(), FileId::zero()))
803+
.with_file_source_code(code);
804+
805+
console.log(markup! {
806+
{PrintDiagnostic::verbose(&error)}
807+
});
808+
}
809+
}
810+
741811
fn error_assertion_not_attached_to_a_declaration(
742812
code: &str,
743813
assertion_range: TextRange,

‎crates/rome_js_semantic/src/tests/references.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,16 @@ assert_semantics! {
194194
}
195195

196196
assert_semantics! {
197-
ok_unmatched_reference, r#"a/*?*/"#,
198-
ok_function_expression_read,"let f/*#F*/ = function g/*#G*/(){}; g/*?*/();",
197+
ok_unresolved_reference, r#"a/*?*/"#,
198+
ok_unresolved_function_expression_read,"let f/*#F*/ = function g/*#G*/(){}; g/*?*/();",
199+
ok_unresolved_reference_arguments,
200+
r#"function f() {
201+
console.log(arguments/*?*/);
202+
203+
for(let i = 0;i < arguments/*?*/.length; ++i) {
204+
console.log(arguments/*?*/[i]);
205+
}
206+
}"#,
199207
}
200208

201209
// Exports

0 commit comments

Comments
 (0)