flutter analyze --watch --flutter-repo check package conflicts (#10641)

packages/flutter_tools/lib/src/commands/analyze.dart

@@ -57,7 +57,7 @@ class AnalyzeCommand extends FlutterCommand {
   @override
   Future<Null> runCommand() {
     if (argResults['watch']) {
-      return new AnalyzeContinuously(argResults, runner.getRepoAnalysisEntryPoints()).analyze();
+      return new AnalyzeContinuously(argResults, runner.getRepoPackages()).analyze();
     } else {
       return new AnalyzeOnce(argResults, runner.getRepoPackages(), workingDirectory: workingDirectory).analyze();
     }

packages/flutter_tools/lib/src/commands/analyze_base.dart

@@ -5,7 +5,9 @@
 import 'dart:async';
 
 import 'package:args/args.dart';
+import 'package:yaml/yaml.dart' as yaml;
 
+import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/utils.dart';
 import '../cache.dart';
@@ -65,3 +67,153 @@ bool inRepo(List<String> fileList) {
   }
   return false;
 }
+
+class PackageDependency {
+  // This is a map from dependency targets (lib directories) to a list
+  // of places that ask for that target (.packages or pubspec.yaml files)
+  Map<String, List<String>> values = <String, List<String>>{};
+  String canonicalSource;
+  void addCanonicalCase(String packagePath, String pubSpecYamlPath) {
+    assert(canonicalSource == null);
+    add(packagePath, pubSpecYamlPath);
+    canonicalSource = pubSpecYamlPath;
+  }
+  void add(String packagePath, String sourcePath) {
+    values.putIfAbsent(packagePath, () => <String>[]).add(sourcePath);
+  }
+  bool get hasConflict => values.length > 1;
+  bool get hasConflictAffectingFlutterRepo {
+    assert(fs.path.isAbsolute(Cache.flutterRoot));
+    for (List<String> targetSources in values.values) {
+      for (String source in targetSources) {
+        assert(fs.path.isAbsolute(source));
+        if (fs.path.isWithin(Cache.flutterRoot, source))
+          return true;
+      }
+    }
+    return false;
+  }
+  void describeConflict(StringBuffer result) {
+    assert(hasConflict);
+    final List<String> targets = values.keys.toList();
+    targets.sort((String a, String b) => values[b].length.compareTo(values[a].length));
+    for (String target in targets) {
+      final int count = values[target].length;
+      result.writeln('  $count ${count == 1 ? 'source wants' : 'sources want'} "$target":');
+      bool canonical = false;
+      for (String source in values[target]) {
+        result.writeln('    $source');
+        if (source == canonicalSource)
+          canonical = true;
+      }
+      if (canonical) {
+        result.writeln('    (This is the actual package definition, so it is considered the canonical "right answer".)');
+      }
+    }
+  }
+  String get target => values.keys.single;
+}
+
+class PackageDependencyTracker {
+  /// Packages whose source is defined in the vended SDK.
+  static const List<String> _vendedSdkPackages = const <String>['analyzer', 'front_end', 'kernel'];
+
+  // This is a map from package names to objects that track the paths
+  // involved (sources and targets).
+  Map<String, PackageDependency> packages = <String, PackageDependency>{};
+
+  PackageDependency getPackageDependency(String packageName) {
+    return packages.putIfAbsent(packageName, () => new PackageDependency());
+  }
+
+  /// Read the .packages file in [directory] and add referenced packages to [dependencies].
+  void addDependenciesFromPackagesFileIn(Directory directory) {
+    final String dotPackagesPath = fs.path.join(directory.path, '.packages');
+    final File dotPackages = fs.file(dotPackagesPath);
+    if (dotPackages.existsSync()) {
+      // this directory has opinions about what we should be using
+      dotPackages
+        .readAsStringSync()
+        .split('\n')
+        .where((String line) => !line.startsWith(new RegExp(r'^ *#')))
+        .forEach((String line) {
+          final int colon = line.indexOf(':');
+          if (colon > 0) {
+            final String packageName = line.substring(0, colon);
+            final String packagePath = fs.path.fromUri(line.substring(colon+1));
+            // Ensure that we only add `analyzer` and dependent packages defined in the vended SDK (and referred to with a local
+            // fs.path. directive). Analyzer package versions reached via transitive dependencies (e.g., via `test`) are ignored
+            // since they would produce spurious conflicts.
+            if (!_vendedSdkPackages.contains(packageName) || packagePath.startsWith('..'))
+              add(packageName, fs.path.normalize(fs.path.absolute(directory.path, packagePath)), dotPackagesPath);
+          }
+      });
+    }
+  }
+
+  void addCanonicalCase(String packageName, String packagePath, String pubSpecYamlPath) {
+    getPackageDependency(packageName).addCanonicalCase(packagePath, pubSpecYamlPath);
+  }
+
+  void add(String packageName, String packagePath, String dotPackagesPath) {
+    getPackageDependency(packageName).add(packagePath, dotPackagesPath);
+  }
+
+  void checkForConflictingDependencies(Iterable<Directory> pubSpecDirectories, PackageDependencyTracker dependencies) {
+    for (Directory directory in pubSpecDirectories) {
+      final String pubSpecYamlPath = fs.path.join(directory.path, 'pubspec.yaml');
+      final File pubSpecYamlFile = fs.file(pubSpecYamlPath);
+      if (pubSpecYamlFile.existsSync()) {
+        // we are analyzing the actual canonical source for this package;
+        // make sure we remember that, in case all the packages are actually
+        // pointing elsewhere somehow.
+        final yaml.YamlMap pubSpecYaml = yaml.loadYaml(fs.file(pubSpecYamlPath).readAsStringSync());
+        final String packageName = pubSpecYaml['name'];
+        final String packagePath = fs.path.normalize(fs.path.absolute(fs.path.join(directory.path, 'lib')));
+        dependencies.addCanonicalCase(packageName, packagePath, pubSpecYamlPath);
+      }
+      dependencies.addDependenciesFromPackagesFileIn(directory);
+    }
+
+    // prepare a union of all the .packages files
+    if (dependencies.hasConflicts) {
+      final StringBuffer message = new StringBuffer();
+      message.writeln(dependencies.generateConflictReport());
+      message.writeln('Make sure you have run "pub upgrade" in all the directories mentioned above.');
+      if (dependencies.hasConflictsAffectingFlutterRepo) {
+        message.writeln(
+            'For packages in the flutter repository, try using '
+            '"flutter update-packages --upgrade" to do all of them at once.');
+      }
+      message.write(
+          'If this does not help, to track down the conflict you can use '
+          '"pub deps --style=list" and "pub upgrade --verbosity=solver" in the affected directories.');
+      throwToolExit(message.toString());
+    }
+  }
+
+  bool get hasConflicts {
+    return packages.values.any((PackageDependency dependency) => dependency.hasConflict);
+  }
+
+  bool get hasConflictsAffectingFlutterRepo {
+    return packages.values.any((PackageDependency dependency) => dependency.hasConflictAffectingFlutterRepo);
+  }
+
+  String generateConflictReport() {
+    assert(hasConflicts);
+    final StringBuffer result = new StringBuffer();
+    for (String package in packages.keys.where((String package) => packages[package].hasConflict)) {
+      result.writeln('Package "$package" has conflicts:');
+      packages[package].describeConflict(result);
+    }
+    return result.toString();
+  }
+
+  Map<String, String> asPackageMap() {
+    final Map<String, String> result = <String, String>{};
+    for (String package in packages.keys)
+      result[package] = packages[package].target;
+    return result;
+  }
+}

packages/flutter_tools/lib/src/commands/analyze_continuously.dart

@@ -20,9 +20,9 @@ import '../globals.dart';
 import 'analyze_base.dart';
 
 class AnalyzeContinuously extends AnalyzeBase {
-  AnalyzeContinuously(ArgResults argResults, this.repoAnalysisEntryPoints) : super(argResults);
+  AnalyzeContinuously(ArgResults argResults, this.repoPackages) : super(argResults);
 
-  final List<Directory> repoAnalysisEntryPoints;
+  final List<Directory> repoPackages;
 
   String analysisTarget;
   bool firstAnalysis = true;
@@ -40,7 +40,9 @@ class AnalyzeContinuously extends AnalyzeBase {
       throwToolExit('The --dartdocs option is currently not supported when using --watch.');
 
     if (argResults['flutter-repo']) {
-      directories = repoAnalysisEntryPoints.map((Directory dir) => dir.path).toList();
+      final PackageDependencyTracker dependencies = new PackageDependencyTracker();
+      dependencies.checkForConflictingDependencies(repoPackages, dependencies);
+      directories = repoPackages.map((Directory dir) => dir.path).toList();
       analysisTarget = 'Flutter repository';
       printTrace('Analyzing Flutter repository:');
       for (String projectPath in directories)

packages/flutter_tools/lib/src/commands/analyze_once.dart

@@ -6,7 +6,6 @@ import 'dart:async';
 import 'dart:collection';
 
 import 'package:args/args.dart';
-import 'package:yaml/yaml.dart' as yaml;
 
 import '../base/common.dart';
 import '../base/file_system.dart';
@@ -31,9 +30,6 @@ class AnalyzeOnce extends AnalyzeBase {
   /// The working directory for testing analysis using dartanalyzer
   final Directory workingDirectory;
 
-  /// Packages whose source is defined in the vended SDK. 
-  static const List<String> _vendedSdkPackages = const <String>['analyzer', 'front_end', 'kernel'];
-
   @override
   Future<Null> analyze() async {
     final Stopwatch stopwatch = new Stopwatch()..start();
@@ -136,56 +132,7 @@ class AnalyzeOnce extends AnalyzeBase {
 
     // determine what all the various .packages files depend on
     final PackageDependencyTracker dependencies = new PackageDependencyTracker();
-    for (Directory directory in pubSpecDirectories) {
-      final String pubSpecYamlPath = fs.path.join(directory.path, 'pubspec.yaml');
-      final File pubSpecYamlFile = fs.file(pubSpecYamlPath);
-      if (pubSpecYamlFile.existsSync()) {
-        // we are analyzing the actual canonical source for this package;
-        // make sure we remember that, in case all the packages are actually
-        // pointing elsewhere somehow.
-        final yaml.YamlMap pubSpecYaml = yaml.loadYaml(fs.file(pubSpecYamlPath).readAsStringSync());
-        final String packageName = pubSpecYaml['name'];
-        final String packagePath = fs.path.normalize(fs.path.absolute(fs.path.join(directory.path, 'lib')));
-        dependencies.addCanonicalCase(packageName, packagePath, pubSpecYamlPath);
-      }
-      final String dotPackagesPath = fs.path.join(directory.path, '.packages');
-      final File dotPackages = fs.file(dotPackagesPath);
-      if (dotPackages.existsSync()) {
-        // this directory has opinions about what we should be using
-        dotPackages
-          .readAsStringSync()
-          .split('\n')
-          .where((String line) => !line.startsWith(new RegExp(r'^ *#')))
-          .forEach((String line) {
-            final int colon = line.indexOf(':');
-            if (colon > 0) {
-              final String packageName = line.substring(0, colon);
-              final String packagePath = fs.path.fromUri(line.substring(colon+1));
-              // Ensure that we only add `analyzer` and dependent packages defined in the vended SDK (and referred to with a local 
-              // fs.path. directive). Analyzer package versions reached via transitive dependencies (e.g., via `test`) are ignored 
-              // since they would produce spurious conflicts.
-              if (!_vendedSdkPackages.contains(packageName) || packagePath.startsWith('..'))
-                dependencies.add(packageName, fs.path.normalize(fs.path.absolute(directory.path, packagePath)), dotPackagesPath);
-            }
-        });
-      }
-    }
-
-    // prepare a union of all the .packages files
-    if (dependencies.hasConflicts) {
-      final StringBuffer message = new StringBuffer();
-      message.writeln(dependencies.generateConflictReport());
-      message.writeln('Make sure you have run "pub upgrade" in all the directories mentioned above.');
-      if (dependencies.hasConflictsAffectingFlutterRepo) {
-        message.writeln(
-            'For packages in the flutter repository, try using '
-            '"flutter update-packages --upgrade" to do all of them at once.');
-      }
-      message.write(
-          'If this does not help, to track down the conflict you can use '
-          '"pub deps --style=list" and "pub upgrade --verbosity=solver" in the affected directories.');
-      throwToolExit(message.toString());
-    }
+    dependencies.checkForConflictingDependencies(pubSpecDirectories, dependencies);
     final Map<String, String> packages = dependencies.asPackageMap();
 
     Cache.releaseLockEarly();
@@ -335,92 +282,3 @@ class AnalyzeOnce extends AnalyzeBase {
     return collected;
   }
 }
-
-class PackageDependency {
-  // This is a map from dependency targets (lib directories) to a list
-  // of places that ask for that target (.packages or pubspec.yaml files)
-  Map<String, List<String>> values = <String, List<String>>{};
-  String canonicalSource;
-  void addCanonicalCase(String packagePath, String pubSpecYamlPath) {
-    assert(canonicalSource == null);
-    add(packagePath, pubSpecYamlPath);
-    canonicalSource = pubSpecYamlPath;
-  }
-  void add(String packagePath, String sourcePath) {
-    values.putIfAbsent(packagePath, () => <String>[]).add(sourcePath);
-  }
-  bool get hasConflict => values.length > 1;
-  bool get hasConflictAffectingFlutterRepo {
-    assert(fs.path.isAbsolute(Cache.flutterRoot));
-    for (List<String> targetSources in values.values) {
-      for (String source in targetSources) {
-        assert(fs.path.isAbsolute(source));
-        if (fs.path.isWithin(Cache.flutterRoot, source))
-          return true;
-      }
-    }
-    return false;
-  }
-  void describeConflict(StringBuffer result) {
-    assert(hasConflict);
-    final List<String> targets = values.keys.toList();
-    targets.sort((String a, String b) => values[b].length.compareTo(values[a].length));
-    for (String target in targets) {
-      final int count = values[target].length;
-      result.writeln('  $count ${count == 1 ? 'source wants' : 'sources want'} "$target":');
-      bool canonical = false;
-      for (String source in values[target]) {
-        result.writeln('    $source');
-        if (source == canonicalSource)
-          canonical = true;
-      }
-      if (canonical) {
-        result.writeln('    (This is the actual package definition, so it is considered the canonical "right answer".)');
-      }
-    }
-  }
-  String get target => values.keys.single;
-}
-
-class PackageDependencyTracker {
-  // This is a map from package names to objects that track the paths
-  // involved (sources and targets).
-  Map<String, PackageDependency> packages = <String, PackageDependency>{};
-
-  PackageDependency getPackageDependency(String packageName) {
-    return packages.putIfAbsent(packageName, () => new PackageDependency());
-  }
-
-  void addCanonicalCase(String packageName, String packagePath, String pubSpecYamlPath) {
-    getPackageDependency(packageName).addCanonicalCase(packagePath, pubSpecYamlPath);
-  }
-
-  void add(String packageName, String packagePath, String dotPackagesPath) {
-    getPackageDependency(packageName).add(packagePath, dotPackagesPath);
-  }
-
-  bool get hasConflicts {
-    return packages.values.any((PackageDependency dependency) => dependency.hasConflict);
-  }
-
-  bool get hasConflictsAffectingFlutterRepo {
-    return packages.values.any((PackageDependency dependency) => dependency.hasConflictAffectingFlutterRepo);
-  }
-
-  String generateConflictReport() {
-    assert(hasConflicts);
-    final StringBuffer result = new StringBuffer();
-    for (String package in packages.keys.where((String package) => packages[package].hasConflict)) {
-      result.writeln('Package "$package" has conflicts:');
-      packages[package].describeConflict(result);
-    }
-    return result.toString();
-  }
-
-  Map<String, String> asPackageMap() {
-    final Map<String, String> result = <String, String>{};
-    for (String package in packages.keys)
-      result[package] = packages[package].target;
-    return result;
-  }
-}

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -346,7 +346,10 @@ class FlutterCommandRunner extends CommandRunner<Null> {
 
   /// Get all pub packages in the Flutter repo.
   List<Directory> getRepoPackages() {
-    return _gatherProjectPaths(fs.path.absolute(Cache.flutterRoot))
+    final String root = fs.path.absolute(Cache.flutterRoot);
+    // not bin, and not the root
+    return <String>['dev', 'examples', 'packages']
+      .expand<String>((String path) => _gatherProjectPaths(fs.path.join(root, path)))
       .map((String dir) => fs.directory(dir))
       .toList();
   }
@@ -366,22 +369,6 @@ class FlutterCommandRunner extends CommandRunner<Null> {
       .toList();
   }
 
-  /// Get the entry-points we want to analyze in the Flutter repo.
-  List<Directory> getRepoAnalysisEntryPoints() {
-    final String rootPath = fs.path.absolute(Cache.flutterRoot);
-    final List<Directory> result = <Directory>[
-      // not bin, and not the root
-      fs.directory(fs.path.join(rootPath, 'dev')),
-      fs.directory(fs.path.join(rootPath, 'examples')),
-    ];
-    // And since analyzer refuses to look at paths that end in "packages/":
-    result.addAll(
-      _gatherProjectPaths(fs.path.join(rootPath, 'packages'))
-      .map<Directory>((String path) => fs.directory(path))
-    );
-    return result;
-  }
-
   void _checkFlutterCopy() {
     // If the current directory is contained by a flutter repo, check that it's
     // the same flutter that is currently running.