fix(semantic)!: correct all ReferenceFlags::Write according to the spec (#7388)

close #7323

According to the specification re-design the JavaScript-part ReferenceFlags inferring approach.

* https://tc39.es/ecma262/#sec-assignment-operators-runtime-semantics-evaluation
* https://tc39.es/ecma262/#sec-postfix-increment-operator-runtime-semantics-evaluation
* https://tc39.es/ecma262/#sec-runtime-semantics-restdestructuringassignmentevaluation
* ... See references of https://tc39.es/ecma262/#sec-putvalue

### Changes

1. The left-hand of `AssignmentExpression` is always `ReferenceFlags::Write`
```js
let a = 0;
console.log(a = 0);
            ^ Write only
```

2. The `argument` of `UpdateExpression` is always `ReferenceFlags::Read | Write`
```js
let a = 0;
a++;
^ Read and Write
```

This change might cause some trouble for `Minfier` to remove this code, because ‘a’ is not used elsewhere. I have taken a look at `esbuild` and `Terser`. Only the `Terser` can remove this code.

Author: Dunqing

crates/oxc_linter/src/rules/eslint/no_const_assign.rs

@@ -82,6 +82,7 @@ fn test() {
         ("try {} catch (x) { x = 1; }", None),
         ("const a = 1; { let a = 2; { a += 1; } }", None),
         ("const foo = 1;let bar;bar[foo ?? foo] = 42;", None),
+        ("const FOO = 1; ({ files = FOO } = arg1); ", None),
     ];
 
     let fail = vec![

crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs

@@ -34,7 +34,7 @@ fn test() {
     let pass = vec![
         (
             "var foo = 5;
-			
+
 			label: while (true) {
 			    console.log(foo);
 			    break label;
@@ -43,7 +43,7 @@ fn test() {
         ),
         (
             "var foo = 5;
-			
+
 			while (true) {
 			    console.log(foo);
 			    break;

crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs

@@ -270,7 +270,6 @@ fn test_vars_reassignment() {
         }
         ",
         "let a = 0; let b = a++; f(b);",
-        "let a = 0, b = 1; let c = b = a = 1; f(c+b);",
         // implicit returns
         "
 		let i = 0;
@@ -332,6 +331,7 @@ fn test_vars_reassignment() {
         "let a = 0; let b = (0, (a++, 0)); f(b);",
         "let a = 0; let b = ((0, a++), 0); f(b);",
         "let a = 0; let b = (a, 0) + 1; f(b);",
+        "let a = 0, b = 1; let c = b = a = 1; f(c+b);",
     ];
 
     Tester::new(NoUnusedVars::NAME, pass, fail)

crates/oxc_linter/src/snapshots/no_unused_vars@eslint.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/oxc_linter/src/tester.rs
-snapshot_kind: text
 ---
   ⚠ eslint(no-unused-vars): Function 'foox' is declared but never used.
    ╭─[no_unused_vars.tsx:1:10]
@@ -261,6 +260,15 @@ snapshot_kind: text
    ╰────
   help: Consider removing this declaration.
 
+  ⚠ eslint(no-unused-vars): Variable 'a' is assigned a value but never used.
+   ╭─[no_unused_vars.tsx:1:20]
+ 1 │ function f() { var a = 1; return function(){ f(a = 2); }; }
+   ·                    ┬                           ┬
+   ·                    │                           ╰── it was last assigned here
+   ·                    ╰── 'a' is declared here
+   ╰────
+  help: Did you mean to use this variable?
+
   ⚠ eslint(no-unused-vars): Identifier 'x' is imported but never used.
    ╭─[no_unused_vars.tsx:1:8]
  1 │ import x from "y";

crates/oxc_linter/src/snapshots/no_unused_vars@oxc-vars-reassignment.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/oxc_linter/src/tester.rs
-snapshot_kind: text
 ---
   ⚠ eslint(no-unused-vars): Variable 'a' is assigned a value but never used. Unused variables should start with a '_'.
    ╭─[no_unused_vars.tsx:1:5]
@@ -99,3 +98,12 @@ snapshot_kind: text
    ·     ╰── 'a' is declared here
    ╰────
   help: Consider removing this declaration.
+
+  ⚠ eslint(no-unused-vars): Variable 'a' is assigned a value but never used. Unused variables should start with a '_'.
+   ╭─[no_unused_vars.tsx:1:5]
+ 1 │ let a = 0, b = 1; let c = b = a = 1; f(c+b);
+   ·     ┬                         ┬
+   ·     │                         ╰── it was last assigned here
+   ·     ╰── 'a' is declared here
+   ╰────
+  help: Did you mean to use this variable?

crates/oxc_semantic/src/builder.rs

@@ -15,7 +15,7 @@ use oxc_cfg::{
 };
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_span::{Atom, CompactStr, SourceType, Span};
-use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};
+use oxc_syntax::module_record::ModuleRecord;
 
 use crate::{
     binder::Binder,
@@ -890,8 +890,17 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         let kind = AstKind::AssignmentExpression(self.alloc(expr));
         self.enter_node(kind);
+
+        if !expr.operator.is_assign() {
+            // Only when the operator is not an `=` operator, the left-hand side is both read and write.
+            // <https://tc39.es/ecma262/#sec-assignment-operators-runtime-semantics-evaluation>
+            self.current_reference_flags = ReferenceFlags::read_write();
+        }
+
         self.visit_assignment_target(&expr.left);
 
+        self.current_reference_flags = ReferenceFlags::empty();
+
         /* cfg  */
         let cfg_ixs = control_flow!(self, |cfg| {
             if expr.operator.is_logical() {
@@ -1760,6 +1769,86 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         self.leave_node(kind);
         self.leave_scope();
     }
+
+    fn visit_update_expression(&mut self, it: &UpdateExpression<'a>) {
+        let kind = AstKind::UpdateExpression(self.alloc(it));
+        self.enter_node(kind);
+        // `++a` or `a--`
+        //    ^      ^ We always treat `a` as Read and Write reference,
+        self.current_reference_flags = ReferenceFlags::read_write();
+        self.visit_simple_assignment_target(&it.argument);
+        self.current_reference_flags = ReferenceFlags::empty();
+        self.leave_node(kind);
+    }
+
+    fn visit_member_expression(&mut self, it: &MemberExpression<'a>) {
+        let kind = AstKind::MemberExpression(self.alloc(it));
+        self.enter_node(kind);
+
+        // A.B = 1;
+        // ^^^ Can't treat A as a Write reference since it's A's property(B) that changes.
+        self.current_reference_flags -= ReferenceFlags::Write;
+
+        match it {
+            MemberExpression::ComputedMemberExpression(it) => {
+                self.visit_computed_member_expression(it);
+            }
+            MemberExpression::StaticMemberExpression(it) => self.visit_static_member_expression(it),
+            MemberExpression::PrivateFieldExpression(it) => self.visit_private_field_expression(it),
+        }
+        self.leave_node(kind);
+    }
+
+    fn visit_simple_assignment_target(&mut self, it: &SimpleAssignmentTarget<'a>) {
+        let kind = AstKind::SimpleAssignmentTarget(self.alloc(it));
+        self.enter_node(kind);
+        let prev_reference_flags = self.current_reference_flags;
+        // Except that the read-write flags has been set in visit_assignment_expression
+        // and visit_update_expression, this is always a write-only reference here.
+        if !self.current_reference_flags.is_write() {
+            self.current_reference_flags = ReferenceFlags::Write;
+        }
+
+        match it {
+            SimpleAssignmentTarget::AssignmentTargetIdentifier(it) => {
+                self.visit_identifier_reference(it);
+            }
+            SimpleAssignmentTarget::TSAsExpression(it) => {
+                self.visit_ts_as_expression(it);
+            }
+            SimpleAssignmentTarget::TSSatisfiesExpression(it) => {
+                self.visit_ts_satisfies_expression(it);
+            }
+            SimpleAssignmentTarget::TSNonNullExpression(it) => {
+                self.visit_ts_non_null_expression(it);
+            }
+            SimpleAssignmentTarget::TSTypeAssertion(it) => {
+                self.visit_ts_type_assertion(it);
+            }
+            SimpleAssignmentTarget::TSInstantiationExpression(it) => {
+                self.visit_ts_instantiation_expression(it);
+            }
+            match_member_expression!(SimpleAssignmentTarget) => {
+                self.visit_member_expression(it.to_member_expression());
+            }
+        }
+        self.current_reference_flags = prev_reference_flags;
+        self.leave_node(kind);
+    }
+
+    fn visit_assignment_target_property_identifier(
+        &mut self,
+        it: &AssignmentTargetPropertyIdentifier<'a>,
+    ) {
+        // NOTE: AstKind doesn't exists!
+        let prev_reference_flags = self.current_reference_flags;
+        self.current_reference_flags = ReferenceFlags::Write;
+        self.visit_identifier_reference(&it.binding);
+        self.current_reference_flags = prev_reference_flags;
+        if let Some(init) = &it.init {
+            self.visit_expression(init);
+        }
+    }
 }
 
 impl<'a> SemanticBuilder<'a> {
@@ -1940,29 +2029,6 @@ impl<'a> SemanticBuilder<'a> {
             AstKind::IdentifierReference(ident) => {
                 self.reference_identifier(ident);
             }
-            AstKind::UpdateExpression(_) => {
-                if !self.current_reference_flags.is_type()
-                    && self.is_not_expression_statement_parent()
-                {
-                    self.current_reference_flags |= ReferenceFlags::Read;
-                }
-                self.current_reference_flags |= ReferenceFlags::Write;
-            }
-            AstKind::AssignmentExpression(expr) => {
-                if expr.operator != AssignmentOperator::Assign
-                    || self.is_not_expression_statement_parent()
-                {
-                    self.current_reference_flags |= ReferenceFlags::Read;
-                }
-            }
-            AstKind::MemberExpression(_) => {
-                // A.B = 1;
-                // ^^^ we can't treat A as Write reference, because it's the property(B) of A that change
-                self.current_reference_flags -= ReferenceFlags::Write;
-            }
-            AstKind::AssignmentTarget(_) => {
-                self.current_reference_flags |= ReferenceFlags::Write;
-            }
             AstKind::LabeledStatement(stmt) => {
                 self.unused_labels.add(stmt.label.name.as_str());
             }
@@ -2018,27 +2084,12 @@ impl<'a> SemanticBuilder<'a> {
             AstKind::TSTypeName(_) => {
                 self.current_reference_flags -= ReferenceFlags::Type;
             }
-            AstKind::UpdateExpression(_) => {
-                if self.is_not_expression_statement_parent() {
-                    self.current_reference_flags -= ReferenceFlags::Read;
-                }
-                self.current_reference_flags -= ReferenceFlags::Write;
-            }
-            AstKind::AssignmentExpression(_) | AstKind::ExportNamedDeclaration(_)
+            AstKind::ExportNamedDeclaration(_)
             | AstKind::TSTypeQuery(_)
             // Clear the reference flags that are set in AstKind::PropertySignature
             | AstKind::PropertyKey(_) => {
                 self.current_reference_flags = ReferenceFlags::empty();
             }
-            AstKind::AssignmentTarget(_) =>{
-                // Handle nested assignment targets like `({a: b} = obj)`
-                if !matches!(
-                    self.nodes.parent_kind(self.current_node_id),
-                    Some(AstKind::ObjectAssignmentTarget(_) | AstKind::ArrayAssignmentTarget(_))
-                ) {
-                    self.current_reference_flags -= ReferenceFlags::Write;
-                }
-            },
             AstKind::LabeledStatement(_) => self.unused_labels.mark_unused(self.current_node_id),
             _ => {}
         }
@@ -2066,34 +2117,12 @@ impl<'a> SemanticBuilder<'a> {
     }
 
     /// Resolve reference flags for the current ast node.
+    #[inline]
     fn resolve_reference_usages(&self) -> ReferenceFlags {
         if self.current_reference_flags.is_empty() {
             ReferenceFlags::Read
         } else {
             self.current_reference_flags
         }
     }
-
-    fn is_not_expression_statement_parent(&self) -> bool {
-        for node in self.nodes.ancestors(self.current_node_id).skip(1) {
-            return match node.kind() {
-                AstKind::ParenthesizedExpression(_) => continue,
-                AstKind::ExpressionStatement(_) => {
-                    if self.current_scope_flags().is_arrow() {
-                        if let Some(node) = self.nodes.ancestors(node.id()).nth(2) {
-                            // (x) => x++
-                            //        ^^^ implicit return, we need to treat `x` as a read reference
-                            if matches!(node.kind(), AstKind::ArrowFunctionExpression(arrow) if arrow.expression)
-                            {
-                                return true;
-                            }
-                        }
-                    }
-                    false
-                }
-                _ => true,
-            };
-        }
-        false
-    }
 }

crates/oxc_semantic/src/lib.rs

@@ -365,9 +365,8 @@ mod tests {
             // assignment expressions count as read-write
             (SourceType::default(), "let a = 1, b; b = a += 5", ReferenceFlags::read_write()),
             (SourceType::default(), "let a = 1; a += 5", ReferenceFlags::read_write()),
-            // note: we consider a to be written, and the read of `1` propagates upwards
-            (SourceType::default(), "let a, b; b = a = 1", ReferenceFlags::read_write()),
-            (SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlags::read_write()),
+            (SourceType::default(), "let a, b; b = a = 1", ReferenceFlags::write()),
+            (SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlags::write()),
             (SourceType::default(), "let a, b, c; b = c = a", ReferenceFlags::read()),
             // sequences return last read_write in sequence
             (SourceType::default(), "let a, b; b = (0, a++)", ReferenceFlags::read_write()),
@@ -404,27 +403,28 @@ mod tests {
             // least, or now)
             (SourceType::default(), "let a, b; b = (a, 0)", ReferenceFlags::read()),
             (SourceType::default(), "let a, b; b = (--a, 0)", ReferenceFlags::read_write()),
-            // other reads after a is written
-            // a = 1 writes, but the CallExpression reads the rhs (1) so a isn't read
             (
                 SourceType::default(),
                 "let a; function foo(a) { return a }; foo(a = 1)",
-                ReferenceFlags::read_write(),
+                //                                        ^ write
+                ReferenceFlags::write(),
             ),
             // member expression
             (SourceType::default(), "let a; a.b = 1", ReferenceFlags::read()),
             (SourceType::default(), "let a; let b; b[a += 1] = 1", ReferenceFlags::read_write()),
             (
                 SourceType::default(),
                 "let a; let b; let c; b[c[a = c['a']] = 'c'] = 'b'",
-                ReferenceFlags::read_write(),
+                //                        ^ write
+                ReferenceFlags::write(),
             ),
             (
                 SourceType::default(),
                 "let a; let b; let c; a[c[b = c['a']] = 'c'] = 'b'",
                 ReferenceFlags::read(),
             ),
-            (SourceType::default(), "console.log;let a=0;a++", ReferenceFlags::write()),
+            (SourceType::default(), "console.log;let a=0;a++", ReferenceFlags::read_write()),
+            //                                           ^^^ UpdateExpression is always a read | write
             // typescript
             (typescript, "let a: number = 1; (a as any) = true", ReferenceFlags::write()),
             (typescript, "let a: number = 1; a = true as any", ReferenceFlags::write()),

crates/oxc_semantic/tests/fixtures/oxc/assignment/array.js

@@ -0,0 +1,3 @@
+let read = {}, write = {};
+
+[write = read, [write = read]] = ref;

crates/oxc_semantic/tests/fixtures/oxc/assignment/array.snap

@@ -0,0 +1,54 @@
+---
+source: crates/oxc_semantic/tests/main.rs
+input_file: crates/oxc_semantic/tests/fixtures/oxc/assignment/array.js
+---
+[
+  {
+    "children": [],
+    "flags": "ScopeFlags(StrictMode | Top)",
+    "id": 0,
+    "node": "Program",
+    "symbols": [
+      {
+        "flags": "SymbolFlags(BlockScopedVariable)",
+        "id": 0,
+        "name": "read",
+        "node": "VariableDeclarator(read)",
+        "references": [
+          {
+            "flags": "ReferenceFlags(Read)",
+            "id": 1,
+            "name": "read",
+            "node_id": 17
+          },
+          {
+            "flags": "ReferenceFlags(Read)",
+            "id": 3,
+            "name": "read",
+            "node_id": 25
+          }
+        ]
+      },
+      {
+        "flags": "SymbolFlags(BlockScopedVariable)",
+        "id": 1,
+        "name": "write",
+        "node": "VariableDeclarator(write)",
+        "references": [
+          {
+            "flags": "ReferenceFlags(Write)",
+            "id": 0,
+            "name": "write",
+            "node_id": 16
+          },
+          {
+            "flags": "ReferenceFlags(Write)",
+            "id": 2,
+            "name": "write",
+            "node_id": 24
+          }
+        ]
+      }
+    ]
+  }
+]