From ee160424d4328bb998e97f3f8fc5697246cc432a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 10:45:01 -0800 Subject: [PATCH 1/7] Start of linting Android embedding --- tools/android_lint/.gitignore | 1 + tools/android_lint/bin/main.dart | 88 ++++++++++++++++++++++++++++++++ tools/android_lint/project.xml | 81 +++++++++++++++++++++++++++++ tools/android_lint/pubspec.yaml | 5 ++ 4 files changed, 175 insertions(+) create mode 100644 tools/android_lint/.gitignore create mode 100644 tools/android_lint/bin/main.dart create mode 100644 tools/android_lint/project.xml create mode 100644 tools/android_lint/pubspec.yaml diff --git a/tools/android_lint/.gitignore b/tools/android_lint/.gitignore new file mode 100644 index 0000000000000..bf4cd096736bf --- /dev/null +++ b/tools/android_lint/.gitignore @@ -0,0 +1 @@ +lint_report/ \ No newline at end of file diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart new file mode 100644 index 0000000000000..3ef7165d55e1c --- /dev/null +++ b/tools/android_lint/bin/main.dart @@ -0,0 +1,88 @@ +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:path/path.dart' as path; +import 'package:process/process.dart'; + +Future main(List args) async { + final ArgParser argParser = ArgParser(); + argParser.addOption( + 'in', + help: 'The path to `engine/src`.', + defaultsTo: path.join(path.current), + ); + 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(''' + + + + + +'''); + for (final FileSystemEntity entity in androidDir.listSync(recursive: true)) { + if (!entity.path.endsWith('.java')) { + continue; + } + projectXml.writeln(' '); + } + + projectXml.write(''' + +'''); + await projectXml.close(); + + print('Wrote project.xml, starting lint...'); + const LocalProcessManager processManager = LocalProcessManager(); + final ProcessResult result = await processManager.run( + [ + 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 + // errors/warnings. + return; +} diff --git a/tools/android_lint/project.xml b/tools/android_lint/project.xml new file mode 100644 index 0000000000000..6d8fffe439f3e --- /dev/null +++ b/tools/android_lint/project.xml @@ -0,0 +1,81 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tools/android_lint/pubspec.yaml b/tools/android_lint/pubspec.yaml new file mode 100644 index 0000000000000..b27131c37c920 --- /dev/null +++ b/tools/android_lint/pubspec.yaml @@ -0,0 +1,5 @@ +name: android_lint +dependencies: + args: 1.5.0 + path: ^1.6.2 + process: ^3.0.9 From 48a93091697c653e4af290ae7838bc153ee154f5 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 11:12:44 -0800 Subject: [PATCH 2/7] review --- tools/android_lint/.gitignore | 2 +- tools/android_lint/bin/main.dart | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tools/android_lint/.gitignore b/tools/android_lint/.gitignore index bf4cd096736bf..e44752dc2001b 100644 --- a/tools/android_lint/.gitignore +++ b/tools/android_lint/.gitignore @@ -1 +1 @@ -lint_report/ \ No newline at end of file +lint_report/ diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart index 3ef7165d55e1c..9a56087b954e1 100644 --- a/tools/android_lint/bin/main.dart +++ b/tools/android_lint/bin/main.dart @@ -1,3 +1,7 @@ +// 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 'dart:io'; import 'package:args/args.dart'; @@ -9,15 +13,33 @@ Future main(List args) async { argParser.addOption( 'in', help: 'The path to `engine/src`.', - defaultsTo: path.join(path.current), + defaultsTo: path.relative( + path.join( + path.dirname( + path.dirname(path.dirname(path.fromUri(Platform.script))), + ), + '..', + '..', + ), + ), ); argParser.addOption( 'out', help: 'The path to write the generated the HTML report to.', defaultsTo: 'lint_report', ); + argParser.addFlag( + 'help', + help: 'Print usage of the command.', + negatable: false, + defaultsTo: false, + ); final ArgResults argResults = argParser.parse(args); + if (argResults['help']) { + print(argParser.usage); + exit(0); + } final Directory androidDir = Directory(path.join( argResults['in'], 'flutter', From 65e4671cc48b572dc5013c9c3602b40bee6957af Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 11:12:57 -0800 Subject: [PATCH 3/7] bump minSdk for androidmanifest.xml --- shell/platform/android/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/AndroidManifest.xml b/shell/platform/android/AndroidManifest.xml index 2c5c91095d2f2..7e10898269888 100644 --- a/shell/platform/android/AndroidManifest.xml +++ b/shell/platform/android/AndroidManifest.xml @@ -5,7 +5,7 @@ --> - + From f41eac72461c54563a9b831cde8411f3c9730aac Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 11:18:00 -0800 Subject: [PATCH 4/7] doc comment --- tools/android_lint/bin/main.dart | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart index 9a56087b954e1..1ac0ee683ee64 100644 --- a/tools/android_lint/bin/main.dart +++ b/tools/android_lint/bin/main.dart @@ -8,6 +8,16 @@ import 'package:args/args.dart'; import 'package:path/path.dart' as path; import 'package:process/process.dart'; +/// Runs the Android SDK Lint tool on flutter/shell/platform/android. +/// +/// This script scans the flutter/shell/platform/android directory for Java +/// files to build a `project.xml` file. This file is then passed to the lint +/// tool and HTML output is reqeusted in the directory for the `--out` +/// parameter, which defaults to `lint_report`. +/// +/// The `--in` parameter may be specified to to force this script to scan a +/// specific location for the engine repository, and expects to be given the +/// `src` directory that contains both `third_party` and `flutter`. Future main(List args) async { final ArgParser argParser = ArgParser(); argParser.addOption( From 9df4d05e1e8c4b82857a148e9818365d355b246a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 11:28:12 -0800 Subject: [PATCH 5/7] check java version --- tools/android_lint/bin/main.dart | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart index 1ac0ee683ee64..d305a684e192f 100644 --- a/tools/android_lint/bin/main.dart +++ b/tools/android_lint/bin/main.dart @@ -18,6 +18,10 @@ import 'package:process/process.dart'; /// The `--in` parameter may be specified to to force this script to scan a /// specific location for the engine repository, and expects to be given the /// `src` directory that contains both `third_party` and `flutter`. +/// +/// At the time of this writing, the Android Lint tool doesn't work well with +/// Java > 1.8. This script will print a warning if you are not running +/// Java 1.8. Future main(List args) async { final ArgParser argParser = ArgParser(); argParser.addOption( @@ -50,6 +54,23 @@ Future main(List args) async { print(argParser.usage); exit(0); } + + print('Checking Java version...'); + const LocalProcessManager processManager = LocalProcessManager(); + final ProcessResult javaResult = await processManager.run( + ['java', '-version'], + ); + if (javaResult.exitCode != 0) { + print('Could not run "java -version". ' + 'Ensure Java is installed and available on your path.'); + print(javaResult.stderr); + } + final String javaVersionStdout = javaResult.stdout; + 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.'); + } + final Directory androidDir = Directory(path.join( argResults['in'], 'flutter', @@ -76,7 +97,8 @@ Future main(List args) async { } final IOSink projectXml = File('./project.xml').openWrite(); - projectXml.write(''' + projectXml.write( + ''' @@ -96,7 +118,6 @@ Future main(List args) async { await projectXml.close(); print('Wrote project.xml, starting lint...'); - const LocalProcessManager processManager = LocalProcessManager(); final ProcessResult result = await processManager.run( [ path.join(androidSdkDir.path, 'tools', 'bin', 'lint'), From a36b8fc80f9e2e54472706429d886e0785fbf60e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 11:33:34 -0800 Subject: [PATCH 6/7] exit code --- tools/android_lint/bin/main.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart index d305a684e192f..6b4ad92e319cc 100644 --- a/tools/android_lint/bin/main.dart +++ b/tools/android_lint/bin/main.dart @@ -126,16 +126,16 @@ Future main(List args) async { '--html', argResults['out'], '--showall', + '--exitcode', // Set non-zero exit code on errors + '-Wall', + '-Werror', ], ); - if (result.exitCode != 0) { + if (result.stderr != null) { + print('Lint tool had internal errors:'); 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 - // errors/warnings. + exit(result.exitCode); return; } From 68b5212def3db01b32357126eecd8f98c2b33727 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 4 Mar 2019 13:13:41 -0800 Subject: [PATCH 7/7] refactor script to use methods --- tools/android_lint/bin/main.dart | 111 ++++++++++++++++--------------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/tools/android_lint/bin/main.dart b/tools/android_lint/bin/main.dart index 6b4ad92e319cc..4254fd2d20124 100644 --- a/tools/android_lint/bin/main.dart +++ b/tools/android_lint/bin/main.dart @@ -8,6 +8,8 @@ import 'package:args/args.dart'; import 'package:path/path.dart' as path; import 'package:process/process.dart'; +const LocalProcessManager processManager = LocalProcessManager(); + /// Runs the Android SDK Lint tool on flutter/shell/platform/android. /// /// This script scans the flutter/shell/platform/android directory for Java @@ -15,7 +17,7 @@ import 'package:process/process.dart'; /// tool and HTML output is reqeusted in the directory for the `--out` /// parameter, which defaults to `lint_report`. /// -/// The `--in` parameter may be specified to to force this script to scan a +/// The `--in` parameter may be specified to force this script to scan a /// specific location for the engine repository, and expects to be given the /// `src` directory that contains both `third_party` and `flutter`. /// @@ -23,54 +25,13 @@ import 'package:process/process.dart'; /// Java > 1.8. This script will print a warning if you are not running /// Java 1.8. Future main(List args) async { - final ArgParser argParser = ArgParser(); - argParser.addOption( - 'in', - help: 'The path to `engine/src`.', - defaultsTo: path.relative( - path.join( - path.dirname( - path.dirname(path.dirname(path.fromUri(Platform.script))), - ), - '..', - '..', - ), - ), - ); - argParser.addOption( - 'out', - help: 'The path to write the generated the HTML report to.', - defaultsTo: 'lint_report', - ); - argParser.addFlag( - 'help', - help: 'Print usage of the command.', - negatable: false, - defaultsTo: false, - ); - - final ArgResults argResults = argParser.parse(args); - if (argResults['help']) { - print(argParser.usage); - exit(0); - } - - print('Checking Java version...'); - const LocalProcessManager processManager = LocalProcessManager(); - final ProcessResult javaResult = await processManager.run( - ['java', '-version'], - ); - if (javaResult.exitCode != 0) { - print('Could not run "java -version". ' - 'Ensure Java is installed and available on your path.'); - print(javaResult.stderr); - } - final String javaVersionStdout = javaResult.stdout; - 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.'); - } + final ArgParser argParser = setupOptions(); + await checkJava1_8(); + final int exitCode = await runLint(argParser, argParser.parse(args)); + exit(exitCode); +} +Future runLint(ArgParser argParser, ArgResults argResults) async { final Directory androidDir = Directory(path.join( argResults['in'], 'flutter', @@ -82,7 +43,7 @@ Future main(List args) async { 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); + return -1; } final Directory androidSdkDir = Directory( @@ -93,7 +54,7 @@ Future main(List args) async { 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); + return -1; } final IOSink projectXml = File('./project.xml').openWrite(); @@ -136,6 +97,52 @@ Future main(List args) async { print(result.stderr); } print(result.stdout); - exit(result.exitCode); - return; + return result.exitCode; +} + +ArgParser setupOptions() { + final ArgParser argParser = ArgParser(); + argParser.addOption( + 'in', + help: 'The path to `engine/src`.', + defaultsTo: path.relative( + path.join( + path.dirname( + path.dirname(path.dirname(path.fromUri(Platform.script))), + ), + '..', + '..', + ), + ), + ); + argParser.addOption( + 'out', + help: 'The path to write the generated the HTML report to.', + defaultsTo: 'lint_report', + ); + argParser.addFlag( + 'help', + help: 'Print usage of the command.', + negatable: false, + defaultsTo: false, + ); + + return argParser; +} + +Future checkJava1_8() async { + print('Checking Java version...'); + final ProcessResult javaResult = await processManager.run( + ['java', '-version'], + ); + if (javaResult.exitCode != 0) { + print('Could not run "java -version". ' + 'Ensure Java is installed and available on your path.'); + print(javaResult.stderr); + } + final String javaVersionStdout = javaResult.stdout; + 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.'); + } }