Fix `frameworkVersionFor` for flutter doctor and usage (#54217)

dev/bots/test.dart

@@ -1121,7 +1121,8 @@ String get gitHash {
 /// Returns null if the contents are good. Returns a string if they are bad.
 /// The string is an error message.
 Future<String> verifyVersion(File file) async {
-  final RegExp pattern = RegExp(r'^\d+\.\d+\.\d+(\+hotfix\.\d+)?(-pre\.\d+)?$');
+  final RegExp pattern = RegExp(
+    r'^(\d+)\.(\d+)\.(\d+)((-\d+\.\d+)?\.pre(\.\d+)?)?$');
   final String version = await file.readAsString();
   if (!file.existsSync())
     return 'The version logic failed to create the Flutter version file.';

dev/bots/test/test_test.dart

@@ -23,9 +23,9 @@ void main() {
       const List<String> valid_versions = <String>[
         '1.2.3',
         '12.34.56',
-        '1.2.3-pre.1',
-        '1.2.3+hotfix.1',
-        '1.2.3+hotfix.12-pre.12',
+        '1.2.3.pre.1',
+        '1.2.3-4.5.pre',
+        '1.2.3-5.0.pre.12',
       ];
       for (final String version in valid_versions) {
         when(file.readAsString()).thenAnswer((Invocation invocation) => Future<String>.value(version));
@@ -41,8 +41,8 @@ void main() {
       const List<String> invalid_versions = <String>[
         '1.2.3.4',
         '1.2.3.',
-        '1.2-pre.1',
-        '1.2.3-pre',
+        '1.2.pre.1',
+        '1.2.3-pre.1',
         '1.2.3-pre.1+hotfix.1',
         '  1.2.3',
         '1.2.3-hotfix.1',

packages/flutter_tools/lib/src/version.dart

@@ -703,6 +703,7 @@ class GitTagVersion {
     this.devPatch,
     this.commits,
     this.hash,
+    this.gitTag,
   });
   const GitTagVersion.unknown()
     : x = null,
@@ -712,7 +713,8 @@ class GitTagVersion {
       commits = 0,
       devVersion = null,
       devPatch = null,
-      hash = '';
+      hash = '',
+      gitTag = '';
 
   /// The X in vX.Y.Z.
   final int x;
@@ -738,6 +740,9 @@ class GitTagVersion {
   /// The M in X.Y.Z-dev.N.M
   final int devPatch;
 
+  /// The git tag that is this version's closest ancestor.
+  final String gitTag;
+
   static GitTagVersion determine(ProcessUtils processUtils, {String workingDirectory, bool fetchTags = false}) {
     if (fetchTags) {
       final String channel = _runGit('git rev-parse --abbrev-ref HEAD', processUtils, workingDirectory);
@@ -752,38 +757,50 @@ class GitTagVersion {
   }
 
   // TODO(fujino): Deprecate this https://github.com/flutter/flutter/issues/53850
-  /// Check for the release tag format pre-v1.17.0
+  /// Check for the release tag format of the form x.y.z-dev.m.n
   static GitTagVersion parseLegacyVersion(String version) {
     final RegExp versionPattern = RegExp(
-      r'^v([0-9]+)\.([0-9]+)\.([0-9]+)(?:\+hotfix\.([0-9]+))?-([0-9]+)-g([a-f0-9]+)$');
-
+      r'^([0-9]+)\.([0-9]+)\.([0-9]+)(-dev\.[0-9]+\.[0-9]+)?-([0-9]+)-g([a-f0-9]+)$');
     final List<String> parts = versionPattern.matchAsPrefix(version)?.groups(<int>[1, 2, 3, 4, 5, 6]);
     if (parts == null) {
       return const GitTagVersion.unknown();
     }
-    final List<int> parsedParts = parts.take(5).map<int>((String source) => source == null ? null : int.tryParse(source)).toList();
+    final List<int> parsedParts = parts.take(5).map<int>(
+      (String source) => source == null ? null : int.tryParse(source)).toList();
+    List<int> devParts = <int>[null, null];
+    if (parts[3] != null) {
+      devParts = RegExp(r'^-dev\.(\d+)\.(\d+)')
+        .matchAsPrefix(parts[3])
+        ?.groups(<int>[1, 2])
+        ?.map<int>(
+          (String source) => source == null ? null : int.tryParse(source)
+        )?.toList() ?? <int>[null, null];
+    }
     return GitTagVersion(
       x: parsedParts[0],
       y: parsedParts[1],
       z: parsedParts[2],
-      hotfix: parsedParts[3],
+      devVersion: devParts[0],
+      devPatch: devParts[1],
       commits: parsedParts[4],
       hash: parts[5],
+      gitTag: '${parts[0]}.${parts[1]}.${parts[2]}${parts[3] ?? ''}', // x.y.z-dev.m.n
     );
   }
 
-  /// Check for the release tag format from v1.17.0 on
+  /// Check for the release tag format of the form x.y.z-m.n.pre
   static GitTagVersion parseVersion(String version) {
     final RegExp versionPattern = RegExp(
-      r'^([0-9]+)\.([0-9]+)\.([0-9]+)(-dev\.[0-9]+\.[0-9]+)?-([0-9]+)-g([a-f0-9]+)$');
+      r'^(\d+)\.(\d+)\.(\d+)(-\d+\.\d+\.pre)?-(\d+)-g([a-f0-9]+)$');
     final List<String> parts = versionPattern.matchAsPrefix(version)?.groups(<int>[1, 2, 3, 4, 5, 6]);
     if (parts == null) {
       return const GitTagVersion.unknown();
     }
-    final List<int> parsedParts = parts.take(5).map<int>((String source) => source == null ? null : int.tryParse(source)).toList();
+    final List<int> parsedParts = parts.take(5).map<int>(
+      (String source) => source == null ? null : int.tryParse(source)).toList();
     List<int> devParts = <int>[null, null];
     if (parts[3] != null) {
-      devParts = RegExp(r'^-dev\.(\d+)\.(\d+)')
+      devParts = RegExp(r'^-(\d+)\.(\d+)\.pre')
         .matchAsPrefix(parts[3])
         ?.groups(<int>[1, 2])
         ?.map<int>(
@@ -798,6 +815,7 @@ class GitTagVersion {
       devPatch: devParts[1],
       commits: parsedParts[4],
       hash: parts[5],
+      gitTag: '${parts[0]}.${parts[1]}.${parts[2]}${parts[3] ?? ''}', // x.y.z-m.n.pre
     );
   }
 
@@ -821,15 +839,16 @@ class GitTagVersion {
       return '0.0.0-unknown';
     }
     if (commits == 0) {
-      if (hotfix != null) {
-        return '$x.$y.$z+hotfix.$hotfix';
-      }
-      return '$x.$y.$z';
+      return gitTag;
     }
     if (hotfix != null) {
-      return '$x.$y.$z+hotfix.${hotfix + 1}-pre.$commits';
+      // This is an unexpected state where untagged commits exist past a hotfix
+      return '$x.$y.$z+hotfix.${hotfix + 1}.pre.$commits';
+    }
+    if (devPatch != null && devVersion != null) {
+      return '$x.$y.$z-${devVersion + 1}.0.pre.$commits';
     }
-    return '$x.$y.${z + 1}-pre.$commits';
+    return '$x.$y.${z + 1}.pre.$commits';
   }
 }
 

packages/flutter_tools/test/commands.shard/hermetic/version_test.dart

@@ -239,7 +239,7 @@ class MockProcessManager extends Mock implements ProcessManager {
       return ProcessResult(0, 0, 'v10.0.0\r\nv20.0.0\r\n30.0.0-dev.0.0', '');
     }
     if (command[0] == 'git' && command[1] == 'checkout') {
-      version = command[2] as String;
+      version = (command[2] as String).replaceFirst('v', '');
     }
     return ProcessResult(0, 0, '', '');
   }

packages/flutter_tools/test/general.shard/version_test.dart

@@ -394,29 +394,44 @@ void main() {
     const String hash = 'abcdef';
     GitTagVersion gitTagVersion;
 
-    // legacy tag release format
-    gitTagVersion = GitTagVersion.parse('v1.2.3-4-g$hash');
-    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.4-pre.4');
+    // legacy tag format (x.y.z-dev.m.n), master channel
+    gitTagVersion = GitTagVersion.parse('1.2.3-dev.4.5-4-g$hash');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-5.0.pre.4');
+    expect(gitTagVersion.gitTag, '1.2.3-dev.4.5');
+    expect(gitTagVersion.devVersion, 4);
+    expect(gitTagVersion.devPatch, 5);
+
+    // new tag release format, master channel
+    gitTagVersion = GitTagVersion.parse('1.2.3-4.5.pre-13-g$hash');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-5.0.pre.13');
+    expect(gitTagVersion.gitTag, '1.2.3-4.5.pre');
+    expect(gitTagVersion.devVersion, 4);
+    expect(gitTagVersion.devPatch, 5);
+
+    gitTagVersion = GitTagVersion.parse('1.2.3-13-g$hash');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.4.pre.13');
+    expect(gitTagVersion.gitTag, '1.2.3');
     expect(gitTagVersion.devVersion, null);
     expect(gitTagVersion.devPatch, null);
 
-    // new dev tag release format
-    gitTagVersion = GitTagVersion.parse('1.2.3-dev.4.5-13-g$hash');
-    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.4-pre.13');
+    // new tag release format, dev channel
+    gitTagVersion = GitTagVersion.parse('1.2.3-4.5.pre-0-g$hash');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-4.5.pre');
+    expect(gitTagVersion.gitTag, '1.2.3-4.5.pre');
     expect(gitTagVersion.devVersion, 4);
     expect(gitTagVersion.devPatch, 5);
 
-    // new stable tag release format
+    // new tag release format, stable channel
     gitTagVersion = GitTagVersion.parse('1.2.3-13-g$hash');
-    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.4-pre.13');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.4.pre.13');
+    expect(gitTagVersion.gitTag, '1.2.3');
     expect(gitTagVersion.devVersion, null);
     expect(gitTagVersion.devPatch, null);
 
-    expect(GitTagVersion.parse('98.76.54-32-g$hash').frameworkVersionFor(hash), '98.76.55-pre.32');
+    expect(GitTagVersion.parse('98.76.54-32-g$hash').frameworkVersionFor(hash), '98.76.55.pre.32');
     expect(GitTagVersion.parse('10.20.30-0-g$hash').frameworkVersionFor(hash), '10.20.30');
-    expect(GitTagVersion.parse('v1.2.3+hotfix.1-4-g$hash').frameworkVersionFor(hash), '1.2.3+hotfix.2-pre.4');
     expect(testLogger.traceText, '');
-    expect(GitTagVersion.parse('1.2.3+hotfix.1-4-g$hash').frameworkVersionFor(hash), '0.0.0-unknown');
+    expect(GitTagVersion.parse('v1.2.3+hotfix.1-4-g$hash').frameworkVersionFor(hash), '0.0.0-unknown');
     expect(GitTagVersion.parse('x1.2.3-4-g$hash').frameworkVersionFor(hash), '0.0.0-unknown');
     expect(GitTagVersion.parse('1.0.0-unknown-0-g$hash').frameworkVersionFor(hash), '0.0.0-unknown');
     expect(GitTagVersion.parse('beta-1-g$hash').frameworkVersionFor(hash), '0.0.0-unknown');
@@ -425,7 +440,7 @@ void main() {
     expect(testLogger.errorText, '');
     expect(
       testLogger.traceText,
-      'Could not interpret results of "git describe": 1.2.3+hotfix.1-4-gabcdef\n'
+      'Could not interpret results of "git describe": v1.2.3+hotfix.1-4-gabcdef\n'
       'Could not interpret results of "git describe": x1.2.3-4-gabcdef\n'
       'Could not interpret results of "git describe": 1.0.0-unknown-0-gabcdef\n'
       'Could not interpret results of "git describe": beta-1-gabcdef\n'