Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(semantic): improve check super implementation, reduce access nodes #1827

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 51 additions & 46 deletions crates/oxc_semantic/src/checker/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,73 +836,78 @@ fn check_super<'a>(sup: &Super, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
_ => None,
};

let Some(class_id) = ctx.class_table_builder.current_class_id else {
for scope_id in ctx.scope.ancestors(ctx.current_scope_id) {
let flags = ctx.scope.get_flags(scope_id);
if flags.is_function()
&& matches!(
ctx.nodes.parent_kind(ctx.scope.get_node_id(scope_id)),
Some(AstKind::ObjectProperty(_))
)
{
if let Some(super_call_span) = super_call_span {
ctx.error(UnexpectedSuperCall(super_call_span));
}
return;
};
}

// ModuleBody : ModuleItemList
// * It is a Syntax Error if ModuleItemList Contains super.
// ScriptBody : StatementList
// * It is a Syntax Error if StatementList Contains super
return super_call_span.map_or_else(
|| ctx.error(UnexpectedSuperReference(sup.span)),
|super_call_span| ctx.error(UnexpectedSuperCall(super_call_span)),
);
};

// skip(1) is the self `Super`
// skip(2) is the parent `CallExpression` or `NewExpression`
for node_id in ctx.nodes.ancestors(node.id()).skip(2) {
match ctx.nodes.kind(node_id) {
AstKind::Class(class) => {
// ClassTail : ClassHeritageopt { ClassBody }
// It is a Syntax Error if ClassHeritage is not present and the following algorithm returns true:
// 1. Let constructor be ConstructorMethod of ClassBody.
// 2. If constructor is empty, return false.
// 3. Return HasDirectSuper of constructor.
if class.super_class.is_none() {
return ctx.error(SuperWithoutDerivedClass(sup.span, class.span));
}
break;
}
AstKind::MethodDefinition(def) => {
// ClassElement : MethodDefinition
// It is a Syntax Error if PropName of MethodDefinition is not "constructor" and HasDirectSuper of MethodDefinition is true.
if let Some(super_call_span) = super_call_span {
if def.kind == MethodDefinitionKind::Constructor {
// pass through and let AstKind::Class check ClassHeritage
} else {
return ctx.error(UnexpectedSuperCall(super_call_span));
// It is a Syntax Error if SuperCall in nested set/get function
if ctx.scope.get_flags(node.scope_id()).is_set_or_get_accessor() {
return ctx.error(UnexpectedSuperCall(super_call_span));
}

// check ClassHeritage
if let AstKind::Class(class) =
ctx.nodes.kind(ctx.class_table_builder.classes.get_node_id(class_id))
{
// ClassTail : ClassHeritageopt { ClassBody }
// It is a Syntax Error if ClassHeritage is not present and the following algorithm returns true:
// 1. Let constructor be ConstructorMethod of ClassBody.
// 2. If constructor is empty, return false.
// 3. Return HasDirectSuper of constructor.
if class.super_class.is_none() {
return ctx.error(SuperWithoutDerivedClass(sup.span, class.span));
}
}
break;
}
} else {
// super references are allowed in method
break;
return ctx.error(UnexpectedSuperCall(super_call_span));
}
// super references are allowed in method
break;
}
// FieldDefinition : ClassElementName Initializer opt
// * It is a Syntax Error if Initializer is present and Initializer Contains SuperCall is true.
// PropertyDefinition : MethodDefinition
// * It is a Syntax Error if HasDirectSuper of MethodDefinition is true.
AstKind::PropertyDefinition(_) => {
if let Some(super_call_span) = super_call_span {
return ctx.error(UnexpectedSuperCall(super_call_span));
}
break;
}
AstKind::ObjectProperty(prop) => {
if prop.value.is_function() {
match super_call_span {
Some(super_call_span) if super_call_span.start > prop.key.span().end => {
return ctx.error(UnexpectedSuperCall(super_call_span));
}
_ => {
break;
}
}
}
}
AstKind::PropertyDefinition(_)
// ClassStaticBlockBody : ClassStaticBlockStatementList
// * It is a Syntax Error if ClassStaticBlockStatementList Contains SuperCall is true.
AstKind::StaticBlock(_) => {
| AstKind::StaticBlock(_) => {
if let Some(super_call_span) = super_call_span {
return ctx.error(UnexpectedSuperCall(super_call_span));
}
}
// ModuleBody : ModuleItemList
// * It is a Syntax Error if ModuleItemList Contains super.
// ScriptBody : StatementList
// * It is a Syntax Error if StatementList Contains super
AstKind::Program(_) => {
return super_call_span.map_or_else(
|| ctx.error(UnexpectedSuperReference(sup.span)),
|super_call_span| ctx.error(UnexpectedSuperCall(super_call_span)),
);
break;
}
_ => {}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_semantic/src/class/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ impl ClassTable {
self.private_identifiers[class_id].iter()
}

pub fn get_node_id(&self, class_id: ClassId) -> AstNodeId {
self.declarations[class_id]
}

pub fn get_property_id(&self, class_id: ClassId, name: &Atom) -> Option<PropertyId> {
self.properties[class_id].iter_enumerated().find_map(|(property_id, property)| {
if property.name == *name {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ impl ScopeTree {
&self.bindings[scope_id]
}

pub fn get_node_id(&self, scope_id: ScopeId) -> Option<&AstNodeId> {
self.node_ids.get(&scope_id)
pub fn get_node_id(&self, scope_id: ScopeId) -> AstNodeId {
self.node_ids[&scope_id]
}

pub fn iter_bindings(&self) -> impl Iterator<Item = (ScopeId, SymbolId, Atom)> + '_ {
Expand Down
8 changes: 8 additions & 0 deletions crates/oxc_syntax/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ impl ScopeFlags {
self.contains(Self::Function)
}

pub fn is_constructor(&self) -> bool {
self.contains(Self::Constructor)
}

pub fn is_class_static_block(&self) -> bool {
self.contains(Self::ClassStaticBlock)
}
Expand All @@ -55,4 +59,8 @@ impl ScopeFlags {
pub fn is_set_accessor(&self) -> bool {
self.contains(Self::SetAccessor)
}

pub fn is_set_or_get_accessor(&self) -> bool {
self.intersects(Self::SetAccessor | Self::GetAccessor)
}
}