Skip to content

Commit e04b6e4

Browse files
reidbakerreidbaker
authored
[tool] Change gradle-check logic to enforce alignment of java versions and a minimum (17) (#10206)
Hold review/merge until #10201 has merged. This pr is code review feedback from #10201 (comment) - **Update gradle-check to support minimum java version and validating java version alignment** ## Pre-Review Checklist
1 parent f6896e5 commit e04b6e4

File tree

2 files changed

+151
-17
lines changed

2 files changed

+151
-17
lines changed

‎script/tool/lib/src/gradle_check_command.dart‎

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class GradleCheckCommand extends PackageLoopingCommand {
2727
super.gitDir,
2828
});
2929

30+
static const int _minimumJavaVersion = 17;
31+
3032
@override
3133
final String name = 'gradle-check';
3234

@@ -130,6 +132,12 @@ class GradleCheckCommand extends PackageLoopingCommand {
130132
if (!_validateCompatibilityVersions(lines)) {
131133
succeeded = false;
132134
}
135+
if (!_validateKotlinJvmCompatibility(lines)) {
136+
succeeded = false;
137+
}
138+
if (!_validateJavaKotlinCompileOptionsAlignment(lines)) {
139+
succeeded = false;
140+
}
133141
if (!_validateGradleDrivenLintConfig(package, lines)) {
134142
succeeded = false;
135143
}
@@ -356,21 +364,18 @@ build.gradle "namespace" must match the "package" attribute in AndroidManifest.x
356364
/// than using whatever the client's local toolchaing defaults to (which can
357365
/// lead to compile errors that show up for clients, but not in CI).
358366
bool _validateCompatibilityVersions(List<String> gradleLines) {
359-
const String requiredJavaVersion = '17';
360367
final bool hasLanguageVersion = gradleLines.any((String line) =>
361368
line.contains('languageVersion') && !_isCommented(line));
362369
final bool hasCompabilityVersions = gradleLines.any((String line) =>
363-
line.contains(
364-
'sourceCompatibility = JavaVersion.VERSION_$requiredJavaVersion') &&
370+
line.contains('sourceCompatibility = JavaVersion.VERSION_') &&
365371
!_isCommented(line)) &&
366372
// Newer toolchains default targetCompatibility to the same value as
367373
// sourceCompatibility, but older toolchains require it to be set
368374
// explicitly. The exact version cutoff (and of which piece of the
369375
// toolchain; likely AGP) is unknown; for context see
370376
// https://github.com/flutter/flutter/issues/125482
371377
gradleLines.any((String line) =>
372-
line.contains(
373-
'targetCompatibility = JavaVersion.VERSION_$requiredJavaVersion') &&
378+
line.contains('targetCompatibility = JavaVersion.VERSION_') &&
374379
!_isCommented(line));
375380
if (!hasLanguageVersion && !hasCompabilityVersions) {
376381
const String javaErrorMessage = '''
@@ -379,15 +384,15 @@ build.gradle(.kts) must set an explicit Java compatibility version.
379384
This can be done either via "sourceCompatibility"/"targetCompatibility":
380385
android {
381386
compileOptions {
382-
sourceCompatibility = JavaVersion.VERSION_$requiredJavaVersion
383-
targetCompatibility = JavaVersion.VERSION_$requiredJavaVersion
387+
sourceCompatibility = JavaVersion.VERSION_$_minimumJavaVersion
388+
targetCompatibility = JavaVersion.VERSION_$_minimumJavaVersion
384389
}
385390
}
386391
387392
or "toolchain":
388393
java {
389394
toolchain {
390-
languageVersion = JavaLanguageVersion.of($requiredJavaVersion)
395+
languageVersion = JavaLanguageVersion.of($_minimumJavaVersion)
391396
}
392397
}
393398
@@ -399,11 +404,16 @@ for more details.''';
399404
'$indentation${javaErrorMessage.split('\n').join('\n$indentation')}');
400405
return false;
401406
}
407+
408+
return true;
409+
}
410+
411+
bool _validateKotlinJvmCompatibility(List<String> gradleLines) {
402412
bool isKotlinOptions(String line) =>
403413
line.contains('kotlinOptions') && !_isCommented(line);
404414
final bool hasKotlinOptions = gradleLines.any(isKotlinOptions);
405415
final bool kotlinOptionsUsesJavaVersion = gradleLines.any((String line) =>
406-
line.contains('jvmTarget = JavaVersion.VERSION_$requiredJavaVersion') &&
416+
line.contains('jvmTarget = JavaVersion.VERSION_') &&
407417
!_isCommented(line));
408418
// Either does not set kotlinOptions or does and uses non-string based syntax.
409419
if (hasKotlinOptions && !kotlinOptionsUsesJavaVersion) {
@@ -421,7 +431,7 @@ If build.gradle(.kts) sets jvmTarget then it must use JavaVersion syntax.
421431
Good:
422432
android {
423433
kotlinOptions {
424-
jvmTarget = JavaVersion.VERSION_$requiredJavaVersion.toString()
434+
jvmTarget = JavaVersion.VERSION_$_minimumJavaVersion.toString()
425435
}
426436
}
427437
BAD:
@@ -431,6 +441,46 @@ If build.gradle(.kts) sets jvmTarget then it must use JavaVersion syntax.
431441
'$indentation${kotlinErrorMessage.split('\n').join('\n$indentation')}');
432442
return false;
433443
}
444+
// No error condition.
445+
return true;
446+
}
447+
448+
bool _validateJavaKotlinCompileOptionsAlignment(List<String> gradleLines) {
449+
final List<String> javaVersions = <String>[];
450+
// Some java versions have the format VERSION_1_8 but we dont need to handle those
451+
// because they are below the minimum.
452+
final RegExp javaVersionMatcher =
453+
RegExp(r'JavaVersion.VERSION_(?<javaVersion>\d+)');
454+
for (final String line in gradleLines) {
455+
final RegExpMatch? match = javaVersionMatcher.firstMatch(line);
456+
if (!_isCommented(line) && match != null) {
457+
final String? foundVersion = match.namedGroup('javaVersion');
458+
if (foundVersion != null) {
459+
javaVersions.add(foundVersion);
460+
}
461+
}
462+
}
463+
if (javaVersions.isNotEmpty) {
464+
final int version = int.parse(javaVersions.first);
465+
if (!javaVersions.every((String element) => element == '$version')) {
466+
const String javaVersionAlignmentError = '''
467+
If build.gradle(.kts) uses JavaVersion.* versions must be the same.
468+
''';
469+
printError(
470+
'$indentation${javaVersionAlignmentError.split('\n').join('\n$indentation')}');
471+
return false;
472+
}
473+
474+
if (version < _minimumJavaVersion) {
475+
final String minimumJavaVersionError = '''
476+
build.gradle(.kts) uses "JavaVersion.VERSION_$version".
477+
Which is below the minimum required. Use at least "JavaVersion.VERSION_$_minimumJavaVersion".
478+
''';
479+
printError(
480+
'$indentation${minimumJavaVersionError.split('\n').join('\n$indentation')}');
481+
return false;
482+
}
483+
}
434484

435485
return true;
436486
}

