Repository: VeryGoodOpenSource/very_good_cli
Pull request #868 (2023-10-23 08:44:46)

fix: ignore empty allowed or forbidden
<!--
  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:
-->

## Description

Ignores empty allow or forbidden.

**After**
```sh
very_good packages check licenses --allowed="" --forbidden="" 
# Reports, equivalent to `very_good packages check licenses`

very_good packages check licenses --allowed="MIT" --forbidden="" 
# Reports with allowed, equivalent to `very_good packages check licenses --allowed="MIT"`

very_good packages check licenses --allowed="" --forbidden="MIT" 
# Reports with forbidden, equivalent to `very_good packages check licenses --forbidden="MIT"`
```

**Before**
```sh
very_good packages check licenses --allowed="" --forbidden="" 
# Throws usage exception, cannot specify both allowed and forbidden options.

very_good packages check licenses --allowed="MIT" --forbidden="" 
# Throws usage exception, cannot specify both allowed and forbidden options.

very_good packages check licenses --allowed="" --forbidden="MIT" 
# Throws usage exception, cannot specify both allowed and forbidden options.
```



## Type of Change

<!--- Put an `x` in all the boxes that apply: -->

- [ ] ✨ New feature (non-breaking change which adds functionality)
- [X] 🛠️ 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 ef2289d..f20c1fe 100644
--- a/lib/src/commands/packages/commands/check/commands/licenses.dart
+++ b/lib/src/commands/packages/commands/check/commands/licenses.dart
@@ -103,6 +103,9 @@ class PackagesCheckLicensesCommand extends Command<int> {
     final forbiddenLicenses = _argResults['forbidden'] as List<String>;
     final skippedPackages = _argResults['skip-packages'] as List<String>;
 
+    allowedLicenses.removeWhere((license) => license.trim().isEmpty);
+    forbiddenLicenses.removeWhere((license) => license.trim().isEmpty);
+
     if (allowedLicenses.isNotEmpty && forbiddenLicenses.isNotEmpty) {
       usageException(
         '''Cannot specify both ${styleItalic.wrap('allowed')} and ${styleItalic.wrap('forbidden')} options.''',


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 4cafcb4..058a101 100644
--- a/test/src/commands/packages/commands/check/commands/licenses_test.dart
+++ b/test/src/commands/packages/commands/check/commands/licenses_test.dart
@@ -36,6 +36,9 @@ void main() {
   group('packages check licenses', () {
     const commandArguments = ['packages', 'check', 'licenses'];
 
+    const forbiddenArgument = '--forbidden';
+    const allowedArgument = '--allowed';
+
     late Progress progress;
 
     setUpAll(() {
@@ -94,10 +97,10 @@ void main() {
     });
 
     group(
-      'reports licenses',
+      'reports licenses correctly',
       () {
         test(
-          '''correctly when there is a single hosted direct dependency and license''',
+          '''when there is a single hosted direct dependency and license''',
           withRunner(
               (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
             final tempDirectory = Directory.systemTemp.createTempSync();
@@ -128,7 +131,7 @@ void main() {
         );
 
         test(
-          '''correctly when there are multiple hosted direct dependency and licenses''',
+          '''when there are multiple hosted direct dependency and licenses''',
           withRunner(
               (commandRunner, logger, pubUpdater, pubLicense, printLogs) async {
             final tempDirectory = Directory.systemTemp.createTempSync();
@@ -165,6 +168,44 @@ void main() {
             expect(result, equals(ExitCode.success.code));
           }),
         );
+
+        test(
+          '''when both allowed and forbidden are specified but left empty''',
+          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);
+
+            final result = await commandRunner.run(
+              [
+                ...commandArguments,
+                allowedArgument,
+                '',
+                forbiddenArgument,
+                '',
+                tempDirectory.path,
+              ],
+            );
+
+            verify(
+              () => progress.update(
+                'Collecting licenses from 1 out of 1 package',
+              ),
+            ).called(1);
+            verify(
+              () => progress.complete(
+                '''Retrieved 1 license from 1 package of type: MIT (1).''',
+              ),
+            ).called(1);
+
+            expect(result, equals(ExitCode.success.code));
+          }),
+        );
       },
     );
 
@@ -637,8 +678,6 @@ void main() {
     });
 
     group('allowed', () {
-      const allowedArgument = '--allowed';
-
       test(
         'warns when a license is not recognized',
         withRunner(
@@ -744,6 +783,50 @@ void main() {
           }),
         );
 
+        test(
+          'when a single license is not allowed and forbidden is left empty',
+          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',
+                forbiddenArgument,
+                '',
+                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(
@@ -793,8 +876,6 @@ void main() {
     });
 
     group('forbidden', () {
-      const forbiddenArgument = '--forbidden';
-
       test(
         'warns when a license is not recognized',
         withRunner(
@@ -900,6 +981,50 @@ void main() {
           }),
         );
 
+        test(
+          'when a single license is forbidden and allowed is left empty',
+          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,
+                '',
+                forbiddenArgument,
+                'MIT',
+                tempDirectory.path,
+              ],
+            );
+
+            final errorMessage =
+                '''1 dependency has a banned license: $dependency1Name ($license1LinkedMessage).''';
+
+            verify(
+              () => logger.err(errorMessage),
+            ).called(1);
+          }),
+        );
+
         test(
           'when multiple licenses are forbidden',
           withRunner(