Get rid of static version methods which ignore the AppContext (#12971)

* Get rid of static version methods which ignore the AppContext

* Review comments

* Review Comments

* Make branch behave the same as original implementation

* Fix tests

packages/flutter_tools/lib/runner.dart

@@ -89,7 +89,7 @@ Future<int> run(
       await runner.run(args);
       await _exit(0);
     } catch (error, stackTrace) {
-      String getVersion() => flutterVersion ?? FlutterVersion.getVersionString();
+      String getVersion() => flutterVersion ?? FlutterVersion.instance.getVersionString();
       return await _handleToolError(error, stackTrace, verbose, args, reportCrashes, getVersion);
     }
     return 0;

packages/flutter_tools/lib/src/usage.dart

@@ -21,13 +21,14 @@ Usage get flutterUsage => Usage.instance;
 class Usage {
   /// Create a new Usage instance; [versionOverride] is used for testing.
   Usage({ String settingsName: 'flutter', String versionOverride }) {
-    final String version = versionOverride ?? FlutterVersion.getVersionString(whitelistBranchName: true);
+    final FlutterVersion flutterVersion = FlutterVersion.instance;
+    final String version = versionOverride ?? flutterVersion.getVersionString(whitelistBranchName: true);
     _analytics = new AnalyticsIO(_kFlutterUA, settingsName, version);
 
     // Report a more detailed OS version string than package:usage does by default.
     _analytics.setSessionValue('cd1', os.name);
     // Send the branch name as the "channel".
-    _analytics.setSessionValue('cd2', FlutterVersion.getBranchName(whitelistBranchName: true));
+    _analytics.setSessionValue('cd2', flutterVersion.getBranchName(whitelistBranchName: true));
     // Record the host as the application installer ID - the context that flutter_tools is running in.
     if (platform.environment.containsKey('FLUTTER_HOST')) {
       _analytics.setSessionValue('aiid', platform.environment['FLUTTER_HOST']);

packages/flutter_tools/lib/src/version.dart

@@ -28,6 +28,8 @@ class FlutterVersion {
   @visibleForTesting
   FlutterVersion(this._clock) {
     _channel = _runGit('git rev-parse --abbrev-ref --symbolic @{u}');
+    final String branch = _runGit('git rev-parse --abbrev-ref HEAD');
+    _branch = branch == 'HEAD' ? _channel : branch;
 
     final int slash = _channel.indexOf('/');
     if (slash != -1) {
@@ -51,6 +53,9 @@ class FlutterVersion {
   /// `master`, `alpha`, `hackathon`, ...
   String get channel => _channel;
 
+  /// The name of the local branch
+  String _branch;
+
   String _frameworkRevision;
   String get frameworkRevision => _frameworkRevision;
   String get frameworkRevisionShort => _shortGitRevision(frameworkRevision);
@@ -145,30 +150,21 @@ class FlutterVersion {
   static FlutterVersion get instance => context.putIfAbsent(FlutterVersion, () => new FlutterVersion(const Clock()));
 
   /// Return a short string for the version (`alpha/a76bc8e22b`).
-  static String getVersionString({ bool whitelistBranchName: false }) {
-    String commit = _shortGitRevision(_runSync(<String>['git', 'rev-parse', 'HEAD']));
-    commit = commit.isEmpty ? 'unknown' : commit;
-
-    final String branch = getBranchName(whitelistBranchName: whitelistBranchName);
-
-    return '$branch/$commit';
+  String getVersionString({bool whitelistBranchName: false}) {
+    return '${getBranchName(whitelistBranchName: whitelistBranchName)}/$frameworkRevisionShort';
   }
 
   /// Return the branch name.
   ///
-  /// If whitelistBranchName is true and the branch is unknown,
+  /// If [whitelistBranchName] is true and the branch is unknown,
   /// the branch name will be returned as 'dev'.
-  static String getBranchName({ bool whitelistBranchName: false }) {
-    String branch = _runSync(<String>['git', 'rev-parse', '--abbrev-ref', 'HEAD']);
-    branch = branch == 'HEAD' ? 'master' : branch;
-
-    if (whitelistBranchName || branch.isEmpty) {
+  String getBranchName({ bool whitelistBranchName: false }) {
+    if (whitelistBranchName || _branch.isEmpty) {
       // Only return the branch names we know about; arbitrary branch names might contain PII.
-      if (!kKnownBranchNames.contains(branch))
-        branch = 'dev';
+      if (!kKnownBranchNames.contains(_branch))
+        return 'dev';
     }
-
-    return branch;
+    return _branch;
   }
 
   /// The amount of time we wait before pinging the server to check for the

packages/flutter_tools/test/analytics_test.dart

@@ -9,6 +9,7 @@ import 'package:flutter_tools/src/commands/config.dart';
 import 'package:flutter_tools/src/commands/doctor.dart';
 import 'package:flutter_tools/src/doctor.dart';
 import 'package:flutter_tools/src/usage.dart';
+import 'package:flutter_tools/src/version.dart';
 import 'package:mockito/mockito.dart';
 import 'package:quiver/time.dart';
 import 'package:test/test.dart';
@@ -53,6 +54,7 @@ void main() {
       await runner.run(<String>['doctor']);
       expect(count, 0);
     }, overrides: <Type, Generator>{
+      FlutterVersion: () => new FlutterVersion(const Clock()),
       Usage: () => new Usage(),
     });
 
@@ -71,6 +73,7 @@ void main() {
       await runner.run(<String>['config']);
       expect(count, 0);
     }, overrides: <Type, Generator>{
+      FlutterVersion: () => new FlutterVersion(const Clock()),
       Usage: () => new Usage(),
     });
   });
@@ -101,7 +104,7 @@ void main() {
       verify(mockClock.now()).called(2);
 
       expect(
-        verify(mockUsage.sendTiming(captureAny, captureAny, captureAny, label: captureAny)).captured, 
+        verify(mockUsage.sendTiming(captureAny, captureAny, captureAny, label: captureAny)).captured,
         <dynamic>['flutter', 'doctor', const Duration(milliseconds: 1000), 'success']
       );
     }, overrides: <Type, Generator>{
@@ -120,7 +123,7 @@ void main() {
       verify(mockClock.now()).called(2);
 
       expect(
-        verify(mockUsage.sendTiming(captureAny, captureAny, captureAny, label: captureAny)).captured, 
+        verify(mockUsage.sendTiming(captureAny, captureAny, captureAny, label: captureAny)).captured,
         <dynamic>['flutter', 'doctor', const Duration(milliseconds: 1000), 'warning']
       );
     }, overrides: <Type, Generator>{