Open to thoughts on if we should take a different approach for replacing. This was inspired by the PR that updates the minimum iOS version, but could be replaced with a regex or other method.

You could rewrite this as a regular expression like 'minSdkVersion [1-9][0-9]+' but that might end up catching future version numbers that we don't want to migrate. It would also match (and thus replace) cases where minSdkVersion is > flutterMinSdk.

I would leave this as is.

I think it'd be reasonable to write this as a regular expression that matches exactly the same set of versions that the three strings match: minSdkVersion 1[678]. I agree it shouldn't use a pattern that matches later versions we don't intend to migrate.

A further step that may be useful, and which a regular expression enables, is to make this a bit more robust against accidentally rewriting matches that might occur in comments or strings. Also against lines like minSdkVersion 1600 which have one of these as a substring but mean something different.

That class of issue is less of a concern for the iOS migration: it's applying to an XML file (so the string to replace has built-in delimiters like <key>/</key>), plus it's an XML file that is made of simple data mostly edited by machine. Here, these Gradle scripts contain a fair bit of logic, and humans do edit them, so comments are natural.

Here's a regexp that takes that further step — it avoids rewriting lines that have anything else on them (except a possible trailing comment), and therefore mean something other than what we're intending to migrate:

final RegExp _oldMinSdkVersionRe = RegExp(r'(?<=^\s*)minSdkVersion 1[678](?=\s*(?://|$))', multiLine: true);

Here's a test that exercises the comments case, and fails with the plain-string-pattern version but passes with that regexp:

      testWithoutContext('avoid rewriting comments', () {
        const String code = '// minSdkVersion 16  // old default\n'
                    '        minSdkVersion 23  // new version';
        project.appGradleFile.writeAsStringSync(sampleModuleGradleBuildFile(code));
        migration.migrate();
        expect(project.appGradleFile.readAsStringSync(), sampleModuleGradleBuildFile(code));
      });

For additional context (that I should have included in the PR): @reidbaker and I had talked about this offline and decided initially that we were ok taking the simple approach of find and replace, despite it hitting comments, but this argument:

That class of issue is less of a concern for the iOS migration: it's applying to an XML file (so the string to replace has built-in delimiters like /), plus it's an XML file that is made of simple data mostly edited by machine. Here, these Gradle scripts contain a fair bit of logic, and humans do edit them, so comments are natural.

convinces me that taking a regular expression approach similar to the java-gradle migrator is the better way. I'll change to include the regexp and comment-test-case

You could rewrite this as a regular expression like 'minSdkVersion [1-9][0-9]+' but that might end up catching future version numbers that we don't want to migrate. It would also match (and thus replace) cases where minSdkVersion is > flutterMinSdk.

I would leave this as is.

I think it'd be reasonable to write this as a regular expression that matches exactly the same set of versions that the three strings match: minSdkVersion 1[678]. I agree it shouldn't use a pattern that matches later versions we don't intend to migrate.

A further step that may be useful, and which a regular expression enables, is to make this a bit more robust against accidentally rewriting matches that might occur in comments or strings. Also against lines like minSdkVersion 1600 which have one of these as a substring but mean something different.

That class of issue is less of a concern for the iOS migration: it's applying to an XML file (so the string to replace has built-in delimiters like <key>/</key>), plus it's an XML file that is made of simple data mostly edited by machine. Here, these Gradle scripts contain a fair bit of logic, and humans do edit them, so comments are natural.

Here's a regexp that takes that further step — it avoids rewriting lines that have anything else on them (except a possible trailing comment), and therefore mean something other than what we're intending to migrate:

final RegExp _oldMinSdkVersionRe = RegExp(r'(?<=^\s*)minSdkVersion 1[678](?=\s*(?://|$))', multiLine: true);

Here's a test that exercises the comments case, and fails with the plain-string-pattern version but passes with that regexp:

      testWithoutContext('avoid rewriting comments', () {
        const String code = '// minSdkVersion 16  // old default\n'
                    '        minSdkVersion 23  // new version';
        project.appGradleFile.writeAsStringSync(sampleModuleGradleBuildFile(code));
        migration.migrate();
        expect(project.appGradleFile.readAsStringSync(), sampleModuleGradleBuildFile(code));
      });