Flow analysis: fix first phase handling of pattern assignments.

Prior to this change, the first phase of flow analysis, in which flow
analysis "looks ahead" to see which variables are potentially assigned
in each code block, was failing to properly account for variables that
are assigned by pattern assignment expressions. This "look ahead"
information is used by flow analysis to determine the effects of
nonlinear control flow (e.g. to figure out which variables to demote
at the top of a loop, or to figure out which variables are potentially
assigned inside a closure). As a result, it was possible to construct
correct code that would be rejected by the analyzer and compiler, as
well as incorrect code that would (unsoundly) be accepted.

The fix is to call `AssignedVariables.write` from the analyzer's
`FlowAnalysisVisitor`, and from the CFE's `BodyBuilder`, to let flow
analysis know about variable assignments that occur inside pattern
assignment expressions.

Fixes #52745.

Change-Id: I0e6f426a5c5c36f214d5a206aaf5a2de0bfdaac1
Bug: #52745
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310502
Auto-Submit: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

@@ -532,6 +532,14 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor<void> {
 
   _AssignedVariablesVisitor(this.assignedVariables);
 
+  @override
+  void visitAssignedVariablePattern(AssignedVariablePattern node) {
+    var element = node.element;
+    if (element is PromotableElement) {
+      assignedVariables.write(element);
+    }
+  }
+
   @override
   void visitAssignmentExpression(AssignmentExpression node) {
     var left = node.leftHandSide;

pkg/analyzer/test/src/diagnostics/definitely_unassigned_late_local_variable_test.dart

@@ -44,6 +44,18 @@ void f() {
     ]);
   }
 
