diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 9db94dda37da..17b28927538d 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 093f8143df4f..e68585c44bdf 100644 --- a/script/tool/lib/src/license_check_command.dart +++ b/script/tool/lib/src/license_check_command.dart @@ -107,21 +107,65 @@ class LicenseCheckCommand extends PluginCommand { @override Future run() async { - final Iterable codeFiles = (await _getAllFiles()).where((File file) => + final Iterable allFiles = await _getAllFiles(); + + final Iterable codeFiles = allFiles.where((File file) => _codeFileExtensions.contains(p.extension(file.path)) && !_shouldIgnoreFile(file)); - final Iterable firstPartyLicenseFiles = (await _getAllFiles()).where( - (File file) => - path.basename(file.basename) == 'LICENSE' && !_isThirdParty(file)); + final Iterable 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 licenseFileFailures = await _checkLicenseFiles(firstPartyLicenseFiles); + final Map<_LicenseFailureType, List> codeFileFailures = + await _checkCodeLicenses(codeFiles); + + 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 (!copyrightCheckSucceeded || !licenseCheckSucceeded) { + if (!passed) { throw ToolExit(1); } + + printSuccess('All files passed validation!'); } // Creates the expected copyright+license block for first-party code. @@ -135,9 +179,10 @@ class LicenseCheckCommand extends PluginCommand { '${comment}found in the LICENSE file.$suffix\n'; } - // Checks all license blocks for [codeFiles], returning false if any of them - // fail validation. - Future _checkCodeLicenses(Iterable codeFiles) async { + /// Checks all license blocks for [codeFiles], returning any that fail + /// validation. + Future>> _checkCodeLicenses( + Iterable codeFiles) async { final List incorrectFirstPartyFiles = []; final List unrecognizedThirdPartyFiles = []; @@ -171,7 +216,6 @@ class LicenseCheckCommand extends PluginCommand { } } } - print('\n'); // Sort by path for more usable output. final int Function(File, File) pathCompare = @@ -179,38 +223,14 @@ class LicenseCheckCommand extends PluginCommand { 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>{ + _LicenseFailureType.incorrectFirstParty: incorrectFirstPartyFiles, + _LicenseFailureType.unknownThirdParty: unrecognizedThirdPartyFiles, + }; } - // Checks all provide LICENSE files, returning false if any of them - // fail validation. - Future _checkLicenseFiles(Iterable files) async { + /// Checks all provided LICENSE [files], returning any that fail validation. + Future> _checkLicenseFiles(Iterable files) async { final List incorrectLicenseFiles = []; for (final File file in files) { @@ -219,22 +239,8 @@ class LicenseCheckCommand extends PluginCommand { 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 @@ class LicenseCheckCommand extends PluginCommand { .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 64adc9214d80..288cf4696a59 100644 --- a/script/tool/test/license_check_command_test.dart +++ b/script/tool/test/license_check_command_test.dart @@ -131,8 +131,12 @@ void main() { await runCapturingPrint(runner, ['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([ + contains('Checking checked.cc'), + contains('All files passed validation!'), + ])); }); test('handles the comment styles for all supported languages', () async { @@ -150,10 +154,14 @@ void main() { await runCapturingPrint(runner, ['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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { await runCapturingPrint(runner, ['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([ + 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 @@ void main() { await runCapturingPrint(runner, ['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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { // 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([ + 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 @@ void main() { await runCapturingPrint(runner, ['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([ + contains('Checking LICENSE'), + contains('All files passed validation!'), + ])); }); test('fails if any first-party LICENSE files are incorrectly formatted', @@ -387,7 +418,7 @@ void main() { }); expect(commandError, isA()); - 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 @@ void main() { await runCapturingPrint(runner, ['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 output = await runCapturingPrint( + runner, ['license-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + 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'), + ])); }); }); }