fix(transformer/typescript): typescript syntax within `SimpleAssignmentTarget` with `MemberExpressions` is not stripped by Dunqing · Pull Request #4920 · oxc-project/oxc

Dunqing · 2024-08-15T17:33:08Z

I added the move_identifier_reference and move_member_expression methods used to take ownership in ast_builder_impl. This way can let us get rid of ast.copy.

Another possible approach is to add get_expression_owner to SimpleAssignmentTarget and a get_inner_expression_owner method to Expression. And add an into_xxxxx method for inherit_variants macro

The implementation looks like this

let Some(expression) = self.get_expression_owner() else { return; }
match expr.get_inner_expression_owner() {
    Expression::Identifier(ident) => {
        *target = self.ctx.ast.simple_assignment_target_from_identifier_reference(ident);
    }
    inner_expr @ match_member_expression!(Expression) => {
        *target = SimpleAssignmentTarget::from(
            inner_expr.into_member_expression()
        );
    }
    _ => (),
}

Dunqing · 2024-08-15T17:33:32Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite

codspeed-hq · 2024-08-15T17:40:48Z

CodSpeed Performance Report

Merging #4920 will not alter performance

_{Comparing 08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped (248a757) with main (508644a)}

Summary

✅ 29 untouched benchmarks

crates/oxc_transformer/src/typescript/annotations.rs

graphite-app · 2024-08-16T05:04:39Z

Merge activity

Aug 16, 1:04 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Aug 16, 1:04 AM EDT: Boshen added this pull request to the Graphite merge queue.
Aug 16, 1:08 AM EDT: Boshen merged this pull request with the Graphite merge queue.

