analyzer: Tidy strict-inference-in-parameters check

The existing code had two local functions which were an unnecessary
structuring of the check. The story might be hard to read in the diff.

1. The previous method body for `_checkStrictInferenceInParameters`
   first declared a variable `usedParameterVisitor` which may or not be
   ever used, and then created two local functions.
2. The first function, `isParameterReferenced`, is only called from the
   second function. It has a lot of complexity because it is responsible
   for maybe instantiating that local variable, only once, to cache the
   results of visiting the function body.
3. The second function is called twice, which is why it was nice to
   encapsulate it in a function.
4. _After_ those things are declared, we check if we're even using
   `strict-inference`, or that the parameter list is null. This results
   in many null-asserts on the parameter list.
5. The code that extracts out a NormalFormalParameter from a
   DefaultFormalParameter is a little complicated.

The changes sort of unwind the complexity from the bottom up:

* We can move the `strict-inference` check and the parameter list null-
  check to the tippy top, returning early (4). This removes many
  null-asserts.
* We can simplify the code that extracts the NormalFormalParameter from
  any DefaultFormalParameter (5), which means there would only be one
  call to `checkParameterTypeIsKnown`. With only one call site, the
  encapsulation is silly (3), and we can inline the function, removing
  `checkParameterTypeIsKnown`.
* Most of the complexity of the first function, `isParameterReferenced`
  is around initializing this expensive visitor, so that we only visit
  the function body once. But it turns out, we only do that if we cannot
  know whether the parameters are used, because `initializers` is null,
  and the `body` is null or empty. But none of that knowledge is per-
  parameter, so it can be computed outside this function. So we
  introduce `parameterReferenceIsUnknown`. We never need to call
  `isParameterReferenced` if `parameterReferenceIsUnknown`. Or, put
  another way, we only need to call `isParameterReferenced` if we know
  we need that visitor. So we can now computer the visitor outside the
  function, after which the function is so simple (a single returned
  expression), it can be removed.

All in all, here are the broad simplifications:

* We return early in a few common scenarios, in which case we neither
  declare that visitor variable, nor declare the local functions.
* We compute `parameterReferenceIsUnknown` once, not per parameter.
* We simplify how inner `NormalFormalParameters` are extracted.
* We simplify instantiating the visitor, rather than in a local
  function.
* We inline simple local functions.

Change-Id: I308f328940f809f10cad884e68f064dd0a468a95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442280
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: srawlins

pkg/analyzer/lib/src/error/best_practices_verifier.dart

@@ -1428,65 +1428,61 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
-  /// In "strict-inference" mode, check that each of the [parameters]' type is
-  /// specified.
+  /// In "strict-inference" mode, check that each of the [parameterList]' type
+  /// is specified.
   ///
   /// Only parameters which are referenced in [initializers] or [body] are
   /// reported. If [initializers] and [body] are both null, the parameters are
   /// assumed to originate from a typedef, function-typed parameter, or function
   /// which is abstract or external.
   void _checkStrictInferenceInParameters(
-    FormalParameterList? parameters, {
+    FormalParameterList? parameterList, {
     List<ConstructorInitializer>? initializers,
     FunctionBody? body,
   }) {
-    _UsedParameterVisitor? usedParameterVisitor;
+    if (!_strictInference || parameterList == null) {
+      return;
+    }
 
-    bool isParameterReferenced(SimpleFormalParameter parameter) {
-      if ((body == null || body is EmptyFunctionBody) && initializers == null) {
-        // The parameter is in a typedef, or function that is abstract,
-        // external, etc.
-        return true;
-      }
-      if (usedParameterVisitor == null) {
-        // Visit the function body and initializers once to determine whether
-        // each of the parameters is referenced.
-        usedParameterVisitor = _UsedParameterVisitor(
-          parameters!.parameters
-              .map((p) => p.declaredFragment!.element)
-              .toSet(),
-        );
-        body?.accept(usedParameterVisitor!);
-        for (var initializer in initializers ?? <ConstructorInitializer>[]) {
-          initializer.accept(usedParameterVisitor!);
-        }
-      }
+    var implicitlyTypedParameters =
+        parameterList.parameters
+            .map((p) => p.notDefault)
+            .whereType<SimpleFormalParameter>()
+            .where((p) => p.type == null)
+            .toList();
 
-      return usedParameterVisitor!.isUsed(parameter.declaredFragment!.element);
-    }
+    if (implicitlyTypedParameters.isEmpty) return;
 
-    void checkParameterTypeIsKnown(SimpleFormalParameter parameter) {
-      if (parameter.type == null && isParameterReferenced(parameter)) {
-        var element = parameter.declaredFragment!.element;
-        _diagnosticReporter.atNode(
-          parameter,
-          WarningCode.INFERENCE_FAILURE_ON_UNTYPED_PARAMETER,
-          arguments: [element.displayName],
-        );
-      }
-    }
+    // Whether the parameters are in a typedef, or function that is abstract,
+    // external, etc.
+    var parameterReferenceIsUnknown =
+        (body == null || body is EmptyFunctionBody) && initializers == null;
 
-    if (_strictInference && parameters != null) {
-      for (FormalParameter parameter in parameters.parameters) {
-        if (parameter is SimpleFormalParameter) {
-          checkParameterTypeIsKnown(parameter);
-        } else if (parameter is DefaultFormalParameter) {
-          var nonDefault = parameter.parameter;
-          if (nonDefault is SimpleFormalParameter) {
-            checkParameterTypeIsKnown(nonDefault);
-          }
+    if (!parameterReferenceIsUnknown) {
+      var usedVisitor = _UsedParameterVisitor(
+        implicitlyTypedParameters
+            .map((p) => p.declaredFragment!.element)
+            .toSet(),
+      );
+      body?.accept(usedVisitor);
+      if (initializers != null) {
+        for (var initializer in initializers) {
+          initializer.accept(usedVisitor);
         }
       }
+
+      implicitlyTypedParameters.removeWhere(
+        (p) => !usedVisitor.isUsed(p.declaredFragment!.element),
+      );
+    }
+
+    for (var parameter in implicitlyTypedParameters) {
+      var element = parameter.declaredFragment!.element;
+      _diagnosticReporter.atNode(
+        parameter,
+        WarningCode.INFERENCE_FAILURE_ON_UNTYPED_PARAMETER,
+        arguments: [element.displayName],
+      );
     }
   }