Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci] Enforce a minimum Kotlin version in examples #3979

Merged
merged 4 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/animations/example/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/dynamic_layouts/example/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_markdown/example/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/example/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/rfw/example/hello/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.0'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/rfw/example/local/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.0'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion packages/rfw/example/remote/android/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.0'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.6.21'
ext.kotlin_version = '1.7.10'
repositories {
google()
mavenCentral()
Expand Down
29 changes: 29 additions & 0 deletions script/tool/lib/src/gradle_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
// found in the LICENSE file.

import 'package:file/file.dart';
import 'package:pub_semver/pub_semver.dart';

import 'common/core.dart';
import 'common/package_looping_command.dart';
import 'common/plugin_utils.dart';
import 'common/repository_package.dart';

/// The lowest `ext.kotlin_version` that example apps are allowed to use.
final Version _minKotlinVersion = Version(1, 7, 10);

/// A command to enforce gradle file conventions and best practices.
class GradleCheckCommand extends PackageLoopingCommand {
/// Creates an instance of the gradle check command.
Expand Down Expand Up @@ -125,6 +129,9 @@ class GradleCheckCommand extends PackageLoopingCommand {
if (!_validateJavacLintConfig(package, lines)) {
succeeded = false;
}
if (!_validateKotlinVersion(package, lines)) {
succeeded = false;
}
return succeeded;
}

Expand Down Expand Up @@ -347,4 +354,26 @@ gradle.projectsEvaluated {
}
return true;
}

/// Validates whether the given [example] has its Kotlin version set to at
/// least a minimum value, if it is set at all.
bool _validateKotlinVersion(
RepositoryPackage example, List<String> gradleLines) {
final RegExp kotlinVersionRegex =
RegExp(r"ext\.kotlin_version\s*=\s*'([\d.]+)'");
RegExpMatch? match;
if (gradleLines.any((String line) {
match = kotlinVersionRegex.firstMatch(line);
return match != null;
})) {
final Version version = Version.parse(match!.group(1)!);
if (version < _minKotlinVersion) {
printError('build.gradle sets "ext.kotlin_version" to "$version". The '
'minimum Kotlin version that can be specified is '
'$_minKotlinVersion, for compatibility with modern dependencies.');
return false;
}
}
return true;
}
}
102 changes: 101 additions & 1 deletion script/tool/test/gradle_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ dependencies {
RepositoryPackage package, {
required String pluginName,
required bool warningsConfigured,
String? kotlinVersion,
}) {
final File buildGradle = package
.platformDirectory(FlutterPlatform.android)
Expand All @@ -140,6 +141,7 @@ gradle.projectsEvaluated {
''';
buildGradle.writeAsStringSync('''
buildscript {
${kotlinVersion == null ? '' : "ext.kotlin_version = '$kotlinVersion'"}
repositories {
google()
mavenCentral()
Expand Down Expand Up @@ -228,9 +230,12 @@ dependencies {
bool includeNamespace = true,
bool commentNamespace = false,
bool warningsConfigured = true,
String? kotlinVersion,
}) {
writeFakeExampleTopLevelBuildGradle(package,
pluginName: pluginName, warningsConfigured: warningsConfigured);
pluginName: pluginName,
warningsConfigured: warningsConfigured,
kotlinVersion: kotlinVersion);
writeFakeExampleAppBuildGradle(package,
includeNamespace: includeNamespace, commentNamespace: commentNamespace);
}
Expand Down Expand Up @@ -644,4 +649,99 @@ dependencies {
],
));
});

group('Kotlin version check', () {
test('passes if not set', () async {
const String packageName = 'a_package';
final RepositoryPackage package =
createFakePackage('a_package', packagesDir);
writeFakePluginBuildGradle(package, includeLanguageVersion: true);
writeFakeManifest(package);
final RepositoryPackage example = package.getExamples().first;
writeFakeExampleBuildGradles(example, pluginName: packageName);
writeFakeManifest(example, isApp: true);

final List<String> output =
await runCapturingPrint(runner, <String>['gradle-check']);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Validating android/build.gradle'),
]),
);
});

test('passes if at the minimum allowed version', () async {
const String packageName = 'a_package';
final RepositoryPackage package =
createFakePackage('a_package', packagesDir);
writeFakePluginBuildGradle(package, includeLanguageVersion: true);
writeFakeManifest(package);
final RepositoryPackage example = package.getExamples().first;
writeFakeExampleBuildGradles(example,
pluginName: packageName, kotlinVersion: '1.7.10');
writeFakeManifest(example, isApp: true);

final List<String> output =
await runCapturingPrint(runner, <String>['gradle-check']);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Validating android/build.gradle'),
]),
);
});

test('passes if above the minimum allowed version', () async {
const String packageName = 'a_package';
final RepositoryPackage package =
createFakePackage('a_package', packagesDir);
writeFakePluginBuildGradle(package, includeLanguageVersion: true);
writeFakeManifest(package);
final RepositoryPackage example = package.getExamples().first;
writeFakeExampleBuildGradles(example,
pluginName: packageName, kotlinVersion: '99.99.0');
writeFakeManifest(example, isApp: true);

final List<String> output =
await runCapturingPrint(runner, <String>['gradle-check']);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Validating android/build.gradle'),
]),
);
});

test('fails if below the minimum allowed version', () async {
const String packageName = 'a_package';
final RepositoryPackage package =
createFakePackage('a_package', packagesDir);
writeFakePluginBuildGradle(package, includeLanguageVersion: true);
writeFakeManifest(package);
final RepositoryPackage example = package.getExamples().first;
writeFakeExampleBuildGradles(example,
pluginName: packageName, kotlinVersion: '1.6.21');
writeFakeManifest(example, isApp: true);

Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['gradle-check'], errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains('build.gradle sets "ext.kotlin_version" to "1.6.21". The '
'minimum Kotlin version that can be specified is 1.7.10, for '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test will need to be updated every time the minimum changes, correct? Is that ideal? (This also applies to the "same as" test)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree I think both should use a modified minKotlinVersion that is @visibleForTesting

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point; done.

'compatibility with modern dependencies.'),
]),
);
});
});
}