Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --allow-dependency to ignore disallowed dependency #214

Merged
merged 1 commit into from
Apr 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 31 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ Core principles:

```
usage: index.js [-h] [-v] [-t TARBALL] [-p PKGFILE] [--cache CACHE]
[--allow-types-dependencies] [--ignore-missing-fields]
[--allow-dependency DEPENDENCY] [--allow-types-dependencies]
[--ignore-missing-fields]

Opiniated linter for NPM package tarball and package.json metadata

Expand All @@ -31,8 +32,11 @@ optional arguments:
-p PKGFILE, --pkgfile PKGFILE
specify package.json location
--cache CACHE specify cache directory
--allow-dependency DEPENDENCY
explicitly allow given dependency (can be given
multiple times or as a comma-separated list)
--allow-types-dependencies
allow dependencies to `@types/*`
allow production dependencies to `@types/*`
--ignore-missing-fields
ignore errors for missing fields (but still checks for
empty and valid)
Expand Down Expand Up @@ -99,29 +103,7 @@ Examples of disallowed packages:

By default `@types/*` is disallowed but this can be disabled with `--allow-types-dependencies`.

## Obsolete dependencies

Disallows certain packages from being included as `dependencies`, `devDependencies` or `peerDependencies` entirely.
These dependencies have native replacements supported by all supported NodeJS versions.

**Why?** Obsolete packages have native replacements and thus only clutter the dependency graphs thus increasing the time to install, the size on disk and produces noise with tools analyzing `package-lock.json`.

Examples of obsolete packages:

- `mkdirp` - `fs#mkdir` supports the `recursive` flag since NodeJS v10.
- `stable` - `Array#sort` is stable since NodeJS v12.

## Deprecated dependencies

Disallows deprecated packages from being included as `dependencies`, `devDependencies` or `peerDependencies` entirely.
These dependences are explicitly marked as deprecated by the package author.

**Why?** Deprecated packages should be removed or replaced with alternatives as they are often unmaintained and might contain security vulnerabilities.

Examples of obsolete packages:

- `mkdirp` - `fs#mkdir` supports the `recursive` flag since NodeJS v10.
- `stable` - `Array#sort` is stable since NodeJS v12.
If needed, `--allow-dependency` can be used to ignore one or more dependencies.

### ESLint

Expand Down Expand Up @@ -210,6 +192,30 @@ If your `package.json` contains the `"prettier"` keyword the Prettier packages c
}
```

## Obsolete dependencies

Disallows certain packages from being included as `dependencies`, `devDependencies` or `peerDependencies` entirely.
These dependencies have native replacements supported by all supported NodeJS versions.

**Why?** Obsolete packages have native replacements and thus only clutter the dependency graphs thus increasing the time to install, the size on disk and produces noise with tools analyzing `package-lock.json`.

Examples of obsolete packages:

- `mkdirp` - `fs#mkdir` supports the `recursive` flag since NodeJS v10.
- `stable` - `Array#sort` is stable since NodeJS v12.

## Deprecated dependencies

Disallows deprecated packages from being included as `dependencies`, `devDependencies` or `peerDependencies` entirely.
These dependences are explicitly marked as deprecated by the package author.

**Why?** Deprecated packages should be removed or replaced with alternatives as they are often unmaintained and might contain security vulnerabilities.

Examples of obsolete packages:

- `mkdirp` - `fs#mkdir` supports the `recursive` flag since NodeJS v10.
- `stable` - `Array#sort` is stable since NodeJS v12.

## Shebang

Require all binaries to have UNIX-style shebang at the beginning of the file.
Expand Down
12 changes: 0 additions & 12 deletions src/__snapshots__/package-json.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,3 @@ exports[`should return engines.node supports eol version 1`] = `
},
]
`;

exports[`should return error if dependency is disallowed 1`] = `
[
{
"column": 1,
"line": 1,
"message": "eslint should be a devDependency",
"ruleId": "disallowed-dependency",
"severity": 2,
},
]
`;
11 changes: 10 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface ParsedArgs {
pkgfile: string;
tarball?: string;
ignore_missing_fields?: boolean;
allow_dependency: string[];
allow_types_dependencies?: boolean;
}

Expand Down Expand Up @@ -99,16 +100,23 @@ async function run(): Promise<void> {
parser.add_argument("-t", "--tarball", { help: "specify tarball location" });
parser.add_argument("-p", "--pkgfile", { help: "specify package.json location" });
parser.add_argument("--cache", { help: "specify cache directory" });
parser.add_argument("--allow-dependency", {
action: "append",
default: [],
metavar: "DEPENDENCY",
help: "explicitly allow given dependency (can be given multiple times or as a comma-separated list)",
});
parser.add_argument("--allow-types-dependencies", {
action: "store_true",
help: "allow dependencies to `@types/*`",
help: "allow production dependencies to `@types/*`",
});
parser.add_argument("--ignore-missing-fields", {
action: "store_true",
help: "ignore errors for missing fields (but still checks for empty and valid)",
});

const args = parser.parse_args() as ParsedArgs;
const allowedDependencies = new Set(args.allow_dependency.map((it) => it.split(",")).flat());

if (args.cache) {
await setCacheDirecory(args.cache);
Expand Down Expand Up @@ -144,6 +152,7 @@ async function run(): Promise<void> {
setupBlacklist(pkg.name);

const options: VerifyOptions = {
allowedDependencies,
allowTypesDependencies: args.allow_types_dependencies,
ignoreMissingFields: args.ignore_missing_fields,
};
Expand Down
57 changes: 53 additions & 4 deletions src/package-json.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,38 @@ it("should return error if dependency is disallowed", async () => {
const results = await verifyPackageJson(pkg, "package.json");
expect(results).toHaveLength(1);
expect(results[0].filePath).toBe("package.json");
expect(results[0].messages).toMatchSnapshot();
expect(results[0].messages).toMatchInlineSnapshot(`
[
{
"column": 1,
"line": 1,
"message": ""eslint" should be a devDependency",
"ruleId": "disallowed-dependency",
"severity": 2,
},
]
`);
});

it("should return error if aliased dependency is disallowed", async () => {
expect.assertions(3);
pkg.dependencies = {
aliased: "npm:eslint@1.2.3",
};
const results = await verifyPackageJson(pkg, "package.json");
expect(results).toHaveLength(1);
expect(results[0].filePath).toBe("package.json");
expect(results[0].messages).toMatchInlineSnapshot(`
[
{
"column": 1,
"line": 1,
"message": """aliased" ("npm:eslint")" should be a devDependency",
"ruleId": "disallowed-dependency",
"severity": 2,
},
]
`);
});

it("should return error if dependency is obsolete", async () => {
Expand All @@ -64,7 +95,7 @@ it("should return error if dependency is obsolete", async () => {
{
"column": 1,
"line": 1,
"message": "mkdirp is obsolete and should no longer be used: use native "fs.mkdir(..., { recursive: true })" instead",
"message": ""mkdirp" is obsolete and should no longer be used: use native "fs.mkdir(..., { recursive: true })" instead",
"ruleId": "obsolete-dependency",
"severity": 2,
},
Expand All @@ -81,6 +112,18 @@ it("should not return error if dependency is allowed", async () => {
expect(results).toEqual([]);
});

it("should not return error if explicitly allowed by user", async () => {
expect.assertions(1);
pkg.dependencies = {
eslint: "1.2.3",
aliased: "npm:eslint@1.2.3",
};
const results = await verifyPackageJson(pkg, "package.json", {
allowedDependencies: new Set(["eslint"]),
});
expect(results).toHaveLength(0);
});

it("should return engines.node supports eol version", async () => {
expect.assertions(3);
pkg.engines.node = ">= 8";
Expand All @@ -105,7 +148,10 @@ describe("@types", () => {

it("should not return error if @types is allowed", async () => {
expect.assertions(1);
const results = await verifyPackageJson(pkg, "package.json", { allowTypesDependencies: true });
const results = await verifyPackageJson(pkg, "package.json", {
allowedDependencies: new Set(),
allowTypesDependencies: true,
});
expect(results).toHaveLength(0);
});
});
Expand All @@ -121,7 +167,10 @@ describe("present", () => {
it("should ignore error on missing fields if ignoreMissingFields is set", async () => {
expect.assertions(1);
delete pkg.description;
const results = await verifyPackageJson(pkg, "package.json", { ignoreMissingFields: true });
const results = await verifyPackageJson(pkg, "package.json", {
allowedDependencies: new Set(),
ignoreMissingFields: true,
});
expect(results).toHaveLength(0);
});
});
Expand Down
27 changes: 23 additions & 4 deletions src/package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { verifyEngineConstraint } from "./rules/verify-engine-constraint";
import { typesNodeMatchingEngine } from "./rules/types-node-matching-engine";

export interface VerifyPackageJsonOptions {
allowedDependencies: Set<string>;
allowTypesDependencies?: boolean;
ignoreMissingFields?: boolean;
}
Expand Down Expand Up @@ -69,22 +70,40 @@ function verifyFields(pkg: PackageJson, options: VerifyPackageJsonOptions): Mess
return messages;
}

function getActualDependency(key: string, version: string): string {
/* handle npm: prefix */
if (version.startsWith("npm:")) {
const [name] = version.slice("npm:".length).split("@", 2);
return name;
}

