From 35640af9fae5d216cf0cdd12a86fe25bcb7ce338 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 18 Aug 2020 13:25:30 -0700 Subject: [PATCH] Convert format script to Dart (#20572) This converts the ci/format.sh script to a Dart script that uses process_runner and isolates to multi-process the clang-format, diffs, and grepping needed to do the formatting changes. It also will (by default) only check the formatting of changed files. The user can optionally check all files (--all-files) or do only some types of checks with --check. --verbose prints the versions of the tools used for Clang format and Java format. Specifying --fix will cause any formatting errors that would have been detected to be fixed. --- build/generate_coverage.py | 8 +- ci/bin/format.dart | 936 ++++++++++++++++++++++ ci/check_gn_format.py | 49 -- ci/format.bat | 32 + ci/format.sh | 236 +----- ci/pubspec.yaml | 1 + testing/scenario_app/android/build.gradle | 3 +- 7 files changed, 985 insertions(+), 280 deletions(-) create mode 100644 ci/bin/format.dart create mode 100644 ci/format.bat diff --git a/build/generate_coverage.py b/build/generate_coverage.py index fb6ad9d74..6286d30a4 100755 --- a/build/generate_coverage.py +++ b/build/generate_coverage.py @@ -42,7 +42,7 @@ def RemoveIfExists(path): def main(): parser = argparse.ArgumentParser(); - + parser.add_argument('-t', '--tests', nargs='+', dest='tests', required=True, help='The unit tests to run and gather coverage data on.') parser.add_argument('-o', '--output', dest='output', @@ -64,19 +64,19 @@ def main(): # Run all unit tests and collect raw profiles. for test in args.tests: absolute_test_path = os.path.abspath(test) - + if not os.path.exists(absolute_test_path): print("Path %s does not exist." % absolute_test_path) return -1 binaries.append(absolute_test_path) - + raw_profile = absolute_test_path + ".rawprofile" RemoveIfExists(raw_profile) print "Running test %s to gather profile." % os.path.basename(absolute_test_path) - + subprocess.check_call([absolute_test_path], env={ "LLVM_PROFILE_FILE": raw_profile }) diff --git a/ci/bin/format.dart b/ci/bin/format.dart new file mode 100644 index 000000000..1f96409d4 --- /dev/null +++ b/ci/bin/format.dart @@ -0,0 +1,936 @@ +// 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. + +// Checks and fixes format on files with changes. +// +// Run with --help for usage. + +// TODO(gspencergoog): Support clang formatting on Windows. +// TODO(gspencergoog): Support Java formatting on Windows. +// TODO(gspencergoog): Convert to null safety. + +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:isolate/isolate.dart'; +import 'package:meta/meta.dart'; +import 'package:path/path.dart' as path; +import 'package:process_runner/process_runner.dart'; +import 'package:process/process.dart'; + +class FormattingException implements Exception { + FormattingException(this.message, [this.result]); + + final String message; + final ProcessResult /*?*/ result; + + int get exitCode => result?.exitCode ?? -1; + + @override + String toString() { + final StringBuffer output = StringBuffer(runtimeType.toString()); + output.write(': $message'); + final String stderr = result?.stderr as String ?? ''; + if (stderr.isNotEmpty) { + output.write(':\n$stderr'); + } + return output.toString(); + } +} + +enum MessageType { + message, + error, + warning, +} + +enum FormatCheck { + clang, + java, + whitespace, + gn, +} + +FormatCheck nameToFormatCheck(String name) { + switch (name) { + case 'clang': + return FormatCheck.clang; + case 'java': + return FormatCheck.java; + case 'whitespace': + return FormatCheck.whitespace; + case 'gn': + return FormatCheck.gn; + } + assert(false, 'Unknown FormatCheck type $name'); + return null; +} + +String formatCheckToName(FormatCheck check) { + switch (check) { + case FormatCheck.clang: + return 'C++/ObjC'; + case FormatCheck.java: + return 'Java'; + case FormatCheck.whitespace: + return 'Trailing whitespace'; + case FormatCheck.gn: + return 'GN'; + } + assert(false, 'Unhandled FormatCheck type $check'); + return null; +} + +List formatCheckNames() { + List allowed; + if (!Platform.isWindows) { + allowed = FormatCheck.values; + } else { + allowed = [FormatCheck.gn, FormatCheck.whitespace]; + } + return allowed + .map((FormatCheck check) => check.toString().replaceFirst('$FormatCheck.', '')) + .toList(); +} + +Future _runGit( + List args, + ProcessRunner processRunner, { + bool failOk = false, +}) async { + final ProcessRunnerResult result = await processRunner.runProcess( + ['git', ...args], + failOk: failOk, + ); + return result.stdout; +} + +typedef MessageCallback = Function(String message, {MessageType type}); + +/// Base class for format checkers. +/// +/// Provides services that all format checkers need. +abstract class FormatChecker { + FormatChecker({ + ProcessManager /*?*/ processManager, + @required this.baseGitRef, + @required this.repoDir, + @required this.srcDir, + this.allFiles = false, + this.messageCallback, + }) : _processRunner = ProcessRunner( + defaultWorkingDirectory: repoDir, + processManager: processManager ?? const LocalProcessManager(), + ); + + /// Factory method that creates subclass format checkers based on the type of check. + factory FormatChecker.ofType( + FormatCheck check, { + ProcessManager /*?*/ processManager, + @required String baseGitRef, + @required Directory repoDir, + @required Directory srcDir, + bool allFiles = false, + MessageCallback messageCallback, + }) { + switch (check) { + case FormatCheck.clang: + return ClangFormatChecker( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); + break; + case FormatCheck.java: + return JavaFormatChecker( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); + break; + case FormatCheck.whitespace: + return WhitespaceFormatChecker( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); + break; + case FormatCheck.gn: + return GnFormatChecker( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); + break; + } + assert(false, 'Unhandled FormatCheck type $check'); + return null; + } + + final ProcessRunner _processRunner; + final Directory srcDir; + final Directory repoDir; + final bool allFiles; + MessageCallback /*?*/ messageCallback; + final String baseGitRef; + + /// Override to provide format checking for a specific type. + Future checkFormatting(); + + /// Override to provide format fixing for a specific type. + Future fixFormatting(); + + @protected + void message(String string) => messageCallback?.call(string, type: MessageType.message); + + @protected + void error(String string) => messageCallback?.call(string, type: MessageType.error); + + @protected + Future runGit(List args) async => _runGit(args, _processRunner); + + /// Converts a given raw string of code units to a stream that yields those + /// code units. + /// + /// Uses to convert the stdout of a previous command into an input stream for + /// the next command. + @protected + Stream> codeUnitsAsStream(List input) async* { + yield input; + } + + @protected + Future applyPatch(List patches) async { + final ProcessPool patchPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('patch'), + ); + final List jobs = patches.map((String patch) { + return WorkerJob( + ['patch', '-p0'], + stdinRaw: codeUnitsAsStream(patch.codeUnits), + failOk: true, + ); + }).toList(); + final List completedJobs = await patchPool.runToCompletion(jobs); + if (patchPool.failedJobs != 0) { + error('${patchPool.failedJobs} patch${patchPool.failedJobs > 1 ? 'es' : ''} ' + 'failed to apply.'); + completedJobs + .where((WorkerJob job) => job.result.exitCode != 0) + .map((WorkerJob job) => job.result.output) + .forEach(message); + } + return patchPool.failedJobs == 0; + } + + /// Gets the list of files to operate on. + /// + /// If [allFiles] is true, then returns all git controlled files in the repo + /// of the given types. + /// + /// If [allFiles] is false, then only return those files of the given types + /// that have changed between the current working tree and the [baseGitRef]. + @protected + Future> getFileList(List types) async { + String output; + if (allFiles) { + output = await runGit([ + 'ls-files', + '--', + ...types, + ]); + } else { + output = await runGit([ + 'diff', + '-U0', + '--no-color', + '--name-only', + baseGitRef, + '--', + ...types, + ]); + } + return output.split('\n').where((String line) => line.isNotEmpty).toList(); + } + + /// Generates a reporting function to supply to ProcessRunner to use instead + /// of the default reporting function. + @protected + ProcessPoolProgressReporter namedReport(String name) { + return (int total, int completed, int inProgress, int pending, int failed) { + final String percent = + total == 0 ? '100' : ((100 * completed) ~/ total).toString().padLeft(3); + final String completedStr = completed.toString().padLeft(3); + final String totalStr = total.toString().padRight(3); + final String inProgressStr = inProgress.toString().padLeft(2); + final String pendingStr = pending.toString().padLeft(3); + final String failedStr = failed.toString().padLeft(3); + + stderr.write('$name Jobs: $percent% done, ' + '$completedStr/$totalStr completed, ' + '$inProgressStr in progress, ' + '$pendingStr pending, ' + '$failedStr failed.${' ' * 20}\r'); + }; + } + + /// Clears the last printed report line so garbage isn't left on the terminal. + @protected + void reportDone() { + stderr.write('\r${' ' * 100}\r'); + } +} + +/// Checks and formats C++/ObjC files using clang-format. +class ClangFormatChecker extends FormatChecker { + ClangFormatChecker({ + ProcessManager /*?*/ processManager, + @required String baseGitRef, + @required Directory repoDir, + @required Directory srcDir, + bool allFiles = false, + MessageCallback messageCallback, + }) : super( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ) { + /*late*/ String clangOs; + if (Platform.isLinux) { + clangOs = 'linux-x64'; + } else if (Platform.isMacOS) { + clangOs = 'mac-x64'; + } else { + throw FormattingException( + "Unknown operating system: don't know how to run clang-format here."); + } + clangFormat = File( + path.join( + srcDir.absolute.path, + 'buildtools', + clangOs, + 'clang', + 'bin', + 'clang-format', + ), + ); + } + + /*late*/ File clangFormat; + + @override + Future checkFormatting() async { + final List failures = await _getCFormatFailures(); + failures.map(stdout.writeln); + return failures.isEmpty; + } + + @override + Future fixFormatting() async { + message('Fixing C++/ObjC formatting...'); + final List failures = await _getCFormatFailures(fixing: true); + if (failures.isEmpty) { + return true; + } + return await applyPatch(failures); + } + + Future _getClangFormatVersion() async { + final ProcessRunnerResult result = + await _processRunner.runProcess([clangFormat.path, '--version']); + return result.stdout.trim(); + } + + Future> _getCFormatFailures({bool fixing = false}) async { + message('Checking C++/ObjC formatting...'); + const List clangFiletypes = [ + '*.c', + '*.cc', + '*.cxx', + '*.cpp', + '*.h', + '*.m', + '*.mm', + ]; + final List files = await getFileList(clangFiletypes); + if (files.isEmpty) { + message('No C++/ObjC files with changes, skipping C++/ObjC format check.'); + return []; + } + if (verbose) { + message('Using ${await _getClangFormatVersion()}'); + } + final List clangJobs = []; + for (String file in files) { + if (file.trim().isEmpty) { + continue; + } + clangJobs.add(WorkerJob([clangFormat.path, '--style=file', file.trim()])); + } + final ProcessPool clangPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('clang-format'), + ); + final Stream completedClangFormats = clangPool.startWorkers(clangJobs); + final List diffJobs = []; + await for (final WorkerJob completedJob in completedClangFormats) { + if (completedJob.result != null && completedJob.result.exitCode == 0) { + diffJobs.add( + WorkerJob(['diff', '-u', completedJob.command.last, '-'], + stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw), failOk: true), + ); + } + } + final ProcessPool diffPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('diff'), + ); + final List completedDiffs = await diffPool.runToCompletion(diffJobs); + final Iterable failed = completedDiffs.where((WorkerJob job) { + return job.result.exitCode != 0; + }); + reportDone(); + if (failed.isNotEmpty) { + final bool plural = failed.length > 1; + if (fixing) { + message('Fixing ${failed.length} C++/ObjC file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + } else { + error('Found ${failed.length} C++/ObjC file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + for (final WorkerJob job in failed) { + stdout.write(job.result.stdout); + } + } + } else { + message('Completed checking ${diffJobs.length} C++/ObjC files with no formatting problems.'); + } + return failed.map((WorkerJob job) { + return job.result.stdout; + }).toList(); + } +} + +/// Checks the format of Java files uing the Google Java format checker. +class JavaFormatChecker extends FormatChecker { + JavaFormatChecker({ + ProcessManager /*?*/ processManager, + @required String baseGitRef, + @required Directory repoDir, + @required Directory srcDir, + bool allFiles = false, + MessageCallback messageCallback, + }) : super( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ) { + googleJavaFormatJar = File( + path.absolute( + path.join( + srcDir.absolute.path, + 'third_party', + 'android_tools', + 'google-java-format', + 'google-java-format-1.7-all-deps.jar', + ), + ), + ); + } + + /*late*/ File googleJavaFormatJar; + + Future _getGoogleJavaFormatVersion() async { + final ProcessRunnerResult result = await _processRunner + .runProcess(['java', '-jar', googleJavaFormatJar.path, '--version']); + return result.stderr.trim(); + } + + @override + Future checkFormatting() async { + final List failures = await _getJavaFormatFailures(); + failures.map(stdout.writeln); + return failures.isEmpty; + } + + @override + Future fixFormatting() async { + message('Fixing Java formatting...'); + final List failures = await _getJavaFormatFailures(fixing: true); + if (failures.isEmpty) { + return true; + } + return await applyPatch(failures); + } + + Future _getJavaVersion() async { + final ProcessRunnerResult result = + await _processRunner.runProcess(['java', '-version']); + return result.stderr.trim().split('\n')[0]; + } + + Future> _getJavaFormatFailures({bool fixing = false}) async { + message('Checking Java formatting...'); + final List formatJobs = []; + final List files = await getFileList(['*.java']); + if (files.isEmpty) { + message('No Java files with changes, skipping Java format check.'); + return []; + } + String javaVersion = ''; + String javaFormatVersion = ''; + try { + javaVersion = await _getJavaVersion(); + } on ProcessRunnerException { + error('Cannot run Java, skipping Java file formatting!'); + return const []; + } + try { + javaFormatVersion = await _getGoogleJavaFormatVersion(); + } on ProcessRunnerException { + error('Cannot find google-java-format, skipping Java format check.'); + return const []; + } + if (verbose) { + message('Using $javaFormatVersion with Java $javaVersion'); + } + for (String file in files) { + if (file.trim().isEmpty) { + continue; + } + formatJobs.add( + WorkerJob( + ['java', '-jar', googleJavaFormatJar.path, file.trim()], + ), + ); + } + final ProcessPool formatPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('Java format'), + ); + final Stream completedClangFormats = formatPool.startWorkers(formatJobs); + final List diffJobs = []; + await for (final WorkerJob completedJob in completedClangFormats) { + if (completedJob.result != null && completedJob.result.exitCode == 0) { + diffJobs.add( + WorkerJob( + ['diff', '-u', completedJob.command.last, '-'], + stdinRaw: codeUnitsAsStream(completedJob.result.stdoutRaw), + failOk: true, + ), + ); + } + } + final ProcessPool diffPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('diff'), + ); + final List completedDiffs = await diffPool.runToCompletion(diffJobs); + final Iterable failed = completedDiffs.where((WorkerJob job) { + return job.result.exitCode != 0; + }); + reportDone(); + if (failed.isNotEmpty) { + final bool plural = failed.length > 1; + if (fixing) { + error('Fixing ${failed.length} Java file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + } else { + error('Found ${failed.length} Java file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + for (final WorkerJob job in failed) { + stdout.write(job.result.stdout); + } + } + } else { + message('Completed checking ${diffJobs.length} Java files with no formatting problems.'); + } + return failed.map((WorkerJob job) { + return job.result.stdout; + }).toList(); + } +} + +/// Checks the format of any BUILD.gn files using the "gn format" command. +class GnFormatChecker extends FormatChecker { + GnFormatChecker({ + ProcessManager /*?*/ processManager, + @required String baseGitRef, + @required Directory repoDir, + @required Directory srcDir, + bool allFiles = false, + MessageCallback messageCallback, + }) : super( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ) { + gnBinary = File( + path.join( + repoDir.absolute.path, + 'third_party', + 'gn', + Platform.isWindows ? 'gn.exe' : 'gn', + ), + ); + } + + /*late*/ File gnBinary; + + @override + Future checkFormatting() async { + message('Checking GN formatting...'); + return (await _runGnCheck(fixing: false)) == 0; + } + + @override + Future fixFormatting() async { + message('Fixing GN formatting...'); + await _runGnCheck(fixing: true); + // The GN script shouldn't fail when fixing errors. + return true; + } + + Future _runGnCheck({@required bool fixing}) async { + final List filesToCheck = await getFileList(['*.gn', '*.gni']); + + final List cmd = [ + gnBinary.path, + 'format', + if (!fixing) '--dry-run', + ]; + final List jobs = []; + for (final String file in filesToCheck) { + jobs.add(WorkerJob([...cmd, file])); + } + final ProcessPool gnPool = ProcessPool( + processRunner: _processRunner, + printReport: namedReport('gn format'), + ); + final List completedJobs = await gnPool.runToCompletion(jobs); + reportDone(); + final List incorrect = []; + for (final WorkerJob job in completedJobs) { + if (job.result.exitCode == 2) { + incorrect.add(' ${job.command.last}'); + } + if (job.result.exitCode == 1) { + // GN has exit code 1 if it had some problem formatting/checking the + // file. + throw FormattingException( + 'Unable to format ${job.command.last}:\n${job.result.output}', + ); + } + } + if (incorrect.isNotEmpty) { + final bool plural = incorrect.length > 1; + if (fixing) { + message('Fixed ${incorrect.length} GN file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly.'); + } else { + error('Found ${incorrect.length} GN file${plural ? 's' : ''}' + ' which ${plural ? 'were' : 'was'} formatted incorrectly:'); + incorrect.forEach(stderr.writeln); + } + } else { + message('All GN files formatted correctly.'); + } + return incorrect.length; + } +} + +@immutable +class _GrepResult { + const _GrepResult(this.file, this.hits, this.lineNumbers); + final File file; + final List hits; + final List lineNumbers; +} + +/// Checks for trailing whitspace in Dart files. +class WhitespaceFormatChecker extends FormatChecker { + WhitespaceFormatChecker({ + ProcessManager /*?*/ processManager, + @required String baseGitRef, + @required Directory repoDir, + @required Directory srcDir, + bool allFiles = false, + MessageCallback messageCallback, + }) : super( + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: allFiles, + messageCallback: messageCallback, + ); + + @override + Future checkFormatting() async { + final List failures = await _getWhitespaceFailures(); + return failures.isEmpty; + } + + static final RegExp trailingWsRegEx = RegExp(r'[ \t]+$', multiLine: true); + + @override + Future fixFormatting() async { + final List failures = await _getWhitespaceFailures(); + if (failures.isNotEmpty) { + for (File file in failures) { + stderr.writeln('Fixing $file'); + String contents = file.readAsStringSync(); + contents = contents.replaceAll(trailingWsRegEx, ''); + file.writeAsStringSync(contents); + } + } + return true; + } + + static Future<_GrepResult> _hasTrailingWhitespace(File file) async { + final List hits = []; + final List lineNumbers = []; + int lineNumber = 0; + for (final String line in file.readAsLinesSync()) { + if (trailingWsRegEx.hasMatch(line)) { + hits.add(line); + lineNumbers.add(lineNumber); + } + lineNumber++; + } + if (hits.isEmpty) { + return null; + } + return _GrepResult(file, hits, lineNumbers); + } + + Stream<_GrepResult> _whereHasTrailingWhitespace(Iterable files) async* { + final LoadBalancer pool = + await LoadBalancer.create(Platform.numberOfProcessors, IsolateRunner.spawn); + for (final File file in files) { + yield await pool.run<_GrepResult, File>(_hasTrailingWhitespace, file); + } + } + + Future> _getWhitespaceFailures() async { + final List files = await getFileList([ + '*.c', + '*.cc', + '*.cpp', + '*.cxx', + '*.dart', + '*.gn', + '*.gni', + '*.gradle', + '*.h', + '*.java', + '*.json', + '*.m', + '*.mm', + '*.py', + '*.sh', + '*.yaml', + ]); + if (files.isEmpty) { + message('No files that differ, skipping whitespace check.'); + return []; + } + message('Checking for trailing whitespace on ${files.length} source ' + 'file${files.length > 1 ? 's' : ''}...'); + + final ProcessPoolProgressReporter reporter = namedReport('whitespace'); + final List<_GrepResult> found = <_GrepResult>[]; + final int total = files.length; + int completed = 0; + int inProgress = Platform.numberOfProcessors; + int pending = total; + int failed = 0; + await for (final _GrepResult result in _whereHasTrailingWhitespace( + files.map( + (String file) => File( + path.join(repoDir.absolute.path, file), + ), + ), + )) { + if (result == null) { + completed++; + } else { + failed++; + found.add(result); + } + pending--; + inProgress = pending < Platform.numberOfProcessors ? pending : Platform.numberOfProcessors; + reporter(total, completed, inProgress, pending, failed); + } + reportDone(); + if (found.isNotEmpty) { + error('Whitespace check failed. The following files have trailing spaces:'); + for (final _GrepResult result in found) { + for (int i = 0; i < result.hits.length; ++i) { + message(' ${result.file.path}:${result.lineNumbers[i]}:${result.hits[i]}'); + } + } + } else { + message('No trailing whitespace found.'); + } + return found.map((_GrepResult result) => result.file).toList(); + } +} + +Future _getDiffBaseRevision(ProcessManager processManager, Directory repoDir) async { + final ProcessRunner processRunner = ProcessRunner( + defaultWorkingDirectory: repoDir, + processManager: processManager ?? const LocalProcessManager(), + ); + String upstream = 'upstream'; + final String upstreamUrl = await _runGit( + ['remote', 'get-url', upstream], + processRunner, + failOk: true, + ); + if (upstreamUrl.isEmpty) { + upstream = 'origin'; + } + await _runGit(['fetch', upstream, 'master'], processRunner); + String result = ''; + try { + // This is the preferred command to use, but developer checkouts often do + // not have a clear fork point, so we fall back to just the regular + // merge-base in that case. + result = await _runGit( + ['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], + processRunner, + ); + } on ProcessRunnerException { + result = await _runGit(['merge-base', 'FETCH_HEAD', 'HEAD'], processRunner); + } + return result.trim(); +} + +void _usage(ArgParser parser, {int exitCode = 1}) { + stderr.writeln('format.dart [--help] [--fix] [--all-files] ' + '[--check <${formatCheckNames().join('|')}>]'); + stderr.writeln(parser.usage); + exit(exitCode); +} + +bool verbose = false; + +Future main(List arguments) async { + final ArgParser parser = ArgParser(); + parser.addFlag('help', help: 'Print help.', abbr: 'h'); + parser.addFlag('fix', + abbr: 'f', + help: 'Instead of just checking for formatting errors, fix them in place.', + defaultsTo: false); + parser.addFlag('all-files', + abbr: 'a', + help: 'Instead of just checking for formatting errors in changed files, ' + 'check for them in all files.', + defaultsTo: false); + parser.addMultiOption('check', + abbr: 'c', + allowed: formatCheckNames(), + defaultsTo: formatCheckNames(), + help: 'Specifies which checks will be performed. Defaults to all checks. ' + 'May be specified more than once to perform multiple types of checks. ' + 'On Windows, only whitespace and gn checks are currently supported.'); + parser.addFlag('verbose', help: 'Print verbose output.', defaultsTo: verbose); + + ArgResults options; + try { + options = parser.parse(arguments); + } on FormatException catch (e) { + stderr.writeln('ERROR: $e'); + _usage(parser, exitCode: 0); + } + + verbose = options['verbose'] as bool; + + if (options['help'] as bool) { + _usage(parser, exitCode: 0); + } + + final File script = File.fromUri(Platform.script).absolute; + final Directory repoDir = script.parent.parent.parent; + final Directory srcDir = repoDir.parent; + if (verbose) { + stderr.writeln('Repo: $repoDir'); + stderr.writeln('Src: $srcDir'); + } + + void message(String message, {MessageType type = MessageType.message}) { + switch (type) { + case MessageType.message: + stderr.writeln(message); + break; + case MessageType.error: + stderr.writeln('ERROR: $message'); + break; + case MessageType.warning: + stderr.writeln('WARNING: $message'); + break; + } + } + + const ProcessManager processManager = LocalProcessManager(); + final String baseGitRef = await _getDiffBaseRevision(processManager, repoDir); + + bool result = true; + final List checks = options['check'] as List; + try { + for (final String checkName in checks) { + final FormatCheck check = nameToFormatCheck(checkName); + final String humanCheckName = formatCheckToName(check); + final FormatChecker checker = FormatChecker.ofType(check, + processManager: processManager, + baseGitRef: baseGitRef, + repoDir: repoDir, + srcDir: srcDir, + allFiles: options['all-files'] as bool, + messageCallback: message); + bool stepResult; + if (options['fix'] as bool) { + message('Fixing any $humanCheckName format problems'); + stepResult = await checker.fixFormatting(); + if (!stepResult) { + message('Unable to apply $humanCheckName format fixes.'); + } + } else { + message('Performing $humanCheckName format check'); + stepResult = await checker.checkFormatting(); + if (!stepResult) { + message('Found $humanCheckName format problems.'); + } + } + result = result && stepResult; + } + } on FormattingException catch (e) { + message('ERROR: $e', type: MessageType.error); + } + + exit(result ? 0 : 1); +} diff --git a/ci/check_gn_format.py b/ci/check_gn_format.py index 8867849cd..e69de29bb 100755 --- a/ci/check_gn_format.py +++ b/ci/check_gn_format.py @@ -1,49 +0,0 @@ -#!/usr/bin/env python -# -# 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 sys -import subprocess -import os -import argparse - -def GetGNFiles(directory): - directory = os.path.abspath(directory) - gn_files = [] - assert os.path.exists(directory), "Directory must exist %s" % directory - for root, dirs, files in os.walk(directory): - for file in files: - if file.endswith(".gn") or file.endswith(".gni"): - gn_files.append(os.path.join(root, file)) - return gn_files - -def main(): - parser = argparse.ArgumentParser(); - - parser.add_argument('--gn-binary', dest='gn_binary', required=True, type=str) - parser.add_argument('--dry-run', dest='dry_run', default=False, action='store_true') - parser.add_argument('--root-directory', dest='root_directory', required=True, type=str) - - args = parser.parse_args() - - gn_binary = os.path.abspath(args.gn_binary) - assert os.path.exists(gn_binary), "GN Binary must exist %s" % gn_binary - - gn_command = [ gn_binary, 'format'] - - if args.dry_run: - gn_command.append('--dry-run') - - for gn_file in GetGNFiles(args.root_directory): - if subprocess.call(gn_command + [ gn_file ]) != 0: - print "ERROR: '%s' is incorrectly formatted." % os.path.relpath(gn_file, args.root_directory) - print "Format the same with 'gn format' using the 'gn' binary in third_party/gn/gn." - print "Or, run ./ci/check_gn_format.py without '--dry-run'" - return 1 - - return 0 - -if __name__ == '__main__': - sys.exit(main()) diff --git a/ci/format.bat b/ci/format.bat new file mode 100644 index 000000000..12a6bb8d7 --- /dev/null +++ b/ci/format.bat @@ -0,0 +1,32 @@ +@ECHO off +REM Copyright 2013 The Flutter Authors. All rights reserved. +REM Use of this source code is governed by a BSD-style license that can be +REM found in the LICENSE file. + +REM ---------------------------------- NOTE ---------------------------------- +REM +REM Please keep the logic in this file consistent with the logic in the +REM `format.sh` script in the same directory to ensure that it continues to +REM work across all platforms! +REM +REM -------------------------------------------------------------------------- + +SETLOCAL ENABLEDELAYEDEXPANSION + +FOR %%i IN ("%~dp0..\..") DO SET SRC_DIR=%%~fi + +REM Test if Git is available on the Host +where /q git || ECHO Error: Unable to find git in your PATH. && EXIT /B 1 + +SET repo_dir=%SRC_DIR%\flutter +SET ci_dir=%repo_dir%\flutter\ci +SET dart_sdk_path=%SRC_DIR%\third_party\dart\tools\sdks\dart-sdk +SET dart=%dart_sdk_path%\bin\dart.exe +SET pub=%dart_sdk_path%\bin\pub.bat + +cd "%ci_dir%" + +REM Do not use the CALL command in the next line to execute Dart. CALL causes +REM Windows to re-read the line from disk after the CALL command has finished +REM regardless of the ampersand chain. +"%pub%" get & "%dart%" --disable-dart-dev bin\format.dart %* & exit /B !ERRORLEVEL! diff --git a/ci/format.sh b/ci/format.sh index 18e327921..672563558 100755 --- a/ci/format.sh +++ b/ci/format.sh @@ -4,14 +4,6 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -# Code formatting presubmit script. -# -# This presubmit script ensures that code under the src/flutter directory is -# formatted according to the Flutter engine style requirements. -# -# If failures are found, they can be fixed by re-running the script with the -# --fix option. - set -e # Needed because if it is set, cd may print the path it changed to. @@ -35,221 +27,15 @@ function follow_links() ( echo "$file" ) -SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")") -SRC_DIR="$( - cd "$SCRIPT_DIR/../.." - pwd -P -)" -FLUTTER_DIR="$SRC_DIR/flutter" - -function message() { - echo "$*" 1>&2 -} - -function error() { - echo "ERROR: $*" 1>&2 -} - -function warning() { - echo "WARNING: $*" 1>&2 -} - -function get_base_sha() ( - local upstream - if git remote get-url upstream >/dev/null 2>&1; then - upstream=upstream - else - upstream=origin - fi - - cd "$FLUTTER_DIR" - git fetch "$upstream" master >/dev/null 2>&1 - git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD -) - -function check_clang_format() ( - cd "$FLUTTER_DIR" - message "Checking C++/ObjC formatting..." - - case "$(uname -s)" in - Darwin) - OS="mac-x64" - ;; - Linux) - OS="linux-x64" - ;; - *) - error "Unknown operating system." - return 255 - ;; - esac - - # Tools - local clang_format="$SRC_DIR/buildtools/$OS/clang/bin/clang-format" - "$clang_format" --version 1>&2 - - local current_diff - local clang_files_to_check - - # Compute the diffs. - local clang_filetypes=("*.c" "*.cc" "*.cpp" "*.h" "*.m" "*.mm") - clang_files_to_check="$(git ls-files "${clang_filetypes[@]}")" - local failed_clang_checks=0 - for file in $clang_files_to_check; do - set +e - current_diff="$(diff -u "$file" <("$clang_format" --style=file "$file"))" - set -e - if [[ -n "$current_diff" ]]; then - echo "$current_diff" - failed_clang_checks=$((failed_clang_checks + 1)) - fi - done - - if [[ $failed_clang_checks -ne 0 ]]; then - error "$failed_clang_checks C++/ObjC files are formatted incorrectly." - fi - return $failed_clang_checks -) - -function check_java_format() ( - cd "$FLUTTER_DIR" - local diff_opts=("-U0" "--no-color" "--name-only") - message "Checking Java formatting..." - local google_java_format="$SRC_DIR/third_party/android_tools/google-java-format/google-java-format-1.7-all-deps.jar" - local failed_java_checks=0 - if [[ -f "$google_java_format" && -n "$(command -v java)" ]]; then - java -jar "$google_java_format" --version 1>&2 - local java_filetypes=("*.java") - local java_files_to_check - java_files_to_check="$(git diff "${diff_opts[@]}" "$BASE_SHA" -- "${java_filetypes[@]}")" - for file in $java_files_to_check; do - set +e - current_diff="$(diff -u "$file" <(java -jar "$google_java_format" "$file"))" - set -e - if [[ -n "$current_diff" ]]; then - echo "$current_diff" - failed_java_checks=$((failed_java_checks + 1)) - fi - done - if [[ $failed_java_checks -ne 0 ]]; then - error "$failed_java_checks Java files are formatted incorrectly." - fi - else - warning "Cannot find google-java-format, skipping Java file formatting!" - fi - return $failed_java_checks -) -# Strips off the "advice" at the end, since this script has different advice. -function do_gn_check() { - local output - output="$("$SCRIPT_DIR/check_gn_format.py" --dry-run --root-directory . --gn-binary "third_party/gn/gn")" - local result=$? - echo "$output" | grep "ERROR" - return $result -} - -function check_gn_format() ( - cd "$FLUTTER_DIR" - message "Checking GN formatting..." - if ! do_gn_check 1>&2; then - error "The gn file format check failed." - return 1 - fi -) - -function check_whitespace() ( - local diff_opts=("-U0" "--no-color" "--name-only") - local filetypes="*.dart" - local trailing_spaces - - message "Checking for trailing whitespace in $filetypes files..." 1>&2 - set +e - trailing_spaces=$(git diff "${diff_opts[@]}" "$BASE_SHA" -- "$filetypes" | xargs grep --line-number --with-filename '[[:blank:]]\+$') - set -e - - if [[ -n "$trailing_spaces" ]]; then - message "$trailing_spaces" - error "Whitespace check failed. The above files have trailing spaces." - return 1 - fi -) - -function fix_clang_format() { - local tmpfile - tmpfile=$(mktemp "fix_clang_format.XXXXXX") - if check_clang_format >"$tmpfile"; then - message "No C++/ObjC formatting issues found." - else - message "Fixing C++/ObjC formatting issues." - ( - cd "$SRC_DIR/flutter" - patch -p0 <"$tmpfile" - ) - fi - command rm -f "$tmpfile" -} - -function fix_java_format() { - local tmpfile - tmpfile=$(mktemp "fix_java_format.XXXXXX") - if check_java_format >"$tmpfile"; then - message "No Java formatting issues found." - else - message "Fixing Java formatting issues." - ( - cd "$SRC_DIR/flutter" - patch -p0 <"$tmpfile" - ) - fi - command rm -f "$tmpfile" -} - -function fix_gn_format() ( - cd "$FLUTTER_DIR" - message "Fixing GN formatting..." - # Check GN format consistency and fix it. - if ! "$SCRIPT_DIR/check_gn_format.py" --root-directory . --gn-binary "third_party/gn/gn"; then - error "The GN file format fix failed." - return 1 - fi -) - -function fix_whitespace() { - if ! check_whitespace; then - message "Fixing trailing whitespace problems." - find "$FLUTTER_DIR" -type f -name "*.dart" -print0 | xargs -0 sed -i -e 's/\s\+$//' - fi -} - -BASE_SHA=$(get_base_sha) -message "Checking formatting for files with differences from $BASE_SHA in $FLUTTER_DIR" - -if [[ $1 == "--fix" ]]; then - fix_clang_format - fix_java_format - fix_gn_format - fix_whitespace - message "Formatting fixed." -else - failures=0 - if ! check_clang_format; then - failures=$((failures + 1)) - fi - if ! check_java_format; then - failures=$((failures + 1)) - fi - if ! check_gn_format; then - failures=$((failures + 1)) - fi - if ! check_whitespace; then - failures=$((failures + 1)) - fi - if [[ $failures -eq 0 ]]; then - message "No formatting issues found." - else - error "Formatting check failed." - error "To fix, run \"$SCRIPT_DIR/format.sh --fix\"" - fi - exit $failures -fi +SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")") +SRC_DIR="$(cd "$SCRIPT_DIR/../.."; pwd -P)" +DART_SDK_DIR="${SRC_DIR}/third_party/dart/tools/sdks/dart-sdk" +DART="${DART_SDK_DIR}/bin/dart" +PUB="${DART_SDK_DIR}/bin/pub" + +cd "$SCRIPT_DIR" +"$PUB" get && "$DART" \ + --disable-dart-dev \ + bin/format.dart \ + "$@" diff --git a/ci/pubspec.yaml b/ci/pubspec.yaml index d1fdcd859..eba8dd49b 100644 --- a/ci/pubspec.yaml +++ b/ci/pubspec.yaml @@ -7,6 +7,7 @@ name: ci_scripts dependencies: args: ^1.6.0 path: ^1.7.0 + isolate: ^2.0.3 process_runner: ^3.0.0 environment: diff --git a/testing/scenario_app/android/build.gradle b/testing/scenario_app/android/build.gradle index 7f9bc6036..11451c5bb 100644 --- a/testing/scenario_app/android/build.gradle +++ b/testing/scenario_app/android/build.gradle @@ -4,12 +4,11 @@ buildscript { repositories { google() jcenter() - } dependencies { classpath 'com.android.tools.build:gradle:3.6.0' classpath 'com.facebook.testing.screenshot:plugin:0.12.0' - + // NOTE: Do not place your application dependencies here; they belong // in the individual module build.gradle files } -- GitLab