Repository: VeryGoodOpenSource/very_good_cli
Pull request #848 (2023-10-11 10:49:24)

feat: add `--allowed` to `check licenses`
<!--
  Thanks for contributing!

  Provide a description of your changes below and a general summary in the title

  Please look at the following checklist to ensure that your PR can be accepted quickly:
-->

> Should be merged after #847 and #851

## Description

Adds a multi option (--allowed) on very_good packages check licenses.

This change anticipates https://github.com/VeryGoodOpenSource/very_good_cli/issues/771

## Demo

https://github.com/VeryGoodOpenSource/very_good_cli/assets/44524995/4109d681-4a40-49fc-8fd6-77bbbad015bc

## Type of Change

<!--- Put an `x` in all the boxes that apply: -->

- [X] ✨ New feature (non-breaking change which adds functionality)
- [ ] 🛠️ Bug fix (non-breaking change which fixes an issue)
- [ ] ❌ Breaking change (fix or feature that would cause existing functionality to change)
- [ ] 🧹 Code refactor
- [ ] ✅ Build configuration change
- [ ] 📝 Documentation
- [ ] 🗑️ Chore



Patch:
diff --git a/lib/src/commands/packages/commands/check/commands/licenses.dart b/lib/src/commands/packages/commands/check/commands/licenses.dart
index d18d06b..5c4bdf1 100644
--- a/lib/src/commands/packages/commands/check/commands/licenses.dart
+++ b/lib/src/commands/packages/commands/check/commands/licenses.dart
@@ -7,11 +7,26 @@ import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 import 'package:pubspec_lock/pubspec_lock.dart';
 import 'package:very_good_cli/src/pub_license/pub_license.dart';
+import 'package:very_good_cli/src/pub_license/spdx_license.gen.dart';
 
 /// The basename of the pubspec lock file.
 @visibleForTesting
 const pubspecLockBasename = 'pubspec.lock';
 
+/// The URI for the pub.dev license page for the given [packageName].
+@visibleForTesting
+Uri pubLicenseUri(String packageName) =>
+    Uri.parse('https://pub.dev/packages/$packageName/license');
+
+/// Defines a [Map] with dependencies as keys and their licenses as values.
+///
+/// If a dependency's license failed to be retrieved its license will be `null`.
+typedef _DependencyLicenseMap = Map<String, Set<String>?>;
+
+/// Defines a [Map] with banned dependencies as keys and their banned licenses
+/// as values.
+typedef _BannedDependencyLicenseMap = Map<String, Set<String>>;
+
 /// {@template packages_check_licenses_command}
 /// `very_good packages check licenses` command for checking packages licenses.
 /// {@endtemplate}
@@ -42,6 +57,10 @@ class PackagesCheckLicensesCommand extends Command<int> {
           'transitive': 'Check for transitive dependencies.',
         },
         defaultsTo: ['direct-main'],
+      )
+      ..addMultiOption(
+        'allowed',
+        help: 'Whitelist of allowed licenses.',
       );
   }
 
