refactor(semantic): use LabelBuilder instead of UnusedLabeled (#2184)

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`
This commit is contained in:
Dunqing 2024-01-29 15:15:07 +08:00 committed by GitHub
parent 972be831e9
commit 56adfb1a86
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 227 additions and 96 deletions

View file

@ -19,6 +19,7 @@ use crate::{
}, },
diagnostics::Redeclaration, diagnostics::Redeclaration,
jsdoc::JSDocBuilder, jsdoc::JSDocBuilder,
label::LabelBuilder,
module_record::ModuleRecordBuilder, module_record::ModuleRecordBuilder,
node::{AstNode, AstNodeId, AstNodes, NodeFlags}, node::{AstNode, AstNodeId, AstNodes, NodeFlags},
pg::replicate_tree_to_leaves, pg::replicate_tree_to_leaves,
@ -28,18 +29,6 @@ use crate::{
Semantic, 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)] #[derive(Debug, Clone)]
pub struct VariableInfo { pub struct VariableInfo {
pub span: Span, pub span: Span,
@ -83,7 +72,7 @@ pub struct SemanticBuilder<'a> {
pub(crate) module_record: Arc<ModuleRecord>, pub(crate) module_record: Arc<ModuleRecord>,
unused_labels: UnusedLabels<'a>, pub label_builder: LabelBuilder<'a>,
jsdoc: JSDocBuilder<'a>, jsdoc: JSDocBuilder<'a>,
@ -123,7 +112,7 @@ impl<'a> SemanticBuilder<'a> {
scope, scope,
symbols: SymbolTable::default(), symbols: SymbolTable::default(),
module_record: Arc::new(ModuleRecord::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), jsdoc: JSDocBuilder::new(source_text, &trivias),
check_syntax_error: false, check_syntax_error: false,
redeclare_variables: RedeclareVariables { variables: vec![] }, redeclare_variables: RedeclareVariables { variables: vec![] },
@ -185,7 +174,7 @@ impl<'a> SemanticBuilder<'a> {
classes: self.class_table_builder.build(), classes: self.class_table_builder.build(),
module_record: Arc::clone(&self.module_record), module_record: Arc::clone(&self.module_record),
jsdoc: self.jsdoc.build(), jsdoc: self.jsdoc.build(),
unused_labels: self.unused_labels.labels, unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables, redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg, cfg: self.cfg,
}; };
@ -203,7 +192,7 @@ impl<'a> SemanticBuilder<'a> {
classes: self.class_table_builder.build(), classes: self.class_table_builder.build(),
module_record: Arc::new(ModuleRecord::default()), module_record: Arc::new(ModuleRecord::default()),
jsdoc: self.jsdoc.build(), jsdoc: self.jsdoc.build(),
unused_labels: self.unused_labels.labels, unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables, redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg, cfg: self.cfg,
} }
@ -1563,9 +1552,11 @@ impl<'a> SemanticBuilder<'a> {
decl.bind(self); decl.bind(self);
self.make_all_namespaces_valuelike(); self.make_all_namespaces_valuelike();
} }
AstKind::StaticBlock(_) => self.label_builder.enter_function_or_static_block(),
AstKind::Function(func) => { AstKind::Function(func) => {
self.function_stack.push(self.current_node_id); self.function_stack.push(self.current_node_id);
func.bind(self); func.bind(self);
self.label_builder.enter_function_or_static_block();
self.add_current_node_id_to_current_scope(); self.add_current_node_id_to_current_scope();
self.make_all_namespaces_valuelike(); self.make_all_namespaces_valuelike();
} }
@ -1641,29 +1632,12 @@ impl<'a> SemanticBuilder<'a> {
self.reference_jsx_element_name(elem); self.reference_jsx_element_name(elem);
} }
AstKind::LabeledStatement(stmt) => { AstKind::LabeledStatement(stmt) => {
self.unused_labels.scopes.push(LabeledScope { self.label_builder.enter(stmt, self.current_node_id);
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) => { AstKind::ContinueStatement(ContinueStatement { label, .. })
if let Some(label) = &stmt.label { | AstKind::BreakStatement(BreakStatement { label, .. }) => {
let scope = if let Some(label) = &label {
self.unused_labels.scopes.iter_mut().rev().find(|x| x.name == label.name); self.label_builder.mark_as_used(label);
if let Some(scope) = scope {
scope.used = true;
}
}
}
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::YieldExpression(_) => { AstKind::YieldExpression(_) => {
@ -1683,14 +1657,15 @@ impl<'a> SemanticBuilder<'a> {
AstKind::ModuleDeclaration(decl) => { AstKind::ModuleDeclaration(decl) => {
self.current_symbol_flags -= Self::symbol_flag_from_module_declaration(decl); self.current_symbol_flags -= Self::symbol_flag_from_module_declaration(decl);
} }
AstKind::LabeledStatement(_) => { AstKind::LabeledStatement(_) => self.label_builder.leave(),
let scope = &self.unused_labels.scopes[self.unused_labels.curr_scope]; AstKind::StaticBlock(_) => {
if !scope.used { self.label_builder.leave_function_or_static_block();
self.unused_labels.labels.push(self.current_node_id);
}
self.unused_labels.curr_scope = scope.parent;
} }
AstKind::Function(_) | AstKind::ArrowExpression(_) => { AstKind::Function(_) => {
self.label_builder.leave_function_or_static_block();
self.function_stack.pop();
}
AstKind::ArrowExpression(_) => {
self.function_stack.pop(); self.function_stack.pop();
} }
AstKind::TSModuleBlock(_) => { AstKind::TSModuleBlock(_) => {

View file

@ -15,6 +15,7 @@ use oxc_syntax::{
NumberBase, NumberBase,
}; };
use phf::{phf_set, Set}; use phf::{phf_set, Set};
use rustc_hash::FxHashMap;
use crate::{builder::SemanticBuilder, diagnostics::Redeclaration, scope::ScopeFlags, AstNode}; use crate::{builder::SemanticBuilder, diagnostics::Redeclaration, scope::ScopeFlags, AstNode};
@ -25,6 +26,7 @@ impl EarlyErrorJavaScript {
let kind = node.kind(); let kind = node.kind();
match kind { match kind {
AstKind::Program(_) => check_labeled_statement(ctx),
AstKind::BindingIdentifier(ident) => { AstKind::BindingIdentifier(ident) => {
check_identifier(&ident.name, ident.span, node, ctx); check_identifier(&ident.name, ident.span, node, ctx);
check_binding_identifier(ident, node, ctx); check_binding_identifier(ident, node, ctx);
@ -54,7 +56,6 @@ impl EarlyErrorJavaScript {
AstKind::ContinueStatement(stmt) => check_continue_statement(stmt, node, ctx), AstKind::ContinueStatement(stmt) => check_continue_statement(stmt, node, ctx),
AstKind::LabeledStatement(stmt) => { AstKind::LabeledStatement(stmt) => {
check_function_declaration(&stmt.body, true, ctx); check_function_declaration(&stmt.body, true, ctx);
check_labeled_statement(stmt, node, ctx);
} }
AstKind::ForInStatement(stmt) => { AstKind::ForInStatement(stmt) => {
check_function_declaration(&stmt.body, false, ctx); check_function_declaration(&stmt.body, false, ctx);
@ -593,6 +594,20 @@ struct InvalidLabelJumpTarget(#[label] Span);
#[diagnostic()] #[diagnostic()]
struct InvalidLabelTarget(#[label("This label is used, but not defined")] Span); 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>) { fn check_break_statement<'a>(stmt: &BreakStatement, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
#[derive(Debug, Error, Diagnostic)] #[derive(Debug, Error, Diagnostic)]
#[error("Illegal break statement")] #[error("Illegal break statement")]
@ -601,29 +616,15 @@ fn check_break_statement<'a>(stmt: &BreakStatement, node: &AstNode<'a>, ctx: &Se
))] ))]
struct InvalidBreak(#[label] Span); 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. // 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) { for node_id in ctx.nodes.ancestors(node.id()).skip(1) {
match ctx.nodes.kind(node_id) { match ctx.nodes.kind(node_id) {
AstKind::Program(_) => { AstKind::Program(_) | AstKind::Function(_) | AstKind::StaticBlock(_) => {
return stmt.label.as_ref().map_or_else( ctx.error(InvalidBreak(stmt.span));
|| 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;
}
} }
kind if (kind.is_iteration_statement() kind if (kind.is_iteration_statement()
|| matches!(kind, AstKind::SwitchStatement(_))) || matches!(kind, AstKind::SwitchStatement(_)))
@ -701,26 +702,18 @@ fn check_continue_statement<'a>(
} }
} }
fn check_labeled_statement<'a>( #[allow(clippy::option_if_let_else)]
stmt: &LabeledStatement, fn check_labeled_statement(ctx: &SemanticBuilder) {
node: &AstNode<'a>, ctx.label_builder.labels.iter().for_each(|labels| {
ctx: &SemanticBuilder<'a>, let mut defined = FxHashMap::default();
) { for labeled in labels {
for node_id in ctx.nodes.ancestors(node.id()).skip(1) { if let Some(span) = defined.get(labeled.name) {
match ctx.nodes.kind(node_id) { ctx.error(Redeclaration(labeled.name.into(), *span, labeled.span));
// label cannot cross boundary on function or static block } else {
AstKind::Function(_) | AstKind::StaticBlock(_) | AstKind::Program(_) => break, defined.insert(labeled.name, labeled.span);
// 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,
));
} }
_ => {}
} }
} });
} }
fn check_for_statement_left<'a>( fn check_for_statement_left<'a>(

View file

@ -0,0 +1,156 @@
use oxc_ast::ast::LabeledStatement;
use oxc_span::Span;
use rustc_hash::FxHashSet;
use crate::AstNodeId;
#[derive(Debug)]
pub struct Label<'a> {
id: AstNodeId,
pub name: &'a str,
pub span: Span,
used: bool,
/// depth is the number of nested labeled statements
depth: usize,
/// is accessible means that the label is accessible from the current position
is_accessible: bool,
/// is_inside_function_or_static_block means that the label is inside a function or static block
is_inside_function_or_static_block: bool,
}
impl<'a> Label<'a> {
pub fn new(
id: AstNodeId,
name: &'a str,
span: Span,
depth: usize,
is_inside_function_or_static_block: bool,
) -> Self {
Self {
id,
name,
span,
depth,
is_inside_function_or_static_block,
used: false,
is_accessible: true,
}
}
}
#[derive(Default)]
pub struct LabelBuilder<'a> {
pub labels: Vec<Vec<Label<'a>>>,
depth: usize,
pub unused_node_ids: FxHashSet<AstNodeId>,
}
impl<'a> LabelBuilder<'a> {
pub fn enter(&mut self, stmt: &'a LabeledStatement<'a>, current_node_id: AstNodeId) {
let is_empty = self.labels.last().map_or(false, Vec::is_empty);
if !self.is_inside_labeled_statement() {
self.labels.push(vec![]);
}
self.depth += 1;
self.labels.last_mut().unwrap_or_else(|| unreachable!()).push(Label::new(
current_node_id,
stmt.label.name.as_str(),
stmt.label.span,
self.depth,
is_empty,
));
}
pub fn leave(&mut self) {
let depth = self.depth;
// Mark labels at the current depth as inaccessible
// ```ts
// label: {} // leave here, mark label as inaccessible
// break label // So we cannot find label here
// ```
for label in self.get_accessible_labels_mut() {
if depth == label.depth {
label.is_accessible = false;
}
}
// If depth is 0, move last labels to the front of `labels` and set `depth` to the length of the last labels.
// We need to do this because we're currently inside a function or static block
while self.depth == 0 {
if let Some(last_labels) = self.labels.pop() {
if !last_labels.is_empty() {
self.labels.insert(0, last_labels);
}
}
self.depth = self.labels.last().unwrap().len();
}
self.depth -= 1;
// insert unused labels into `unused_node_ids`
if self.depth == 0 {
if let Some(labels) = self.labels.last() {
for label in labels {
if !label.used {
self.unused_node_ids.insert(label.id);
}
}
}
}
}
pub fn enter_function_or_static_block(&mut self) {
if self.is_inside_labeled_statement() {
self.depth = 0;
self.labels.push(vec![]);
}
}
pub fn leave_function_or_static_block(&mut self) {
if self.is_inside_labeled_statement() {
let labels = self.labels.pop().unwrap_or_else(|| unreachable!());
if !labels.is_empty() {
self.labels.insert(0, labels);
}
self.depth = self.labels.last().map_or(0, Vec::len);
}
}
pub fn is_inside_labeled_statement(&self) -> bool {
self.depth != 0 || self.labels.last().is_some_and(Vec::is_empty)
}
pub fn is_inside_function_or_static_block(&self) -> bool {
self.labels
.last()
.is_some_and(|labels| labels.is_empty() || labels[0].is_inside_function_or_static_block)
}
pub fn get_accessible_labels(&self) -> impl DoubleEndedIterator<Item = &Label<'a>> {
return self.labels.last().unwrap().iter().filter(|label| label.is_accessible).rev();
}
pub fn get_accessible_labels_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut Label<'a>> {
return self
.labels
.last_mut()
.unwrap()
.iter_mut()
.filter(|label| label.is_accessible)
.rev();
}
pub fn mark_as_used(&mut self, label: &oxc_ast::ast::LabelIdentifier) {
if self.is_inside_labeled_statement() {
let label = self.get_accessible_labels_mut().find(|x| x.name == label.name);
if let Some(label) = label {
label.used = true;
}
}
}
}

