DAS: only attempt to compute fixes for fix-supporting producers

The diff makes this change look bigger than it is.

Here's how the code worked before: `compute()` called `_addFromProducers()`. `_addFromProducers()` had a local function, also
called `compute()`, which is called per CorrectionProducer. The local
`compute()` function would unconditionally create a ChangeBuilder and
call each CorrectionProducer's `compute()` method, then call back out
to an instance method called `_addFixFromBuilder()`, which would drop
the computed change on the floor if `fixKind` was `null`.

This CL contains the following changes to the above system:

* The local `compute()` function and the instance method,
  `_addFixFromBuilder()` are combined together, into one instance
  method, `_addFromProducer()`.
* Then, the `fixKind` check is moved to the very top of that
  method, bailing out _before_ computing changes if `fixKind == null`.

Change-Id: I6c61059fc36ec1587b15ca01b43830a5b179da9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417326
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analysis_server_plugin/lib/src/correction/fix_processor.dart

@@ -64,20 +64,48 @@ class FixProcessor {
     return _fixes;
   }
 
-  void _addFixFromBuilder(ChangeBuilder builder, CorrectionProducer producer) {
-    var change = builder.sourceChange;
-    if (change.edits.isEmpty) {
-      return;
-    }
-
+  Future<void> _addFromProducer(CorrectionProducer producer) async {
     var kind = producer.fixKind;
+    // If this producer is not actually designed to work as an fix, ignore it.
     if (kind == null) {
       return;
     }
 
-    change.id = kind.id;
-    change.message = formatList(kind.message, producer.fixArguments);
-    _fixes.add(Fix(kind: kind, change: change));
+    var builder =
+        ChangeBuilder(workspace: _fixContext.workspace, eol: producer.eol);
+    try {
+      var fixKind = producer.fixKind;
+
+      if (_performance != null) {
+        var startTime = _timer.elapsedMilliseconds;
+        await producer.compute(builder);
+        _performance.producerTimings.add((
+          className: producer.runtimeType.toString(),
+          elapsedTime: _timer.elapsedMilliseconds - startTime,
+        ));
+      } else {
+        await producer.compute(builder);
+      }
+
+      assert(
+        !producer.canBeAppliedAcrossSingleFile || producer.fixKind == fixKind,
+        'Producers used in bulk fixes must not modify the FixKind during '
+        'computation. $producer changed from $fixKind to ${producer.fixKind}.',
+      );
+
+      var change = builder.sourceChange;
+      if (change.edits.isEmpty) {
+        return;
+      }
+
+      change.id = kind.id;
+      change.message = formatList(kind.message, producer.fixArguments);
+      _fixes.add(Fix(kind: kind, change: change));
+    } on ConflictingEditException catch (exception, stackTrace) {
+      // Handle the exception by (a) not adding a fix based on the producer
+      // and (b) logging the exception.
+      _fixContext.instrumentationService.logException(exception, stackTrace);
+    }
   }
 
   Future<void> _addFromProducers() async {
@@ -91,37 +119,6 @@ class FixProcessor {
       selectionLength: _fixContext.diagnostic.length,
     );
 
-    Future<void> compute(CorrectionProducer producer) async {
-      var builder =
-          ChangeBuilder(workspace: _fixContext.workspace, eol: producer.eol);
-      try {
-        var fixKind = producer.fixKind;
-
-        if (_performance != null) {
-          var startTime = _timer.elapsedMilliseconds;
-          await producer.compute(builder);
-          _performance.producerTimings.add((
-            className: producer.runtimeType.toString(),
-            elapsedTime: _timer.elapsedMilliseconds - startTime,
-          ));
-        } else {
-          await producer.compute(builder);
-        }
-
-        assert(
-          !producer.canBeAppliedAcrossSingleFile || producer.fixKind == fixKind,
-          'Producers used in bulk fixes must not modify the FixKind during '
-          'computation. $producer changed from $fixKind to ${producer.fixKind}.',
-        );
-
-        _addFixFromBuilder(builder, producer);
-      } on ConflictingEditException catch (exception, stackTrace) {
-        // Handle the exception by (a) not adding a fix based on the producer
-        // and (b) logging the exception.
-        _fixContext.instrumentationService.logException(exception, stackTrace);
-      }
-    }
-
     var diagnosticCode = diagnostic.diagnosticCode;
     List<ProducerGenerator>? generators;
     List<MultiProducerGenerator>? multiGenerators;
@@ -137,14 +134,14 @@ class FixProcessor {
 
     if (generators != null) {
       for (var generator in generators) {
-        await compute(generator(context: context));
+        await _addFromProducer(generator(context: context));
       }
     }
     if (multiGenerators != null) {
       for (var multiGenerator in multiGenerators) {
         var multiProducer = multiGenerator(context: context);
         for (var producer in await multiProducer.producers) {
-          await compute(producer);
+          await _addFromProducer(producer);
         }
       }
     }
@@ -163,7 +160,7 @@ class FixProcessor {
             continue;
           }
         }
-        await compute(producer);
+        await _addFromProducer(generator(context: context));
       }
     }
   }