improve analysis server notifications

pkg/analysis_server/lib/src/analysis_server.dart

 // Copyright (c) 2014, 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.
 
 library analysis.server;
 
 import 'dart:async';
 
 import 'package:analysis_server/src/analysis_logger.dart';
 import 'package:analysis_server/src/channel.dart';
 import 'package:analysis_server/src/protocol.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/error.dart';
 import 'package:analyzer/src/generated/java_core.dart';
 
 /**
  * Instances of the class [AnalysisServer] implement a server that listens on a
  * [CommunicationChannel] for analysis requests and process them.
  */
 class AnalysisServer {
   /**
    * The name of the notification of new errors associated with a source.
    */
   static const String ERROR_NOTIFICATION_NAME = 'context.errors';
 
   /**
+   * The name of the notification sent when analysis is complete.
+   */
+  static const String ANALYSIS_COMPLETE = "analysis.complete";
+
+  /**
    * The name of the contextId parameter.
    */
   static const String CONTEXT_ID_PARAM = 'contextId';
 
   /**
    * The name of the parameter whose value is a list of errors.
    */
   static const String ERRORS_PARAM = 'errors';
 
   /**
@@ skipping 24 matching lines @@
    * server.
    */
   List<RequestHandler> handlers;
 
   /**
    * A table mapping context id's to the analysis contexts associated with them.
    */
   final Map<String, AnalysisContext> contextMap = new Map<String, AnalysisContext>();
 
   /**
+   * A table mapping analysis contexts to the context id's associated with them.
+   */
+  final Map<AnalysisContext, String> contextIdMap = new Map<AnalysisContext, String>();
+
+  /**
    * A list of the analysis contexts for which analysis work needs to be
    * performed.
    *
    * Invariant: when this list is non-empty, there is exactly one pending call
    * to [performTask] on the event queue.  When this list is empty, there are
    * no calls to [performTask] on the event queue.
    */
   final List<AnalysisContext> contextWorkQueue = new List<AnalysisContext>();
 
   /**
@@ skipping 74 matching lines @@
       contextWorkQueue.clear();
     }
     if (contextWorkQueue.isEmpty) {
       // Nothing to do.
       return;
     }
     //
     // Look for a context that has work to be done and then perform one task.
     //
     List<ChangeNotice> notices = null;
+    String contextId;
     try {
       AnalysisContext context = contextWorkQueue[0];
+      contextId = contextIdMap[context];
       AnalysisResult result = context.performAnalysisTask();
       notices = result.changeNotices;
     } finally {
       if (notices == null) {
         // Either we have no more work to do for this context, or there was an
         // unhandled exception trying to perform the analysis.  In either case,
         // remove the context form the work queue so we won't try to do more
         // analysis on it.
-        contextWorkQueue.removeAt(0);
+        AnalysisContext context = contextWorkQueue.removeAt(0);
+        var notification = new Notification(ANALYSIS_COMPLETE);

[Paul Berry, 2014/05/20 15:34:58]

I'm concerned that this is going to lead to bugs, because it provides no way for
the client to reliably know when analysis has started, so it's going to have to
guess.  If it fails to guess that analysis has started when it actually has, the
user will see no indication that analysis is going on.  If it falsely guesses
that analysis has started when it actually hasn't, it will appear to the user
that analysis is stuck in an infinite loop.

Also, based on features present in other refactoring editors, I suspect we're
eventually going to want to also communicate information like:

- What file is currently being analyzed
- Analysis progress (e.g. we have analyzed N files out of M)
- Estimated time until analysis completes

I would rather see an "analysis status" notification, containing this sort of
information in a map.  Initially we can just populate the map with a single
boolean to indicate whether analysis is in progress or not, and later we can add
other fields.

+        notification.setParameter(CONTEXT_ID_PARAM, contextId);
+        sendNotification(notification);
       }
       //
       // Schedule this method to be run again if there is any more work to be
       // done.
       //
       if (!contextWorkQueue.isEmpty) {
         _scheduleTask();
       }
     }
     if (notices != null) {
-      sendNotices(notices);
+      sendNotices(contextId, notices);
     }
   }
 
   /**
    * Send the information in the given list of notices back to the client.
    */
-  void sendNotices(List<ChangeNotice> notices) {
+  void sendNotices(String contextId, List<ChangeNotice> notices) {
     for (int i = 0; i < notices.length; i++) {
       ChangeNotice notice = notices[i];
       Notification notification = new Notification(ERROR_NOTIFICATION_NAME);
+      notification.setParameter(CONTEXT_ID_PARAM, contextId);
       notification.setParameter(SOURCE_PARAM, notice.source.encoding);
       notification.setParameter(ERRORS_PARAM, notice.errors.map(
           errorToJson).toList());
       sendNotification(notification);
     }
   }
 
   static Map<String, Object> errorToJson(AnalysisError analysisError) {
     // TODO(paulberry): move this function into the AnalysisError class.
 
@@ skipping 19 matching lines @@
   void sendNotification(Notification notification) {
     channel.sendNotification(notification);
   }
 
   void _scheduleTask() {
     new Future(performTask).catchError((ex, st) {
       AnalysisEngine.instance.logger.logError("${ex}\n${st}");
     });
   }
 }