[tool] Move Android lint checks (#3816)

Moves the checks for Android warning configuration from `lint-android`, where it made sense to put them at the time, to the new `check-gradle`, which is a newer command specifically for validating that our Gradle files are following best practices.

Makes minor changes to the Pigeon platform test projects to make them conform to the checks, since they are now included in the run. The changes shouldn't actually change the behavior of the Pigeon tests.
This commit is contained in:
stuartmorgan
2023-04-25 19:11:12 -07:00
committed by GitHub
parent 501db272c1
commit 3e88d03a16
9 changed files with 370 additions and 253 deletions

View File

@ -6,6 +6,7 @@ import 'package:file/file.dart';
import 'common/core.dart';
import 'common/package_looping_command.dart';
import 'common/plugin_utils.dart';
import 'common/repository_package.dart';
/// A command to enforce gradle file conventions and best practices.
@ -84,6 +85,8 @@ class GradleCheckCommand extends PackageLoopingCommand {
.childFile('AndroidManifest.xml');
}
bool _isCommented(String line) => line.trim().startsWith('//');
/// Validates the build.gradle file for a plugin
/// (some_plugin/android/build.gradle).
bool _validatePluginBuildGradle(RepositoryPackage package, File gradleFile) {
@ -101,6 +104,9 @@ class GradleCheckCommand extends PackageLoopingCommand {
if (!_validateSourceCompatibilityVersion(lines)) {
succeeded = false;
}
if (!_validateGradleDrivenLintConfig(package, lines)) {
succeeded = false;
}
return succeeded;
}
@ -110,9 +116,16 @@ class GradleCheckCommand extends PackageLoopingCommand {
RepositoryPackage package, File gradleFile) {
print('${indentation}Validating '
'${getRelativePosixPath(gradleFile, from: package.directory)}.');
// TODO(stuartmorgan): Move the -Xlint validation from lint_android_command
// to here.
return true;
final String contents = gradleFile.readAsStringSync();
final List<String> lines = contents.split('\n');
// This is tracked as a variable rather than a sequence of &&s so that all
// failures are reported at once, not just the first one.
bool succeeded = true;
if (!_validateJavacLintConfig(package, lines)) {
succeeded = false;
}
return succeeded;
}
/// Validates the app-level build.gradle for an example app (e.g.,
@ -193,11 +206,9 @@ build.gradle "namespace" must match the "package" attribute in AndroidManifest.x
/// lead to compile errors that show up for clients, but not in CI).
bool _validateSourceCompatibilityVersion(List<String> gradleLines) {
if (!gradleLines.any((String line) =>
line.contains('languageVersion') &&
!line.trim().startsWith('//')) &&
line.contains('languageVersion') && !_isCommented(line)) &&
!gradleLines.any((String line) =>
line.contains('sourceCompatibility') &&
!line.trim().startsWith('//'))) {
line.contains('sourceCompatibility') && !_isCommented(line))) {
const String errorMessage = '''
build.gradle must set an explicit Java compatibility version.
@ -225,4 +236,75 @@ for more details.''';
}
return true;
}
/// Returns whether the given gradle content is configured to enable all
/// Gradle-driven lints (those checked by ./gradlew lint) and treat them as
/// errors.
bool _validateGradleDrivenLintConfig(
RepositoryPackage package, List<String> gradleLines) {
final List<String> gradleBuildContents = package
.platformDirectory(FlutterPlatform.android)
.childFile('build.gradle')
.readAsLinesSync();
if (!gradleBuildContents.any((String line) =>
line.contains('checkAllWarnings true') && !_isCommented(line)) ||
!gradleBuildContents.any((String line) =>
line.contains('warningsAsErrors true') && !_isCommented(line))) {
printError('${indentation}This package is not configured to enable all '
'Gradle-driven lint warnings and treat them as errors. '
'Please add the following to the lintOptions section of '
'android/build.gradle:');
print('''
checkAllWarnings true
warningsAsErrors true
''');
return false;
}
return true;
}
/// Validates whether the given [example]'s gradle content is configured to
/// build its plugin target with javac lints enabled and treated as errors,
/// if the enclosing package is a plugin.
///
/// This can only be called on example packages. (Plugin packages should not
/// be configured this way, since it would affect clients.)
///
/// If [example]'s enclosing package is not a plugin package, this just
/// returns true.
bool _validateJavacLintConfig(
RepositoryPackage example, List<String> gradleLines) {
final RepositoryPackage enclosingPackage = example.getEnclosingPackage()!;
if (!pluginSupportsPlatform(platformAndroid, enclosingPackage,
requiredMode: PlatformSupport.inline)) {
return true;
}
final String enclosingPackageName = enclosingPackage.directory.basename;
// The check here is intentionally somewhat loose, to allow for the
// possibility of variations (e.g., not using Xlint:all in some cases, or
// passing other arguments).
if (!(gradleLines.any((String line) =>
line.contains('project(":$enclosingPackageName")')) &&
gradleLines.any((String line) =>
line.contains('options.compilerArgs') &&
line.contains('-Xlint') &&
line.contains('-Werror')))) {
printError('The example '
'"${getRelativePosixPath(example.directory, from: enclosingPackage.directory)}" '
'is not configured to treat javac lints and warnings as errors. '
'Please add the following to its build.gradle:');
print('''
gradle.projectsEvaluated {
project(":$enclosingPackageName") {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
''');
return false;
}
return true;
}
}