Context invalidation on option changes (Implements #24574).

pkg/analysis_server/lib/src/context_manager.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 context.directory.manager;
 
 import 'dart:async';
 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';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
 import 'package:analyzer/plugin/options.dart';
 import 'package:analyzer/source/analysis_options_provider.dart';
 import 'package:analyzer/source/package_map_provider.dart';
 import 'package:analyzer/source/package_map_resolver.dart';
 import 'package:analyzer/source/path_filter.dart';
@@ skipping 282 matching lines @@
   void updateContextPackageUriResolver(
       Folder contextFolder, FolderDisposition disposition);
 }
 
 /**
  * Class that maintains a mapping from included/excluded paths to a set of
  * folders that should correspond to analysis contexts.
  */
 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(
       'ContextManagerImpl.ENABLE_PACKAGESPEC_SUPPORT',
       defaultValue: true);
 
   /**
    * The name of the `lib` directory.
    */
   static const String LIB_DIR_NAME = 'lib';
@@ skipping 135 matching lines @@
         return true;
       }
     }
     // no
     return false;
   }
 
   /**
    * 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.
       return;
     }
 
     // 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.
     YamlList exclude = analyzer['exclude'];
     if (exclude != null) {
       setIgnorePatternsForContext(info, exclude);
     }
   }
 
   @override
@@ skipping 332 matching lines @@
     // Next resort to a package uri resolver.
     if (disposition == null) {
       disposition =
           _computeFolderDisposition(folder, dependencies.add, packagespecFile);
     }
 
     info.setDependencies(dependencies);
     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;
   }
 
   /**
    * Potentially create a new context associated with the given [folder].
    *
    * If there are subfolders with 'pubspec.yaml' files, separate contexts are
    * created for them and excluded from the context associated with the
    * [folder].
@@ skipping 282 matching lines @@
         List<Source> sources = info.context.getSourcesWithFullName(path);
         if (!sources.isEmpty) {
           ChangeSet changeSet = new ChangeSet();
           sources.forEach((Source source) {
             changeSet.changedSource(source);
           });
           callbacks.applyChangesToContext(info.folder, changeSet);
         }
         break;
     }
+    _checkForPackagespecUpdate(path, info, info.folder);
+    _checkForAnalysisOptionsUpdate(path, info, event.type);
+  }
 
-    //TODO(pquitslund): find the right place for this
-    _checkForPackagespecUpdate(path, info, info.folder);
+  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());
+      }
+    }
   }
 
   /**
    * Determine whether the given [path], when interpreted relative to the
    * context root [root], contains a folder whose name starts with '.'.
    */
   bool _isContainedInDotFolder(String root, String path) {
     String relativePath =
         pathContext.relative(pathContext.dirname(path), from: root);
     for (String pathComponent in pathContext.split(relativePath)) {
@@ skipping 265 matching lines @@
           var path = resourceProvider.pathContext.fromUri(uri);
           packageMap[name] = <Folder>[resourceProvider.getFolder(path)];
         }
       });
       return <UriResolver>[new SdkExtUriResolver(packageMap)];
     } else {
       return const <UriResolver>[];
     }
   }
 }