[flow analysis] Remove skipDuplicateCheck hack.

In https://dart-review.googlesource.com/c/sdk/+/278649, I added
assertions to flow analysis to make sure that `FlowAnalysis.declare`
wasn't called more times than necessary. To work around a few instance
in which it _was_ being called more than necessary, I added a
parameter `skipDuplicateCheck` that caused the assertion to be
skipped.

This change fixes the one remaining case in which skipping the check
was necessary, by moving the CFE logic that calls
`FlowAnalysis.declare` for formal parameters. Previously, this logic
was in both `BodyBuilder.finishFunction` and
`BodyBuilder.finishConstructor`, resulting in some duplication because
`BodyBuilder.finishFunction` sometimes calls
`BodyBuilder.finishConstructor`. I've moved the logic into a new
method, `BodyBuilder._declareFormals`, which is called from
`BodyBuilder.finishFunction` and `BodyBuilder.parseInitializers`,
ensuring that the calls to `FlowAnalysis.declare` only occur once.

Fixing this case allows the `skipDuplicateCheck` hack to be removed
entirely.

Note that there was also one place where the analyzer was also using
`skipDuplicateCheck`, but it was doing so unnecessarily.

Change-Id: Ic34d1b7120f05d94c53774d52eb90e4ada67415f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/434261
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: stereotype441

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

@@ -245,13 +245,10 @@ class FlowAnalysisHelper {
         // a `FormalParameterListImpl`
         var declaredElement =
             parameter.declaredFragment!.element as PromotableElementImpl2;
-        // TODO(paulberry): `skipDuplicateCheck` is currently needed to work
-        // around a failure in duplicate_definition_test.dart; fix this.
         flow!.declare(
           declaredElement,
           SharedTypeView(declaredElement.type),
           initialized: true,
-          skipDuplicateCheck: true,
         );
       }
     }

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

@@ -1201,20 +1201,9 @@ class BodyBuilder extends StackListenerImpl
     typeInferrer.assignedVariables.finish();
 
     FunctionNode function = _context.function;
-    if (thisVariable != null) {
-      typeInferrer.flowAnalysis.declare(
-          thisVariable!, new SharedTypeView(thisVariable!.type),
-          initialized: true);
-    }
+    _declareFormals();
     if (formals?.parameters != null) {
       for (int i = 0; i < formals!.parameters!.length; i++) {
-        FormalParameterBuilder parameter = formals.parameters![i];
-        VariableDeclaration variable = parameter.variable!;
-        typeInferrer.flowAnalysis.declare(
-            variable, new SharedTypeView(variable.type),
-            initialized: true);
-      }
-      for (int i = 0; i < formals.parameters!.length; i++) {
         FormalParameterBuilder parameter = formals.parameters![i];
         Expression? initializer = parameter.variable!.initializer;
         bool inferInitializer;
@@ -1705,10 +1694,11 @@ class BodyBuilder extends StackListenerImpl
     }
     if (doFinishConstructor) {
       List<FormalParameterBuilder>? formals = _context.formals;
+      List<Object>? superParametersAsArguments =
+          formals != null ? createSuperParametersAsArguments(formals) : null;
+      _declareFormals();
       finishConstructor(AsyncMarker.Sync, null,
-          superParametersAsArguments: formals != null
-              ? createSuperParametersAsArguments(formals)
-              : null);
+          superParametersAsArguments: superParametersAsArguments);
     }
     return _initializers;
   }
@@ -1755,6 +1745,26 @@ class BodyBuilder extends StackListenerImpl
     return arguments;
   }
 
+  void _declareFormals() {
+    if (thisVariable != null && _context.isConstructor) {
+      // `thisVariable` usually appears in `_context.formals`, but for a
+      // constructor, it doesn't. So declare it separately.
+      typeInferrer.flowAnalysis.declare(
+          thisVariable!, new SharedTypeView(thisVariable!.type),
+          initialized: true);
+    }
+    List<FormalParameterBuilder>? formals = _context.formals;
+    if (formals != null) {
+      for (int i = 0; i < formals.length; i++) {
+        FormalParameterBuilder parameter = formals[i];
+        VariableDeclaration variable = parameter.variable!;
+        typeInferrer.flowAnalysis.declare(
+            variable, new SharedTypeView(variable.type),
+            initialized: true);
+      }
+    }
+  }
+
   void finishConstructor(AsyncMarker asyncModifier, Statement? body,
       {required List<Object /* Expression | NamedExpression */ >?
           superParametersAsArguments}) {
@@ -1800,24 +1810,11 @@ class BodyBuilder extends StackListenerImpl
         "to be sorted by occurrence in file.");
 
     FunctionNode function = _context.function;
-    List<FormalParameterBuilder>? formals = _context.formals;
-    if (formals != null) {
-      for (int i = 0; i < formals.length; i++) {
-        FormalParameterBuilder parameter = formals[i];
-        VariableDeclaration variable = parameter.variable!;
-        // TODO(paulberry): `skipDuplicateCheck` is currently needed to work
-        // around a failure in
-        // co19/Language/Expressions/Postfix_Expressions/conditional_increment_t02;
-        // fix this.
-        typeInferrer.flowAnalysis.declare(
-            variable, new SharedTypeView(variable.type),
-            initialized: true, skipDuplicateCheck: true);
-      }
-    }
 
     Set<String>? namedSuperParameterNames;
     List<Expression>? positionalSuperParametersAsArguments;
     List<NamedExpression>? namedSuperParametersAsArguments;
+    List<FormalParameterBuilder>? formals = _context.formals;
     if (superParametersAsArguments != null) {
       for (Object superParameterAsArgument in superParametersAsArguments) {
         if (superParameterAsArgument is Expression) {