@@ -69,6 +88,14 @@ class PackagesCheckLicensesCommand extends Command<int> {
 
     final ignoreFailures = _argResults['ignore-failures'] as bool;
     final dependencyTypes = _argResults['dependency-type'] as List<String>;
+    final allowedLicenses = _argResults['allowed'] as List<String>;
+
+    final invalidLicenses = _invalidLicenses(allowedLicenses);
+    if (invalidLicenses.isNotEmpty) {
+      _logger.warn(
+        '''Some ${styleItalic.wrap('allowed')} licenses failed to be recognized: ${invalidLicenses.stringify()}. Refer to the documentation for a list of valid licenses.''',
+      );
+    }
 
     final target = _argResults.rest.length == 1 ? _argResults.rest[0] : '.';
     final targetPath = path.normalize(Directory(target).absolute.path);
@@ -114,7 +141,7 @@ class PackagesCheckLicensesCommand extends Command<int> {
     final licenses = <String, Set<String>?>{};
     for (final dependency in filteredDependencies) {
       progress.update(
-        'Collecting licenses of ${licenses.length}/${filteredDependencies.length} packages',
+        'Collecting licenses of ${licenses.length}/${filteredDependencies.length} packages.',
       );
 
       final dependencyName = dependency.package();
@@ -145,7 +172,21 @@ class PackagesCheckLicensesCommand extends Command<int> {
       }
     }
 
-    progress.complete(_composeReport(licenses));
+    final bannedDependencies = allowedLicenses.isNotEmpty
+        ? _bannedDependencies(licenses, allowedLicenses.contains)
+        : null;
+
+    progress.complete(
+      _composeReport(
+        licenses: licenses,
+        bannedDependencies: bannedDependencies,
+      ),
+    );
+
+    if (bannedDependencies != null) {
+      _logger.err(_composeBannedReport(bannedDependencies));
+      return ExitCode.config.code;
+    }
 
     return ExitCode.success.code;
   }
@@ -163,14 +204,77 @@ PubspecLock? _tryParsePubspecLock(File pubspecLockFile) {
   }
 }
 
+/// Verifies that all [licenses] are valid license inputs.
+///
+/// Valid license inputs are:
+/// - [SpdxLicense] values.
+///
+/// Returns a [List] of invalid licenses, if all licenses are valid the list
+/// will be empty.
+List<String> _invalidLicenses(List<String> licenses) {
+  final invalidLicenses = <String>[];
+  for (final license in licenses) {
+    final parsedLicense = SpdxLicense.tryParse(license);
+    if (parsedLicense == null) {
+      invalidLicenses.add(license);
+    }
+  }
+
+  return invalidLicenses;
+}
+
+/// Returns a [Map] of banned dependencies and their banned licenses.
+///
+/// The [Map] is lazily initialized, if no dependencies are banned `null` is
+/// returned.
+_BannedDependencyLicenseMap? _bannedDependencies(
+  _DependencyLicenseMap licenses,
+  bool Function(String license) isAllowed,
+) {
+  _BannedDependencyLicenseMap? bannedDependencies;
+  for (final dependency in licenses.entries) {
+    final name = dependency.key;
+    final license = dependency.value;
+    if (license == null) continue;
+
+    for (final licenseType in license) {
+      if (isAllowed(licenseType)) continue;
+
+      bannedDependencies ??= <String, Set<String>>{};
+      bannedDependencies.putIfAbsent(name, () => <String>{});
+      bannedDependencies[name]!.add(licenseType);
+    }
+  }
+
+  return bannedDependencies;
+}
+
 /// Composes a human friendly [String] to report the result of the retrieved
 /// licenses.
