This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Start of linting Android embedding #8023
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| lint_report/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import 'dart:io'; | ||
|
dnfield marked this conversation as resolved.
|
||
|
|
||
| import 'package:args/args.dart'; | ||
| import 'package:path/path.dart' as path; | ||
| import 'package:process/process.dart'; | ||
|
|
||
| Future<void> main(List<String> args) async { | ||
|
mklim marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Can we break this function down? Something like: request = parseArgs(args);
writeProjectFile(request);
result = runLint();
return result;
Contributor
Author
There was a problem hiding this comment. More or less done - I could definitely get fancier with this, but right now I just want something that works. |
||
| final ArgParser argParser = ArgParser(); | ||
| argParser.addOption( | ||
|
dnfield marked this conversation as resolved.
Outdated
|
||
| 'in', | ||
| help: 'The path to `engine/src`.', | ||
| defaultsTo: path.join(path.current), | ||
|
Contributor
There was a problem hiding this comment. You could have it find the default based on the location of the script, using final String flutterRoot = path.dirname(path.dirname(path.dirname(path.fromUri(Platform.script))));This makes it possible to run it from any dir without needing to specify
Contributor
Author
There was a problem hiding this comment. Done with a little twist to make sure it's still relative so that project.xml comes out ok on other systems |
||
| ); | ||
| argParser.addOption( | ||
| 'out', | ||
| help: 'The path to write the generated the HTML report to.', | ||
| defaultsTo: 'lint_report', | ||
| ); | ||
|
|
||
| final ArgResults argResults = argParser.parse(args); | ||
| final Directory androidDir = Directory(path.join( | ||
| argResults['in'], | ||
| 'flutter', | ||
| 'shell', | ||
| 'platform', | ||
| 'android', | ||
| )); | ||
| if (!androidDir.existsSync()) { | ||
| print('This command must be run from the engine/src directory, ' | ||
| 'or be passed that directory as the --in parameter.\n'); | ||
| print(argParser.usage); | ||
| exit(-1); | ||
| } | ||
|
|
||
| final Directory androidSdkDir = Directory( | ||
| path.join(argResults['in'], 'third_party', 'android_tools', 'sdk'), | ||
| ); | ||
|
|
||
| if (!androidSdkDir.existsSync()) { | ||
| print('The Android SDK for this engine is missing from the ' | ||
| 'third_party/android_tools directory. Have you run gclient sync?\n'); | ||
| print(argParser.usage); | ||
| exit(-1); | ||
| } | ||
|
|
||
| final IOSink projectXml = File('./project.xml').openWrite(); | ||
| projectXml.write('''<!-- THIS FILE IS GENERATED. PLEASE USE THE INCLUDED DART PROGRAM WHICH --> | ||
|
mklim marked this conversation as resolved.
Outdated
|
||
| <!-- 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"> | ||
| <manifest file="${path.join(androidDir.path, 'AndroidManifest.xml')}" /> | ||
| '''); | ||
| for (final FileSystemEntity entity in androidDir.listSync(recursive: true)) { | ||
| if (!entity.path.endsWith('.java')) { | ||
| continue; | ||
| } | ||
| projectXml.writeln(' <src file="${entity.path}" />'); | ||
| } | ||
|
|
||
| projectXml.write(''' </module> | ||
| </project> | ||
| '''); | ||
| await projectXml.close(); | ||
|
|
||
| print('Wrote project.xml, starting lint...'); | ||
| const LocalProcessManager processManager = LocalProcessManager(); | ||
| final ProcessResult result = await processManager.run( | ||
| <String>[ | ||
| path.join(androidSdkDir.path, 'tools', 'bin', 'lint'), | ||
| '--project', | ||
| './project.xml', | ||
| '--html', | ||
| argResults['out'], | ||
| '--showall', | ||
| ], | ||
| ); | ||
| if (result.exitCode != 0) { | ||
| print(result.stderr); | ||
| exit(result.exitCode); | ||
| } | ||
| print(result.stdout); | ||
| // TODO(dnfield): once we know what a clean lint will look like for this | ||
| // project, we should detect it and set the exit code based on it. | ||
| // Android Lint does _not_ set the exit code to non-zero if it detects lint | ||
|
Contributor
There was a problem hiding this comment. Whoa. Seems like quite an oversight in the lint tool to not report its results via exit code.
Contributor
Author
There was a problem hiding this comment. Ah! There's an option for it. |
||
| // errors/warnings. | ||
| return; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| <!-- THIS FILE IS GENERATED. PLEASE USE THE INCLUDED DART PROGRAM WHICH --> | ||
| <!-- WILL AUTOMATICALLY FIND ALL .java FILES AND INCLUDE THEM HERE --> | ||
|
Contributor
There was a problem hiding this comment. Should you find all Java files, or only the Java files listed in BUILD.gn?
Contributor
Author
There was a problem hiding this comment. We could use BUILD.gn - in which case we could probably write this as a build rule. I don't really have a strong opinion on that - would we want unlinted files in the repo that aren't part of the build though?
Contributor
There was a problem hiding this comment. I guess the question is, why would a Java file exist without being a part of BUILD.gn....and if no such situation can occur, then why not just yank the Java file paths out of BUILD.gn? |
||
| <project> | ||
| <sdk dir="../../../third_party/android_tools/sdk" /> | ||
| <module name="FlutterEngine" android="true" library="true" compile-sdk-version="android-P"> | ||
| <manifest file="../../../flutter/shell/platform/android/AndroidManifest.xml" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterPluginRegistry.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterFragmentActivity.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterActivity.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterActivityEvents.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterApplication.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/util/Preconditions.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/util/Predicate.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/util/PathUtils.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/renderer/OnFirstFrameRenderedListener.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/PlatformMessageHandler.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/dart/DartMessenger.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/NavigationChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/LocalizationChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/AccessibilityChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SystemChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/LifecycleChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/AndroidTouchProcessor.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/FlutterActivity.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/FlutterView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/FlutterTextureView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/FlutterFragment.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/FlutterSurfaceView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/android/AndroidKeyProcessor.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRegistry.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/VirtualDisplayController.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRegistryImpl.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewFactory.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/BasicMessageChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/JSONMethodCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/JSONUtil.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/PluginRegistry.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/MessageCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/ErrorLogResult.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/JSONMessageCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/ActivityLifecycleListener.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/BinaryCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/FlutterException.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/StringCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/StandardMethodCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/MethodCodec.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/BinaryMessenger.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/MethodCall.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/EventChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/common/MethodChannel.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/ResourceUpdater.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/FlutterCallbackInformation.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/FlutterMain.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/ResourceExtractor.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/ResourcePaths.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/ResourceCleaner.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArguments.java" /> | ||
| <src file="../../../flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java" /> | ||
| </module> | ||
| </project> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| name: android_lint | ||
| dependencies: | ||
| args: 1.5.0 | ||
| path: ^1.6.2 | ||
| process: ^3.0.9 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.