Fix a bug with a disjunctive license (#192)

This PR fixes the behavior with a disjunctive license (e.g. "MIT, WTFPL, GPL-3.0-only"). Disjunctive license means that the user can choose any license from the list. Therefore, `composer-license-checker` should allow the package if _at least one_ license is allowed. Before, it was checking all licenses from the list and demanding all of them to be allowed. --------- Co-authored-by: vasily.pyatykh <[email protected]>
Lendable · Sep 20, 2023 · fdbb6ad · fdbb6ad
1 parent f03a720
commit fdbb6ad
Show file tree

Hide file tree

Showing 19 changed files with 141 additions and 111 deletions.
diff --git a/src/Event/PackageWithViolatingLicense.php b/src/Event/PackageWithViolatingLicense.php
@@ -8,10 +8,7 @@
 
 final class PackageWithViolatingLicense implements Event
 {
-    /**
-     * @param non-empty-string $license
-     */
-    public function __construct(public readonly Package $package, public readonly string $license)
+    public function __construct(public readonly Package $package)
     {
     }
 }
diff --git a/src/LicenseChecker.php b/src/LicenseChecker.php
@@ -167,21 +167,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 continue;
             }
 
-            if ($package->licenses === []) {
+            if ($package->isUnlicensed()) {
                 $violation = true;
                 $this->dispatcher->dispatch(new UnlicensedPackageNotExplicitlyAllowed($package));
 
                 continue;
             }
 
-            foreach ($package->licenses as $license) {
-                if ($config->allowsLicense($license)) {
-                    continue;
-                }
-
-                $violation = true;
-                $this->dispatcher->dispatch(new PackageWithViolatingLicense($package, $license));
+            if ($config->allowsLicenseOfPackage($package)) {
+                continue;
             }
+
+            $violation = true;
+            $this->dispatcher->dispatch(new PackageWithViolatingLicense($package));
         }
 
         if ($violation) {

diff --git a/src/LicenseConfiguration.php b/src/LicenseConfiguration.php
@@ -17,9 +17,15 @@ public function __construct(
     ) {
     }
 
-    public function allowsLicense(string $license): bool
+    public function allowsLicenseOfPackage(Package $package): bool
     {
-        return \in_array($license, $this->allowedLicenses, true);
+        foreach ($package->licenses as $license) {
+            if ($this->allowsLicense($license)) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     public function allowsPackage(string $package): bool
@@ -32,4 +38,9 @@ public function allowsPackage(string $package): bool
 
         return false;
     }
+
+    private function allowsLicense(string $license): bool
+    {
+        return \in_array($license, $this->allowedLicenses, true);
+    }
 }
diff --git a/src/Licenses.php b/src/Licenses.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Lendable\ComposerLicenseChecker;
+
+/**
+ * @implements \IteratorAggregate<int, string>
+ */
+final class Licenses implements \IteratorAggregate
+{
+    /**
+     * @param list<non-empty-string> $licenses
+     */
+    public function __construct(
+        private readonly array $licenses,
+    ) {
+    }
+
+    public function toString(): string
+    {
+        return \implode(', ', $this->licenses);
+    }
+
+    public function isEmpty(): bool
+    {
+        return $this->licenses === [];
+    }
+
+    public function getIterator(): \Traversable
+    {
+        return new \ArrayIterator($this->licenses);
+    }
+
+    public function isDisjunctive(): bool
+    {
+        return \count($this->licenses) > 1;
+    }
+}
diff --git a/src/Output/Display.php b/src/Output/Display.php
@@ -19,10 +19,7 @@ public function onOutcomeFailure(): void;
 
     public function onOutcomeSuccess(): void;
 
-    /**
-     * @param non-empty-string $license
-     */
-    public function onPackageWithViolatingLicense(Package $package, string $license): void;
+    public function onPackageWithViolatingLicense(Package $package): void;
 
     public function onUnlicensedPackageNotExplicitlyAllowed(Package $package): void;
 

diff --git a/src/Output/DisplayOutputSubscriber.php b/src/Output/DisplayOutputSubscriber.php
@@ -53,7 +53,7 @@ private function onOutcomeSuccess(OutcomeSuccess $event): void
 
     private function onPackageWithViolatingLicense(PackageWithViolatingLicense $event): void
     {
-        $this->display->onPackageWithViolatingLicense($event->package, $event->license);
+        $this->display->onPackageWithViolatingLicense($event->package);
     }
 
     private function onUnlicensedPackageNotExplicitlyAllowed(UnlicensedPackageNotExplicitlyAllowed $event): void

diff --git a/src/Output/HumanReadableDisplay.php b/src/Output/HumanReadableDisplay.php
@@ -37,15 +37,25 @@ public function onOutcomeSuccess(): void
         $this->style->success('All dependencies have allowed licenses.');
     }
 
-    public function onPackageWithViolatingLicense(Package $package, string $license): void
+    public function onPackageWithViolatingLicense(Package $package): void
     {
-        $this->style->error(
-            \sprintf(
-                'Dependency "%s" has license "%s" which is not in the allowed list.',
-                $package->name->toString(),
-                $license,
-            ),
-        );
+        if ($package->licenses->isDisjunctive()) {
+            $this->style->error(
+                \sprintf(
+                    'Dependency "%s" is licensed under any of "%s", none of which are allowed.',
+                    $package->name->toString(),
+                    $package->licenses->toString(),
+                ),
+            );
+        } else {
+            $this->style->error(
+                \sprintf(
+                    'Dependency "%s" is licensed under "%s" which is not in the allowed list.',
+                    $package->name->toString(),
+                    $package->licenses->toString(),
+                ),
+            );
+        }
     }
 
     public function onUnlicensedPackageNotExplicitlyAllowed(Package $package): void

diff --git a/src/Output/JsonDisplay.php b/src/Output/JsonDisplay.php
@@ -43,9 +43,11 @@ public function onFatalError(string $message): void
         );
     }
 
