Fix JSON deserialization of AssignExpr

[overlookmotel]

Description:

Fix for #5168. However:

1. Surely there's an easier way to do this?
2. I imagine there is a more idiomatic way to use match { } here:

https://github.com/overlookmotel/swc/blob/assign-expr-deserialize/crates/swc_ecma_ast/src/expr.rs#L568-L576

I would appreciate any pointers as to how to improve this.

Related issue (if exists):

#5168

[kdy1]

Please remove images

[overlookmotel]

Oops sorry I based this on wrong branch. Have pushed again with just the relevant commit.

[magic-akari]

Is there a case where PatOrExpr::Expr is not Expr::Ident? @kdy1
I wonder if PatOrExpr is redundant?

---

Oh, it may be Expr::Member.

[kdy1]

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.

[magic-akari]

It seems a lot of work to do if I remove the PatOrExpr, I will try it later.
And I don't have strong opinions on this pull request change. It's ok to fix #5168.

[kdy1]

Thank you!

[kdy1]

Thanks!

---

swc-bump:

• swc_ecma_ast

[swc-bot]

Automated review comment generated by auto-rebase script

[overlookmotel]

I've made the changes requested and tidied up some other bits.

I'm still not completely happy with this:

if let PatOrExpr::Pat(ref pat) = left {
  if let Pat::Ident(ident) = &**pat {
    left = PatOrExpr::Expr(Box::new(Expr::Ident(ident.id.clone())));
  }
}

The .clone() call should be unnecessary - ident.id can just be moved from the old structure to the new. But I couldn't find a way to get the compiler to accept this, since pat is borrowed.

My first attempt was:

left = match left {
  PatOrExpr::Pat(pat) => match *pat {
    Pat::Ident(ident) => PatOrExpr::Expr(Box::new(Expr::Ident(ident.id))),
    pat => PatOrExpr::Pat(Box::new(pat)),
  },
  left => left,
}

This avoids cloning as it takes ownership, but has the worse consequences of unclear code and re-creating PatOrExpr::Pat(Box::new(pat)) unnecessarily in the majority of cases.

Or am I worrying too much? Will the compiler compile away the .clone() call?

[kdy1]

I think so?
Even if the compiler fails to optimize it, it will not affect performance in almost all cases.
clone() of Ident is typically very cheap.

[overlookmotel]

Is there no way to tell compiler "give me ownership of ident, and make left and pat uninitialized"? Sorry, this has nothing to do with SWC specifically, I'm just still learning Rust. This feels like it'd be a common situation.