View file

@ -5,6 +5,7 @@ mod class;
mod control_flow; mod control_flow;
mod diagnostics; mod diagnostics;
mod jsdoc; mod jsdoc;
mod label;
mod module_record; mod module_record;
mod node; mod node;
pub mod pg; pub mod pg;
@ -27,6 +28,7 @@ pub use oxc_syntax::{
scope::{ScopeFlags, ScopeId}, scope::{ScopeFlags, ScopeId},
symbol::{SymbolFlags, SymbolId}, symbol::{SymbolFlags, SymbolId},
}; };
use rustc_hash::FxHashSet;
pub use crate::{ pub use crate::{
builder::VariableInfo, builder::VariableInfo,
@ -61,7 +63,7 @@ pub struct Semantic<'a> {
jsdoc: JSDoc<'a>, jsdoc: JSDoc<'a>,
unused_labels: Vec<AstNodeId>, unused_labels: FxHashSet<AstNodeId>,
redeclare_variables: Vec<VariableInfo>, redeclare_variables: Vec<VariableInfo>,
@ -113,7 +115,7 @@ impl<'a> Semantic<'a> {
&self.symbols &self.symbols
} }
pub fn unused_labels(&self) -> &Vec<AstNodeId> { pub fn unused_labels(&self) -> &FxHashSet<AstNodeId> {
&self.unused_labels &self.unused_labels
} }

View file

@ -21736,27 +21736,30 @@ Expect Syntax Error: "language/import/import-attributes/json-named-bindings.js"
24 │ var y=2; 24 │ var y=2;
╰──── ╰────
× Jump target cannot cross function boundary. × Use of undefined label
╭─[language/statements/break/S12.8_A5_T1.js:22:1] ╭─[language/statements/break/S12.8_A5_T1.js:22:1]
22 │ return; 22 │ return;
23 │ break LABEL_ANOTHER_LOOP; 23 │ break LABEL_ANOTHER_LOOP;
· ────────────────── · ─────────┬────────
· ╰── This label is used, but not defined
24 │ LABEL_IN_2 : y++; 24 │ LABEL_IN_2 : y++;
╰──── ╰────
× Jump target cannot cross function boundary. × Use of undefined label
╭─[language/statements/break/S12.8_A5_T2.js:24:1] ╭─[language/statements/break/S12.8_A5_T2.js:24:1]
24 │ return; 24 │ return;
25 │ break IN_DO_FUNC; 25 │ break IN_DO_FUNC;
· ────────── · ─────┬────
· ╰── This label is used, but not defined
26 │ LABEL_IN_2 : y++; 26 │ LABEL_IN_2 : y++;
╰──── ╰────
× Jump target cannot cross function boundary. × Use of undefined label
╭─[language/statements/break/S12.8_A5_T3.js:24:1] ╭─[language/statements/break/S12.8_A5_T3.js:24:1]
24 │ return; 24 │ return;
25 │ break LABEL_IN; 25 │ break LABEL_IN;
· ──────── · ────┬───
· ╰── This label is used, but not defined
26 │ LABEL_IN_2 : y++; 26 │ LABEL_IN_2 : y++;
╰──── ╰────
@ -28280,11 +28283,12 @@ Expect Syntax Error: "language/import/import-attributes/json-named-bindings.js"
21 │ } 21 │ }
╰──── ╰────
× Jump target cannot cross function boundary. × Use of undefined label
╭─[language/statements/class/static-init-invalid-undefined-break-target.js:21:1] ╭─[language/statements/class/static-init-invalid-undefined-break-target.js:21:1]
21 │ x: while (false) { 21 │ x: while (false) {
22 │ break y; 22 │ break y;
· ─ · ┬
· ╰── This label is used, but not defined
23 │ } 23 │ }
╰──── ╰────

View file

@ -18236,11 +18236,12 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
32 │ return toFixed() 32 │ return toFixed()
╰──── ╰────
× Jump target cannot cross function boundary. × Use of undefined label
╭─[conformance/salsa/plainJSBinderErrors.ts:37:1] ╭─[conformance/salsa/plainJSBinderErrors.ts:37:1]
37 │ label: var x = 1 37 │ label: var x = 1
38 │ break label 38 │ break label
· ───── · ──┬──
· ╰── This label is used, but not defined
39 │ } 39 │ }
╰──── ╰────