…ntTarget` with `MemberExpressions` is not stripped (#4920) close: #4870 I added the `move_identifier_reference` and `move_member_expression` methods used to take ownership in `ast_builder_impl`. This way can let us get rid of `ast.copy`. Another possible approach is to add `get_expression_owner` to `SimpleAssignmentTarget` and a `get_inner_expression_owner` method to `Expression`. And add an `into_xxxxx` method for `inherit_variants` macro The implementation looks like this ```rs let Some(expression) = self.get_expression_owner() else { return; } match expr.get_inner_expression_owner() { Expression::Identifier(ident) => { *target = self.ctx.ast.simple_assignment_target_from_identifier_reference(ident); } inner_expr @ match_member_expression!(Expression) => { *target = SimpleAssignmentTarget::from( inner_expr.into_member_expression() ); } _ => (), } ```

## [0.24.3] - 2024-08-18 ### Features - d49fb16 oxc_codegen: Support generate range leading comments (#4898) (IWANABETHATGUY) - 80d0d1f semantic: Check for invalid interface heritage clauses (#4928) (DonIsaac) - 48821c0 semantic,syntax: Add SymbolFlags::ArrowFunction (#4946) (DonIsaac) - f1fcdde transformer: Support react fast refresh (#4587) (Dunqing) - 0d79122 transformer: Support logical-assignment-operators plugin (#4890) (Dunqing) - ab1d08c transformer: Support `optional-catch-binding` plugin (#4885) (Dunqing) - 69da9fd transformer: Support nullish-coalescing-operator plugin (#4884) (Dunqing) - 3a66e58 transformer: Support exponentiation operator plugin (#4876) (Dunqing) - f88cbcd transformer: Add `BoundIdentifier::new_uid_in_current_scope` method (#4903) (overlookmotel) - 1e6d0fe transformer: Add methods to `BoundIdentifier` (#4897) (overlookmotel) - fd34640 traverse: Support `generate_uid_based_on_node` method in `TraverseCtx` (#4940) (Dunqing) - 72a37fc traverse: Support `clone_identifier_reference` method in `TraverseCtx` (#4880) (Dunqing) ### Bug Fixes - c0b26f4 ast: Do not include `scope_id` fields in JSON AST (#4858) (overlookmotel) - bbf9ec0 codegen: Add missing `declare` to `PropertyDefinition` (#4937) (Boshen) - f210cf7 codegen: Print `TSSatisfiesExpression` and `TSInstantiationExpression` (#4936) (Boshen) - 21f5762 codegen: Minify large numbers (#4889) (Boshen) - e8de4bd codegen: Fix whitespace issue when minifying `x in new Error()` (#4886) (Boshen) - a226962 codegen: Print `TSNonNullExpression` (#4869) (Boshen) - 3da33d3 codegen: Missing parenthesis for `PrivateInExpression` (#4865) (Boshen) - 1808529 codegen: Dedupe pure annotation comments (#4862) (IWANABETHATGUY) - d3bbc62 isolated-declarations: Declare modifier of PropertyDefinition should not be retained (#4941) (Dunqing) - 8e80f59 isolated_declarations: Class properties should still be lifted from private constructors (#4934) (michaelm) - b3ec9e5 isolated_declarations: Always emit module declarations that perform augmentation (#4919) (michaelm) - 0fb0b71 isolated_declarations: Always emit module declarations (#4911) (michaelm) - 4a16916 isolated_declarations: Support expando functions (#4910) (michaelm) - 508644a linter/tree-shaking: Correct the calculation of `>>`, `<<` and `>>>` (#4932) (mysteryven) - 46cb1c1 minifier: Handle `Object.definedPropert(exports` for @babel/types/lib/index.js (#4933) (Boshen) - 81fd637 minifier: Do not fold `0 && (module.exports = {})` for `cjs-module-lexer` (#4878) (Boshen) - 879a271 minifier: Do not join `require` calls for `cjs-module-lexer` (#4875) (Boshen) - 1bdde2c parser: Detect @flow in `/** @flow */ comment (#4861) (Boshen) - 2476dce transformer: Remove an `ast.copy` from `NullishCoalescingOperator` transform (#4913) (overlookmotel) - 248a757 transformer/typescript: Typescript syntax within `SimpleAssignmentTarget` with `MemberExpressions` is not stripped (#4920) (Dunqing) ### Documentation - 47c9552 ast, ast_macros, ast_tools: Better documentation for `Ast` helper attributes. (#4856) (rzvxa) - 0a01a47 semantic: Improve documentation (#4850) (DonIsaac) - 9c700ed transformer: Add README including style guide (#4899) (overlookmotel) ### Refactor - a6967b3 allocator: Correct code comment (#4904) (overlookmotel) - 90d0b2b allocator, ast, span, ast_tools: Use `allocator` as var name for `Allocator` (#4900) (overlookmotel) - 1eb59d2 ast, isolated_declarations, transformer: Mark `AstBuilder::copy` as an unsafe function (#4907) (overlookmotel) - 8e8fcd0 ast_tools: Rename `oxc_ast_codegen` to `oxc_ast_tools`. (#4846) (rzvxa) - 786bf07 index: Shorten code and correct comment (#4905) (overlookmotel) - ea1e64a semantic: Make SemanticBuilder opaque (#4851) (DonIsaac) - 5fd1701 sourcemap: Lower the `msrv`. (#4873) (rzvxa) - 48a1c32 syntax: Inline trivial bitflags methods (#4877) (overlookmotel) - 452187a transformer: Rename `BoundIdentifier::new_uid_in_root_scope` (#4902) (overlookmotel) - 707a01f transformer: Re-order `BoundIdentifier` methods (#4896) (overlookmotel) - 117dff2 transformer: Improve comments for `BoundIdentifier` helper (#4895) (overlookmotel) Co-authored-by: Boshen <[email protected]>

…e empty string (#4977) #4920 introduced `AstBuilder::move_identifier_reference`. Make this slightly cheaper by using a static empty `Atom` rather than relying on `"".into_in(allocator)` which allocates an empty string into arena.

…#4982) Follow on after #4920. #4920 removed a usage of `AstBuilder::copy` and replaced with 2 new methods `AstBuilder::move_identifier_reference` and `AstBuilder:: move_member_expression`. We can instead use `AstBuilder::move_expression` earlier and then unpack the enum again. Hopefully the compiler can see that the 2 `unreachable!()` branches are indeed unreachable and elide them. `move_expression` is preferable to `move_member_expression` as it only creates 1 temporary `Box<Expression::NullLiteral>` instead of 2.

Remove one usage of unsound `AstBuilder::copy` method, using the new `get_inner_expression_mut` method introduced in #4920.

github-actions bot added A-ast Area - AST A-transformer Area - Transformer / Transpiler labels Aug 15, 2024

Dunqing requested a review from overlookmotel August 15, 2024 17:49

Dunqing changed the title ~~feat(transformer/typescript): typescript syntax within SimpleAssignmentTarget with MemberExpressions is not stripped~~ fix(transformer/typescript): typescript syntax within SimpleAssignmentTarget with MemberExpressions is not stripped Aug 15, 2024

Dunqing force-pushed the 08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped branch from 56296d4 to 1493b4f Compare August 15, 2024 17:53

Dunqing requested a review from Boshen August 15, 2024 17:58

Boshen reviewed Aug 16, 2024

View reviewed changes

crates/oxc_transformer/src/typescript/annotations.rs Outdated Show resolved Hide resolved

Dunqing force-pushed the 08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped branch from 1493b4f to 2549549 Compare August 16, 2024 04:21

Dunqing requested a review from Boshen August 16, 2024 04:21

Boshen added the 0-merge Merge with Graphite Merge Queue label Aug 16, 2024

Boshen force-pushed the 08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped branch from 2549549 to 248a757 Compare August 16, 2024 05:05

graphite-app bot merged commit 248a757 into main Aug 16, 2024

graphite-app bot deleted the 08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped branch August 16, 2024 05:08

oxc-bot mentioned this pull request Aug 18, 2024

Release crates v0.24.3 #4950

Merged

overlookmotel mentioned this pull request Aug 19, 2024

refactor(ast): AstBuilder::move_identifier_reference do not allocate empty string #4977

Merged

This was referenced Aug 19, 2024

refactor(transformer): TS annotations transform use move_expression #4982

Merged

fix(transformer): remove an AstBuilder::copy call #4983

Merged

Dunqing pushed a commit that referenced this pull request Aug 19, 2024

fix(transformer): remove an AstBuilder::copy call (#4983)

edacf93

Remove one usage of unsound `AstBuilder::copy` method, using the new `get_inner_expression_mut` method introduced in #4920.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix(transformer/typescript): typescript syntax within `SimpleAssignmentTarget` with `MemberExpressions` is not stripped#4920

fix(transformer/typescript): typescript syntax within `SimpleAssignmentTarget` with `MemberExpressions` is not stripped#4920
graphite-app[bot] merged 1 commit intomainfrom
08-16-feat_transformer_typescript_typescript_syntax_within_simpleassignmenttarget_with_memberexpressions_is_not_stripped

Dunqing commented Aug 15, 2024 •

edited

Loading

Uh oh!

Dunqing commented Aug 15, 2024

Uh oh!

codspeed-hq bot commented Aug 15, 2024 •

edited

Loading

Uh oh!

Uh oh!

graphite-app bot commented Aug 16, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Comments

Conversation

Dunqing commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dunqing commented Aug 15, 2024

Uh oh!

codspeed-hq bot commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #4920 will not alter performance

Summary

Uh oh!

Uh oh!

graphite-app bot commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dunqing commented Aug 15, 2024 •

edited

Loading

codspeed-hq bot commented Aug 15, 2024 •

edited

Loading

graphite-app bot commented Aug 16, 2024 •

edited

Loading