[AST] PatOrExpr

[magic-akari]

PatOrExpr::Expr stores MemberExpr and some more, and more (I think) with operator assignments, but I'm not sure about it at the moment.

I think Pat::Expr can be used instead of it.

Originally posted by @kdy1 in #5179 (comment)

I found a lot of patterns like the following code, PatOrExpr::Expr and PatOrExpr::Pat(Pat::Expr) are handled the same way.
I am trying to remove PatOrExpr . These logics will be simplified.

swc/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs

Lines 1344 to 1376 in 0a13105

	match &mut b.left {
	PatOrExpr::Expr(b_left) => {
	trace_op!("seq: Try lhs of assign");
	if self.merge_sequential_expr(a, &mut **b_left)? {
	return Ok(true);
	}
	match &**b_left {
	Expr::Ident(..) => {}
	_ => {
	return Ok(false);
	}
	}
	}
	PatOrExpr::Pat(b_left) => match &mut **b_left {
	Pat::Expr(b_left) => {
	trace_op!("seq: Try lhs of assign");
	if self.merge_sequential_expr(a, &mut **b_left)? {
	return Ok(true);
	}
	match &**b_left {
	Expr::Ident(..) => {}
	_ => {
	return Ok(false);
	}
	}
	}
	Pat::Ident(..) => {}
	_ => return Ok(false),
	},
	}

---

If I remove PatOrExpr , PatOrExpr::Expr will be merged into Pat::Expr, and all Idents will be merged into Pat::Ident.
It means that left of AssignExpr will be represented as Pat::Ident whether a = b or a += b. Is this expected?

[magic-akari]

If there is an ident in PatOrExpr, there are 3 cases.

• PatOrExpr::Expr(Expr::Ident)
• PatOrExpr::Pat(Pat::Ident)
• PatOrExpr::Pat(Pat::Expr(Expr::Ident))

After removing PatOrExpr, there are still two cases, Pat::Ident and Pat::Expr(Expr::Ident).

Should Pat::Ident be removed and only Pat::Expr(Expr::Ident) remain?

It's easy to forget about the case of Pat::Expr(Expr::Ident) when we're dealing with Pat::Ident.

If there is the following code, we replace a.b with x, we will get Pat::Expr(Expr::Ident) in the AST.

for(a.b of c){}

//
for(x of c){
    a.b = x;
}

[kdy1]

I think we should not remove Pat::Ident.
We should produce Pat::Ident from the parser, though.

[kdy1]

Alternatively we can reduce Pat::Expr to Pat::ParenExpr, Pat::MemberExpr, Pat::OptChainExpr, Pat::SuperPropExpr

[Austaras]

Alternatively we can reduce Pat::Expr to Pat::ParenExpr, Pat::MemberExpr, Pat::OptChainExpr, Pat::SuperPropExpr

This seems to be better. OptChainExpr cannot be lhs though

[magic-akari]

If a transformation needs to change Expr::Member into Expr::Ident, then it is likely to need to change Pat::Member into Pat::Ident as well. It's easy to forget.

[Austaras]

Oh, I tried to remove Pat::Express and found that it's not only aforementioned four kinds of expressions but also lots of TS expressions. So that's not viable.

[magic-akari]

Another confusing issue.

a.b = 1;

The a.b is parsed as PatOrExpr->Pat->Expr not PatOrExpr->Expr

[kdy1]

Oh yeah... I found that a.b += 1 is PatOrExpr->Expr while a.b = 1 is PatOrExpr->Pat->Expr, which seems wrong

[magic-akari]

Concise design from oxc: AssignmentTarget

OXC Playground
SWC AST

https://github.com/web-infra-dev/oxc/blob/09ebe961d2a6b859f627af86f666d57d28b6a9b3/crates/oxc_ast/src/ast/js.rs#L746-L763

In swc, this structure is called PatOrExpr, while in oxc, it is referred to AssignmentTarget.

AssignmentTarget is an enum with two variants: SimpleAssignmentTarget and AssignmentTargetPattern.

SimpleAssignmentTarget wraps an identifier, or member expression (or TypeScript expression).
AssignmentTargetPattern corresponds to Pat in swc.

This design offers the following advantages:

1. Explicit lhs. By marking the lhs during the parsing phase, we can easily determine whether an expression is an lhs during the transformation process. With the presence of AssignmentTarget, the lhs can only be an AssignmentTarget.
2. Non-overlapping structures. When an identifier appears within AssignmentTarget, it can only be under SimpleAssignmentTarget. (However, in TypeScript, there is an additional layer of wrapping, which can be disregarded if the TS pass has already been executed.)
3. Convenient conversion. AssignmentTarget is a limited expression, not arbitrary. This is particularly useful when converting between foo and foo.bar, as there is no need to cross the boundaries of Pat and Expr.

[kdy1]

Let's do this. Will you do it? If not, I'll do it.

[kdy1]

Name should be

• AssignTarget
• SimpleAssigTarget
• AssignTargetPat

I guess?

[magic-akari]

Let's do this. Will you do it? If not, I'll do it.

I gave it a go, however it didn't work out. Good luck to you.