[flutter_plugin_tools] Improve license-check output (#4154)
Currently each type of check handles its output in isolation, which
creates confusing output when the last check succeeds but an earlier
check fails, since the end of the output will just be a success message.
This makes the output follow the same basic approach as the package
looper commands, where all failures are collected, and then a final
summary is presented at the end, so the last message will always reflect
the important details.
It also adopts the colorized output now used by most other commands.
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index 9db94dd..17b2892 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,3 +1,7 @@
+## NEXT
+
+- Improved `license-check` output.
+
## 0.4.0
- Modified the output format of many commands
diff --git a/script/tool/lib/src/license_check_command.dart b/script/tool/lib/src/license_check_command.dart
index 093f814..e68585c 100644
--- a/script/tool/lib/src/license_check_command.dart
+++ b/script/tool/lib/src/license_check_command.dart
@@ -107,21 +107,65 @@
@override
Future<void> run() async {
- final Iterable<File> codeFiles = (await _getAllFiles()).where((File file) =>
+ final Iterable<File> allFiles = await _getAllFiles();
+
+ final Iterable<File> codeFiles = allFiles.where((File file) =>
_codeFileExtensions.contains(p.extension(file.path)) &&
!_shouldIgnoreFile(file));
- final Iterable<File> firstPartyLicenseFiles = (await _getAllFiles()).where(
- (File file) =>
- path.basename(file.basename) == 'LICENSE' && !_isThirdParty(file));
+ final Iterable<File> firstPartyLicenseFiles = allFiles.where((File file) =>
+ path.basename(file.basename) == 'LICENSE' && !_isThirdParty(file));
- final bool copyrightCheckSucceeded = await _checkCodeLicenses(codeFiles);
- print('\n=======================================\n');
- final bool licenseCheckSucceeded =
+ final List<File> licenseFileFailures =
await _checkLicenseFiles(firstPartyLicenseFiles);
+ final Map<_LicenseFailureType, List<File>> codeFileFailures =
+ await _checkCodeLicenses(codeFiles);
- if (!copyrightCheckSucceeded || !licenseCheckSucceeded) {
+ bool passed = true;
+
+ print('\n=======================================\n');
+
+ if (licenseFileFailures.isNotEmpty) {
+ passed = false;
+ printError(
+ 'The following LICENSE files do not follow the expected format:');
+ for (final File file in licenseFileFailures) {
+ printError(' ${file.path}');
+ }
+ printError('Please ensure that they use the exact format used in this '
+ 'repository".\n');
+ }
+
+ if (codeFileFailures[_LicenseFailureType.incorrectFirstParty]!.isNotEmpty) {
+ passed = false;
+ printError('The license block for these files is missing or incorrect:');
+ for (final File file
+ in codeFileFailures[_LicenseFailureType.incorrectFirstParty]!) {
+ printError(' ${file.path}');
+ }
+ printError(
+ 'If this third-party code, move it to a "third_party/" directory, '
+ 'otherwise ensure that you are using the exact copyright and license '
+ 'text used by all first-party files in this repository.\n');
+ }
+
+ if (codeFileFailures[_LicenseFailureType.unknownThirdParty]!.isNotEmpty) {
+ passed = false;
+ printError(
+ 'No recognized license was found for the following third-party files:');
+ for (final File file
+ in codeFileFailures[_LicenseFailureType.unknownThirdParty]!) {
+ printError(' ${file.path}');
+ }
+ print('Please check that they have a license at the top of the file. '
+ 'If they do, the license check needs to be updated to recognize '
+ 'the new third-party license block.\n');
+ }
+
+ if (!passed) {
throw ToolExit(1);
}
+
+ printSuccess('All files passed validation!');
}
// Creates the expected copyright+license block for first-party code.
@@ -135,9 +179,10 @@
'${comment}found in the LICENSE file.$suffix\n';
}
- // Checks all license blocks for [codeFiles], returning false if any of them
- // fail validation.
- Future<bool> _checkCodeLicenses(Iterable<File> codeFiles) async {
+ /// Checks all license blocks for [codeFiles], returning any that fail
+ /// validation.
+ Future<Map<_LicenseFailureType, List<File>>> _checkCodeLicenses(
+ Iterable<File> codeFiles) async {
final List<File> incorrectFirstPartyFiles = <File>[];
final List<File> unrecognizedThirdPartyFiles = <File>[];
@@ -171,7 +216,6 @@
}
}
}
- print('\n');
// Sort by path for more usable output.
final int Function(File, File) pathCompare =
@@ -179,38 +223,14 @@
incorrectFirstPartyFiles.sort(pathCompare);
unrecognizedThirdPartyFiles.sort(pathCompare);
- if (incorrectFirstPartyFiles.isNotEmpty) {
- print('The license block for these files is missing or incorrect:');
- for (final File file in incorrectFirstPartyFiles) {
- print(' ${file.path}');
- }
- print('If this third-party code, move it to a "third_party/" directory, '
- 'otherwise ensure that you are using the exact copyright and license '
- 'text used by all first-party files in this repository.\n');
- }
-
- if (unrecognizedThirdPartyFiles.isNotEmpty) {
- print(
- 'No recognized license was found for the following third-party files:');
- for (final File file in unrecognizedThirdPartyFiles) {
- print(' ${file.path}');
- }
- print('Please check that they have a license at the top of the file. '
- 'If they do, the license check needs to be updated to recognize '
- 'the new third-party license block.\n');
- }
-
- final bool succeeded =
- incorrectFirstPartyFiles.isEmpty && unrecognizedThirdPartyFiles.isEmpty;
- if (succeeded) {
- print('All source files passed validation!');
- }
- return succeeded;
+ return <_LicenseFailureType, List<File>>{
+ _LicenseFailureType.incorrectFirstParty: incorrectFirstPartyFiles,
+ _LicenseFailureType.unknownThirdParty: unrecognizedThirdPartyFiles,
+ };
}
- // Checks all provide LICENSE files, returning false if any of them
- // fail validation.
- Future<bool> _checkLicenseFiles(Iterable<File> files) async {
+ /// Checks all provided LICENSE [files], returning any that fail validation.
+ Future<List<File>> _checkLicenseFiles(Iterable<File> files) async {
final List<File> incorrectLicenseFiles = <File>[];
for (final File file in files) {
@@ -219,22 +239,8 @@
incorrectLicenseFiles.add(file);
}
}
- print('\n');
- if (incorrectLicenseFiles.isNotEmpty) {
- print('The following LICENSE files do not follow the expected format:');
- for (final File file in incorrectLicenseFiles) {
- print(' ${file.path}');
- }
- print(
- 'Please ensure that they use the exact format used in this repository".\n');
- }
-
- final bool succeeded = incorrectLicenseFiles.isEmpty;
- if (succeeded) {
- print('All LICENSE files passed validation!');
- }
- return succeeded;
+ return incorrectLicenseFiles;
}
bool _shouldIgnoreFile(File file) {
@@ -255,3 +261,5 @@
.map((FileSystemEntity file) => file as File)
.toList();
}
+
+enum _LicenseFailureType { incorrectFirstParty, unknownThirdParty }
diff --git a/script/tool/test/license_check_command_test.dart b/script/tool/test/license_check_command_test.dart
index 64adc92..288cf46 100644
--- a/script/tool/test/license_check_command_test.dart
+++ b/script/tool/test/license_check_command_test.dart
@@ -131,8 +131,12 @@
await runCapturingPrint(runner, <String>['license-check']);
// Sanity check that the test did actually check a file.
- expect(output, contains('Checking checked.cc'));
- expect(output, contains('All source files passed validation!'));
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking checked.cc'),
+ contains('All files passed validation!'),
+ ]));
});
test('handles the comment styles for all supported languages', () async {
@@ -150,10 +154,14 @@
await runCapturingPrint(runner, <String>['license-check']);
// Sanity check that the test did actually check the files.
- expect(output, contains('Checking file_a.cc'));
- expect(output, contains('Checking file_b.sh'));
- expect(output, contains('Checking file_c.html'));
- expect(output, contains('All source files passed validation!'));
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking file_a.cc'),
+ contains('Checking file_b.sh'),
+ contains('Checking file_c.html'),
+ contains('All files passed validation!'),
+ ]));
});
test('fails if any checked files are missing license blocks', () async {
@@ -176,12 +184,14 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'The license block for these files is missing or incorrect:'));
- expect(output, contains(' bad.cc'));
- expect(output, contains(' bad.h'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'The license block for these files is missing or incorrect:'),
+ contains(' bad.cc'),
+ contains(' bad.h'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('fails if any checked files are missing just the copyright', () async {
@@ -202,11 +212,13 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'The license block for these files is missing or incorrect:'));
- expect(output, contains(' bad.cc'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'The license block for these files is missing or incorrect:'),
+ contains(' bad.cc'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('fails if any checked files are missing just the license', () async {
@@ -227,11 +239,13 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'The license block for these files is missing or incorrect:'));
- expect(output, contains(' bad.cc'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'The license block for these files is missing or incorrect:'),
+ contains(' bad.cc'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('fails if any third-party code is not in a third_party directory',
@@ -250,11 +264,13 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'The license block for these files is missing or incorrect:'));
- expect(output, contains(' third_party.cc'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'The license block for these files is missing or incorrect:'),
+ contains(' third_party.cc'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('succeeds for third-party code in a third_party directory', () async {
@@ -276,8 +292,12 @@
await runCapturingPrint(runner, <String>['license-check']);
// Sanity check that the test did actually check the file.
- expect(output, contains('Checking a_plugin/lib/src/third_party/file.cc'));
- expect(output, contains('All source files passed validation!'));
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking a_plugin/lib/src/third_party/file.cc'),
+ contains('All files passed validation!'),
+ ]));
});
test('allows first-party code in a third_party directory', () async {
@@ -294,9 +314,12 @@
await runCapturingPrint(runner, <String>['license-check']);
// Sanity check that the test did actually check the file.
- expect(output,
- contains('Checking a_plugin/lib/src/third_party/first_party.cc'));
- expect(output, contains('All source files passed validation!'));
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking a_plugin/lib/src/third_party/first_party.cc'),
+ contains('All files passed validation!'),
+ ]));
});
test('fails for licenses that the tool does not expect', () async {
@@ -320,11 +343,13 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'No recognized license was found for the following third-party files:'));
- expect(output, contains(' third_party/bad.cc'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'No recognized license was found for the following third-party files:'),
+ contains(' third_party/bad.cc'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('Apache is not recognized for new authors without validation changes',
@@ -353,11 +378,13 @@
// Failure should give information about the problematic files.
expect(
output,
- contains(
- 'No recognized license was found for the following third-party files:'));
- expect(output, contains(' third_party/bad.cc'));
+ containsAllInOrder(<Matcher>[
+ contains(
+ 'No recognized license was found for the following third-party files:'),
+ contains(' third_party/bad.cc'),
+ ]));
// Failure shouldn't print the success message.
- expect(output, isNot(contains('All source files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('passes if all first-party LICENSE files are correctly formatted',
@@ -370,8 +397,12 @@
await runCapturingPrint(runner, <String>['license-check']);
// Sanity check that the test did actually check the file.
- expect(output, contains('Checking LICENSE'));
- expect(output, contains('All LICENSE files passed validation!'));
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking LICENSE'),
+ contains('All files passed validation!'),
+ ]));
});
test('fails if any first-party LICENSE files are incorrectly formatted',
@@ -387,7 +418,7 @@
});
expect(commandError, isA<ToolExit>());
- expect(output, isNot(contains('All LICENSE files passed validation!')));
+ expect(output, isNot(contains(contains('All files passed validation!'))));
});
test('ignores third-party LICENSE format', () async {
@@ -400,8 +431,42 @@
await runCapturingPrint(runner, <String>['license-check']);
// The file shouldn't be checked.
- expect(output, isNot(contains('Checking third_party/LICENSE')));
- expect(output, contains('All LICENSE files passed validation!'));
+ expect(output, isNot(contains(contains('Checking third_party/LICENSE'))));
+ });
+
+ test('outputs all errors at the end', () async {
+ root.childFile('bad.cc').createSync();
+ root
+ .childDirectory('third_party')
+ .childFile('bad.cc')
+ .createSync(recursive: true);
+ final File license = root.childFile('LICENSE');
+ license.createSync();
+ license.writeAsStringSync(_incorrectLicenseFileText);
+
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['license-check'], errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('Checking LICENSE'),
+ contains('Checking bad.cc'),
+ contains('Checking third_party/bad.cc'),
+ contains(
+ 'The following LICENSE files do not follow the expected format:'),
+ contains(' LICENSE'),
+ contains(
+ 'The license block for these files is missing or incorrect:'),
+ contains(' bad.cc'),
+ contains(
+ 'No recognized license was found for the following third-party files:'),
+ contains(' third_party/bad.cc'),
+ ]));
});
});
}
