Fix CLI SDK option config timing (#26129).

pkg/analyzer_cli/lib/src/driver.dart

 // Copyright (c) 2015, 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 analyzer_cli.src.driver;
 
 import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
@@ skipping 55 matching lines @@
 
 typedef ErrorSeverity _BatchRunnerHandler(List<String> args);
 
 class Driver implements CommandLineStarter {
   static final PerformanceTag _analyzeAllTag =
       new PerformanceTag("Driver._analyzeAll");
 
   /// The plugins that are defined outside the `analyzer_cli` package.
   List<Plugin> _userDefinedPlugins = <Plugin>[];
 
-  /// Indicates whether the analyzer is running in batch mode.
-  bool _isBatch;
-
   /// The context that was most recently created by a call to [_analyzeAll], or
   /// `null` if [_analyzeAll] hasn't been called yet.
   AnalysisContext _context;
 
   /// The total number of source files loaded by an AnalysisContext.
   int _analyzedFileCount = 0;
 
   /// If [_context] is not `null`, the [CommandLineOptions] that guided its
   /// creation.
   CommandLineOptions _previousOptions;
 
   @override
   EmbeddedResolverProvider embeddedUriResolverProvider;
 
   @override
   ResolverProvider packageResolverProvider;
 
+  /// SDK instance.
+  DirectoryBasedDartSdk sdk;
+
   /// Collected analysis statistics.
   final AnalysisStats stats = new AnalysisStats();
 
   /// This Driver's current analysis context.
   ///
   /// *Visible for testing.*
   AnalysisContext get context => _context;
 
   @override
   void set userDefinedPlugins(List<Plugin> plugins) {
     _userDefinedPlugins = plugins ?? <Plugin>[];
   }
 
   @override
   void start(List<String> args) {
     if (_context != null) {
       throw new StateError("start() can only be called once");
     }
     int startTime = new DateTime.now().millisecondsSinceEpoch;
 
     StringUtilities.INTERNER = new MappedInterner();
 
     _processPlugins();
 
     // Parse commandline options.
     CommandLineOptions options = CommandLineOptions.parse(args);
 
-    // Cache options of interest to inform analysis.
-    _setupEnv(options);
-
     // Do analysis.
     if (options.buildMode) {
       ErrorSeverity severity = _buildModeAnalyze(options);
       // In case of error propagate exit code.
       if (severity == ErrorSeverity.ERROR) {
         exitCode = severity.ordinal;
       }
-    } else if (_isBatch) {
+    } else if (options.shouldBatch) {
       _BatchRunner.runAsBatch(args, (List<String> args) {
         CommandLineOptions options = CommandLineOptions.parse(args);
         return _analyzeAll(options);
       });
     } else {
       ErrorSeverity severity = _analyzeAll(options);
       // In case of error propagate exit code.
       if (severity == ErrorSeverity.ERROR) {
         exitCode = severity.ordinal;
       }
@@ skipping 155 matching lines @@
       return false;
     }
     return true;
   }
 
   /// Decide on the appropriate policy for which files need to be fully parsed
   /// and which files need to be diet parsed, based on [options], and return an
   /// [AnalyzeFunctionBodiesPredicate] that implements this policy.
   AnalyzeFunctionBodiesPredicate _chooseDietParsingPolicy(
       CommandLineOptions options) {
-    if (_isBatch) {
+    if (options.shouldBatch) {
       // As analyzer is currently implemented, once a file has been diet
       // parsed, it can't easily be un-diet parsed without creating a brand new
       // context and losing caching.  In batch mode, we can't predict which
       // files we'll need to generate errors and warnings for in the future, so
       // we can't safely diet parse anything.
       return (Source source) => true;
     }
 
     return (Source source) {
       if (options.sourceFiles.contains(source.fullName)) {
@@ skipping 180 matching lines @@
     _previousOptions = options;
 
     // Save stats from previous context before clobbering it.
     if (_context != null) {
       _analyzedFileCount += _context.sources.length;
     }
 
     // Create a context.
     _context = AnalysisEngine.instance.createAnalysisContext();
 
+    AnalyzeFunctionBodiesPredicate dietParsingPolicy =
+        _chooseDietParsingPolicy(options);
+    setAnalysisContextOptions(_context, options,
+        (AnalysisOptionsImpl contextOptions) {
+      contextOptions.analyzeFunctionBodiesPredicate = dietParsingPolicy;
+    });
+
+    // Once options are processed, setup the SDK.
+    _setupSdk(path: options.dartSdkPath);
+
     // Choose a package resolution policy and a diet parsing policy based on
     // the command-line options.
     SourceFactory sourceFactory = _chooseUriResolutionPolicy(
         options, (_context as InternalAnalysisContext).embedderYamlLocator);
-    AnalyzeFunctionBodiesPredicate dietParsingPolicy =
-        _chooseDietParsingPolicy(options);
 
     _context.sourceFactory = sourceFactory;
-
-    setAnalysisContextOptions(_context, options,
-        (AnalysisOptionsImpl contextOptions) {
-      contextOptions.analyzeFunctionBodiesPredicate = dietParsingPolicy;
-    });
   }
 
   /// Return discovered packagespec, or `null` if none is found.
   Packages _discoverPackagespec(Uri root) {
     try {
       Packages packages = pkgDiscovery.findPackagesFromFile(root);
       if (packages != Packages.noPackages) {
         return packages;
       }
     } catch (_) {
@@ skipping 42 matching lines @@
     var errorSeverity = analyzer.analyzeSync();
     if (errorSeverity == ErrorSeverity.ERROR) {
       exitCode = errorSeverity.ordinal;
     }
     if (options.warningsAreFatal && errorSeverity == ErrorSeverity.WARNING) {
       exitCode = errorSeverity.ordinal;
     }
     return errorSeverity;
   }
 
-  void _setupEnv(CommandLineOptions options) {
-    // In batch mode, SDK is specified on the main command line rather than in
-    // the command lines sent to stdin.  So process it before deciding whether
-    // to activate batch mode.
+  void _setupSdk({String path}) {

[Paul Berry, 2016/05/10 18:33:24]

Nit: it seems weird for [path] to be an optional parameter, when AFAICT the only
caller always passes a value for it.

[pquitslund, 2016/05/10 21:22:39]

Yeah.  I just really like named params! Would that I could make them required. 
After merging with master I did away with this anyway since I need the options
handy to scan source files to check whether to use summaries.

Thanks!

     if (sdk == null) {
-      sdk = new DirectoryBasedDartSdk(new JavaFile(options.dartSdkPath));
+      sdk = new DirectoryBasedDartSdk(new JavaFile(path));
       sdk.useSummary = true;
-      sdk.analysisOptions = createAnalysisOptionsForCommandLineOptions(options);
+      sdk.analysisOptions = context.analysisOptions;
     }
-    _isBatch = options.shouldBatch;
   }
 
   static AnalysisOptionsImpl createAnalysisOptionsForCommandLineOptions(
       CommandLineOptions options) {
     AnalysisOptionsImpl contextOptions = new AnalysisOptionsImpl();
     contextOptions.hint = !options.disableHints;
     contextOptions.enableStrictCallChecks = options.enableStrictCallChecks;
     contextOptions.enableSuperMixins = options.enableSuperMixins;
     contextOptions.generateImplicitErrors = options.showPackageWarnings;
     contextOptions.generateSdkErrors = options.showSdkWarnings;
@@ skipping 173 matching lines @@
     for (var package in packages) {
       var packageName = path.basename(package.path);
       var realPath = package.resolveSymbolicLinksSync();
       result[packageName] = [
         PhysicalResourceProvider.INSTANCE.getFolder(realPath)
       ];
     }
     return result;
   }
 }