dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 04:08:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(semantic): rewrite handling of label statement errors (#5138)

Browse source 
This reverts the previous changes to handling labels so that all tests can pass.

This passes all false postivies found in `monitor-oxc` (node_modules/flow-parser/flow_parser.js)

As it turns out this requires less code and produces better diagnostics.
This commit is contained in:
Boshen 2024-08-24 02:37:49 +00:00
parent 960e1d5c60
commit d5a494023e
7 changed files with 109 additions and 235 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
crates/oxc_semantic/src/builder.rs
View file

@ -24,7 +24,7 @@ use crate::{
counter::Counter,
diagnostics::redeclaration,
jsdoc::JSDocBuilder,
label::LabelBuilder,
label::UnusedLabels,
module_record::ModuleRecordBuilder,
node::{AstNodeId, AstNodes, NodeFlags},
reference::{Reference, ReferenceFlags, ReferenceId},
@ -94,8 +94,7 @@ pub struct SemanticBuilder<'a> {
pub(crate) module_record: Arc<ModuleRecord>,
pub(crate) label_builder: LabelBuilder<'a>,
unused_labels: UnusedLabels<'a>,
build_jsdoc: bool,
jsdoc: JSDocBuilder<'a>,
@ -141,7 +140,7 @@ impl<'a> SemanticBuilder<'a> {
symbols: SymbolTable::default(),
unresolved_references: UnresolvedReferencesStack::new(),
module_record: Arc::new(ModuleRecord::default()),
label_builder: LabelBuilder::default(),
unused_labels: UnusedLabels::default(),
build_jsdoc: false,
jsdoc: JSDocBuilder::new(source_text, trivias),
check_syntax_error: false,
@ -271,7 +270,7 @@ impl<'a> SemanticBuilder<'a> {
classes: self.class_table_builder.build(),
module_record: Arc::clone(&self.module_record),
jsdoc,
unused_labels: self.label_builder.unused_node_ids,
unused_labels: self.unused_labels.labels,
cfg: self.cfg.map(ControlFlowGraphBuilder::build),
};
SemanticBuilderReturn { semantic, errors: self.errors.into_inner() }
@ -1755,13 +1754,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);
if func.is_declaration() {
func.bind(self);
}
self.label_builder.enter_function_or_static_block();
self.make_all_namespaces_valuelike();
}
AstKind::ArrowFunctionExpression(_) => {
@ -1894,12 +1891,12 @@ impl<'a> SemanticBuilder<'a> {
self.current_reference_flags |= ReferenceFlags::Write;
}
AstKind::LabeledStatement(stmt) => {
self.label_builder.enter(stmt, self.current_node_id);
self.unused_labels.add(stmt.label.name.as_str());
}
AstKind::ContinueStatement(ContinueStatement { label, .. })
| AstKind::BreakStatement(BreakStatement { label, .. }) => {
if let Some(label) = &label {
self.label_builder.mark_as_used(label);
self.unused_labels.reference(&label.name);
}
}
AstKind::YieldExpression(_) => {
@ -1927,15 +1924,7 @@ impl<'a> SemanticBuilder<'a> {
self.current_reference_flags = ReferenceFlags::empty();
}
}
AstKind::LabeledStatement(_) => self.label_builder.leave(),
AstKind::StaticBlock(_) => {
self.label_builder.leave_function_or_static_block();
}
AstKind::Function(_) => {
self.label_builder.leave_function_or_static_block();
self.function_stack.pop();
}
AstKind::ArrowFunctionExpression(_) => {
AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) => {
self.function_stack.pop();
}
AstKind::FormalParameters(parameters) => {
@ -1975,6 +1964,7 @@ impl<'a> SemanticBuilder<'a> {
self.current_reference_flags = ReferenceFlags::empty();
}
AstKind::AssignmentTarget(_) => self.current_reference_flags -= ReferenceFlags::Write,
AstKind::LabeledStatement(_) => self.unused_labels.mark_unused(self.current_node_id),
_ => {}
}
}

128
crates/oxc_semantic/src/checker/javascript.rs
View file

@ -554,31 +554,6 @@ fn invalid_label_non_iteration(x0: &str, span1: Span, span2: Span) -> OxcDiagnos
])
}
fn check_label(label: &LabelIdentifier, ctx: &SemanticBuilder, is_continue: bool) {
if ctx.label_builder.is_inside_labeled_statement() {
for labeled in ctx.label_builder.get_accessible_labels() {
if label.name == labeled.name {
if is_continue
&& matches!(ctx.nodes.kind(labeled.id), AstKind::LabeledStatement(stmt) if {
let mut body = &stmt.body;
while let Statement::LabeledStatement(stmt) = body {
body = &stmt.body;
}
!body.is_iteration_statement()
})
{
ctx.error(invalid_label_non_iteration("continue", labeled.span, label.span));
}
return;
}
}
if ctx.label_builder.is_inside_function_or_static_block() {
return ctx.error(invalid_label_jump_target(label.span));
}
}
ctx.error(invalid_label_target(label.span));
}
fn invalid_break(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::error("Illegal break statement")
.with_help("A `break` statement can only be used within an enclosing iteration or switch statement.")
@ -590,15 +565,29 @@ pub fn check_break_statement<'a>(
node: &AstNode<'a>,
ctx: &SemanticBuilder<'a>,
) {
if let Some(label) = &stmt.label {
return check_label(label, ctx, false);
}
// 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(_) | AstKind::Function(_) | AstKind::StaticBlock(_) => {
ctx.error(invalid_break(stmt.span));
AstKind::Program(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(invalid_break(stmt.span)),
|label| ctx.error(invalid_label_target(label.span)),
);
}
AstKind::Function(_) | AstKind::StaticBlock(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(invalid_break(stmt.span)),
|label| ctx.error(invalid_label_jump_target(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()
|| matches!(kind, AstKind::SwitchStatement(_)))
@ -622,16 +611,42 @@ pub fn check_continue_statement<'a>(
node: &AstNode<'a>,
ctx: &SemanticBuilder<'a>,
) {
if let Some(label) = &stmt.label {
return check_label(label, ctx, true);
}
// It is a Syntax Error if this ContinueStatement is not nested, directly or indirectly (but not crossing function or static initialization block boundaries), within an IterationStatement.
for node_id in ctx.nodes.ancestors(node.id()).skip(1) {
match ctx.nodes.kind(node_id) {
AstKind::Program(_) | AstKind::Function(_) | AstKind::StaticBlock(_) => {
ctx.error(invalid_continue(stmt.span));
AstKind::Program(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(invalid_continue(stmt.span)),
|label| ctx.error(invalid_label_target(label.span)),
);
}
AstKind::Function(_) | AstKind::StaticBlock(_) => {
return stmt.label.as_ref().map_or_else(
|| ctx.error(invalid_continue(stmt.span)),
|label| ctx.error(invalid_label_jump_target(label.span)),
);
}
AstKind::LabeledStatement(labeled_statement) => match &stmt.label {
Some(label) if label.name == labeled_statement.label.name => {
if matches!(
labeled_statement.body,
Statement::LabeledStatement(_)
| Statement::DoWhileStatement(_)
| Statement::WhileStatement(_)
| Statement::ForStatement(_)
| Statement::ForInStatement(_)
| Statement::ForOfStatement(_)
) {
break;
}
return ctx.error(invalid_label_non_iteration(
"continue",
labeled_statement.label.span,
label.span,
));
}
_ => {}
},
kind if kind.is_iteration_statement() && stmt.label.is_none() => break,
_ => {}
}
@ -645,29 +660,26 @@ fn label_redeclaration(x0: &str, span1: Span, span2: Span) -> OxcDiagnostic {
])
}
#[allow(clippy::option_if_let_else)]
pub fn check_labeled_statement(ctx: &SemanticBuilder) {
ctx.label_builder.labels.iter().for_each(|labels| {
let mut defined = FxHashMap::default();
//only need to care about the monotone increasing depth of the array
let mut increase_depth = vec![];
for labeled in labels {
increase_depth.push(labeled);
// have to traverse because HashMap can only delete one by one
while increase_depth.len() > 2
&& increase_depth.iter().rev().nth(1).unwrap().depth
>= increase_depth.iter().next_back().unwrap().depth
{
defined.remove(increase_depth[increase_depth.len() - 2].name);
increase_depth.remove(increase_depth.len() - 2);
}
if let Some(span) = defined.get(labeled.name) {
ctx.error(label_redeclaration(labeled.name, *span, labeled.span));
} else {
defined.insert(labeled.name, labeled.span);
pub 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(label_redeclaration(
stmt.label.name.as_str(),
label_stmt.label.span,
stmt.label.span,
));
}
_ => {}
}
});
}
}
fn multiple_declaration_in_for_loop_head(x0: &str, span1: Span) -> OxcDiagnostic {

3
crates/oxc_semantic/src/checker/mod.rs
View file

@ -16,7 +16,7 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
match kind {
AstKind::Program(_) => {
js::check_labeled_statement(ctx);
// js::check_labeled_statement(ctx);
js::check_duplicate_class_elements(ctx);
}
AstKind::BindingIdentifier(ident) => {
@ -47,6 +47,7 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
AstKind::BreakStatement(stmt) => js::check_break_statement(stmt, node, ctx),
AstKind::ContinueStatement(stmt) => js::check_continue_statement(stmt, node, ctx),
AstKind::LabeledStatement(stmt) => {
js::check_labeled_statement(stmt, node, ctx);
js::check_function_declaration(&stmt.body, true, ctx);
}
AstKind::ForInStatement(stmt) => {

161
crates/oxc_semantic/src/label.rs
View file

@ -1,156 +1,37 @@
use oxc_ast::ast::LabeledStatement;
use oxc_span::Span;
use rustc_hash::FxHashSet;
use crate::AstNodeId;
#[derive(Debug)]
pub struct Label<'a> {
pub id: AstNodeId,
pub struct LabeledScope<'a> {
pub name: &'a str,
pub span: Span,
used: bool,
/// depth is the number of nested labeled statements
pub 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,
pub used: bool,
pub parent: usize,
}
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(Debug, Default)]
pub struct UnusedLabels<'a> {
pub scopes: Vec<LabeledScope<'a>>,
pub curr_scope: usize,
pub labels: Vec<AstNodeId>,
}
#[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,
));
impl<'a> UnusedLabels<'a> {
pub fn add(&mut self, name: &'a str) {
self.scopes.push(LabeledScope { name, used: false, parent: self.curr_scope });
self.curr_scope = self.scopes.len() - 1;
}
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 reference(&mut self, name: &'a str) {
let scope = self.scopes.iter_mut().rev().find(|x| x.name == name);
if let Some(scope) = scope {
scope.used = true;
}
}
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;
}
pub fn mark_unused(&mut self, current_node_id: AstNodeId) {
let scope = &self.scopes[self.curr_scope];
if !scope.used {
self.labels.push(current_node_id);
}
self.curr_scope = scope.parent;
}
}

