Context invalidation on option changes (Implements #24574).

pkg/analysis_server/lib/src/context_manager.dart

Index: pkg/analysis_server/lib/src/context_manager.dart
diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart
index 3e72ad50054c00c2ed03b97f0e3bea255cc94222..465729d9d7b6856c50d319b42821edc45140c73d 100644
--- a/pkg/analysis_server/lib/src/context_manager.dart
+++ b/pkg/analysis_server/lib/src/context_manager.dart
@@ -9,6 +9,7 @@ import 'dart:collection';
 import 'dart:convert';
 import 'dart:core' hide Resource;
 
+import 'package:analyzer/src/context/context.dart' as context;
 import 'package:analysis_server/plugin/analysis/resolver_provider.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/server_options.dart';
@@ -311,6 +312,11 @@ abstract class ContextManagerCallbacks {
  */
 class ContextManagerImpl implements ContextManager {
   /**
+   * File name of analysis options files.
+   */
+  static const String ANALYSIS_OPTIONS_FILE = '.analysis_options';

[Brian Wilkerson, 2015/10/15 00:20:36]

Do we have other definitions of this constant (or other references to this file
name)? If so, it would be nice to unify them.

[pquitslund, 2015/10/15 03:39:06]

Agreed.  The only other ones are in tests and in analyzer (and probably not
where they should live long term).  Do you have a convention for where constants
such as these should live?

[Brian Wilkerson, 2015/10/15 13:15:31]

Not really, no.

+
+  /**
    * Temporary flag to hide WIP .packages support (DEP 5).
    */
   static bool ENABLE_PACKAGESPEC_SUPPORT = serverOptions.isSet(
@@ -466,18 +472,46 @@ class ContextManagerImpl implements ContextManager {
   /**
    * Process [options] for the given context [info].
    */
-  void processOptionsForContext(
-      ContextInfo info, Map<String, YamlNode> options) {
-    if (options == null) {
+  void processOptionsForContext(ContextInfo info, Folder folder,
+      {bool optionsRemoved: false}) {
+    Map<String, YamlNode> options;
+    try {
+      options = analysisOptionsProvider.getOptions(folder);
+    } catch (e, stacktrace) {
+      AnalysisEngine.instance.logger.logError(
+          'Error processing .analysis_options',
+          new CaughtException(e, stacktrace));
+      // TODO(pquitslund): contribute plugin that sends error notification on
+      // options file.
+      // Related test:
+      //   context_manager_test.test_analysis_options_parse_failure()
+      // AnalysisEngine.instance.optionsPlugin.optionsProcessors
+      //      .forEach((OptionsProcessor p) => p.onError(e));
+    }
+
+    if (options == null && !optionsRemoved) {
       return;
     }
 
     // Notify options processors.
-    AnalysisEngine.instance.optionsPlugin.optionsProcessors.forEach(
-        (OptionsProcessor p) => p.optionsProcessed(info.context, options));
+    AnalysisEngine.instance.optionsPlugin.optionsProcessors
+        .forEach((OptionsProcessor p) {
+      try {
+        p.optionsProcessed(info.context, options);
+      } catch (e, stacktrace) {
+        AnalysisEngine.instance.logger.logError(
+            'Error processing .analysis_options',
+            new CaughtException(e, stacktrace));
+      }
+    });
+
+    // In case options files are removed, revert to default options.
+    if (optionsRemoved) {
+      info.context.analysisOptions = new AnalysisOptionsImpl();
+      return;
+    }
 
     // Analysis options are processed 'in-line'.
-    // TODO(pq): consider pushing exclude handling into a plugin.
     YamlMap analyzer = options['analyzer'];
     if (analyzer == null) {
       // No options for analyzer.
@@ -486,12 +520,14 @@ class ContextManagerImpl implements ContextManager {
 
     // Set strong mode.
     var strongMode = analyzer['strong-mode'];
-    if (strongMode == true) {
+    if (strongMode is bool) {

[Brian Wilkerson, 2015/10/15 00:20:36]

If the key isn't there I think we want to treat it as "false". Hence, we
probably want to do something like "strongMode ??= false;" before the test.

[pquitslund, 2015/10/15 03:39:06]

Ah.  Good catch!   I took this approach:

    bool strongMode = analyzer['strong-mode'] ?? false;

Thanks!

       AnalysisContext context = info.context;
-      AnalysisOptionsImpl options =
-          new AnalysisOptionsImpl.from(context.analysisOptions);
-      options.strongMode = true;
-      context.analysisOptions = options;
+      if (context.analysisOptions.strongMode != strongMode) {
+        AnalysisOptionsImpl options =
+            new AnalysisOptionsImpl.from(context.analysisOptions);
+        options.strongMode = strongMode;
+        context.analysisOptions = options;
+      }
     }
 
     // Set ignore patterns.
@@ -844,21 +880,7 @@ class ContextManagerImpl implements ContextManager {
     info.context = callbacks.addContext(folder, disposition);
     info.context.name = folder.path;
 
-    try {
-      Map<String, YamlNode> options =
-          analysisOptionsProvider.getOptions(folder);
-      processOptionsForContext(info, options);
-    } catch (e, stacktrace) {
-      AnalysisEngine.instance.logger.logError(
-          'Error processing .analysis_options',
-          new CaughtException(e, stacktrace));
-      // TODO(pquitslund): contribute plugin that sends error notification on
-      // options file.
-      // Related test:
-      //   context_manager_test.test_analysis_options_parse_failure()
-      // AnalysisEngine.instance.optionsPlugin.optionsProcessors
-      //      .forEach((OptionsProcessor p) => p.onError(e));
-    }
+    processOptionsForContext(info, folder);
 
     return info;
   }
@@ -1161,9 +1183,21 @@ class ContextManagerImpl implements ContextManager {
         }
         break;
     }
-
-    //TODO(pquitslund): find the right place for this
     _checkForPackagespecUpdate(path, info, info.folder);
+    _checkForAnalysisOptionsUpdate(path, info, event.type);
+  }
+
+  void _checkForAnalysisOptionsUpdate(
+      String path, ContextInfo info, ChangeType changeType) {
+    if (pathContext.basename(path) == ANALYSIS_OPTIONS_FILE) {
+      var analysisContext = info.context;
+      if (analysisContext is context.AnalysisContextImpl) {
+        processOptionsForContext(info, info.folder,
+            optionsRemoved: changeType == ChangeType.REMOVE);
+        analysisContext.invalidateCachedResults();
+        callbacks.applyChangesToContext(info.folder, new ChangeSet());
+      }
+    }
   }
 
   /**