Skip to content

Commit

Permalink
refactor(semantic): use LabelBuilder instead of UnusedLabeled (#2184)
Browse files Browse the repository at this point in the history 
I think `UnusedLabeled` can do more than that.

1. Collect unused label
2. Support check duplication label
3. Support check label in `BreakStatement`
4. Support check label in `ContinueStatement` (Not yet)

But then the `UnusedLabeled` name wouldn't fit, so I renamed it
`LabelBuilder` and moved it to `label.rs`
  • Loading branch information
@Dunqing
Dunqing authored Jan 29, 2024
1 parent 972be83 commit 56adfb1
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 96 deletions.
65 changes: 20 additions & 45 deletions crates/oxc_semantic/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::{
},
diagnostics::Redeclaration,
jsdoc::JSDocBuilder,
label::LabelBuilder,
module_record::ModuleRecordBuilder,
node::{AstNode, AstNodeId, AstNodes, NodeFlags},
pg::replicate_tree_to_leaves,
Expand All @@ -28,18 +29,6 @@ use crate::{
Semantic,
};

pub struct LabeledScope<'a> {
name: &'a str,
used: bool,
parent: usize,
}

struct UnusedLabels<'a> {
scopes: Vec<LabeledScope<'a>>,
curr_scope: usize,
labels: Vec<AstNodeId>,
}