5
crates/oxc_semantic/src/lib.rs
View file

@ -36,7 +36,6 @@ pub use oxc_syntax::{
scope::{ScopeFlags, ScopeId},
symbol::{SymbolFlags, SymbolId},
};
use rustc_hash::FxHashSet;
pub use crate::{
reference::{Reference, ReferenceFlags, ReferenceId},
@ -83,7 +82,7 @@ pub struct Semantic<'a> {
/// Parsed JSDoc comments.
jsdoc: JSDocFinder<'a>,
unused_labels: FxHashSet<AstNodeId>,
unused_labels: Vec<AstNodeId>,
/// Control flow graph. Only present if [`Semantic`] is built with cfg
/// creation enabled using [`SemanticBuilder::with_cfg`].
@ -150,7 +149,7 @@ impl<'a> Semantic<'a> {
&self.symbols
}
pub fn unused_labels(&self) -> &FxHashSet<AstNodeId> {
pub fn unused_labels(&self) -> &Vec<AstNodeId> {
&self.unused_labels
}

10
tasks/coverage/parser_test262.snap
View file

@ -23861,7 +23861,7 @@ Negative Passed: 4220/4220 (100.00%)
24 │ var y=2;
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[test262/test/language/statements/break/S12.8_A5_T1.js:23:15]
22 │ return;
23 │ break LABEL_ANOTHER_LOOP;
@ -23869,7 +23869,7 @@ Negative Passed: 4220/4220 (100.00%)
24 │ LABEL_IN_2 : y++;
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[test262/test/language/statements/break/S12.8_A5_T2.js:25:15]
24 │ return;
25 │ break IN_DO_FUNC;
@ -23877,7 +23877,7 @@ Negative Passed: 4220/4220 (100.00%)
26 │ LABEL_IN_2 : y++;
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[test262/test/language/statements/break/S12.8_A5_T3.js:25:15]
24 │ return;
25 │ break LABEL_IN;
@ -30352,7 +30352,7 @@ Negative Passed: 4220/4220 (100.00%)
21 │ }
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[test262/test/language/statements/class/static-init-invalid-undefined-break-target.js:22:13]
21 │ x: while (false) {
22 │ break y;
@ -30360,7 +30360,7 @@ Negative Passed: 4220/4220 (100.00%)
23 │ }
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[test262/test/language/statements/class/static-init-invalid-undefined-continue-target.js:22:16]
21 │ x: while (false) {
22 │ continue y;

11
tasks/coverage/parser_typescript.snap
View file

@ -8118,15 +8118,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/typeFro
╰────
help: A `continue` statement can only be used within an enclosing `for`, `while` or `do while`
× Illegal continue statement: no surrounding iteration statement
╭─[typescript/tests/cases/compiler/invalidContinueInDownlevelAsync.ts:3:9]
2 │ if (true) {
3 │ continue;
· ─────────
4 │ }
╰────
help: A `continue` statement can only be used within an enclosing `for`, `while` or `do while`
× Expected `;` but found `[`
╭─[typescript/tests/cases/compiler/invalidLetInForOfAndForIn_ES5.ts:5:13]
4 │ var let = 10;
@ -21438,7 +21429,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/typeFro
28 │ return toFixed()
╰────
× Use of undefined label
× Jump target cannot cross function boundary.
╭─[typescript/tests/cases/conformance/salsa/plainJSBinderErrors.ts:34:19]
33 │ label: var x = 1
34 │ break label