+  test_mightBeAssigned_byPatternAssignment() async {
+    await assertNoErrorsInCode('''
+void main() {
+  late String s;
+  () {
+    (s,) = ('',);
+  }();
+  s;
+}
+''');
+  }
+
   test_mightBeAssigned_if_else() async {
     await assertNoErrorsInCode(r'''
 void f(bool c) {

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

@@ -9401,8 +9401,10 @@ class BodyBuilder extends StackListenerImpl
     String name = variable.lexeme;
     Expression variableUse = toValue(scopeLookup(scope, name, variable));
     if (variableUse is VariableGet) {
+      VariableDeclaration variableDeclaration = variableUse.variable;
       pattern = forest.createAssignedVariablePattern(
-          variable.charOffset, variableUse.variable);
+          variable.charOffset, variableDeclaration);
+      registerVariableAssignment(variableDeclaration);
     } else {
       addProblem(fasta.messagePatternAssignmentNotLocalVariable,
           variable.charOffset, variable.charCount);

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart

@@ -0,0 +1,11 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+void main() {
+  late String s;
+  () {
+    (s,) = ('',);
+  }();
+  s;
+}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.strong.expect

@@ -0,0 +1,17 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method main() → void {
+  late core::String s;
+  (() → Null {
+    block {
+      core::String #t1;
+      final synthesized dynamic #0#0 = ("");
+      if(!(let final dynamic #t2 = #t1 = #0#0{(core::String)}.$1{core::String} in true))
+        throw new core::StateError::•("Pattern matching error");
+      s = #t1;
+    } =>#0#0;
+  })(){() → Null};
+  s;
+}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.strong.transformed.expect

@@ -0,0 +1,22 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method main() → void {
+  late core::String s;
+  (() → Null {
+    block {
+      core::String #t1;
+      final synthesized dynamic #0#0 = ("");
+      if(!(let final core::String #t2 = #t1 = #0#0{(core::String)}.$1{core::String} in true))
+        throw new core::StateError::•("Pattern matching error");
+      s = #t1;
+    } =>#0#0;
+  })(){() → Null};
+  s;
+}
+
+
+Extra constant evaluation status:
+Evaluated: RecordLiteral @ org-dartlang-testcase:///variable_might_be_assigned_by_variable_pattern.dart:8:12 -> RecordConstant(const (""))
+Extra constant evaluation: evaluated: 15, effectively constant: 1

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.textual_outline.expect

@@ -0,0 +1 @@
+void main() {}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.textual_outline_modelled.expect

@@ -0,0 +1 @@
+void main() {}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.weak.expect

@@ -0,0 +1,17 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method main() → void {
+  late core::String s;
+  (() → Null {
+    block {
+      core::String #t1;
+      final synthesized dynamic #0#0 = ("");
+      if(!(let final dynamic #t2 = #t1 = #0#0{(core::String)}.$1{core::String} in true))
+        throw new core::StateError::•("Pattern matching error");
+      s = #t1;
+    } =>#0#0;
+  })(){() → Null};
+  s;
+}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.weak.modular.expect

@@ -0,0 +1,17 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method main() → void {
+  late core::String s;
+  (() → Null {
+    block {
+      core::String #t1;
+      final synthesized dynamic #0#0 = ("");
+      if(!(let final dynamic #t2 = #t1 = #0#0{(core::String)}.$1{core::String} in true))
+        throw new core::StateError::•("Pattern matching error");
+      s = #t1;
+    } =>#0#0;
+  })(){() → Null};
+  s;
+}

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.weak.outline.expect

@@ -0,0 +1,5 @@
+library;
+import self as self;
+
+static method main() → void
+  ;

pkg/front_end/testcases/patterns/variable_might_be_assigned_by_variable_pattern.dart.weak.transformed.expect

@@ -0,0 +1,22 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method main() → void {
+  late core::String s;
+  (() → Null {
+    block {
+      core::String #t1;
+      final synthesized dynamic #0#0 = ("");
+      if(!(let final core::String #t2 = #t1 = #0#0{(core::String)}.$1{core::String} in true))
+        throw new core::StateError::•("Pattern matching error");
+      s = #t1;
+    } =>#0#0;
+  })(){() → Null};
+  s;
+}
+
+
+Extra constant evaluation status:
+Evaluated: RecordLiteral @ org-dartlang-testcase:///variable_might_be_assigned_by_variable_pattern.dart:8:12 -> RecordConstant(const (""))
+Extra constant evaluation: evaluated: 15, effectively constant: 1

tests/language/patterns/flow_analysis/pattern_assignment_lookahead_error_test.dart

@@ -0,0 +1,31 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Verifies that the first phase of flow analysis (used for "lookahead" to see
+// what variables are assigned inside loops and closures) properly detects
+// variables assigned inside pattern assignments.
+
+int f(int? i) {
+  if (i == null) return 0;
+  int k = 0;
+  // `i` is promoted to non-nullable `int` now.
+  for (int j = 0; j < 2; j++) {
+    // `i` should be demoted at this point, because it's assigned later in the
+    // loop, so this statement should be an error.
+    k += i;
+    //^
+    // [cfe] A value of type 'num' can't be assigned to a variable of type 'int'.
+    //   ^
+    // [analyzer] COMPILE_TIME_ERROR.ARGUMENT_TYPE_NOT_ASSIGNABLE
+    // [cfe] A value of type 'int?' can't be assigned to a variable of type 'num' because 'int?' is nullable and 'num' isn't.
+
+    // Now assign a nullable value to `i`.
+    (i,) = (null,);
+  }
+  return k;
+}
+
+void main() {
+  print(f(0));
+}

tests/language/patterns/flow_analysis/pattern_assignment_lookahead_test.dart

@@ -0,0 +1,23 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Verifies that the first phase of flow analysis (used for "lookahead" to see
+// what variables are assigned inside loops and closures) properly detects
+// variables assigned inside pattern assignments.
+
+void main() {
+  late String s;
+  // `s` is definitely unassigned at this point.
+  () {
+    (s,) = ('',);
+  }();
+  // `s` should be considered potentially assigned at this point, since there's
+  // an assignment to it inside the closure. (In point of fact, it's definitely
+  // assigned, because the closure has definitely been called at this point, and
+  // all control paths through the closure definitely assign to `s`. But flow
+  // analysis only tracks closure creation, not calls to closures, so all it
+  // knows is that `s` is potentially assigned). Therefore it should be legal to
+  // read from `s` now.
+  print(s);
+}