Skip to content

Commit

Permalink
refactor(js_semantic): use range start to identify the declaration in…
Browse files Browse the repository at this point in the history 
… read/write (#3404)
  • Loading branch information
@Conaclos
Conaclos committed Jul 10, 2024
1 parent b791bb3 commit 08f0c8f
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 79 deletions.
56 changes: 32 additions & 24 deletions crates/biome_js_semantic/src/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use biome_js_syntax::{
TsTypeParameterName,
};
use biome_js_syntax::{AnyJsImportClause, AnyJsNamedImportSpecifier, AnyTsType};
use biome_rowan::TextSize;
use biome_rowan::{syntax::Preorder, AstNode, SyntaxNodeOptionExt, TokenText};
use rustc_hash::FxHashMap;
use std::collections::VecDeque;
Expand Down Expand Up @@ -33,7 +34,7 @@ pub enum SemanticEvent {
/// - All reference identifiers
Read {
range: TextRange,
declared_at: TextRange,
declaration_at: TextSize,
scope_id: u32,
},

Expand All @@ -42,7 +43,7 @@ pub enum SemanticEvent {
/// - All reference identifiers
HoistedRead {
range: TextRange,
declared_at: TextRange,
declaration_at: TextSize,
scope_id: u32,
},

Expand All @@ -51,7 +52,7 @@ pub enum SemanticEvent {
/// - All identifier assignments
Write {
range: TextRange,
declared_at: TextRange,
declaration_at: TextSize,
scope_id: u32,
},

Expand All @@ -61,7 +62,7 @@ pub enum SemanticEvent {
/// - All identifier assignments
HoistedWrite {
range: TextRange,
declared_at: TextRange,
declaration_at: TextSize,
scope_id: u32,
},

Expand Down Expand Up @@ -94,7 +95,10 @@ pub enum SemanticEvent {

/// Tracks where a symbol is exported.
/// The range points to the binding that is being exported.
Exported { range: TextRange },
Export {
range: TextRange,
declaration_at: TextSize,
},
}

impl SemanticEvent {
Expand All @@ -108,7 +112,7 @@ impl SemanticEvent {
| Self::Write { range, .. }
| Self::HoistedWrite { range, .. }
| Self::UnresolvedReference { range, .. }
| Self::Exported { range } => *range,
| Self::Export { range, .. } => *range,
}
}
}
Expand Down Expand Up @@ -537,14 +541,16 @@ impl SemanticEventExtractor {
false
};
let scope_id = self.current_scope_mut().scope_id;
let range = node.syntax().text_trimmed_range();
self.stash.push_back(SemanticEvent::DeclarationFound {
scope_id,
hoisted_scope_id,
range: node.syntax().text_trimmed_range(),
range,
});
if is_exported {
self.stash.push_back(SemanticEvent::Exported {
range: node.syntax().text_trimmed_range(),
self.stash.push_back(SemanticEvent::Export {
range,
declaration_at: range.start(),
});
}
}
Expand Down Expand Up @@ -804,28 +810,30 @@ impl SemanticEventExtractor {
// Bind references to declarations
for (name, mut references) in scope.references {
if let Some(&BindingInfo {
range: declared_at,
range: declaration_range,
declaration_kind,
}) = self.bindings.get(&name)
{
let declaration_at = declaration_range.start();
// We know the declaration of these reference.
for reference in references {
let declaration_before_reference =
declared_at.start() < reference.range().start();
let declaration_before_reference = declaration_at < reference.range().start();
let event = match reference {
Reference::Export(range) => {
self.stash
.push_back(SemanticEvent::Exported { range: declared_at });
self.stash.push_back(SemanticEvent::Export {
range,
declaration_at,
});
if declaration_before_reference {
SemanticEvent::Read {
range,
declared_at,
declaration_at,
scope_id,
}
} else {
SemanticEvent::HoistedRead {
range,
declared_at,
declaration_at,
scope_id,
}
}
Expand All @@ -848,13 +856,13 @@ impl SemanticEventExtractor {
if declaration_before_reference {
SemanticEvent::Read {
range,
declared_at,
declaration_at,
scope_id,
}
} else {
SemanticEvent::HoistedRead {
range,
declared_at,
declaration_at,
scope_id,
}
}
Expand All @@ -863,13 +871,13 @@ impl SemanticEventExtractor {
if declaration_before_reference {
SemanticEvent::Write {
range,
declared_at,
declaration_at,
scope_id,
}
} else {
SemanticEvent::HoistedWrite {
range,
declared_at,
declaration_at,
scope_id,
}
}
Expand Down Expand Up @@ -898,19 +906,19 @@ impl SemanticEventExtractor {
Reference::AmbientRead(range) if info.is_imported() => {
// An ambient read can only read a value,
// but also an imported value as a type (with the `type` modifier)
let declared_at = info.range;
let declaration_at = info.range.start();
let declaration_before_reference =
declared_at.start() < reference.range().start();
declaration_at < reference.range().start();
let event = if declaration_before_reference {
SemanticEvent::Read {
range,
declared_at,
declaration_at,
scope_id: 0,
}
} else {
SemanticEvent::HoistedRead {
range,
declared_at,
declaration_at,
scope_id: 0,
}
};
Expand Down
22 changes: 11 additions & 11 deletions crates/biome_js_semantic/src/semantic_model/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ impl SemanticModelBuilder {
}
Read {
range,
declared_at: declaration_at, //TODO change to binding_id like we do with scope_id
declaration_at,
scope_id,
} => {
let binding_id = self.bindings_by_start[&declaration_at.start()];
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id as usize];
let reference_index = binding.references.len() as u32;

Expand All @@ -224,13 +224,13 @@ impl SemanticModelBuilder {
}
HoistedRead {
range,
declared_at: declaration_at,
declaration_at,
scope_id,
} => {
let binding_id = self.bindings_by_start[&declaration_at.start()];
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id as usize];

let reference_index = binding.references.len() as u32;

binding.references.push(SemanticModelReference {
index: (binding.id, reference_index).into(),
range,
Expand All @@ -247,10 +247,10 @@ impl SemanticModelBuilder {
}
Write {
range,
declared_at: declaration_at,
declaration_at,
scope_id,
} => {
let binding_id = self.bindings_by_start[&declaration_at.start()];
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id as usize];

let reference_index = binding.references.len() as u32;
Expand All @@ -270,10 +270,10 @@ impl SemanticModelBuilder {
}
HoistedWrite {
range,
declared_at: declaration_at,
declaration_at,
scope_id,
} => {
let binding_id = self.bindings_by_start[&declaration_at.start()];
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id as usize];

let reference_index = binding.references.len() as u32;
Expand Down Expand Up @@ -327,8 +327,8 @@ impl SemanticModelBuilder {
.push(SemanticModelUnresolvedReference { range }),
}
}
Exported { range } => {
self.exported.insert(range.start());
Export { declaration_at, .. } => {
self.exported.insert(declaration_at);
}
}
}
Expand Down
72 changes: 41 additions & 31 deletions crates/biome_js_semantic/src/tests/assertions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,27 @@ pub fn assert(code: &str, test_name: &str) {
// Extract semantic events and index by range

let mut events_by_pos: FxHashMap<TextSize, Vec<SemanticEvent>> = FxHashMap::default();
let mut scope_start_by_id: FxHashMap<u32, TextSize> = FxHashMap::default();
let mut declaration_range_by_start: FxHashMap<TextSize, TextRange> = FxHashMap::default();
let mut scope_range_by_id: FxHashMap<u32, TextRange> = FxHashMap::default();
for event in semantic_events(r.syntax()) {
let pos = if let SemanticEvent::ScopeEnded {
range, scope_id, ..
} = event
{
scope_start_by_id.insert(scope_id, range.start());
range.end()
} else {
event.range().start()
let pos = match event {
SemanticEvent::DeclarationFound { range, .. } => {
declaration_range_by_start.insert(range.start(), range);
range.start()
}
SemanticEvent::ScopeStarted {
range, scope_id, ..
} => {
scope_range_by_id.insert(scope_id, range);
range.start()
}
SemanticEvent::Read { range, .. }
| SemanticEvent::HoistedRead { range, .. }
| SemanticEvent::Write { range, .. }
| SemanticEvent::HoistedWrite { range, .. }
| SemanticEvent::UnresolvedReference { range, .. } => range.start(),
SemanticEvent::ScopeEnded { range, .. } => range.end(),
SemanticEvent::Export { .. } => continue,
};
let v = events_by_pos.entry(pos).or_default();
v.push(event);
Expand All @@ -143,7 +154,13 @@ pub fn assert(code: &str, test_name: &str) {

// check

assertions.check(code, test_name, events_by_pos, scope_start_by_id);
assertions.check(
code,
test_name,
events_by_pos,
declaration_range_by_start,
scope_range_by_id,
);
}

#[derive(Debug, Diagnostic)]
Expand Down Expand Up @@ -447,7 +464,8 @@ impl SemanticAssertions {
code: &str,
test_name: &str,
events_by_pos: FxHashMap<TextSize, Vec<SemanticEvent>>,
scope_start: FxHashMap<u32, TextSize>,
declaration_range_by_start: FxHashMap<TextSize, TextRange>,
scope_range_by_id: FxHashMap<u32, TextRange>,
) {
// Check every declaration assertion is ok

Expand Down Expand Up @@ -503,23 +521,19 @@ impl SemanticAssertions {
let mut unused_match = None;
let at_least_one_match = events.iter().any(|e| {
let declaration_at_range = match &e {
SemanticEvent::Read {
declared_at: declaration_at,
..
} => Some(*declaration_at),
SemanticEvent::HoistedRead {
declared_at: declaration_at,
..
} => Some(*declaration_at),
SemanticEvent::Read { declaration_at, .. }
| SemanticEvent::HoistedRead { declaration_at, .. } => {
declaration_range_by_start.get(declaration_at)
}
_ => None,
};

if let Some(declaration_at_range) = declaration_at_range {
unused_match = Some(format!(
"{} != {}",
&code[declaration_at_range], &code[decl.range]
&code[*declaration_at_range], &code[decl.range]
));
code[declaration_at_range] == code[decl.range]
code[*declaration_at_range] == code[decl.range]
} else {
false
}
Expand Down Expand Up @@ -567,19 +581,15 @@ impl SemanticAssertions {

let at_least_one_match = events.iter().any(|e| {
let declaration_at_range = match &e {
SemanticEvent::Write {
declared_at: declaration_at,
..
} => Some(*declaration_at),
SemanticEvent::HoistedWrite {
declared_at: declaration_at,
..
} => Some(*declaration_at),
SemanticEvent::Write { declaration_at, .. }
| SemanticEvent::HoistedWrite { declaration_at, .. } => {
declaration_range_by_start.get(declaration_at)
}
_ => None,
};

if let Some(declaration_at_range) = declaration_at_range {
code[declaration_at_range] == code[decl.range]
code[*declaration_at_range] == code[decl.range]
} else {
false
}
Expand Down Expand Up @@ -614,7 +624,7 @@ impl SemanticAssertions {
{
Some(scope_start_assertion) => {
let scope_started_at =
&scope_start[&hoisted_scope_id.unwrap_or(*scope_id)];
&scope_range_by_id[&hoisted_scope_id.unwrap_or(*scope_id)].start();
if scope_start_assertion.range.start() != *scope_started_at {
show_all_events(test_name, code, events_by_pos, is_scope_event);
show_unmatched_assertion(
Expand Down
Loading

0 comments on commit 08f0c8f

Please sign in to comment.