-String _composeReport(Map<String, Set<String>?> licenses) {
+///
+/// If [bannedDependencies] is provided those banned licenses will be
+/// highlighted in red.
+String _composeReport({
+  required _DependencyLicenseMap licenses,
+  required _BannedDependencyLicenseMap? bannedDependencies,
+}) {
+  final bannedLicenseTypes =
+      bannedDependencies?.values.fold(<String>{}, (previousValue, licenses) {
+    if (licenses.isEmpty) return previousValue;
+    return previousValue..addAll(licenses);
+  });
   final licenseTypes =
-      licenses.values.fold(<String>{}, (previousValue, element) {
-    if (element == null) return previousValue;
-    return previousValue..addAll(element);
+      licenses.values.fold(<String>{}, (previousValue, licenses) {
+    if (licenses == null) return previousValue;
+    return previousValue..addAll(licenses);
+  });
+  final coloredLicenseTypes = licenseTypes.map((license) {
+    if (bannedLicenseTypes != null && bannedLicenseTypes.contains(license)) {
+      return red.wrap(license)!;
+    }
+    return green.wrap(license)!;
   });
+
   final licenseCount = licenses.values.fold<int>(0, (previousValue, element) {
     if (element == null) return previousValue;
     return previousValue + element.length;
@@ -178,13 +282,41 @@ String _composeReport(Map<String, Set<String>?> licenses) {
 
   final licenseWord = licenseCount == 1 ? 'license' : 'licenses';
   final packageWord = licenses.length == 1 ? 'package' : 'packages';
-  final suffix = licenseTypes.isEmpty
+  final suffix = coloredLicenseTypes.isEmpty
       ? ''
-      : ' of type: ${licenseTypes.toList().stringify()}';
+      : ' of type: ${coloredLicenseTypes.toList().stringify()}';
 
   return '''Retrieved $licenseCount $licenseWord from ${licenses.length} $packageWord$suffix.''';
 }
 
+String _composeBannedReport(_BannedDependencyLicenseMap bannedDependencies) {
+  final bannedDependenciesList = bannedDependencies.entries.fold(
+    <String>[],
+    (previousValue, element) {
+      final dependencyName = element.key;
+      final dependencyLicenses = element.value;
+
+      final text = '$dependencyName (${link(
+        uri: pubLicenseUri(dependencyName),
+        message: dependencyLicenses.toList().stringify(),
+      )})';
+      return previousValue..add(text);
+    },
+  );
+  final bannedLicenseTypes =
+      bannedDependencies.values.fold(<String>{}, (previousValue, licenses) {
+    if (licenses.isEmpty) return previousValue;
+    return previousValue..addAll(licenses);
+  });
+
+  final prefix =
+      bannedDependencies.length == 1 ? 'dependency has' : 'dependencies have';
+  final suffix =
+      bannedLicenseTypes.length == 1 ? 'a banned license' : 'banned licenses';
+
+  return '''${bannedDependencies.length} $prefix $suffix: ${bannedDependenciesList.stringify()}.''';
+}
+
 extension on List<Object> {
   String stringify() {
     if (isEmpty) return '';


Test patch:
diff --git a/test/src/commands/packages/commands/check/commands/licenses_test.dart b/test/src/commands/packages/commands/check/commands/licenses_test.dart
index d5b0d7c..1794ed1 100644
--- a/test/src/commands/packages/commands/check/commands/licenses_test.dart
+++ b/test/src/commands/packages/commands/check/commands/licenses_test.dart
@@ -25,6 +25,8 @@ const _expectedPackagesCheckLicensesUsage = [
       '''          [direct-main] (default)    Check for direct main dependencies.\n'''
       '''          [transitive]               Check for transitive dependencies.\n'''
       '\n'
+      '    --allowed                        Whitelist of allowed licenses.\n'
+      '\n'
       'Run "very_good help" to see global options.'
 ];
 
@@ -96,7 +98,7 @@ void main() {
             );
 
             verify(
-              () => progress.update('Collecting licenses of 0/1 packages'),
+              () => progress.update('Collecting licenses of 0/1 packages.'),
             ).called(1);
             verify(
               () => progress.complete(
@@ -128,10 +130,10 @@ void main() {
             );
 
             verify(
-              () => progress.update('Collecting licenses of 0/2 packages'),
+              () => progress.update('Collecting licenses of 0/2 packages.'),
             ).called(1);
             verify(
-              () => progress.update('Collecting licenses of 1/2 packages'),
+              () => progress.update('Collecting licenses of 1/2 packages.'),
             ).called(1);
             verify(
               () => progress.complete(
@@ -175,10 +177,10 @@ void main() {
             verify(() => logger.err(errorMessage)).called(1);
 
             verify(
-              () => progress.update('Collecting licenses of 0/2 packages'),
+              () => progress.update('Collecting licenses of 0/2 packages.'),
             ).called(1);
             verify(
-              () => progress.update('Collecting licenses of 1/2 packages'),
+              () => progress.update('Collecting licenses of 1/2 packages.'),
             ).called(1);
             verify(
               () => progress.complete(
@@ -216,10 +218,10 @@ void main() {
             verify(() => logger.err(errorMessage)).called(1);
 
             verify(
-              () => progress.update('Collecting licenses of 0/2 packages'),
+              () => progress.update('Collecting licenses of 0/2 packages.'),
             ).called(1);
             verify(
-              () => progress.update('Collecting licenses of 1/2 packages'),
+              () => progress.update('Collecting licenses of 1/2 packages.'),
             ).called(1);
             verify(
               () => progress.complete(
@@ -267,10 +269,10 @@ void main() {
           ).called(1);
 
           verify(
-            () => progress.update('Collecting licenses of 0/2 packages'),
+            () => progress.update('Collecting licenses of 0/2 packages.'),
           ).called(1);
           verify(
-            () => progress.update('Collecting licenses of 1/2 packages'),
+            () => progress.update('Collecting licenses of 1/2 packages.'),
           ).called(1);
           verify(
             () => progress.complete(
@@ -353,7 +355,7 @@ void main() {
                 );
 
                 verify(
-                  () => progress.update('Collecting licenses of 0/1 packages'),
+                  () => progress.update('Collecting licenses of 0/1 packages.'),
                 ).called(1);
                 verify(
                   () => progress.complete(
@@ -402,7 +404,7 @@ void main() {
                 );
 
                 verify(
-                  () => progress.update('Collecting licenses of 0/1 packages'),
+                  () => progress.update('Collecting licenses of 0/1 packages.'),
                 ).called(1);
                 verify(
                   () => progress.complete(
@@ -452,7 +454,7 @@ void main() {
               );
 
               verify(
-                () => progress.update('Collecting licenses of 0/1 packages'),
+                () => progress.update('Collecting licenses of 0/1 packages.'),
               ).called(1);
               verify(
                 () => progress.complete(
@@ -501,7 +503,7 @@ void main() {
               );
 
               verify(
-                () => progress.update('Collecting licenses of 0/1 packages'),
+                () => progress.update('Collecting licenses of 0/1 packages.'),
               ).called(1);
               verify(
                 () => progress.complete(
@@ -554,13 +556,13 @@ void main() {
               );
 
               verify(
-                () => progress.update('Collecting licenses of 0/3 packages'),
+                () => progress.update('Collecting licenses of 0/3 packages.'),
               ).called(1);
               verify(
-                () => progress.update('Collecting licenses of 1/3 packages'),
+                () => progress.update('Collecting licenses of 1/3 packages.'),
               ).called(1);
               verify(
-                () => progress.update('Collecting licenses of 2/3 packages'),
+                () => progress.update('Collecting licenses of 2/3 packages.'),
               ).called(1);
               verify(
                 () => progress.complete(
@@ -575,6 +577,158 @@ void main() {
       });
     });
 
+    group('allowed', () {
+      const allowedArgument = '--allowed';
+
+      test(
+        'warns when a license is not recognized',
+        withRunner(
+            (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
+          final tempDirectory = Directory.systemTemp.createTempSync();
+          addTearDown(() => tempDirectory.deleteSync(recursive: true));
+
+          File(path.join(tempDirectory.path, pubspecLockBasename))
+              .writeAsStringSync(_validPubspecLockContent);
+
+          when(() => logger.progress(any())).thenReturn(progress);
+
+          const invalidLicense = 'not_a_valid_license';
+          await commandRunner.run(
+            [
+              ...commandArguments,
+              allowedArgument,
+              invalidLicense,
+              tempDirectory.path,
+            ],
+          );
+
+          const warningMessage =
+              '''Some allowed licenses failed to be recognized: $invalidLicense. Refer to the documentation for a list of valid licenses.''';
+          verify(
+            () => logger.warn(warningMessage),
+          ).called(1);
+        }),
+      );
+
+      test(
+        'exits when a license is not allowed',
+        withRunner(
+            (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
+          final tempDirectory = Directory.systemTemp.createTempSync();
+          addTearDown(() => tempDirectory.deleteSync(recursive: true));
+
+          File(path.join(tempDirectory.path, pubspecLockBasename))
+              .writeAsStringSync(_validPubspecLockContent);
+
+          when(() => logger.progress(any())).thenReturn(progress);
+
+          when(() => pubLicense.getLicense(any()))
+              .thenAnswer((_) => Future.value({'MIT'}));
+
+          final result = await commandRunner.run(
+            [
+              ...commandArguments,
+              allowedArgument,
+              'BSD',
+              tempDirectory.path,
+            ],
+          );
+
+          expect(result, ExitCode.config.code);
+        }),
+      );
+
+      group('reports', () {
+        test(
+          'when a single license is not allowed',
+          withRunner(
+              (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
+            final tempDirectory = Directory.systemTemp.createTempSync();
+            addTearDown(() => tempDirectory.deleteSync(recursive: true));
+
+            File(path.join(tempDirectory.path, pubspecLockBasename))
+                .writeAsStringSync(_validMultiplePubspecLockContent);
+
+            when(() => logger.progress(any())).thenReturn(progress);
+
+            const dependency1Name = 'very_good_test_runner';
+            when(() => pubLicense.getLicense(dependency1Name))
+                .thenAnswer((_) => Future.value({'MIT'}));
+            final license1LinkedMessage = link(
+              uri: pubLicenseUri(dependency1Name),
+              message: 'MIT',
+            );
+
+            const dependency2Name = 'cli_completion';
+            when(() => pubLicense.getLicense(dependency2Name))
+                .thenAnswer((_) => Future.value({'BSD'}));
+
+            await commandRunner.run(
+              [
+                ...commandArguments,
+                allowedArgument,
+                'BSD',
+                tempDirectory.path,
+              ],
+            );
+
+            final errorMessage =
+                '''1 dependency has a banned license: $dependency1Name ($license1LinkedMessage).''';
+
+            verify(
+              () => logger.err(errorMessage),
+            ).called(1);
+          }),
+        );
+
+        test(
+          'when multiple licenses are not allowed',
+          withRunner(
+              (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
+            final tempDirectory = Directory.systemTemp.createTempSync();
+            addTearDown(() => tempDirectory.deleteSync(recursive: true));
+
+            File(path.join(tempDirectory.path, pubspecLockBasename))
+                .writeAsStringSync(_validMultiplePubspecLockContent);
+
+            when(() => logger.progress(any())).thenReturn(progress);
+
+            const dependency1Name = 'very_good_test_runner';
+            when(() => pubLicense.getLicense(dependency1Name))
+                .thenAnswer((_) => Future.value({'MIT'}));
+            final license1LinkedMessage = link(
+              uri: pubLicenseUri(dependency1Name),
+              message: 'MIT',
+            );
+
+            const dependency2Name = 'cli_completion';
+            when(() => pubLicense.getLicense(dependency2Name))
+                .thenAnswer((_) => Future.value({'BSD'}));
+            final license2LinkedMessage = link(
+              uri: pubLicenseUri(dependency2Name),
+              message: 'BSD',
+            );
+
+            await commandRunner.run(
+              [
+                ...commandArguments,
+                allowedArgument,
+                'Apache-2.0',
+                tempDirectory.path,
+              ],
+            );
+
+            final errorMessage =
+                '''2 dependencies have banned licenses: $dependency1Name ($license1LinkedMessage) and $dependency2Name ($license2LinkedMessage).''';
+
+            verify(
+              () => logger.err(errorMessage),
+            ).called(1);
+          }),
+        );
+      });
+    });
+
     group('exits with error', () {
       test(
         'when it did not find a pubspec.lock file at the target path',