-    public function onPackageWithViolatingLicense(Package $package, string $license): void
+    public function onPackageWithViolatingLicense(Package $package): void
     {
-        $this->violations[$license][] = $package->name->toString();
+        /** @var non-empty-string $licences */
+        $licences = $package->licenses->toString();
+        $this->violations[$licences][] = $package->name->toString();
     }
 
     public function onUnlicensedPackageNotExplicitlyAllowed(Package $package): void

diff --git a/src/Package.php b/src/Package.php
@@ -6,12 +6,14 @@
 
 final class Package
 {
-    /**
-     * @param list<non-empty-string> $licenses
-     */
     public function __construct(
         public readonly PackageName $name,
-        public readonly array $licenses,
+        public readonly Licenses $licenses,
     ) {
     }
+
+    public function isUnlicensed(): bool
+    {
+        return $this->licenses->isEmpty();
+    }
 }
diff --git a/src/PackagesProvider/ComposerInstalledJsonPackagesProvider.php b/src/PackagesProvider/ComposerInstalledJsonPackagesProvider.php
@@ -5,6 +5,7 @@
 namespace Lendable\ComposerLicenseChecker\PackagesProvider;
 
 use Lendable\ComposerLicenseChecker\Exception\FailedProvidingPackages;
+use Lendable\ComposerLicenseChecker\Licenses;
 use Lendable\ComposerLicenseChecker\Package;
 use Lendable\ComposerLicenseChecker\PackageName;
 use Lendable\ComposerLicenseChecker\Packages;
@@ -84,11 +85,10 @@ static function (mixed $package) use ($skipPackages): ?Package {
                             throw FailedProvidingPackages::withReason('Key "license" is not a list');
                         }
 
-                        /** @var list<non-empty-string> $licenses */
                         /** @var PackageData $package */
                         return new Package(
                             new PackageName($package['name']),
-                            $licenses,
+                            new Licenses($licenses),
                         );
                     },
                     $dependencies,

diff --git a/src/PackagesProvider/ComposerLicensesPackagesProvider.php b/src/PackagesProvider/ComposerLicensesPackagesProvider.php
@@ -7,6 +7,7 @@
 use Lendable\ComposerLicenseChecker\ComposerRunner;
 use Lendable\ComposerLicenseChecker\Exception\FailedProvidingPackages;
 use Lendable\ComposerLicenseChecker\Exception\FailedRunningComposer;
+use Lendable\ComposerLicenseChecker\Licenses;
 use Lendable\ComposerLicenseChecker\Package;
 use Lendable\ComposerLicenseChecker\PackageName;
 use Lendable\ComposerLicenseChecker\Packages;