‎script/tool/test/gradle_check_command_test.dart‎

Lines changed: 91 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ void main() {
5050
String compileSdk = '36',
5151
bool includeKotlinOptions = true,
5252
bool commentKotlinOptions = false,
53-
bool useDeprecatedJvmTarget = false,
53+
bool useDeprecatedJvmTargetStyle = false,
54+
int jvmTargetValue = 17,
55+
int kotlinJvmValue = 17,
5456
}) {
5557
final File buildGradle = package
5658
.platformDirectory(FlutterPlatform.android)
@@ -74,16 +76,17 @@ java {
7476
7577
''';
7678
final String sourceCompat =
77-
'${commentSourceLanguage ? '// ' : ''}sourceCompatibility = JavaVersion.VERSION_17';
79+
'${commentSourceLanguage ? '// ' : ''}sourceCompatibility = JavaVersion.VERSION_$jvmTargetValue';
7880
final String targetCompat =
79-
'${commentSourceLanguage ? '// ' : ''}targetCompatibility = JavaVersion.VERSION_17';
81+
'${commentSourceLanguage ? '// ' : ''}targetCompatibility = JavaVersion.VERSION_$jvmTargetValue';
8082
final String namespace =
8183
" ${commentNamespace ? '// ' : ''}namespace = '$_defaultFakeNamespace'";
82-
final String jvmTarget =
83-
useDeprecatedJvmTarget ? '17' : 'JavaVersion.VERSION_17.toString()';
84+
final String kotlinJvmTarget = useDeprecatedJvmTargetStyle
85+
? '$jvmTargetValue'
86+
: 'JavaVersion.VERSION_$kotlinJvmValue.toString()';
8487
final String kotlinConfig = '''
8588
${commentKotlinOptions ? '//' : ''}kotlinOptions {
86-
${commentKotlinOptions ? '//' : ''}jvmTarget = $jvmTarget
89+
${commentKotlinOptions ? '//' : ''}jvmTarget = $kotlinJvmTarget
8790
${commentKotlinOptions ? '//' : ''}}''';
8891

8992
buildGradle.writeAsStringSync('''
@@ -391,6 +394,87 @@ dependencies {
391394
);
392395
});
393396

