Update the plugin API

pkg/analyzer_plugin/lib/utilities/fixes/fixes.dart

 // Copyright (c) 2017, 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.
 
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer_plugin/protocol/protocol.dart';
 import 'package:analyzer_plugin/protocol/protocol_generated.dart';
 import 'package:analyzer_plugin/src/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/generator.dart';
 
 /**
+ * The information about a requested set of fixes when computing fixes in a
+ * `.dart` file.
+ *
+ * Clients may not extend, implement or mix-in this class.
+ */
+abstract class DartFixesRequest implements FixesRequest {
+  /**
+   * The analysis result for the file in which the fixes are being requested.
+   */
+  ResolveResult get result;
+}
+
+/**
  * An object that [FixContributor]s use to record fixes.
  *
  * Clients may not extend, implement or mix-in this class.
  */
 abstract class FixCollector {
   /**
    * Record a new [change] (fix) associated with the given [error].
    */
   void addFix(AnalysisError error, PrioritizedSourceChange change);
 }
 
 /**
  * An object used to produce fixes.
  *
  * Clients may implement this class when implementing plugins.
  */
 abstract class FixContributor {
   /**
    * Contribute fixes for the location in the file specified by the given
    * [request] into the given [collector].
    */
-  void computeFixes(FixesRequest request, FixCollector collector);
+  void computeFixes(covariant FixesRequest request, FixCollector collector);
 }
 
 /**
  * The information about a requested set of fixes.
  *
  * Clients may not extend, implement or mix-in this class.
  */
 abstract class FixesRequest {

[maxkim, 2017/06/22 20:52:57]

Can we also add 'String path' to this object?

   /**
    * The analysis error to be fixed, or `null` if the error has not been
    * determined.
    */
   AnalysisError get error;
 
   /**
    * Return the offset within the source for which fixes are being requested.
    */
   int get offset;
 
   /**
    * Return the resource provider associated with this request.
    */
   ResourceProvider get resourceProvider;
-
-  /**
-   * The analysis result for the file in which the fixes are being requested.
-   */
-  ResolveResult get result;
 }
 
 /**
  * A generator that will generate an 'edit.getFixes' response.
  *
  * Clients may not extend, implement or mix-in this class.
  */
 class FixGenerator {
   /**
    * The contributors to be used to generate the fixes.
    */
   final List<FixContributor> contributors;
 
   /**
    * Initialize a newly created fix generator to use the given [contributors].
    */
   FixGenerator(this.contributors);
 
   /**
    * Create an 'edit.getFixes' response for the location in the file specified
    * by the given [request]. If any of the contributors throws an exception,
    * also create a non-fatal 'plugin.error' notification.
    */
-  GeneratorResult generateFixesResponse(FixesRequest request) {
+  GeneratorResult generateFixesResponse(DartFixesRequest request) {

[Brian Wilkerson, 2017/06/22 17:12:21]

I'm not entirely thrilled with this change. The alternative is to extend
FixesRequest to have getters for 'errors' and 'lineInfo'. Thoughts appreciated.

[maxkim, 2017/06/22 20:52:56]

Adding a 'List<AnalysisError> get errorsAtOffset' makes sense in this context
and let the plugin-developer define how the list of errors is obtained. However,
I'm more in favor of just storing the errors for the offset instead of all
errors in the file.

Naming wise, it would be nice to have additional clarity in the field names: 

AnalysisError get fixedError; 
String path;
int get offset;
ResourceProvider get resourceProvider;
List<AnalysisError> get errorsAtOffset;  // Returns all error in file at offset

Meanwhile LineInfo can just exist in DartFixesRequest.

This way we can set 'generateFixesResponse' to take in a 'FixesRequest request'
instead of DartFixesRequest. Ideally no one should have to overload this
function.

[Brian Wilkerson, 2017/06/22 22:07:01]

Yeah, I'm starting to think that you're right, and that `FixesRequest` should
have an `errorsToFix` getter that returns exactly the errors for which fixes
should be produced and that fix contributors should iterate over the list. (The
current implementation in analysis server moves the iteration outside the
contributors, and I was trying to keep that, but I don't think it's worth the
pain it's causing.)

[maxkim, 2017/06/22 23:18:19]

Acknowledged.

     List<Notification> notifications = <Notification>[];
     FixCollectorImpl collector = new FixCollectorImpl();
     Iterable<AnalysisError> errors = _getErrors(request);
     FixesRequestImpl requestImpl = request;
     for (FixContributor contributor in contributors) {
       try {
         for (AnalysisError error in errors) {
           requestImpl.error = error;
           contributor.computeFixes(request, collector);
         }
       } catch (exception, stackTrace) {
         notifications.add(new PluginErrorParams(
                 false, exception.toString(), stackTrace.toString())
             .toNotification());
       } finally {
         requestImpl.error = null;
       }
     }
     EditGetFixesResult result = new EditGetFixesResult(collector.fixes);
     return new GeneratorResult(result, notifications);
   }
 
-  Iterable<AnalysisError> _getErrors(FixesRequest request) {
+  Iterable<AnalysisError> _getErrors(DartFixesRequest request) {

[maxkim, 2017/06/22 20:52:57]

If we put 'errorsAtOffset' within FixesRequest, this entire logic can
effectively be moved into
DartFixesMixin -> getFixesRequest(...).

And (in generateFixesResponse(...)) errors = _getErrors(request) can just become
errors = request.errors

This way we can even remove LineInfo from FixesRequest dependency from the root
FixesRequest object.

     int offset = request.offset;
     LineInfo lineInfo = request.result.lineInfo;
     int offsetLine = lineInfo.getLocation(offset).lineNumber;
     return request.result.errors.where((AnalysisError error) {
       int errorLine = lineInfo.getLocation(error.offset).lineNumber;
       return errorLine == offsetLine;
     });
   }
 }