#[derive(Debug, Clone)]
pub struct VariableInfo {
pub span: Span,
Expand Down Expand Up @@ -83,7 +72,7 @@ pub struct SemanticBuilder<'a> {

pub(crate) module_record: Arc<ModuleRecord>,

unused_labels: UnusedLabels<'a>,
pub label_builder: LabelBuilder<'a>,

jsdoc: JSDocBuilder<'a>,

Expand Down Expand Up @@ -123,7 +112,7 @@ impl<'a> SemanticBuilder<'a> {
scope,
symbols: SymbolTable::default(),
module_record: Arc::new(ModuleRecord::default()),
unused_labels: UnusedLabels { scopes: vec![], curr_scope: 0, labels: vec![] },
label_builder: LabelBuilder::default(),
jsdoc: JSDocBuilder::new(source_text, &trivias),
check_syntax_error: false,
redeclare_variables: RedeclareVariables { variables: vec![] },
Expand Down Expand Up @@ -185,7 +174,7 @@ impl<'a> SemanticBuilder<'a> {
classes: self.class_table_builder.build(),
module_record: Arc::clone(&self.module_record),
jsdoc: self.jsdoc.build(),
unused_labels: self.unused_labels.labels,
unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg,
};
Expand All @@ -203,7 +192,7 @@ impl<'a> SemanticBuilder<'a> {
classes: self.class_table_builder.build(),
module_record: Arc::new(ModuleRecord::default()),
jsdoc: self.jsdoc.build(),
unused_labels: self.unused_labels.labels,
unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg,
}
Expand Down Expand Up @@ -1563,9 +1552,11 @@ impl<'a> SemanticBuilder<'a> {
decl.bind(self);
self.make_all_namespaces_valuelike();
}
AstKind::StaticBlock(_) => self.label_builder.enter_function_or_static_block(),
AstKind::Function(func) => {
self.function_stack.push(self.current_node_id);
func.bind(self);
self.label_builder.enter_function_or_static_block();
self.add_current_node_id_to_current_scope();
self.make_all_namespaces_valuelike();
}
Expand Down Expand Up @@ -1641,29 +1632,12 @@ impl<'a> SemanticBuilder<'a> {
self.reference_jsx_element_name(elem);
}
AstKind::LabeledStatement(stmt) => {
self.unused_labels.scopes.push(LabeledScope {
name: stmt.label.name.as_str(),
used: false,
parent: self.unused_labels.curr_scope,
});
self.unused_labels.curr_scope = self.unused_labels.scopes.len() - 1;
}
AstKind::ContinueStatement(stmt) => {
if let Some(label) = &stmt.label {
let scope =
self.unused_labels.scopes.iter_mut().rev().find(|x| x.name == label.name);
if let Some(scope) = scope {
scope.used = true;
}
}
self.label_builder.enter(stmt, self.current_node_id);
}
AstKind::BreakStatement(stmt) => {
if let Some(label) = &stmt.label {
let scope =
self.unused_labels.scopes.iter_mut().rev().find(|x| x.name == label.name);
if let Some(scope) = scope {
scope.used = true;
}
AstKind::ContinueStatement(ContinueStatement { label, .. })
| AstKind::BreakStatement(BreakStatement { label, .. }) => {
if let Some(label) = &label {
self.label_builder.mark_as_used(label);
}
}
AstKind::YieldExpression(_) => {
Expand All @@ -1683,14 +1657,15 @@ impl<'a> SemanticBuilder<'a> {
AstKind::ModuleDeclaration(decl) => {
self.current_symbol_flags -= Self::symbol_flag_from_module_declaration(decl);
}
AstKind::LabeledStatement(_) => {
let scope = &self.unused_labels.scopes[self.unused_labels.curr_scope];
if !scope.used {
self.unused_labels.labels.push(self.current_node_id);
}
self.unused_labels.curr_scope = scope.parent;
AstKind::LabeledStatement(_) => self.label_builder.leave(),
AstKind::StaticBlock(_) => {
self.label_builder.leave_function_or_static_block();
}
AstKind::Function(_) | AstKind::ArrowExpression(_) => {
AstKind::Function(_) => {
self.label_builder.leave_function_or_static_block();
self.function_stack.pop();
}
AstKind::ArrowExpression(_) => {
self.function_stack.pop();
}
AstKind::TSModuleBlock(_) => {
Expand Down
71 changes: 32 additions & 39 deletions crates/oxc_semantic/src/checker/javascript.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use oxc_syntax::{
NumberBase,
};
use phf::{phf_set, Set};
use rustc_hash::FxHashMap;

use crate::{builder::SemanticBuilder, diagnostics::Redeclaration, scope::ScopeFlags, AstNode};

Expand All @@ -25,6 +26,7 @@ impl EarlyErrorJavaScript {
let kind = node.kind();

match kind {
AstKind::Program(_) => check_labeled_statement(ctx),
AstKind::BindingIdentifier(ident) => {
check_identifier(&ident.name, ident.span, node, ctx);
check_binding_identifier(ident, node, ctx);
Expand Down Expand Up @@ -54,7 +56,6 @@ impl EarlyErrorJavaScript {
AstKind::ContinueStatement(stmt) => check_continue_statement(stmt, node, ctx),
AstKind::LabeledStatement(stmt) => {
check_function_declaration(&stmt.body, true, ctx);
check_labeled_statement(stmt, node, ctx);
}
AstKind::ForInStatement(stmt) => {
check_function_declaration(&stmt.body, false, ctx);
Expand Down Expand Up @@ -593,6 +594,20 @@ struct InvalidLabelJumpTarget(#[label] Span);
#[diagnostic()]
struct InvalidLabelTarget(#[label("This label is used, but not defined")] Span);

fn check_label(label: &LabelIdentifier, ctx: &SemanticBuilder) {
if ctx.label_builder.is_inside_labeled_statement() {
for labeled in ctx.label_builder.get_accessible_labels() {
if label.name == labeled.name {
return;
}
}
if ctx.label_builder.is_inside_function_or_static_block() {
return ctx.error(InvalidLabelJumpTarget(label.span));
}
}
ctx.error(InvalidLabelTarget(label.span));
}

fn check_break_statement<'a>(stmt: &BreakStatement, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
#[derive(Debug, Error, Diagnostic)]
#[error("Illegal break statement")]
Expand All @@ -601,29 +616,15 @@ fn check_break_statement<'a>(stmt: &BreakStatement, node: &AstNode<'a>, ctx: &Se
))]
struct InvalidBreak(#[label] Span);

if let Some(label) = &stmt.label {
return check_label(label, ctx);
}

// It is a Syntax Error if this BreakStatement is not nested, directly or indirectly (but not crossing function or static initialization block boundaries), within an IterationStatement or a SwitchStatement.
for node_id in ctx.nodes.ancestors(node.id()).skip(1) {
match ctx.nodes.kind(node_id) {
AstKind::Program(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(InvalidBreak(stmt.span)),
|label| ctx.error(InvalidLabelTarget(label.span)),
);
}
AstKind::Function(_) | AstKind::StaticBlock(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(InvalidBreak(stmt.span)),
|label| ctx.error(InvalidLabelJumpTarget(label.span)),
);
}
AstKind::LabeledStatement(labeled_statement) => {
if stmt
.label
.as_ref()
.is_some_and(|label| label.name == labeled_statement.label.name)
{
break;
}
AstKind::Program(_) | AstKind::Function(_) | AstKind::StaticBlock(_) => {
ctx.error(InvalidBreak(stmt.span));
}
kind if (kind.is_iteration_statement()
|| matches!(kind, AstKind::SwitchStatement(_)))
Expand Down Expand Up @@ -701,26 +702,18 @@ fn check_continue_statement<'a>(
}
}

fn check_labeled_statement<'a>(
stmt: &LabeledStatement,
node: &AstNode<'a>,
ctx: &SemanticBuilder<'a>,
) {
for node_id in ctx.nodes.ancestors(node.id()).skip(1) {
match ctx.nodes.kind(node_id) {
// label cannot cross boundary on function or static block
AstKind::Function(_) | AstKind::StaticBlock(_) | AstKind::Program(_) => break,
// check label name redeclaration
AstKind::LabeledStatement(label_stmt) if stmt.label.name == label_stmt.label.name => {
return ctx.error(Redeclaration(
stmt.label.name.clone(),
label_stmt.label.span,
stmt.label.span,
));
#[allow(clippy::option_if_let_else)]
fn check_labeled_statement(ctx: &SemanticBuilder) {
ctx.label_builder.labels.iter().for_each(|labels| {
let mut defined = FxHashMap::default();
for labeled in labels {
if let Some(span) = defined.get(labeled.name) {
ctx.error(Redeclaration(labeled.name.into(), *span, labeled.span));
} else {
defined.insert(labeled.name, labeled.span);
}
_ => {}
}
}
});
}

fn check_for_statement_left<'a>(
Expand Down
Loading

0 comments on commit 56adfb1

Please sign in to comment.