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.

Sign up for GitHub

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

Use Android linter from cmdline-tools #28153

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Use Android linter from cmdline-tools #28153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
910adba
Use Android linter from cmdline-tools
Aug 18, 2021
66a632d
import
Aug 18, 2021
0655587
Fix test
Aug 18, 2021
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
11 changes: 11 additions & 0 deletions DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,17 @@ deps = {
'dep_type': 'cipd',
},

'src/third_party/android_tools/sdk/cmdline-tools': {
'packages': [
{
'package': 'flutter/android/sdk/cmdline-tools',
'version': 'latest',
}
],
'condition': 'download_android_deps',
'dep_type': 'cipd',
},

'src/third_party/android_tools/sdk/licenses': {
'packages': [
{
Expand Down
6 changes: 2 additions & 4 deletions shell/platform/android/io/flutter/app/FlutterActivityDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* The most obvious example of when this may come in handy is if an application wishes to subclass
* the Android v4 support library's {@code FragmentActivity}.
*
* <p><b>Usage:</b></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why the closing

was removed?

Copy link
Author

Choose a reason for hiding this comment

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

The closing tag </p> isn't required. See this thread google/google-java-format#61

* <p><b>Usage:</b>
*
* <p>To wire this class up to your activity, simply forward the events defined in {@link
* FlutterActivityEvents} from your activity to an instance of this class. Optionally, you can make
Expand Down Expand Up @@ -425,9 +425,7 @@ private Boolean showSplashScreenUntilFirstFrame() {
ActivityInfo activityInfo =
activity
.getPackageManager()
.getActivityInfo(
activity.getComponentName(),
PackageManager.GET_META_DATA | PackageManager.GET_ACTIVITIES);
.getActivityInfo(activity.getComponentName(), PackageManager.GET_META_DATA);
Bundle metadata = activityInfo.metaData;
return metadata != null && metadata.getBoolean(SPLASH_SCREEN_META_DATA_KEY);
} catch (NameNotFoundException e) {
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/android/io/flutter/embedding/android/FlutterImageView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.flutter.embedding.android;

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Context;
import android.graphics.Bitmap;
Expand Down Expand Up @@ -99,6 +100,7 @@ private static void logW(String format, Object... args) {
}

@TargetApi(19)
@SuppressLint("WrongConstant") // RGBA_8888 is a valid constant.
blasten marked this conversation as resolved.
Show resolved Hide resolved
@NonNull
private static ImageReader createImageReader(int width, int height) {
if (width <= 0) {
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/android/FlutterActivityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.mockito.Mockito.when;
import static org.robolectric.Shadows.shadowOf;

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Context;
import android.content.Intent;
Expand Down Expand Up @@ -438,6 +439,7 @@ public void itWithMetadataWithoutSplashScreenResourceKeyDoesNotProvideSplashScre

static class FlutterActivityWithProvidedEngine extends FlutterActivity {
@Override
@SuppressLint("MissingSuperCall")
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.delegate = new FlutterActivityAndFragmentDelegate(this);
super.delegate.setupFlutterEngine();
Expand Down
66 changes: 12 additions & 54 deletions tools/android_lint/bin/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const LocalProcessManager processManager = LocalProcessManager();
/// Java 1.8.
Future<void> main(List<String> args) async {
final ArgParser argParser = setupOptions();
await checkJava1_8();
final int exitCode = await runLint(argParser, argParser.parse(args));
exit(exitCode);
}
Expand Down Expand Up @@ -74,7 +73,7 @@ Future<int> runLint(ArgParser argParser, ArgResults argResults) async {
<!-- WILL AUTOMATICALLY FIND ALL .java FILES AND INCLUDE THEM HERE -->
<project>
<sdk dir="${androidSdkDir.path}" />
<module name="FlutterEngine" android="true" library="true" compile-sdk-version="android-P">
<module name="FlutterEngine" android="true" library="true" compile-sdk-version="android-S">
<manifest file="${path.join(androidDir.path, 'AndroidManifest.xml')}" />
''');
for (final FileSystemEntity entity in androidDir.listSync(recursive: true)) {
Expand All @@ -89,12 +88,11 @@ Future<int> runLint(ArgParser argParser, ArgResults argResults) async {
</project>
''');
await projectXml.close();

print('Wrote project.xml, starting lint...');
final List<String> lintArgs = <String>[
path.join(androidSdkDir.path, 'tools', 'bin', 'lint'),
'--project',
projectXmlPath,
path.join(androidSdkDir.path, 'cmdline-tools', 'bin', 'lint'),
'--project', projectXmlPath,
'--compile-sdk-version', '31',
'--showall',
'--exitcode', // Set non-zero exit code on errors
'-Wall',
Expand All @@ -106,14 +104,13 @@ Future<int> runLint(ArgParser argParser, ArgResults argResults) async {
if (html) {
lintArgs.addAll(<String>['--html', argResults['out'] as String]);
}
final String? javaHome = await getJavaHome();
final String javahome = getJavaHome(inArgument);
print('Using JAVA_HOME=$javahome');
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is this print meant to be here?

Copy link
Author

Choose a reason for hiding this comment

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

it's intended. It helps inform the user of this script that this ENV variable is set. Particularly, relevant in CI.

final Process lintProcess = await processManager.start(
lintArgs,
environment: javaHome != null
? <String, String>{
'JAVA_HOME': javaHome,
}
: null,
environment: <String, String>{
'JAVA_HOME': javahome,
},
);
lintProcess.stdout.pipe(stdout);
lintProcess.stderr.pipe(stderr);
Expand Down Expand Up @@ -166,50 +163,11 @@ ArgParser setupOptions() {
return argParser;
}

/// On macOS, we can try to find Java 1.8.
///
/// Otherwise, default to whatever JAVA_HOME is already.
Future<String?> getJavaHome() async {
String getJavaHome(String src) {
if (Platform.isMacOS) {
final ProcessResult result = await processManager.run(
<String>['/usr/libexec/java_home', '-v', '1.8', '-F'],
);
if (result.exitCode == 0) {
return (result.stdout as String).trim();
}
}
return Platform.environment['JAVA_HOME'];
}

/// Checks that `java` points to Java 1.8.
///
/// The SDK lint tool may not work with Java > 1.8.
Future<void> checkJava1_8() async {
print('Checking Java version...');

if (Platform.isMacOS) {
final ProcessResult result = await processManager.run(
<String>['/usr/libexec/java_home', '-v', '1.8', '-F'],
);
if (result.exitCode != 0) {
print('Java 1.8 not available - the linter may not work properly.');
}
return;
}
final ProcessResult javaResult = await processManager.run(
<String>['java', '-version'],
);
if (javaResult.exitCode != 0) {
print('Could not run "java -version". '
'Ensure Java is installed and available on your path.');
print(javaResult.stderr);
}
// `java -version` writes to stderr.
final String javaVersionStdout = javaResult.stderr as String;
if (!javaVersionStdout.contains('"1.8')) {
print('The Android SDK tools may not work properly with your Java version. '
'If this process fails, please retry using Java 1.8.');
return path.normalize('$src/third_party/java/openjdk/Contents/Home/');
}
return path.normalize('$src/third_party/java/openjdk/');
}

/// The root directory of this project.
Expand Down
Loading