From a4ced6bd365fc927ed13f44b710a00089ca7c5aa Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 17 Apr 2023 09:35:22 -0700 Subject: [PATCH] [tool] Add initial gradle validation command (#3715) Adds a repo tool command to validate that all plugins set an explicit Java compatibility version, instead of using whatever the local toolchain happens to be (which creates the potential for issues like https://github.com/flutter/flutter/issues/124839 where our CI passes but builds fail for some clients because our default is newer than theirs). Currently that's all it checks, but we can add any other gradle best practices we want to enforce here in the future. This also enables the new check in CI, and fixes all the violations it found. Fixes https://github.com/flutter/flutter/issues/124839 --- .cirrus.yml | 1 + packages/espresso/CHANGELOG.md | 4 + packages/espresso/android/build.gradle | 5 + packages/espresso/pubspec.yaml | 2 +- .../CHANGELOG.md | 3 +- .../android/build.gradle | 5 + .../pubspec.yaml | 2 +- .../google_sign_in_android/CHANGELOG.md | 4 + .../android/build.gradle | 5 + .../google_sign_in_android/pubspec.yaml | 2 +- .../local_auth_android/CHANGELOG.md | 4 + .../local_auth_android/android/build.gradle | 5 + .../local_auth_android/pubspec.yaml | 2 +- .../url_launcher_android/CHANGELOG.md | 4 + .../url_launcher_android/android/build.gradle | 5 + .../url_launcher_android/pubspec.yaml | 2 +- script/tool/lib/src/gradle_check_command.dart | 92 ++++++++ script/tool/lib/src/main.dart | 2 + .../tool/test/gradle_check_command_test.dart | 219 ++++++++++++++++++ 19 files changed, 362 insertions(+), 6 deletions(-) create mode 100644 script/tool/lib/src/gradle_check_command.dart create mode 100644 script/tool/test/gradle_check_command_test.dart diff --git a/.cirrus.yml b/.cirrus.yml index 38690ed7b8..7d00bcbc75 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -106,6 +106,7 @@ task: # run with --require-excerpts and no exclusions. - ./script/tool_runner.sh readme-check --require-excerpts --exclude=script/configs/temp_exclude_excerpt.yaml dependabot_script: $PLUGIN_TOOL_COMMAND dependabot-check + gradle_script: $PLUGIN_TOOL_COMMAND gradle-check version_script: # For pre-submit, pass the PR labels to the script to allow for # check overrides. diff --git a/packages/espresso/CHANGELOG.md b/packages/espresso/CHANGELOG.md index 5ef1319e9c..2a318f20a2 100644 --- a/packages/espresso/CHANGELOG.md +++ b/packages/espresso/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.0+1 + +* Sets an explicit Java compatibility version. + ## 0.3.0 * **BREAKING CHANGE**: Migrates uses of the deprecated `@Beta` annotation to the new `@ExperimentalApi` annotation. diff --git a/packages/espresso/android/build.gradle b/packages/espresso/android/build.gradle index 2aac3452e0..c169b19dba 100644 --- a/packages/espresso/android/build.gradle +++ b/packages/espresso/android/build.gradle @@ -28,6 +28,11 @@ android { minSdkVersion 16 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + lintOptions { checkAllWarnings true warningsAsErrors true diff --git a/packages/espresso/pubspec.yaml b/packages/espresso/pubspec.yaml index ff2dd7c3ff..baf0b7d2dc 100644 --- a/packages/espresso/pubspec.yaml +++ b/packages/espresso/pubspec.yaml @@ -3,7 +3,7 @@ description: Java classes for testing Flutter apps using Espresso. Allows driving Flutter widgets from a native Espresso test. repository: https://github.com/flutter/packages/tree/main/packages/espresso issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+espresso%22 -version: 0.3.0 +version: 0.3.0+1 environment: sdk: ">=2.17.0 <4.0.0" diff --git a/packages/flutter_plugin_android_lifecycle/CHANGELOG.md b/packages/flutter_plugin_android_lifecycle/CHANGELOG.md index 0ab8296d8e..6e1dad0310 100644 --- a/packages/flutter_plugin_android_lifecycle/CHANGELOG.md +++ b/packages/flutter_plugin_android_lifecycle/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 2.0.10 +* Sets an explicit Java compatibility version. * Aligns Dart and Flutter SDK constraints. ## 2.0.9 diff --git a/packages/flutter_plugin_android_lifecycle/android/build.gradle b/packages/flutter_plugin_android_lifecycle/android/build.gradle index 8431e0bc2c..72c4a127e2 100644 --- a/packages/flutter_plugin_android_lifecycle/android/build.gradle +++ b/packages/flutter_plugin_android_lifecycle/android/build.gradle @@ -29,6 +29,11 @@ android { testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" consumerProguardFiles 'proguard.txt' } + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + lintOptions { checkAllWarnings true warningsAsErrors true diff --git a/packages/flutter_plugin_android_lifecycle/pubspec.yaml b/packages/flutter_plugin_android_lifecycle/pubspec.yaml index 2fcb75d1ed..d8c92d030d 100644 --- a/packages/flutter_plugin_android_lifecycle/pubspec.yaml +++ b/packages/flutter_plugin_android_lifecycle/pubspec.yaml @@ -2,7 +2,7 @@ name: flutter_plugin_android_lifecycle description: Flutter plugin for accessing an Android Lifecycle within other plugins. repository: https://github.com/flutter/packages/tree/main/packages/flutter_plugin_android_lifecycle issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_plugin_android_lifecycle%22 -version: 2.0.9 +version: 2.0.10 environment: sdk: ">=2.17.0 <4.0.0" diff --git a/packages/google_sign_in/google_sign_in_android/CHANGELOG.md b/packages/google_sign_in/google_sign_in_android/CHANGELOG.md index b5c90aa2f0..a419d902e7 100644 --- a/packages/google_sign_in/google_sign_in_android/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 6.1.10 + +* Sets an explicit Java compatibility version. + ## 6.1.9 * Updates play-services-auth version to 20.5.0. diff --git a/packages/google_sign_in/google_sign_in_android/android/build.gradle b/packages/google_sign_in/google_sign_in_android/android/build.gradle index e199535e45..d6872d51b6 100644 --- a/packages/google_sign_in/google_sign_in_android/android/build.gradle +++ b/packages/google_sign_in/google_sign_in_android/android/build.gradle @@ -28,6 +28,11 @@ android { minSdkVersion 16 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + lintOptions { checkAllWarnings true warningsAsErrors true diff --git a/packages/google_sign_in/google_sign_in_android/pubspec.yaml b/packages/google_sign_in/google_sign_in_android/pubspec.yaml index bd4d8853d6..b6e892f50c 100644 --- a/packages/google_sign_in/google_sign_in_android/pubspec.yaml +++ b/packages/google_sign_in/google_sign_in_android/pubspec.yaml @@ -2,7 +2,7 @@ name: google_sign_in_android description: Android implementation of the google_sign_in plugin. repository: https://github.com/flutter/packages/tree/main/packages/google_sign_in/google_sign_in_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22 -version: 6.1.9 +version: 6.1.10 environment: sdk: ">=2.17.0 <4.0.0" diff --git a/packages/local_auth/local_auth_android/CHANGELOG.md b/packages/local_auth/local_auth_android/CHANGELOG.md index e2acc8f4cf..92a44356cd 100644 --- a/packages/local_auth/local_auth_android/CHANGELOG.md +++ b/packages/local_auth/local_auth_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.0.22 + +* Sets an explicit Java compatibility version. + ## 1.0.21 * Clarifies explanation of endorsement in README. diff --git a/packages/local_auth/local_auth_android/android/build.gradle b/packages/local_auth/local_auth_android/android/build.gradle index 6bd8e4d845..53ef4f47d8 100644 --- a/packages/local_auth/local_auth_android/android/build.gradle +++ b/packages/local_auth/local_auth_android/android/build.gradle @@ -28,6 +28,11 @@ android { minSdkVersion 16 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + lintOptions { checkAllWarnings true warningsAsErrors true diff --git a/packages/local_auth/local_auth_android/pubspec.yaml b/packages/local_auth/local_auth_android/pubspec.yaml index b84a763e8d..28d778c73d 100644 --- a/packages/local_auth/local_auth_android/pubspec.yaml +++ b/packages/local_auth/local_auth_android/pubspec.yaml @@ -2,7 +2,7 @@ name: local_auth_android description: Android implementation of the local_auth plugin. repository: https://github.com/flutter/packages/tree/main/packages/local_auth/local_auth_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+local_auth%22 -version: 1.0.21 +version: 1.0.22 environment: sdk: ">=2.17.0 <4.0.0" diff --git a/packages/url_launcher/url_launcher_android/CHANGELOG.md b/packages/url_launcher/url_launcher_android/CHANGELOG.md index 3c8a025bc6..3e46aa9288 100644 --- a/packages/url_launcher/url_launcher_android/CHANGELOG.md +++ b/packages/url_launcher/url_launcher_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 6.0.28 + +* Sets an explicit Java compatibility version. + ## 6.0.27 * Fixes Java warnings. diff --git a/packages/url_launcher/url_launcher_android/android/build.gradle b/packages/url_launcher/url_launcher_android/android/build.gradle index acd30143ae..8b4abcc199 100644 --- a/packages/url_launcher/url_launcher_android/android/build.gradle +++ b/packages/url_launcher/url_launcher_android/android/build.gradle @@ -28,6 +28,11 @@ android { minSdkVersion 16 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + lintOptions { checkAllWarnings true warningsAsErrors true diff --git a/packages/url_launcher/url_launcher_android/pubspec.yaml b/packages/url_launcher/url_launcher_android/pubspec.yaml index 6812385d68..505fdfaf27 100644 --- a/packages/url_launcher/url_launcher_android/pubspec.yaml +++ b/packages/url_launcher/url_launcher_android/pubspec.yaml @@ -2,7 +2,7 @@ name: url_launcher_android description: Android implementation of the url_launcher plugin. repository: https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22 -version: 6.0.27 +version: 6.0.28 environment: sdk: ">=2.18.0 <4.0.0" diff --git a/script/tool/lib/src/gradle_check_command.dart b/script/tool/lib/src/gradle_check_command.dart new file mode 100644 index 0000000000..734180c1f2 --- /dev/null +++ b/script/tool/lib/src/gradle_check_command.dart @@ -0,0 +1,92 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; + +import 'common/core.dart'; +import 'common/package_looping_command.dart'; +import 'common/repository_package.dart'; + +/// A command to enforce gradle file conventions and best practices. +class GradleCheckCommand extends PackageLoopingCommand { + /// Creates an instance of the gradle check command. + GradleCheckCommand(Directory packagesDir) : super(packagesDir); + + @override + final String name = 'gradle-check'; + + @override + final String description = + 'Checks that gradle files follow repository conventions.'; + + @override + bool get hasLongOutput => false; + + @override + PackageLoopingType get packageLoopingType => + PackageLoopingType.includeAllSubpackages; + + @override + Future runForPackage(RepositoryPackage package) async { + if (!package.platformDirectory(FlutterPlatform.android).existsSync()) { + return PackageResult.skip('No android/ directory.'); + } + + const String exampleDirName = 'example'; + final bool isExample = package.directory.basename == exampleDirName || + package.directory.parent.basename == exampleDirName; + if (!_validateBuildGradle(package, isExample: isExample)) { + return PackageResult.fail(); + } + return PackageResult.success(); + } + + bool _validateBuildGradle(RepositoryPackage package, + {required bool isExample}) { + // Currently the only check is not relevant to examples; checks that apply + // to both plugins and examples should go above here. + if (!isExample) { + print('${indentation}Validating android/build.gradle.'); + final String contents = package + .platformDirectory(FlutterPlatform.android) + .childFile('build.gradle') + .readAsStringSync(); + final List lines = contents.split('\n'); + + if (!lines.any((String line) => + line.contains('languageVersion') && + !line.trim().startsWith('//')) && + !lines.any((String line) => + line.contains('sourceCompatibility') && + !line.trim().startsWith('//'))) { + const String errorMessage = ''' +build.gradle must set an explicit Java compatibility version. + +This can be done either via "sourceCompatibility": + android { + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_8 + } + } + +or "toolchain": + java { + toolchain { + languageVersion = JavaLanguageVersion.of(8) + } + } + +See: +https://docs.gradle.org/current/userguide/java_plugin.html#toolchain_and_compatibility +for more details.'''; + + printError( + '$indentation${errorMessage.split('\n').join('\n$indentation')}'); + return false; + } + } + + return true; + } +} diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 64f5135304..1839524a85 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -19,6 +19,7 @@ import 'federation_safety_check_command.dart'; import 'firebase_test_lab_command.dart'; import 'fix_command.dart'; import 'format_command.dart'; +import 'gradle_check_command.dart'; import 'license_check_command.dart'; import 'lint_android_command.dart'; import 'list_command.dart'; @@ -66,6 +67,7 @@ void main(List args) { ..addCommand(FirebaseTestLabCommand(packagesDir)) ..addCommand(FixCommand(packagesDir)) ..addCommand(FormatCommand(packagesDir)) + ..addCommand(GradleCheckCommand(packagesDir)) ..addCommand(LicenseCheckCommand(packagesDir)) ..addCommand(LintAndroidCommand(packagesDir)) ..addCommand(PodspecCheckCommand(packagesDir)) diff --git a/script/tool/test/gradle_check_command_test.dart b/script/tool/test/gradle_check_command_test.dart new file mode 100644 index 0000000000..e7a4b2b9cb --- /dev/null +++ b/script/tool/test/gradle_check_command_test.dart @@ -0,0 +1,219 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:args/command_runner.dart'; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/gradle_check_command.dart'; +import 'package:test/test.dart'; + +import 'util.dart'; + +void main() { + late CommandRunner runner; + late FileSystem fileSystem; + late Directory packagesDir; + + setUp(() { + fileSystem = MemoryFileSystem(); + packagesDir = fileSystem.currentDirectory.childDirectory('packages'); + createPackagesDirectory(parentDir: packagesDir.parent); + final GradleCheckCommand command = GradleCheckCommand( + packagesDir, + ); + + runner = CommandRunner( + 'gradle_check_command', 'Test for gradle_check_command'); + runner.addCommand(command); + }); + + void writeFakeBuildGradle( + RepositoryPackage package, { + bool includeLanguageVersion = false, + bool includeSourceCompat = false, + bool commentRequiredLine = false, + }) { + final File buildGradle = package + .platformDirectory(FlutterPlatform.android) + .childFile('build.gradle'); + buildGradle.createSync(recursive: true); + + final String compileOptionsSection = ''' + compileOptions { + ${commentRequiredLine ? '// ' : ''}sourceCompatibility JavaVersion.VERSION_1_8 + } +'''; + final String javaSection = ''' +java { + toolchain { + ${commentRequiredLine ? '// ' : ''}languageVersion = JavaLanguageVersion.of(8) + } +} + +'''; + + buildGradle.writeAsStringSync(''' +group 'dev.flutter.plugins.fake' +version '1.0-SNAPSHOT' + +buildscript { + repositories { + google() + mavenCentral() + } +} + +apply plugin: 'com.android.library' + +${includeLanguageVersion ? javaSection : ''} +android { + compileSdkVersion 33 + + defaultConfig { + minSdkVersion 30 + } + lintOptions { + checkAllWarnings true + } + testOptions { + unitTests.includeAndroidResources = true + } +${includeSourceCompat ? compileOptionsSection : ''} +} + +dependencies { + implementation 'fake.package:fake:1.0.0' +} +'''); + } + + test('skips when package has no Android directory', () async { + createFakePackage('a_package', packagesDir, examples: []); + + final List output = + await runCapturingPrint(runner, ['gradle-check']); + + expect( + output, + containsAllInOrder([ + contains('Skipped 1 package(s)'), + ]), + ); + }); + + test('fails when build.gradle has no java compatibility version', () async { + final RepositoryPackage package = + createFakePlugin('a_plugin', packagesDir, examples: []); + writeFakeBuildGradle(package); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'build.gradle must set an explicit Java compatibility version.'), + ]), + ); + }); + + test('passes when sourceCompatibility is specified', () async { + final RepositoryPackage package = + createFakePlugin('a_plugin', packagesDir, examples: []); + writeFakeBuildGradle(package, includeSourceCompat: true); + + final List output = + await runCapturingPrint(runner, ['gradle-check']); + + expect( + output, + containsAllInOrder([ + contains('Validating android/build.gradle'), + ]), + ); + }); + + test('passes when toolchain languageVersion is specified', () async { + final RepositoryPackage package = + createFakePlugin('a_plugin', packagesDir, examples: []); + writeFakeBuildGradle(package, includeLanguageVersion: true); + + final List output = + await runCapturingPrint(runner, ['gradle-check']); + + expect( + output, + containsAllInOrder([ + contains('Validating android/build.gradle'), + ]), + ); + }); + + test('does not require java version in examples', () async { + final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir); + writeFakeBuildGradle(package, includeLanguageVersion: true); + writeFakeBuildGradle(package.getExamples().first); + + final List output = + await runCapturingPrint(runner, ['gradle-check']); + + expect( + output, + containsAllInOrder([ + contains('Validating android/build.gradle'), + contains('Ran for 2 package(s)'), + ]), + ); + }); + + test('fails when java compatibility version is commented out', () async { + final RepositoryPackage package = + createFakePlugin('a_plugin', packagesDir, examples: []); + writeFakeBuildGradle(package, + includeSourceCompat: true, commentRequiredLine: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'build.gradle must set an explicit Java compatibility version.'), + ]), + ); + }); + + test('fails when languageVersion is commented out', () async { + final RepositoryPackage package = + createFakePlugin('a_plugin', packagesDir, examples: []); + writeFakeBuildGradle(package, + includeLanguageVersion: true, commentRequiredLine: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'build.gradle must set an explicit Java compatibility version.'), + ]), + ); + }); +}