Skip to content

Commit

Permalink
refactor(js_semantic): reduce semantic data size and make some cleanup (
Browse files Browse the repository at this point in the history 
#3417)
  • Loading branch information
@Conaclos
Conaclos authored Jul 11, 2024
1 parent 4284b19 commit 48387f6
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 164 deletions.
65 changes: 32 additions & 33 deletions crates/biome_js_analyze/src/lint/correctness/no_invalid_use_before_declaration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ impl Rule for NoInvalidUseBeforeDeclaration {
None
};
for reference in binding.all_references() {
let reference_range = reference.range();
if reference_range.start() < declaration_end
if reference.range_start() < declaration_end {
let reference_syntax = reference.syntax();
// References that are exports, such as `export { a }` are always valid,
// even when they appear before the declaration.
// For example:
Expand All @@ -119,41 +119,40 @@ impl Rule for NoInvalidUseBeforeDeclaration {
// export { X };
// const X = 0;
// ```
&& reference
.syntax()
if reference_syntax
.parent()
.kind()
.filter(|parent_kind| AnyJsExportNamedSpecifier::can_cast(*parent_kind))
.is_none()
// Don't report variables used in another control flow root (function, classes, ...)
// For example:
//
// ```js
// function f() { X; }
// const X = 0;
// ```
&& (declaration_control_flow_root.is_none() ||
declaration_control_flow_root == reference
.syntax()
.ancestors()
.skip(1)
.find(|ancestor| AnyJsControlFlowRoot::can_cast(ancestor.kind()))
)
// ignore when used as a type.
// For example:
//
// ```js
// type Y = typeof X;
// const X = 0;
// ```
&& !AnyJsIdentifierUsage::cast_ref(reference.syntax())
.is_some_and(|usage| usage.is_only_type())
{
result.push(InvalidUseBeforeDeclaration {
declaration_kind,
reference_range: *reference_range,
binding_range: id.range(),
});
// Don't report variables used in another control flow root (function, classes, ...)
// For example:
//
// ```js
// function f() { X; }
// const X = 0;
// ```
&& (declaration_control_flow_root.is_none() ||
declaration_control_flow_root == reference_syntax
.ancestors()
.skip(1)
.find(|ancestor| AnyJsControlFlowRoot::can_cast(ancestor.kind()))
)
// ignore when used as a type.
// For example:
//
// ```js
// type Y = typeof X;
// const X = 0;
// ```
&& !AnyJsIdentifierUsage::cast_ref(reference_syntax)
.is_some_and(|usage| usage.is_only_type())
{
result.push(InvalidUseBeforeDeclaration {
declaration_kind,
reference_range: reference_syntax.text_trimmed_range(),
binding_range: id.range(),
});
}
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/biome_js_analyze/src/lint/style/no_arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ impl Rule for NoArguments {
for unresolved_reference in model.all_unresolved_references() {
let name = unresolved_reference.syntax().text_trimmed();
if name == "arguments" {
let range = unresolved_reference.range();
found_arguments.push(*range);
found_arguments.push(unresolved_reference.range());
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_analyze/src/lint/style/use_for_of.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn list_initializer_references(
.all_references(model)
.filter_map(|reference| {
// We only want references within this block / for body
if reference.range().start() < body_range.start() {
if reference.range_start() < body_range.start() {
return None;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_analyze/src/lint/suspicious/no_global_assign.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Rule for NoGlobalAssign {
let text = identifier.to_string();
let is_global_var = is_js_global(text.as_str());
if is_global_var {
result.push(*global_ref.range());
result.push(global_ref.range());
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/biome_js_analyze/src/react/hooks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ impl ReactCallWithDependencyResult {
.and_then(|node| AnyJsFunctionExpression::try_from(node.clone()).ok())
.map(|function_expression| {
let closure = function_expression.closure(model);
let range = *closure.closure_range();
let range = closure.closure_range();
closure
.descendents()
.flat_map(|closure| closure.all_captures())
.filter(move |capture| capture.declaration_range().intersect(range).is_none())
.filter(move |capture| !range.contains(capture.declaration_range().start()))
})
.into_iter()
.flatten()
Expand Down
10 changes: 5 additions & 5 deletions crates/biome_js_semantic/src/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,12 @@ impl Reference {
}

/// Range of the referenced binding
const fn range(&self) -> &TextRange {
const fn range(&self) -> TextRange {
match self {
Self::Export(range)
| Self::Read(range)
| Self::AmbientRead(range)
| Self::Write(range) => range,
| Self::Write(range) => *range,
}
}
}
Expand Down Expand Up @@ -849,7 +849,7 @@ impl SemanticEventExtractor {
// because an import namespace is already in the root scope.
self.stash.push_back(SemanticEvent::UnresolvedReference {
is_read: !reference.is_write(),
range: *reference.range(),
range: reference.range(),
});
continue;
}
Expand Down Expand Up @@ -927,7 +927,7 @@ impl SemanticEventExtractor {
_ => {
self.stash.push_back(SemanticEvent::UnresolvedReference {
is_read: !reference.is_write(),
range: *reference.range(),
range: reference.range(),
});
}
}
Expand All @@ -937,7 +937,7 @@ impl SemanticEventExtractor {
for reference in references {
self.stash.push_back(SemanticEvent::UnresolvedReference {
is_read: !reference.is_write(),
range: *reference.range(),
range: reference.range(),
});
}
}
Expand Down
42 changes: 21 additions & 21 deletions crates/biome_js_semantic/src/semantic_model/binding.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ use biome_js_syntax::{binding_ext::AnyJsIdentifierBinding, TextRange, TsTypePara
/// Internal type with all the semantic data of a specific binding
#[derive(Debug)]
pub(crate) struct SemanticModelBindingData {
pub id: BindingId,
pub range: TextRange,
pub references: Vec<SemanticModelReference>,
pub(crate) range: TextRange,
pub(crate) references: Vec<SemanticModelReference>,
}

#[derive(Clone, Copy, Debug)]
Expand All @@ -18,8 +17,7 @@ pub(crate) enum SemanticModelReferenceType {
/// Internal type with all the semantic data of a specific reference
#[derive(Debug)]
pub(crate) struct SemanticModelReference {
pub(crate) id: ReferenceId,
pub(crate) range: TextRange,
pub(crate) range_start: TextSize,
pub(crate) ty: SemanticModelReferenceType,
}

Expand Down Expand Up @@ -58,10 +56,9 @@ impl Binding {
/// Returns the scope of this binding
pub fn scope(&self) -> Scope {
let binding = self.data.binding(self.id);
let id = self.data.scope(&binding.range); //TODO declaration can have its scope id
Scope {
data: self.data.clone(),
id,
id: self.data.scope(binding.range),
}
}

Expand All @@ -73,19 +70,20 @@ impl Binding {

/// Returns the typed AST node associated with this binding.
pub fn tree(&self) -> AnyJsIdentifierBinding {
let node = self.syntax();
let binding = AnyJsIdentifierBinding::cast_ref(node);
debug_assert!(binding.is_some());
binding.unwrap()
AnyJsIdentifierBinding::unwrap_cast(self.syntax().clone())
}

/// Returns an iterator to all references of this binding.
pub fn all_references(&self) -> AllBindingReferencesIter {
let binding = self.data.binding(self.id);
let first = binding.references.first().map(|reference| Reference {
data: self.data.clone(),
index: reference.id,
});
let first = if binding.references.is_empty() {
None
} else {
Some(Reference {
data: self.data.clone(),
id: ReferenceId::new(self.id, 0),
})
};
std::iter::successors(first, Reference::find_next)
}

Expand All @@ -95,10 +93,11 @@ impl Binding {
let first = binding
.references
.iter()
.find(|x| x.is_read())
.map(|reference| Reference {
.enumerate()
.find(|(_, x)| x.is_read())
.map(|(index, _)| Reference {
data: self.data.clone(),
index: reference.id,
id: ReferenceId::new(self.id, index),
});
std::iter::successors(first, Reference::find_next_read)
}
Expand All @@ -109,10 +108,11 @@ impl Binding {
let first = binding
.references
.iter()
.find(|x| x.is_write())
.map(|reference| Reference {
.enumerate()
.find(|(_, x)| x.is_write())
.map(|(index, _)| Reference {
data: self.data.clone(),
index: reference.id,
id: ReferenceId::new(self.id, index),
});
std::iter::successors(first, Reference::find_next_write)
}
Expand Down
32 changes: 15 additions & 17 deletions crates/biome_js_semantic/src/semantic_model/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ impl SemanticModelBuilder {

let binding_id = BindingId::new(self.bindings.len());
self.bindings.push(SemanticModelBindingData {
id: binding_id,
range,
references: vec![],
});
Expand Down Expand Up @@ -203,10 +202,9 @@ impl SemanticModelBuilder {
} => {
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id.index()];
let reference_id = ReferenceId::new(binding.id, binding.references.len());
let reference_id = ReferenceId::new(binding_id, binding.references.len());
binding.references.push(SemanticModelReference {
id: reference_id,
range,
range_start: range.start(),
ty: SemanticModelReferenceType::Read { hoisted: false },
});

Expand All @@ -222,10 +220,9 @@ impl SemanticModelBuilder {
} => {
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id.index()];
let reference_id = ReferenceId::new(binding.id, binding.references.len());
let reference_id = ReferenceId::new(binding_id, binding.references.len());
binding.references.push(SemanticModelReference {
id: reference_id,
range,
range_start: range.start(),
ty: SemanticModelReferenceType::Read { hoisted: true },
});

Expand All @@ -241,10 +238,9 @@ impl SemanticModelBuilder {
} => {
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id.index()];
let reference_id = ReferenceId::new(binding.id, binding.references.len());
let reference_id = ReferenceId::new(binding_id, binding.references.len());
binding.references.push(SemanticModelReference {
id: reference_id,
range,
range_start: range.start(),
ty: SemanticModelReferenceType::Write { hoisted: false },
});

Expand All @@ -260,10 +256,9 @@ impl SemanticModelBuilder {
} => {
let binding_id = self.bindings_by_start[&declaration_at];
let binding = &mut self.bindings[binding_id.index()];
let reference_id = ReferenceId::new(binding.id, binding.references.len());
let reference_id = ReferenceId::new(binding_id, binding.references.len());
binding.references.push(SemanticModelReference {
id: reference_id,
range,
range_start: range.start(),
ty: SemanticModelReferenceType::Write { hoisted: true },
});

Expand All @@ -287,15 +282,18 @@ impl SemanticModelBuilder {
let entry = entry.get_mut();
match entry {
Some(index) => {
self.globals[(*index) as usize]
.references
.push(SemanticModelGlobalReferenceData { range, ty });
self.globals[(*index) as usize].references.push(
SemanticModelGlobalReferenceData {
range_start: range.start(),
ty,
},
);
}
None => {
let id = self.globals.len() as u32;
self.globals.push(SemanticModelGlobalBindingData {
references: vec![SemanticModelGlobalReferenceData {
range,
range_start: range.start(),
ty,
}],
});
Expand Down
Loading

0 comments on commit 48387f6

Please sign in to comment.