update flutter analyze to call dartanalyzer

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

@@ -14,9 +14,8 @@ bool isDartFile(FileSystemEntity entry) => entry is File && entry.path.endsWith(
 typedef bool FileFilter(FileSystemEntity entity);
 
 class AnalyzeCommand extends FlutterCommand {
-  AnalyzeCommand({bool verboseHelp: false}) {
+  AnalyzeCommand({ bool verboseHelp: false, this.workingDirectory }) {
     argParser.addFlag('flutter-repo', help: 'Include all the examples and tests from the Flutter repository.', defaultsTo: false);
-    argParser.addFlag('current-directory', help: 'Include all the Dart files in the current directory, if any.', defaultsTo: true);
     argParser.addFlag('current-package', help: 'Include the lib/main.dart file from the current directory, if any.', defaultsTo: true);
     argParser.addFlag('dartdocs', help: 'List every public member that is lacking documentation (only examines files in the Flutter repository).', defaultsTo: false);
     argParser.addFlag('watch', help: 'Run analysis continuously, watching the filesystem for changes.', negatable: false);
@@ -29,10 +28,13 @@ class AnalyzeCommand extends FlutterCommand {
     usesPubOption();
 
     // Not used by analyze --watch
-    argParser.addFlag('congratulate', help: 'Show output even when there are no errors, warnings, hints, or lints.', defaultsTo: true);
-    argParser.addFlag('preamble', help: 'Display the number of files that will be analyzed.', defaultsTo: true);
+    argParser.addFlag('congratulate', help: 'When analyzing the flutter repository, show output even when there are no errors, warnings, hints, or lints.', defaultsTo: true);
+    argParser.addFlag('preamble', help: 'When analyzing the flutter repository, display the number of files that will be analyzed.', defaultsTo: true);
   }
 
+  /// The working directory for testing analysis using dartanalyzer.
+  final Directory workingDirectory;
+
   @override
   String get name => 'analyze';
 
@@ -57,7 +59,7 @@ class AnalyzeCommand extends FlutterCommand {
     if (argResults['watch']) {
       return new AnalyzeContinuously(argResults, runner.getRepoAnalysisEntryPoints()).analyze();
     } else {
-      return new AnalyzeOnce(argResults, runner.getRepoPackages()).analyze();
+      return new AnalyzeOnce(argResults, runner.getRepoPackages(), workingDirectory: workingDirectory).analyze();
     }
   }
 }

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

@@ -10,6 +10,7 @@ import 'package:yaml/yaml.dart' as yaml;
 
 import '../base/common.dart';
 import '../base/file_system.dart';
+import '../base/process.dart';
 import '../cache.dart';
 import '../dart/analysis.dart';
 import '../globals.dart';
@@ -22,9 +23,12 @@ typedef bool FileFilter(FileSystemEntity entity);
 
 /// An aspect of the [AnalyzeCommand] to perform once time analysis.
 class AnalyzeOnce extends AnalyzeBase {
+  AnalyzeOnce(ArgResults argResults, this.repoPackages, { this.workingDirectory }) : super(argResults);
+
   final List<Directory> repoPackages;
 
-  AnalyzeOnce(ArgResults argResults, this.repoPackages) : super(argResults);
+  /// The working directory for testing analysis using dartanalyzer
+  final Directory workingDirectory;
 
   @override
   Future<Null> analyze() async {
@@ -45,38 +49,70 @@ class AnalyzeOnce extends AnalyzeBase {
       }
     }
 
-    final bool currentDirectory = argResults['current-directory'] && (argResults.wasParsed('current-directory') || dartFiles.isEmpty);
     final bool currentPackage = argResults['current-package'] && (argResults.wasParsed('current-package') || dartFiles.isEmpty);
-    final bool flutterRepo = argResults['flutter-repo'] || inRepo(argResults.rest);
-
-    //TODO (pq): revisit package and directory defaults
-
-    if (currentDirectory  && !flutterRepo) {
-      // ./*.dart
-      final Directory currentDirectory = fs.directory('.');
-      bool foundOne = false;
-      for (FileSystemEntity entry in currentDirectory.listSync()) {
-        if (isDartFile(entry)) {
-          dartFiles.add(entry);
-          foundOne = true;
+    final bool flutterRepo = argResults['flutter-repo'] || (workingDirectory == null && inRepo(argResults.rest));
+
+    // Use dartanalyzer directly except when analyzing the Flutter repository.
+    // Analyzing the repository requires a more complex report than dartanalyzer
+    // currently supports (e.g. missing member dartdoc summary).
+    // TODO(danrubel): enhance dartanalyzer to provide this type of summary
+    if (!flutterRepo) {
+      final List<String> arguments = <String>[];
+      arguments.addAll(dartFiles.map((FileSystemEntity f) => f.path));
+
+      if (arguments.isEmpty || currentPackage) {
+        // workingDirectory is non-null only when testing flutter analyze
+        final Directory currentDirectory = workingDirectory ?? fs.currentDirectory.absolute;
+        final Directory projectDirectory = await projectDirectoryContaining(currentDirectory);
+        if (projectDirectory != null) {
+          arguments.add(projectDirectory.path);
+        } else if (arguments.isEmpty) {
+          arguments.add(currentDirectory.path);
         }
       }
-      if (foundOne)
-        pubSpecDirectories.add(currentDirectory);
-    }
 
-    if (currentPackage && !flutterRepo) {
-      // **/.*dart
-      final Directory currentDirectory = fs.directory('.');
-      _collectDartFiles(currentDirectory, dartFiles);
-      pubSpecDirectories.add(currentDirectory);
+      // If the files being analyzed are outside of the current directory hierarchy
+      // then dartanalyzer does not yet know how to find the ".packages" file.
+      // TODO(danrubel): fix dartanalyzer to find the .packages file
+      final File packagesFile = await packagesFileFor(arguments);
+      if (packagesFile != null) {
+        arguments.insert(0, '--packages');
+        arguments.insert(1, packagesFile.path);
+      }
+
+      final String dartanalyzer = fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'dart-sdk', 'bin', 'dartanalyzer');
+      arguments.insert(0, dartanalyzer);
+      bool noErrors = false;
+      int exitCode = await runCommandAndStreamOutput(
+          arguments,
+          workingDirectory: workingDirectory?.path,
+          mapFunction: (String line) {
+            // Workaround for the fact that dartanalyzer does not exit with a non-zero exit code
+            // when errors are found.
+            // TODO(danrubel): Fix dartanalyzer to return non-zero exit code
+            if (line == 'No issues found!')
+              noErrors = true;
+            return line;
+          },
+      );
+      stopwatch.stop();
+      final String elapsed = (stopwatch.elapsedMilliseconds / 1000.0).toStringAsFixed(1);
+      // Workaround for the fact that dartanalyzer does not exit with a non-zero exit code
+      // when errors are found.
+      // TODO(danrubel): Fix dartanalyzer to return non-zero exit code
+      if (exitCode == 0 && !noErrors)
+        exitCode = 1;
+      if (exitCode != 0)
+        throwToolExit('(Ran in ${elapsed}s)', exitCode: exitCode);
+      printStatus('Ran in ${elapsed}s');
+      return;
     }
 
-    if (flutterRepo) {
-      for (Directory dir in repoPackages) {
-        _collectDartFiles(dir, dartFiles);
-        pubSpecDirectories.add(dir);
-      }
+    //TODO (pq): revisit package and directory defaults
+
+    for (Directory dir in repoPackages) {
+      _collectDartFiles(dir, dartFiles);
+      pubSpecDirectories.add(dir);
     }
 
     // determine what all the various .packages files depend on
@@ -145,9 +181,7 @@ class AnalyzeOnce extends AnalyzeBase {
     final DriverOptions options = new DriverOptions();
     options.dartSdkPath = argResults['dart-sdk'];
     options.packageMap = packages;
-    options.analysisOptionsFile = flutterRepo
-        ? fs.path.join(Cache.flutterRoot, '.analysis_options_repo')
-        : fs.path.join(Cache.flutterRoot, 'packages', 'flutter', 'lib', 'analysis_options_user.yaml');
+    options.analysisOptionsFile = fs.path.join(Cache.flutterRoot, '.analysis_options_repo');
     final AnalysisDriver analyzer = new AnalysisDriver(options);
 
     // TODO(pq): consider error handling
@@ -183,13 +217,13 @@ class AnalyzeOnce extends AnalyzeBase {
 
     if (errorCount > 0) {
       // we consider any level of error to be an error exit (we don't report different levels)
-      if (membersMissingDocumentation > 0 && flutterRepo)
+      if (membersMissingDocumentation > 0)
         throwToolExit('[lint] $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation (ran in ${elapsed}s)');
       else
         throwToolExit('(Ran in ${elapsed}s)');
     }
     if (argResults['congratulate']) {
-      if (membersMissingDocumentation > 0 && flutterRepo) {
+      if (membersMissingDocumentation > 0) {
         printStatus('No analyzer warnings! (ran in ${elapsed}s; $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation)');
       } else {
         printStatus('No analyzer warnings! (ran in ${elapsed}s)');
@@ -197,6 +231,54 @@ class AnalyzeOnce extends AnalyzeBase {
     }
   }
 
+  /// Return a path to the ".packages" file for use by dartanalyzer when analyzing the specified files.
+  /// Report an error if there are file paths that belong to different projects.
+  Future<File> packagesFileFor(List<String> filePaths) async {
+    String projectPath = await projectPathContaining(filePaths.first);
+    if (projectPath != null) {
+      if (projectPath.endsWith(fs.path.separator))
+        projectPath = projectPath.substring(0, projectPath.length - 1);
+      final String projectPrefix = projectPath + fs.path.separator;
+      // Assert that all file paths are contained in the same project directory
+      for (String filePath in filePaths) {
+        if (!filePath.startsWith(projectPrefix) && filePath != projectPath)
+          throwToolExit('Files in different projects cannot be analyzed at the same time.\n'
+              '  Project: $projectPath\n  File outside project:  $filePath');
+      }
+    } else {
+      // Assert that all file paths are not contained in any project
+      for (String filePath in filePaths) {
+        final String otherProjectPath = await projectPathContaining(filePath);
+        if (otherProjectPath != null)
+          throwToolExit('Files inside a project cannot be analyzed at the same time as files not in any project.\n'
+              '  File inside a project: $filePath');
+      }
+    }
+
+    if (projectPath == null)
+      return null;
+    final File packagesFile = fs.file(fs.path.join(projectPath, '.packages'));
+    return await packagesFile.exists() ? packagesFile : null;
+  }
+
+  Future<String> projectPathContaining(String targetPath) async {
+    final FileSystemEntity target = await fs.isDirectory(targetPath) ? fs.directory(targetPath) : fs.file(targetPath);
+    final Directory projectDirectory = await projectDirectoryContaining(target);
+    return projectDirectory?.path;
+  }
+
+  Future<Directory> projectDirectoryContaining(FileSystemEntity entity) async {
+    Directory dir = entity is Directory ? entity : entity.parent;
+    dir = dir.absolute;
+    while (!await dir.childFile('pubspec.yaml').exists()) {
+      final Directory parent = dir.parent;
+      if (parent == null || parent.path == dir.path)
+        return null;
+      dir = parent;
+    }
+    return dir;
+  }
+
   List<String> flutterRootComponents;
   bool isFlutterLibrary(String filename) {
     flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator);

packages/flutter_tools/test/analyze_duplicate_names_test.dart

@@ -37,9 +37,10 @@ void main() {
       final AnalyzeCommand command = new AnalyzeCommand();
       applyMocksToCommand(command);
       return createTestCommandRunner(command).run(
-        <String>['analyze', '--no-current-package', '--no-current-directory', dartFileA.path, dartFileB.path]
+        <String>['analyze', '--no-current-package', dartFileA.path, dartFileB.path]
       ).then<Null>((Null value) {
-        expect(testLogger.statusText, startsWith('Analyzing 2 files...\nNo analyzer warnings!'));
+        expect(testLogger.statusText, contains('Analyzing'));
+        expect(testLogger.statusText, contains('No issues found!'));
       });
 
     });

packages/flutter_tools/test/analyze_once_test.dart

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:flutter_tools/src/base/common.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/cache.dart';
+import 'package:flutter_tools/src/commands/analyze.dart';
+import 'package:flutter_tools/src/commands/create.dart';
+import 'package:flutter_tools/src/runner/flutter_command.dart';
+import 'package:test/test.dart';
+
+import 'src/common.dart';
+import 'src/context.dart';
+
+void main() {
+  group('analyze once', () {
+    Directory tempDir;
+    File libMain;
+
+    setUpAll(() {
+      Cache.disableLocking();
+      tempDir = fs.systemTempDirectory.createTempSync('analyze_once_test_').absolute;
+      libMain = fs.file(fs.path.join(tempDir.path, 'lib', 'main.dart'));
+    });
+
+    tearDownAll(() {
+      tempDir?.deleteSync(recursive: true);
+    });
+
+    // Create a project to be analyzed
+    testUsingContext('flutter create', () async {
+      await runCommand(
+        command: new CreateCommand(),
+        arguments: <String>['create', tempDir.path],
+        statusTextContains: <String>[
+          'All done!',
+          'Your main program file is lib/main.dart',
+        ],
+      );
+      expect(libMain.existsSync(), isTrue);
+    });
+
+    // Analyze in the current directory - no arguments
+    testUsingContext('flutter analyze working directory', () async {
+      await runCommand(
+        command: new AnalyzeCommand(workingDirectory: tempDir),
+        arguments: <String>['analyze'],
+        statusTextContains: <String>['No issues found!'],
+      );
+    });
+
+    // Analyze a specific file outside the current directory
+    testUsingContext('flutter analyze one file', () async {
+      await runCommand(
+        command: new AnalyzeCommand(),
+        arguments: <String>['analyze', libMain.path],
+        statusTextContains: <String>['No issues found!'],
+      );
+    });
+
+    // Analyze in the current directory - no arguments
+    testUsingContext('flutter analyze working directory with errors', () async {
+
+      // Break the code to produce the "The parameter 'child' is required" hint
+      // that is upgraded to a warning in package:flutter/analysis_options_user.yaml
+      // to assert that we are using the default Flutter analysis options.
+      // Also insert a statement that should not trigger a lint here
+      // but will trigger a lint later on when an analysis_options.yaml is added.
+      String source = await libMain.readAsString();
+      source = source.replaceFirst(
+        'child: new Icon(Icons.add),',
+        '// child: new Icon(Icons.add),',
+      );
+      source = source.replaceFirst(
+        '_counter++;',
+        '_counter++; throw "an error message";',
+      );
+      await libMain.writeAsString(source);
+
+      /// Analyze in the current directory - no arguments
+      await runCommand(
+        command: new AnalyzeCommand(workingDirectory: tempDir),
+        arguments: <String>['analyze'],
+        statusTextContains: <String>[
+          'Analyzing',
+          '[warning] The parameter \'child\' is required',
+          '1 warning found.',
+        ],
+        toolExit: true,
+      );
+    });
+
+    // Analyze a specific file outside the current directory
+    testUsingContext('flutter analyze one file with errors', () async {
+      await runCommand(
+        command: new AnalyzeCommand(),
+        arguments: <String>['analyze', libMain.path],
+        statusTextContains: <String>[
+          'Analyzing',
+          '[warning] The parameter \'child\' is required',
+          '1 warning found.',
+        ],
+        toolExit: true,
+      );
+    });
+
+    // Analyze in the current directory - no arguments
+    testUsingContext('flutter analyze working directory with local options', () async {
+
+      // Insert an analysis_options.yaml file in the project
+      // which will trigger a lint for broken code that was inserted earlier
+      final File optionsFile = fs.file(fs.path.join(tempDir.path, 'analysis_options.yaml'));
+      await optionsFile.writeAsString('''
+  include: package:flutter/analysis_options_user.yaml
+  linter:
+    rules:
+      - only_throw_errors
+  ''');
+
+      /// Analyze in the current directory - no arguments
+      await runCommand(
+        command: new AnalyzeCommand(workingDirectory: tempDir),
+        arguments: <String>['analyze'],
+        statusTextContains: <String>[
+          'Analyzing',
+          '[warning] The parameter \'child\' is required',
+          '[lint] Only throw instances of classes extending either Exception or Error',
+          '1 warning and 1 lint found.',
+        ],
+        toolExit: true,
+      );
+    });
+
+    // Analyze a specific file outside the current directory
+    testUsingContext('flutter analyze one file with local options', () async {
+      await runCommand(
+        command: new AnalyzeCommand(),
+        arguments: <String>['analyze', libMain.path],
+        statusTextContains: <String>[
+          'Analyzing',
+          '[warning] The parameter \'child\' is required',
+          '[lint] Only throw instances of classes extending either Exception or Error',
+          '1 warning and 1 lint found.',
+        ],
+        toolExit: true,
+      );
+    });
+  });
+}
+
+void assertContains(String text, List<String> patterns) {
+  if (patterns == null) {
+    expect(text, isEmpty);
+  } else {
+    for (String pattern in patterns) {
+      expect(text, contains(pattern));
+    }
+  }
+}
+
+Future<Null> runCommand({
+  FlutterCommand command,
+  List<String> arguments,
+  List<String> statusTextContains,
+  List<String> errorTextContains,
+  bool toolExit: false,
+}) async {
+  try {
+    arguments.insert(0, '--flutter-root=${Cache.flutterRoot}');
+    await createTestCommandRunner(command).run(arguments);
+    expect(toolExit, isFalse, reason: 'Expected ToolExit exception');
+  } on ToolExit {
+    if (!toolExit) {
+      testLogger.clear();
+      rethrow;
+    }
+  }
+  assertContains(testLogger.statusText, statusTextContains);
+  assertContains(testLogger.errorText, errorTextContains);
+  testLogger.clear();
+}