return key;
}

function verifyDependencies(pkg: PackageJson, options: VerifyPackageJsonOptions): Message[] {
const messages: Message[] = [];
const { dependencies = {}, devDependencies = {}, peerDependencies = {} } = pkg;
const allDependencies = { ...dependencies, ...devDependencies, ...peerDependencies };

for (const dependency of Object.keys(dependencies)) {
for (const [key, version] of Object.entries(dependencies)) {
const dependency = getActualDependency(key, version);

/* skip dependencies explicitly allowed by the user */
if (options.allowedDependencies.has(dependency)) {
continue;
}

/* skip @types/* if explicitly allowed by user */
if (options.allowTypesDependencies && dependency.match(/^@types\//)) {
continue;
}

if (isDisallowedDependency(pkg, dependency)) {
const name = key === dependency ? dependency : `"${key}" ("npm:${dependency}")`;
messages.push({
ruleId: "disallowed-dependency",
severity: 2,
message: `${dependency} should be a devDependency`,
message: `"${name}" should be a devDependency`,
line: 1,
column: 1,
});
Expand All @@ -97,7 +116,7 @@ function verifyDependencies(pkg: PackageJson, options: VerifyPackageJsonOptions)
messages.push({
ruleId: "obsolete-dependency",
severity: 2,
message: `${dependency} is obsolete and should no longer be used: ${obsolete.message}`,
message: `"${dependency}" is obsolete and should no longer be used: ${obsolete.message}`,
line: 1,
column: 1,
});
Expand All @@ -110,7 +129,7 @@ function verifyDependencies(pkg: PackageJson, options: VerifyPackageJsonOptions)
export async function verifyPackageJson(
pkg: PackageJson,
filePath: string,
options: VerifyPackageJsonOptions = {},
options: VerifyPackageJsonOptions = { allowedDependencies: new Set() },
): Promise<Result[]> {
const messages: Message[] = [
...(await deprecatedDependency(pkg)),
Expand Down
2 changes: 1 addition & 1 deletion src/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export async function verify(
pkg: PackageJson,
pkgPath: string,
tarball: TarballMeta,
options: VerifyOptions = {},
options: VerifyOptions,
): Promise<Result[]> {
return [
...(await verifyTarball(pkg, tarball)),
Expand Down
2 changes: 1 addition & 1 deletion tests/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ it.each(fixtures)("%s", async (fixture) => {
const pkgPath = path.relative(ROOT_DIRECTORY, path.join(dir, "package.json"));
const pkg: PackageJson = JSON.parse(await fs.readFile(pkgPath, "utf-8"));
const tarball = { filePath: await npmPack(pkg, fixture) };
const result = await verify(pkg, pkgPath, tarball);
const result = await verify(pkg, pkgPath, tarball, { allowedDependencies: new Set() });
expect(result).toMatchSnapshot();
});