diff --git a/ci/bin/lint.dart b/ci/bin/lint.dart new file mode 100644 index 0000000000000..658388f4e9773 --- /dev/null +++ b/ci/bin/lint.dart @@ -0,0 +1,256 @@ +/// Runs clang-tidy on files with changes. +/// +/// usage: +/// dart lint.dart [clang-tidy checks] +/// +/// User environment variable FLUTTER_LINT_ALL to run on all files. + +import 'dart:async' show Completer; +import 'dart:convert' show jsonDecode, utf8, LineSplitter; +import 'dart:io' show File, exit, Directory, FileSystemEntity, Platform, stderr; + +import 'package:args/args.dart'; +import 'package:path/path.dart' as path; +import 'package:process_runner/process_runner.dart'; + +String _linterOutputHeader = ''' +┌──────────────────────────┐ +│ Engine Clang Tidy Linter │ +└──────────────────────────┘ +The following errors have been reported by the Engine Clang Tidy Linter. For +more information on addressing these issues please see: +https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter +'''; + +class Command { + Directory directory = Directory(''); + String command = ''; + File file = File(''); +} + +Command parseCommand(Map map) { + final Directory dir = Directory(map['directory'] as String).absolute; + return Command() + ..directory = dir + ..command = map['command'] as String + ..file = File(path.normalize(path.join(dir.path, map['file'] as String))); +} + +String calcTidyArgs(Command command) { + String result = command.command; + result = result.replaceAll(RegExp(r'\S*clang/bin/clang'), ''); + result = result.replaceAll(RegExp(r'-MF \S*'), ''); + return result; +} + +String calcTidyPath(Command command) { + final RegExp regex = RegExp(r'\S*clang/bin/clang'); + return regex + .stringMatch(command.command) + ?.replaceAll('clang/bin/clang', 'clang/bin/clang-tidy') ?? + ''; +} + +bool isNonEmptyString(String str) => str.isNotEmpty; + +bool containsAny(File file, Iterable queries) { + return queries.where((File query) => path.equals(query.path, file.path)).isNotEmpty; +} + +/// Returns a list of all non-deleted files which differ from the nearest +/// merge-base with `master`. If it can't find a fork point, uses the default +/// merge-base. +Future> getListOfChangedFiles(Directory repoPath) async { + final ProcessRunner processRunner = ProcessRunner(defaultWorkingDirectory: repoPath); + final ProcessRunnerResult fetchResult = await processRunner.runProcess( + ['git', 'fetch', 'upstream', 'master'], + failOk: true, + ); + if (fetchResult.exitCode != 0) { + await processRunner.runProcess(['git', 'fetch', 'origin', 'master']); + } + final Set result = {}; + ProcessRunnerResult mergeBaseResult = await processRunner.runProcess( + ['git', 'merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], + failOk: true); + if (mergeBaseResult.exitCode != 0) { + if (verbose) { + stderr.writeln("Didn't find a fork point, falling back to default merge base."); + } + mergeBaseResult = await processRunner + .runProcess(['git', 'merge-base', 'FETCH_HEAD', 'HEAD'], failOk: false); + } + final String mergeBase = mergeBaseResult.stdout.trim(); + final ProcessRunnerResult masterResult = await processRunner + .runProcess(['git', 'diff', '--name-only', '--diff-filter=ACMRT', mergeBase]); + result.addAll(masterResult.stdout.split('\n').where(isNonEmptyString)); + return result.map((String filePath) => File(path.join(repoPath.path, filePath))).toList(); +} + +Future> dirContents(Directory dir) { + final List files = []; + final Completer> completer = Completer>(); + final Stream lister = dir.list(recursive: true); + lister.listen((FileSystemEntity file) => file is File ? files.add(file) : null, + onError: (Object e) => completer.completeError(e), onDone: () => completer.complete(files)); + return completer.future; +} + +File buildFileAsRepoFile(String buildFile, Directory repoPath) { + // Removes the "../../flutter" from the build files to make it relative to the flutter + // dir. + final String relativeBuildFile = path.joinAll(path.split(buildFile).sublist(3)); + final File result = File(path.join(repoPath.absolute.path, relativeBuildFile)); + print('Build file: $buildFile => ${result.path}'); + return result; +} + +Future shouldIgnoreFile(File file) async { + if (path.split(file.path).contains('third_party')) { + return 'third_party'; + } else { + final RegExp exp = RegExp(r'//\s*FLUTTER_NOLINT'); + await for (String line + in file.openRead().transform(utf8.decoder).transform(const LineSplitter())) { + if (exp.hasMatch(line)) { + return 'FLUTTER_NOLINT'; + } else if (line.isNotEmpty && line[0] != '\n' && line[0] != '/') { + // Quick out once we find a line that isn't empty or a comment. The + // FLUTTER_NOLINT must show up before the first real code. + return ''; + } + } + return ''; + } +} + +void _usage(ArgParser parser, {int exitCode = 1}) { + stderr.writeln('lint.dart [--help] [--lint-all] [--verbose] [--diff-branch]'); + stderr.writeln(parser.usage); + exit(exitCode); +} + +bool verbose = false; + +void main(List arguments) async { + final ArgParser parser = ArgParser(); + parser.addFlag('help', help: 'Print help.'); + parser.addFlag('lint-all', + help: 'lint all of the sources, regardless of FLUTTER_NOLINT.', defaultsTo: false); + parser.addFlag('verbose', help: 'Print verbose output.', defaultsTo: verbose); + parser.addOption('repo', help: 'Use the given path as the repo path'); + parser.addOption('compile-commands', + help: 'Use the given path as the source of compile_commands.json. This ' + 'file is created by running tools/gn'); + parser.addOption('checks', + help: 'Perform the given checks on the code. Defaults to the empty ' + 'string, indicating all checks should be performed.', + defaultsTo: ''); + final ArgResults options = parser.parse(arguments); + + verbose = options['verbose'] as bool; + + if (options['help'] as bool) { + _usage(parser, exitCode: 0); + } + + if (!options.wasParsed('compile-commands')) { + stderr.writeln('ERROR: The --compile-commands argument is requried.'); + _usage(parser); + } + + if (!options.wasParsed('repo')) { + stderr.writeln('ERROR: The --repo argument is requried.'); + _usage(parser); + } + + final File buildCommandsPath = File(options['compile-commands'] as String); + if (!buildCommandsPath.existsSync()) { + stderr.writeln("ERROR: Build commands path ${buildCommandsPath.absolute.path} doesn't exist."); + _usage(parser); + } + + final Directory repoPath = Directory(options['repo'] as String); + if (!repoPath.existsSync()) { + stderr.writeln("ERROR: Repo path ${repoPath.absolute.path} doesn't exist."); + _usage(parser); + } + + print(_linterOutputHeader); + + final String checksArg = options.wasParsed('checks') ? options['checks'] as String : ''; + final String checks = checksArg.isNotEmpty ? '--checks=$checksArg' : '--config='; + final bool lintAll = + Platform.environment['FLUTTER_LINT_ALL'] != null || options['lint-all'] as bool; + final List changedFiles = + lintAll ? await dirContents(repoPath) : await getListOfChangedFiles(repoPath); + + if (verbose) { + print('Checking lint in repo at $repoPath.'); + if (checksArg.isNotEmpty) { + print('Checking for specific checks: $checks.'); + } + if (lintAll) { + print('Checking all ${changedFiles.length} files the repo dir.'); + } else { + print('Dectected ${changedFiles.length} files that have changed'); + } + } + + final List buildCommandMaps = + jsonDecode(await buildCommandsPath.readAsString()) as List; + final List buildCommands = buildCommandMaps + .map((dynamic x) => parseCommand(x as Map)) + .toList(); + final Command firstCommand = buildCommands[0]; + final String tidyPath = calcTidyPath(firstCommand); + assert(tidyPath.isNotEmpty); + final List changedFileBuildCommands = + buildCommands.where((Command x) => containsAny(x.file, changedFiles)).toList(); + + if (changedFileBuildCommands.isEmpty) { + print('No changed files that have build commands associated with them ' + 'were found.'); + exit(0); + } + + if (verbose) { + print('Found ${changedFileBuildCommands.length} files that have build ' + 'commands associated with them and can be lint checked.'); + } + + int exitCode = 0; + final List jobs = []; + for (Command command in changedFileBuildCommands) { + final String relativePath = path.relative(command.file.path, from: repoPath.parent.path); + final String ignoreReason = await shouldIgnoreFile(command.file); + if (ignoreReason.isEmpty) { + final String tidyArgs = calcTidyArgs(command); + final List args = [command.file.path, checks, '--']; + args.addAll(tidyArgs?.split(' ') ?? []); + print('🔶 linting $relativePath'); + jobs.add(WorkerJob( + [tidyPath, ...args], + workingDirectory: command.directory, + name: 'clang-tidy on ${command.file.path}', + )); + } else { + print('🔷 ignoring $relativePath ($ignoreReason)'); + } + } + final ProcessPool pool = ProcessPool(); + + await for (final WorkerJob job in pool.startWorkers(jobs)) { + if (job.result.stdout.isEmpty) { + continue; + } + print('❌ Failures for ${job.name}:'); + print(job.result.stdout); + exitCode = 1; + } + print('\n'); + if (exitCode == 0) { + print('No lint problems found.'); + } + exit(exitCode); +} diff --git a/ci/lint.dart b/ci/lint.dart deleted file mode 100644 index 933da7d8a3c06..0000000000000 --- a/ci/lint.dart +++ /dev/null @@ -1,168 +0,0 @@ -/// Runs clang-tidy on files with changes. -/// -/// usage: -/// dart lint.dart [clang-tidy checks] -/// -/// User environment variable FLUTTER_LINT_ALL to run on all files. - -import 'dart:io' - show - File, - Process, - ProcessResult, - exit, - Directory, - FileSystemEntity, - Platform; -import 'dart:convert' show jsonDecode, utf8, LineSplitter; -import 'dart:async' show Completer; - -String _linterOutputHeader = '''┌──────────────────────────┐ -│ Engine Clang Tidy Linter │ -└──────────────────────────┘ -The following errors have been reported by the Engine Clang Tidy Linter. For -more information on addressing these issues please see: -https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter -'''; - -class Command { - String directory; - String command; - String file; -} - -Command parseCommand(Map map) { - return Command() - ..directory = map['directory'] - ..command = map['command'] - ..file = map['file']; -} - -String calcTidyArgs(Command command) { - String result = command.command; - result = result.replaceAll(RegExp(r'\S*clang/bin/clang'), ''); - result = result.replaceAll(RegExp(r'-MF \S*'), ''); - return result; -} - -String calcTidyPath(Command command) { - final RegExp regex = RegExp(r'\S*clang/bin/clang'); - return regex - .stringMatch(command.command) - .replaceAll('clang/bin/clang', 'clang/bin/clang-tidy'); -} - -bool isNonEmptyString(String str) => str.length > 0; - -bool containsAny(String str, List queries) { - for (String query in queries) { - if (str.contains(query)) { - return true; - } - } - return false; -} - -/// Returns a list of all files with current changes or differ from `master`. -List getListOfChangedFiles(String repoPath) { - final Set result = Set(); - final ProcessResult diffResult = Process.runSync( - 'git', ['diff', '--name-only'], - workingDirectory: repoPath); - final ProcessResult diffCachedResult = Process.runSync( - 'git', ['diff', '--cached', '--name-only'], - workingDirectory: repoPath); - - final ProcessResult fetchResult = - Process.runSync('git', ['fetch', 'upstream', 'master']); - if (fetchResult.exitCode != 0) { - Process.runSync('git', ['fetch', 'origin', 'master']); - } - final ProcessResult mergeBaseResult = Process.runSync( - 'git', ['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], - workingDirectory: repoPath); - final String mergeBase = mergeBaseResult.stdout.trim(); - final ProcessResult masterResult = Process.runSync( - 'git', ['diff', '--name-only', mergeBase], - workingDirectory: repoPath); - result.addAll(diffResult.stdout.split('\n').where(isNonEmptyString)); - result.addAll(diffCachedResult.stdout.split('\n').where(isNonEmptyString)); - result.addAll(masterResult.stdout.split('\n').where(isNonEmptyString)); - return result.toList(); -} - -Future> dirContents(String repoPath) { - Directory dir = Directory(repoPath); - var files = []; - var completer = new Completer>(); - var lister = dir.list(recursive: true); - lister.listen((FileSystemEntity file) => files.add(file.path), - // should also register onError - onDone: () => completer.complete(files)); - return completer.future; -} - -Future shouldIgnoreFile(String path) async { - if (path.contains('/third_party/')) { - return true; - } else { - final RegExp exp = RegExp(r'//.*FLUTTER_NOLINT'); - await for (String line in File(path.substring(6)) - .openRead() - .transform(utf8.decoder) - .transform(const LineSplitter())) { - if (exp.hasMatch(line)) { - return true; - } else if (line.length > 0 && line[0] != '\n' && line[0] != '/') { - // Quick out once we find a line that isn't empty or a comment. The - // FLUTTER_NOLINT must show up before the first real code. - return false; - } - } - return false; - } -} - -void main(List arguments) async { - final String buildCommandsPath = arguments[0]; - final String repoPath = arguments[1]; - final String checks = - arguments.length >= 3 ? '--checks=${arguments[2]}' : '--config='; - final List changedFiles = - Platform.environment['FLUTTER_LINT_ALL'] != null - ? await dirContents(repoPath) - : getListOfChangedFiles(repoPath); - /// TODO(gaaclarke): Convert FLUTTER_LINT_ALL to a command-line flag and add - /// `--verbose` flag. - - final List buildCommandMaps = - jsonDecode(await new File(buildCommandsPath).readAsString()); - final List buildCommands = - buildCommandMaps.map((x) => parseCommand(x)).toList(); - final Command firstCommand = buildCommands[0]; - final String tidyPath = calcTidyPath(firstCommand); - final List changedFileBuildCommands = - buildCommands.where((x) => containsAny(x.file, changedFiles)).toList(); - - print(_linterOutputHeader); - int exitCode = 0; - //TODO(aaclarke): Coalesce this into one call using the `-p` arguement. - for (Command command in changedFileBuildCommands) { - if (!(await shouldIgnoreFile(command.file))) { - final String tidyArgs = calcTidyArgs(command); - final List args = [command.file, checks, '--']; - args.addAll(tidyArgs.split(' ')); - print('🔶 linting ${command.file}'); - final Process process = await Process.start(tidyPath, args, - workingDirectory: command.directory, runInShell: false); - process.stdout.transform(utf8.decoder).listen((data) { - print(data); - exitCode = 1; - }); - await process.exitCode; - } else { - print('🔷 ignoring ${command.file}'); - } - } - exit(exitCode); -} diff --git a/ci/lint.sh b/ci/lint.sh index 023d98292ae30..c3292d8e369fe 100755 --- a/ci/lint.sh +++ b/ci/lint.sh @@ -2,9 +2,47 @@ set -e -COMPILE_COMMANDS="out/compile_commands.json" -if [ ! -f $COMPILE_COMMANDS ]; then - ./flutter/tools/gn +# Needed because if it is set, cd may print the path it changed to. +unset CDPATH + +# On Mac OS, readlink -f doesn't work, so follow_links traverses the path one +# link at a time, and then cds into the link destination and find out where it +# ends up. +# +# The returned filesystem path must be a format usable by Dart's URI parser, +# since the Dart command line tool treats its argument as a file URI, not a +# filename. For instance, multiple consecutive slashes should be reduced to a +# single slash, since double-slashes indicate a URI "authority", and these are +# supposed to be filenames. There is an edge case where this will return +# multiple slashes: when the input resolves to the root directory. However, if +# that were the case, we wouldn't be running this shell, so we don't do anything +# about it. +# +# The function is enclosed in a subshell to avoid changing the working directory +# of the caller. +function follow_links() ( + cd -P "$(dirname -- "$1")" + file="$PWD/$(basename -- "$1")" + while [[ -h "$file" ]]; do + cd -P "$(dirname -- "$file")" + file="$(readlink -- "$file")" + cd -P "$(dirname -- "$file")" + file="$PWD/$(basename -- "$file")" + done + echo "$file" +) +PROG_NAME="$(follow_links "${BASH_SOURCE[0]}")" +CI_DIR="$(cd "${PROG_NAME%/*}" ; pwd -P)" +SRC_DIR="$(cd "$CI_DIR/../.."; pwd -P)" + +COMPILE_COMMANDS="$SRC_DIR/out/compile_commands.json" +if [ ! -f "$COMPILE_COMMANDS" ]; then + (cd $SRC_DIR; ./flutter/tools/gn) fi -dart flutter/ci/lint.dart $COMPILE_COMMANDS flutter/ +cd "$CI_DIR" +pub get && dart \ + bin/lint.dart \ + --compile-commands="$COMPILE_COMMANDS" \ + --repo="$SRC_DIR/flutter" \ + "$@" diff --git a/ci/pubspec.yaml b/ci/pubspec.yaml new file mode 100644 index 0000000000000..d345cab6e78f1 --- /dev/null +++ b/ci/pubspec.yaml @@ -0,0 +1,9 @@ +name: ci_scripts + +dependencies: + args: ^1.6.0 + path: ^1.7.0 + process_runner: ^2.0.3 + +environment: + sdk: '>=2.8.0 <3.0.0'