Skip to content

Commit

Permalink
fix: added errorResults handling in cli (#140)
Browse files Browse the repository at this point in the history
fix: added errorResults handling in cli, fixed issue with duplicated license names in results in cli, renamed describe-license-count to more generic pluralize
  • Loading branch information
Lukasz-pluszczewski authored Dec 11, 2024
1 parent 869d917 commit ce13c9f
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 53 deletions.
17 changes: 16 additions & 1 deletion packages/cli/src/components/audit-licenses/audit-result.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import IncludingUnknownResult from "./including-unknown-result.js";
import NeedsUserVerificationResult from "./needs-user-verification-result.js";
import NoLicensesFoundResult from "./no-licenses-found-result.js";
import NotFoundResult from "./not-found-result.js";
import ResultsList from "./results-list.js";
import SuccessResult from "./success-result.js";
import VerboseView from "./verbose-view.js";

Expand Down Expand Up @@ -66,9 +67,9 @@ export default function AuditResult({
warning,
overrides,
}: AuditResultProps) {
//TODO handle errorResults here
const hasNotFound = result.notFound.size > 0;
const hasNeedsUserVerification = result.needsUserVerification.size > 0;
const hasErrorResults = result.errorResults.size > 0;

return (
<Box flexDirection="column">
Expand All @@ -91,6 +92,20 @@ export default function AuditResult({
verbose={verbose}
/>
)}
{hasErrorResults && (
<ResultsList
message={["package returned error", "packages returned error"]}
results={[...result.errorResults.entries()].map(
([packageName, { errorMessage }]) => ({
packageName,
message: errorMessage,
licenses: [],
}),
)}
type="error"
renderMessages={verbose}
/>
)}
</Box>
);
}
10 changes: 9 additions & 1 deletion packages/cli/src/components/audit-licenses/license-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ export default function LicenseList({
<Text color="cyan">{licenseExpression}</Text>
) : (
<Text color="cyan">
{licenses.map(({ licenseId }) => licenseId).join(", ")}
{licenses
.filter(
(license, index, self) =>
self.findIndex(
(l) => l.licenseId === license.licenseId,
) === index,
)
.map(({ licenseId }) => licenseId)
.join(", ")}
</Text>
)}
{verbose && <Text>: {licensePath}</Text>}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { LicenseAuditResult } from "@license-auditor/data";
import figures from "figures";
import { Box, Text } from "ink";
import { describeLicenseCount } from "../../utils/describe-license-count.js";
import { pluralize } from "../../utils/pluralize.js";

export default function NeedsUserVerificationResult({
needsUserVerification,
Expand All @@ -12,7 +12,7 @@ export default function NeedsUserVerificationResult({
> & {
verbose: boolean;
}) {
const describePackagesCount = describeLicenseCount(
const describePackagesCount = pluralize(
needsUserVerification.size,
"package is",
"packages are",
Expand Down
12 changes: 4 additions & 8 deletions packages/cli/src/components/audit-licenses/result-messages.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import figures from "figures";
import { Text } from "ink";
import { describeLicenseCount } from "../../utils/describe-license-count.js";
import { pluralize } from "../../utils/pluralize.js";

interface MessageProps {
count: number;
Expand All @@ -10,9 +10,7 @@ export function CompliantMessage({ count }: MessageProps) {
return (
<>
<Text color="green">{figures.tick}</Text>
<Text>
{describeLicenseCount(count, "license is", "licenses are")} compliant
</Text>
<Text>{pluralize(count, "license is", "licenses are")} compliant</Text>
</>
);
}
Expand All @@ -22,7 +20,7 @@ export function BlacklistedMessage({ count }: MessageProps) {
<>
<Text color="red">{figures.cross}</Text>
<Text>
{describeLicenseCount(count, "license is", "licenses are")} blacklisted:
{` ${pluralize(count, "license is", "licenses are")} blacklisted:`}
</Text>
</>
);
Expand All @@ -32,9 +30,7 @@ export function UnknownMessage({ count }: MessageProps) {
return (
<>
<Text color="yellow">{figures.warning}</Text>
<Text>
{describeLicenseCount(count, "license is", "licenses are")} unknown:
</Text>
<Text>{pluralize(count, "license is", "licenses are")} unknown:</Text>
</>
);
}
Expand Down
46 changes: 46 additions & 0 deletions packages/cli/src/components/audit-licenses/results-list.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import figures from "figures";
import { Box, Text } from "ink";
import { pluralize } from "../../utils/pluralize.js";

type ResultsListProps = {
message: [string, string];
results: { packageName: string; licenses: string[]; message: string }[];
type: "ok" | "warning" | "error";
renderMessages: boolean;
};

const ICONS = {
ok: <Text color="green">{figures.tick}</Text>,
warning: <Text color="yellow">{figures.warning}</Text>,
error: <Text color="red">{figures.cross}</Text>,
};

export default function ResultsList({
message,
results,
type,
renderMessages,
}: ResultsListProps) {
const pluralizedMessage = pluralize(results.length, message[0], message[1]);

return (
<Box flexDirection="column">
<Box>
{ICONS[type]}
<Text>{` ${pluralizedMessage}`}:</Text>
</Box>
<Box flexDirection="column" marginLeft={2}>
{results.map(({ packageName, message }) => (
<Box key={packageName} marginBottom={renderMessages ? 1 : 0}>
<Text color="gray">{figures.pointerSmall}</Text>
{renderMessages ? (
<Text> {message} </Text>
) : (
<Text> {packageName} </Text>
)}
</Box>
))}
</Box>
</Box>
);
}
5 changes: 5 additions & 0 deletions packages/cli/src/components/audit-licenses/verbose-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ export default function VerboseView({ result, filter }: VerboseViewProps) {
license: detectedLicense.licenseExpression
? detectedLicense.licenseExpression
: detectedLicense.licenses
.filter(
(license, index, self) =>
self.findIndex((l) => l.licenseId === license.licenseId) ===
index,
)
.map((license) => license.licenseId)
.join(", "),
deprecated: detectedLicense.licenses.some(
Expand Down
7 changes: 3 additions & 4 deletions packages/cli/src/components/override-result.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ConfigType, LicenseAuditResult } from "@license-auditor/data";
import figures from "figures";
import { Box, Text } from "ink";
import { describeLicenseCount } from "../utils/describe-license-count.js";
import { pluralize } from "../utils/pluralize.js";

export function OverrideResult({
configOverrides,
Expand Down Expand Up @@ -29,9 +29,8 @@ export function OverrideResult({
<Box>
<Text color="grey">{figures.warning} </Text>
<Text>
Skipped audit for{" "}
{describeLicenseCount(overrideCount, "license", "licenses")} defined
in the config file overrides field.
Skipped audit for {pluralize(overrideCount, "license", "licenses")}{" "}
defined in the config file overrides field.
</Text>
</Box>
{warns.length > 0 ? (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function describeLicenseCount(
export function pluralize(
count: number,
singular: string,
plural: string,
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/audit-licenses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export async function auditLicenses({
await mapLicensesToStatus(licenses, config);

return {
groupedByStatus: groupedByStatus,
notFound: notFound,
overrides: overrides,
warning: warning,
needsUserVerification: needsUserVerification,
errorResults: errorResults,
groupedByStatus,
notFound,
overrides,
warning,
needsUserVerification,
errorResults,
};
}
1 change: 0 additions & 1 deletion packages/core/src/get-all-licenses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export async function getAllLicenses({
const packageJsonResult = readPackageJson(packagePath);

if (!packageJsonResult.success) {
//To code reviewer: Should we handle in in this or different way?
errorResults.set(packageNameFromPath, {
packagePath,
errorMessage: packageJsonResult.errorMessage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,31 +47,3 @@ describe("retrieveLicenseFromLicenseFileContent", () => {
expect(sortedLicenses).toEqual(expectedLicenses);
});
});

// describe("retrieveLicenseFromLicenseFileName", () => {
// it("should return an empty array when file name does not match any licenses", () => {
// const filePath = "README.md";
// const result = retrieveLicenseFromLicenseFileName(filePath);
// expect(result.licenses).toEqual([]);
// });

// it("should return the correct license when file name matches a license key", () => {
// const filePath = "LICENSE-MIT.txt";
// const expectedLicense = LicenseSchema.parse(licenseMap.get("MIT"));
// const result = retrieveLicenseFromLicenseFileName(filePath);
// expect(result.licenses).toEqual([expectedLicense]);
// });

// it("should return the correct license when file name matches a license name", () => {
// const filePath = "LICENSE-Apache-2.0";
// const expectedLicense = LicenseSchema.parse(licenseMap.get("Apache-2.0"));
// const result = retrieveLicenseFromLicenseFileName(filePath);
// expect(result.licenses).toEqual([expectedLicense]);
// });

// it("should return an empty array when file name does not match any known license pattern", () => {
// const filePath = "LICENSE-UNKNOWN";
// const result = retrieveLicenseFromLicenseFileName(filePath);
// expect(result.licenses).toEqual([]);
// });
// });
2 changes: 1 addition & 1 deletion packages/core/src/map-licenses-to-statuses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export async function mapLicensesToStatus(
),
});

// we don't continue here because we want this package to appear in the blacklisted results
// we don't "continue" here because we want this package to appear in the blacklisted results
}

const statusOfAllLicenses = resolveLicenseStatus(licensesWithPath, config);
Expand Down

0 comments on commit ce13c9f

Please sign in to comment.