From 89cb3687ad97fe790921828183f99d6a6d3d068b Mon Sep 17 00:00:00 2001 From: therewillbecode Date: Sun, 23 Mar 2025 06:27:41 +0000 Subject: [PATCH 1/2] fix(ast/estree): Add decorators field to `AssignmentPattern` (#9967) Adds a `decorators` field with an empty array to align our AST's ESTree output with that of TS-ESLint's for the `AssignmentPattern` node. Relates to #9705 --------- Co-authored-by: Hiroshi Ogawa --- crates/oxc_ast/src/ast/js.rs | 1 + crates/oxc_ast/src/generated/derive_estree.rs | 1 + napi/parser/deserialize-ts.js | 1 + npm/oxc-types/types.d.ts | 1 + tasks/coverage/snapshots/estree_typescript.snap | 6 +----- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 6cf1d2cb49fa3..ad1ca827bd4ee 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -1547,6 +1547,7 @@ pub enum BindingPatternKind<'a> { #[ast(visit)] #[derive(Debug)] #[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ESTree)] +#[estree(add_fields(decorators = TsEmptyArray))] pub struct AssignmentPattern<'a> { pub span: Span, pub left: BindingPattern<'a>, diff --git a/crates/oxc_ast/src/generated/derive_estree.rs b/crates/oxc_ast/src/generated/derive_estree.rs index 76aaa4527630c..c4abf2f5fdcc7 100644 --- a/crates/oxc_ast/src/generated/derive_estree.rs +++ b/crates/oxc_ast/src/generated/derive_estree.rs @@ -1279,6 +1279,7 @@ impl ESTree for AssignmentPattern<'_> { state.serialize_field("end", &self.span.end); state.serialize_field("left", &self.left); state.serialize_field("right", &self.right); + state.serialize_ts_field("decorators", &crate::serialize::TsEmptyArray(self)); state.end(); } } diff --git a/napi/parser/deserialize-ts.js b/napi/parser/deserialize-ts.js index bb26b16c48f20..314ec7fe8f6bf 100644 --- a/napi/parser/deserialize-ts.js +++ b/napi/parser/deserialize-ts.js @@ -720,6 +720,7 @@ function deserializeAssignmentPattern(pos) { end: deserializeU32(pos + 4), left: deserializeBindingPattern(pos + 8), right: deserializeExpression(pos + 40), + decorators: [], }; } diff --git a/npm/oxc-types/types.d.ts b/npm/oxc-types/types.d.ts index 60296d2c07b9a..a7fde01f4b628 100644 --- a/npm/oxc-types/types.d.ts +++ b/npm/oxc-types/types.d.ts @@ -516,6 +516,7 @@ export interface AssignmentPattern extends Span { type: 'AssignmentPattern'; left: BindingPattern; right: Expression; + decorators?: []; } export interface ObjectPattern extends Span { diff --git a/tasks/coverage/snapshots/estree_typescript.snap b/tasks/coverage/snapshots/estree_typescript.snap index 23bf6b45e21b4..4bc9c86b49e77 100644 --- a/tasks/coverage/snapshots/estree_typescript.snap +++ b/tasks/coverage/snapshots/estree_typescript.snap @@ -2,7 +2,7 @@ commit: 15392346 estree_typescript Summary: AST Parsed : 10623/10725 (99.05%) -Positive Passed: 1979/10725 (18.45%) +Positive Passed: 1983/10725 (18.49%) Mismatch: tasks/coverage/typescript/tests/cases/compiler/APILibCheck.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/APISample_Watch.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/APISample_WatchWithDefaults.ts @@ -2738,7 +2738,6 @@ Mismatch: tasks/coverage/typescript/tests/cases/compiler/inheritedOverloadedSpec Mismatch: tasks/coverage/typescript/tests/cases/compiler/inheritedStringIndexersFromDifferentBaseTypes.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/inheritedStringIndexersFromDifferentBaseTypes2.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/initializePropertiesWithRenamedLet.ts -Mismatch: tasks/coverage/typescript/tests/cases/compiler/initializedDestructuringAssignmentTypes.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/initializedParameterBeforeNonoptionalNotOptional.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/initializerWithThisPropertyAccess.ts Mismatch: tasks/coverage/typescript/tests/cases/compiler/initializersInAmbientEnums.ts @@ -8767,15 +8766,12 @@ Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofSta Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of19.ts Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of20.ts Missing initializer in const declaration -Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of26.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of27.ts -Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of28.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of29.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of30.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of31.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of34.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of35.ts -Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of36.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of37.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of8.ts Mismatch: tasks/coverage/typescript/tests/cases/conformance/statements/for-ofStatements/ES5For-of9.ts From 9992559f84519d73c85da7e19fbf132d6fd70862 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sun, 23 Mar 2025 06:43:37 +0000 Subject: [PATCH 2/2] feat(mangler): collect symbols that is used to set `name` property (#9897) Same as #9872 but implemented in the mangler instead. close #9872 --- Cargo.lock | 1 + crates/oxc_mangler/Cargo.toml | 3 + crates/oxc_mangler/src/keep_names.rs | 269 +++++++++++++++++++++++++++ crates/oxc_mangler/src/lib.rs | 1 + 4 files changed, 274 insertions(+) create mode 100644 crates/oxc_mangler/src/keep_names.rs diff --git a/Cargo.lock b/Cargo.lock index bc6eaaac2fac0..79d6e25d970b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1867,6 +1867,7 @@ dependencies = [ "oxc_ast", "oxc_data_structures", "oxc_index", + "oxc_parser", "oxc_semantic", "oxc_span", "rustc-hash", diff --git a/crates/oxc_mangler/Cargo.toml b/crates/oxc_mangler/Cargo.toml index ed07ebdc3153b..3b934ae4b142c 100644 --- a/crates/oxc_mangler/Cargo.toml +++ b/crates/oxc_mangler/Cargo.toml @@ -31,3 +31,6 @@ oxc_span = { workspace = true } fixedbitset = { workspace = true } itertools = { workspace = true } rustc-hash = { workspace = true } + +[dev-dependencies] +oxc_parser = { workspace = true } diff --git a/crates/oxc_mangler/src/keep_names.rs b/crates/oxc_mangler/src/keep_names.rs new file mode 100644 index 0000000000000..eb5969031b4d4 --- /dev/null +++ b/crates/oxc_mangler/src/keep_names.rs @@ -0,0 +1,269 @@ +use itertools::Itertools; +use oxc_ast::{AstKind, ast::*}; +use oxc_semantic::{AstNode, AstNodes, ReferenceId, Scoping, SymbolId}; +use rustc_hash::FxHashSet; + +#[cfg_attr(not(test), expect(dead_code))] +pub fn collect_name_symbols(scoping: &Scoping, ast_nodes: &AstNodes) -> FxHashSet { + let collector = NameSymbolCollector::new(scoping, ast_nodes); + collector.collect() +} + +/// Collects symbols that are used to set `name` properties of functions and classes. +struct NameSymbolCollector<'a, 'b> { + scoping: &'b Scoping, + ast_nodes: &'b AstNodes<'a>, +} + +impl<'a, 'b: 'a> NameSymbolCollector<'a, 'b> { + fn new(scoping: &'b Scoping, ast_nodes: &'b AstNodes<'a>) -> Self { + Self { scoping, ast_nodes } + } + + fn collect(self) -> FxHashSet { + self.scoping + .symbol_ids() + .filter(|symbol_id| { + let decl_node = + self.ast_nodes.get_node(self.scoping.symbol_declaration(*symbol_id)); + self.is_name_set_declare_node(decl_node, *symbol_id) + || self.has_name_set_reference_node(*symbol_id) + }) + .collect() + } + + fn has_name_set_reference_node(&self, symbol_id: SymbolId) -> bool { + self.scoping.get_resolved_reference_ids(symbol_id).into_iter().any(|reference_id| { + let node = self.ast_nodes.get_node(self.scoping.get_reference(*reference_id).node_id()); + self.is_name_set_reference_node(node, *reference_id) + }) + } + + fn is_name_set_declare_node(&self, node: &'a AstNode, symbol_id: SymbolId) -> bool { + match node.kind() { + AstKind::Function(function) => { + function.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id) + } + AstKind::Class(cls) => cls.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id), + AstKind::VariableDeclarator(decl) => { + if let BindingPatternKind::BindingIdentifier(id) = &decl.id.kind { + if id.symbol_id() == symbol_id { + return decl.init.as_ref().is_some_and(|init| { + self.is_expression_whose_name_needs_to_be_kept(init) + }); + } + } + if let Some(assign_pattern) = + Self::find_assign_binding_pattern_kind_of_specific_symbol( + &decl.id.kind, + symbol_id, + ) + { + return self.is_expression_whose_name_needs_to_be_kept(&assign_pattern.right); + } + false + } + _ => false, + } + } + + fn is_name_set_reference_node(&self, node: &AstNode, reference_id: ReferenceId) -> bool { + let Some(parent_node) = self.ast_nodes.parent_node(node.id()) else { return false }; + match parent_node.kind() { + AstKind::SimpleAssignmentTarget(_) => { + let Some((grand_parent_node_kind, grand_grand_parent_node_kind)) = + self.ast_nodes.ancestor_kinds(parent_node.id()).skip(1).take(2).collect_tuple() + else { + return false; + }; + debug_assert!(matches!(grand_parent_node_kind, AstKind::AssignmentTarget(_))); + + match grand_grand_parent_node_kind { + AstKind::AssignmentExpression(assign_expr) => { + Self::is_assignment_target_id_of_specific_reference( + &assign_expr.left, + reference_id, + ) && self.is_expression_whose_name_needs_to_be_kept(&assign_expr.right) + } + AstKind::AssignmentTargetWithDefault(assign_target) => { + Self::is_assignment_target_id_of_specific_reference( + &assign_target.binding, + reference_id, + ) && self.is_expression_whose_name_needs_to_be_kept(&assign_target.init) + } + _ => false, + } + } + AstKind::ObjectAssignmentTarget(assign_target) => { + assign_target.properties.iter().any(|property| { + if let AssignmentTargetProperty::AssignmentTargetPropertyIdentifier(prop_id) = + &property + { + if prop_id.binding.reference_id() == reference_id { + return prop_id.init.as_ref().is_some_and(|init| { + self.is_expression_whose_name_needs_to_be_kept(init) + }); + } + } + false + }) + } + _ => false, + } + } + + fn find_assign_binding_pattern_kind_of_specific_symbol( + kind: &'a BindingPatternKind, + symbol_id: SymbolId, + ) -> Option<&'a AssignmentPattern<'a>> { + match kind { + BindingPatternKind::BindingIdentifier(_) => None, + BindingPatternKind::ObjectPattern(object_pattern) => { + for property in &object_pattern.properties { + if let Some(value) = Self::find_assign_binding_pattern_kind_of_specific_symbol( + &property.value.kind, + symbol_id, + ) { + return Some(value); + } + } + None + } + BindingPatternKind::ArrayPattern(array_pattern) => { + for element in &array_pattern.elements { + let Some(binding) = element else { continue }; + + if let Some(value) = Self::find_assign_binding_pattern_kind_of_specific_symbol( + &binding.kind, + symbol_id, + ) { + return Some(value); + } + } + None + } + BindingPatternKind::AssignmentPattern(assign_pattern) => { + if Self::is_binding_id_of_specific_symbol(&assign_pattern.left.kind, symbol_id) { + return Some(assign_pattern); + } + Self::find_assign_binding_pattern_kind_of_specific_symbol( + &assign_pattern.left.kind, + symbol_id, + ) + } + } + } + + fn is_binding_id_of_specific_symbol( + pattern_kind: &BindingPatternKind, + symbol_id: SymbolId, + ) -> bool { + if let BindingPatternKind::BindingIdentifier(id) = pattern_kind { + id.symbol_id() == symbol_id + } else { + false + } + } + + fn is_assignment_target_id_of_specific_reference( + target_kind: &AssignmentTarget, + reference_id: ReferenceId, + ) -> bool { + if let AssignmentTarget::AssignmentTargetIdentifier(id) = target_kind { + id.reference_id() == reference_id + } else { + false + } + } + + #[expect(clippy::unused_self)] + fn is_expression_whose_name_needs_to_be_kept(&self, expr: &Expression) -> bool { + expr.is_anonymous_function_definition() + } +} + +#[cfg(test)] +mod test { + use oxc_allocator::Allocator; + use oxc_parser::Parser; + use oxc_semantic::SemanticBuilder; + use oxc_span::SourceType; + use rustc_hash::FxHashSet; + use std::iter::once; + + use super::collect_name_symbols; + + fn collect(source_text: &str) -> FxHashSet { + let allocator = Allocator::default(); + let ret = Parser::new(&allocator, source_text, SourceType::mjs()).parse(); + assert!(!ret.panicked, "{source_text}"); + assert!(ret.errors.is_empty(), "{source_text}"); + let ret = SemanticBuilder::new().build(&ret.program); + assert!(ret.errors.is_empty(), "{source_text}"); + let semantic = ret.semantic; + let symbols = collect_name_symbols(semantic.scoping(), semantic.nodes()); + symbols + .into_iter() + .map(|symbol_id| semantic.scoping().symbol_name(symbol_id).to_string()) + .collect() + } + + #[test] + fn test_declarations() { + assert_eq!(collect("function foo() {}"), once("foo".to_string()).collect()); + assert_eq!(collect("class Foo {}"), once("Foo".to_string()).collect()); + } + + #[test] + fn test_simple_declare_init() { + assert_eq!(collect("var foo = function() {}"), once("foo".to_string()).collect()); + assert_eq!(collect("var foo = () => {}"), once("foo".to_string()).collect()); + assert_eq!(collect("var Foo = class {}"), once("Foo".to_string()).collect()); + } + + #[test] + fn test_simple_assign() { + assert_eq!(collect("var foo; foo = function() {}"), once("foo".to_string()).collect()); + assert_eq!(collect("var foo; foo = () => {}"), once("foo".to_string()).collect()); + assert_eq!(collect("var Foo; Foo = class {}"), once("Foo".to_string()).collect()); + + assert_eq!(collect("var foo; foo ||= function() {}"), once("foo".to_string()).collect()); + assert_eq!( + collect("var foo = 1; foo &&= function() {}"), + once("foo".to_string()).collect() + ); + assert_eq!(collect("var foo; foo ??= function() {}"), once("foo".to_string()).collect()); + } + + #[test] + fn test_default_declarations() { + assert_eq!(collect("var [foo = function() {}] = []"), once("foo".to_string()).collect()); + assert_eq!(collect("var [foo = () => {}] = []"), once("foo".to_string()).collect()); + assert_eq!(collect("var [Foo = class {}] = []"), once("Foo".to_string()).collect()); + assert_eq!(collect("var { foo = function() {} } = {}"), once("foo".to_string()).collect()); + } + + #[test] + fn test_default_assign() { + assert_eq!( + collect("var foo; [foo = function() {}] = []"), + once("foo".to_string()).collect() + ); + assert_eq!(collect("var foo; [foo = () => {}] = []"), once("foo".to_string()).collect()); + assert_eq!(collect("var Foo; [Foo = class {}] = []"), once("Foo".to_string()).collect()); + assert_eq!( + collect("var foo; ({ foo = function() {} } = {})"), + once("foo".to_string()).collect() + ); + } + + #[test] + fn test_for_in_declaration() { + assert_eq!( + collect("for (var foo = function() {} in []) {}"), + once("foo".to_string()).collect() + ); + assert_eq!(collect("for (var foo = () => {} in []) {}"), once("foo".to_string()).collect()); + assert_eq!(collect("for (var Foo = class {} in []) {}"), once("Foo".to_string()).collect()); + } +} diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index 99389f4ae05bd..3c9080df55e2c 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -13,6 +13,7 @@ use oxc_semantic::{Scoping, Semantic, SemanticBuilder, SymbolId}; use oxc_span::Atom; pub(crate) mod base54; +mod keep_names; #[derive(Default, Debug, Clone, Copy)] pub struct MangleOptions {