@@ -45,7 +46,7 @@ public function provide(string $projectPath, bool $ignoreDev): Packages
 
         return new Packages(
             \array_map(
-                static fn (string $name, array $package): Package => new Package(new PackageName($name), $package['license']),
+                static fn (string $name, array $package): Package => new Package(new PackageName($name), new Licenses($package['license'])),
                 \array_keys($data['dependencies']),
                 $data['dependencies'],
             ),

diff --git a/tests/e2e/LicenseCheckerCase.php b/tests/e2e/LicenseCheckerCase.php
@@ -155,7 +155,6 @@ public function test_report_not_allowed_licenses(): void
             ->foundLicensingIssues(
                 [
                     'lendable/apache' => 'Apache-2.0',
-                    'lendable/bsd3_mit' => 'BSD-3-Clause',
                     'package/bsd3' => 'BSD-3-Clause',
                 ],
             );

diff --git a/tests/e2e/Output/DisplayOutputSubscriberTest.php b/tests/e2e/Output/DisplayOutputSubscriberTest.php
@@ -12,6 +12,7 @@
 use Lendable\ComposerLicenseChecker\Event\Started;
 use Lendable\ComposerLicenseChecker\Event\TraceInformation;
 use Lendable\ComposerLicenseChecker\Event\UnlicensedPackageNotExplicitlyAllowed;
+use Lendable\ComposerLicenseChecker\Licenses;
 use Lendable\ComposerLicenseChecker\Output\Display;
 use Lendable\ComposerLicenseChecker\Output\DisplayOutputSubscriber;
 use Lendable\ComposerLicenseChecker\Package;
@@ -75,20 +76,19 @@ public function test_delegates_on_outcome_success(): void
 
     public function test_delegates_on_package_with_violating_license(): void
     {
-        $package = new Package(new PackageName('foo/bar'), ['MIT']);
-        $license = 'MIT';
+        $package = new Package(new PackageName('foo/bar'), new Licenses(['MIT']));
 
         $this->display
             ->expects(self::once())
             ->method('onPackageWithViolatingLicense')
-            ->with($package, $license);
+            ->with($package);
 
-        $this->dispatcher->dispatch(new PackageWithViolatingLicense($package, $license));
+        $this->dispatcher->dispatch(new PackageWithViolatingLicense($package));
     }
 
     public function test_delegates_on_unlicensed_package_not_explicitly_allowed(): void
     {
-        $package = new Package(new PackageName('foo/bar'), ['MIT']);
+        $package = new Package(new PackageName('foo/bar'), new Licenses(['MIT']));
 
         $this->display
             ->expects(self::once())

diff --git a/tests/support/CommandTesterAsserter.php b/tests/support/CommandTesterAsserter.php
@@ -81,11 +81,18 @@ public function foundLicensingIssues(array $issues): self
             }
 
             if (\is_array($license)) {
-                foreach ($license as $element) {
+                if (\count($license) > 1) {
                     $expectedOutput[] = \sprintf(
-                        ' [ERROR] Dependency "%s" has license "%s" which is not in the allowed list.',
+                        ' [ERROR] Dependency "%s" is licensed under any of "%s", none of which are allowed.',
                         $package,
-                        $element,
+                        \implode(', ', $license),
+                    );
+                    $expectedOutput[] = '';
+                } else {
+                    $expectedOutput[] = \sprintf(
+                        ' [ERROR] Dependency "%s" is licensed under "%s" which is not in the allowed list.',
+                        $package,
+                        \implode(', ', $license),
                     );
                     $expectedOutput[] = '';
                 }

diff --git a/tests/support/PackageAsserter.php b/tests/support/PackageAsserter.php
@@ -4,6 +4,7 @@
 
 namespace Tests\Support\Lendable\ComposerLicenseChecker;
 
+use Lendable\ComposerLicenseChecker\Licenses;
 use Lendable\ComposerLicenseChecker\Package;
 use Lendable\ComposerLicenseChecker\PackageName;
 use PHPUnit\Framework\Assert;
@@ -30,32 +31,25 @@ public function hasName(string|PackageName $name): self
         return $this;
     }
 
-    public function hasLicense(string $license): self
+    public function equals(Package $package): self
     {
-        Assert::assertContains($license, $this->package->licenses);
+        $this->hasName($package->name);
+        $this->hasExactLicenses($package->licenses);
 
         return $this;
     }
 
-    /**
-     * @param list<string> $licenses
-     */
-    public function hasExactLicenses(array $licenses): self
+    private function hasExactLicenses(Licenses $licenses): void
     {
         Assert::assertSameSize($licenses, $this->package->licenses);
 
         foreach ($licenses as $license) {
             $this->hasLicense($license);
         }
-
-        return $this;
     }
 
-    public function equals(Package $package): self
+    private function hasLicense(string $license): void
     {
-        $this->hasName($package->name);
-        $this->hasExactLicenses($package->licenses);
-
-        return $this;
+        Assert::assertContains($license, \explode(', ', $this->package->licenses->toString()));
     }
 }