397+
test('fails when sourceCompatibility/targetCompatibility are below minimum',
398+
() async {
399+
final RepositoryPackage package =
400+
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
401+
writeFakePluginBuildGradle(
402+
package,
403+
includeSourceCompat: true,
404+
includeTargetCompat: true,
405+
jvmTargetValue: 11,
406+
kotlinJvmValue: 11,
407+
);
408+
writeFakeManifest(package);
409+
410+
Error? commandError;
411+
final List<String> output = await runCapturingPrint(
412+
runner, <String>['gradle-check'], errorHandler: (Error e) {
413+
commandError = e;
414+
});
415+
416+
expect(commandError, isA<ToolExit>());
417+
expect(
418+
output,
419+
containsAllInOrder(<Matcher>[
420+
contains(
421+
'Which is below the minimum required. Use at least "JavaVersion.VERSION_'),
422+
]),
423+
);
424+
});
425+
426+
test('fails when compatibility values do not match kotlinOptions', () async {
427+
final RepositoryPackage package =
428+
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
429+
writeFakePluginBuildGradle(
430+
package,
431+
includeSourceCompat: true,
432+
includeTargetCompat: true,
433+
jvmTargetValue: 21,
434+
// ignore: avoid_redundant_argument_values
435+
kotlinJvmValue: 17,
436+
);
437+
writeFakeManifest(package);
438+
439+
Error? commandError;
440+
final List<String> output = await runCapturingPrint(
441+
runner, <String>['gradle-check'], errorHandler: (Error e) {
442+
commandError = e;
443+
});
444+
445+
expect(commandError, isA<ToolExit>());
446+
expect(
447+
output,
448+
containsAllInOrder(<Matcher>[
449+
contains(
450+
'If build.gradle(.kts) uses JavaVersion.* versions must be the same.'),
451+
]),
452+
);
453+
});
454+
455+
test('passes when jvmValues are higher than minimim', () async {
456+
final RepositoryPackage package =
457+
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
458+
writeFakePluginBuildGradle(
459+
package,
460+
includeSourceCompat: true,
461+
includeTargetCompat: true,
462+
jvmTargetValue: 21,
463+
kotlinJvmValue: 21,
464+
);
465+
writeFakeManifest(package);
466+
467+
final List<String> output =
468+
await runCapturingPrint(runner, <String>['gradle-check']);
469+
470+
expect(
471+
output,
472+
containsAllInOrder(<Matcher>[
473+
contains('Validating android/build.gradle'),
474+
]),
475+
);
476+
});
477+
394478
test('passes when sourceCompatibility and targetCompatibility are specified',
395479
() async {
396480
final RepositoryPackage package =
@@ -1242,7 +1326,7 @@ dependencies {
12421326
writeFakePluginBuildGradle(
12431327
package,
12441328
includeLanguageVersion: true,
1245-
useDeprecatedJvmTarget: true,
1329+
useDeprecatedJvmTargetStyle: true,
12461330
);
12471331
writeFakeManifest(package);
12481332

0 commit comments

Comments
 (0)