Skip to content

Commit

Permalink
Improve messaging for no-archive-* rules (#607)
Browse files Browse the repository at this point in the history
  • Loading branch information
tclindner authored Mar 19, 2022
1 parent 5c33064 commit 85a8424
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 50 deletions.
16 changes: 12 additions & 4 deletions src/rules/no-archive-dependencies.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import {PackageJson} from 'type-fest';
import {doVersContainArchiveUrl} from '../validators/dependency-audit';
import {auditDependenciesForArchiveUrlVersion} from '../validators/dependency-audit';
import {LintIssue} from '../lint-issue';
import {RuleType} from '../types/rule-type';
import {Severity} from '../types/severity';

const lintId = 'no-archive-dependencies';
const nodeName = 'dependencies';
const message = 'You are using dependencies via url to archive file. Please use dependencies from npm.';

export const ruleType = RuleType.OptionalObject;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const lint = (packageJsonData: PackageJson | any, severity: Severity, config: any): LintIssue | null => {
if (packageJsonData.hasOwnProperty(nodeName) && doVersContainArchiveUrl(packageJsonData, nodeName, config)) {
return new LintIssue(lintId, severity, nodeName, message);
const auditResult = auditDependenciesForArchiveUrlVersion(packageJsonData, nodeName, config);

if (packageJsonData.hasOwnProperty(nodeName) && auditResult.hasArchiveUrlVersions) {
return new LintIssue(
lintId,
severity,
nodeName,
`You are using ${nodeName} via url to archive file. Please use ${nodeName} from npm. Invalid ${nodeName} include: ${auditResult.dependenciesWithArchiveUrlVersion.join(
', '
)}`
);
}

return null;
Expand Down
16 changes: 12 additions & 4 deletions src/rules/no-archive-devDependencies.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import {PackageJson} from 'type-fest';
import {LintIssue} from '../lint-issue';
import {doVersContainArchiveUrl} from '../validators/dependency-audit';
import {auditDependenciesForArchiveUrlVersion} from '../validators/dependency-audit';
import {RuleType} from '../types/rule-type';
import {Severity} from '../types/severity';

const lintId = 'no-archive-devDependencies';
const nodeName = 'devDependencies';
const message = 'You are using devDependencies via url to archive file. Please use devDependencies from npm.';

export const ruleType = RuleType.OptionalObject;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const lint = (packageJsonData: PackageJson | any, severity: Severity, config: any): LintIssue | null => {
if (packageJsonData.hasOwnProperty(nodeName) && doVersContainArchiveUrl(packageJsonData, nodeName, config)) {
return new LintIssue(lintId, severity, nodeName, message);
const auditResult = auditDependenciesForArchiveUrlVersion(packageJsonData, nodeName, config);

if (packageJsonData.hasOwnProperty(nodeName) && auditResult.hasArchiveUrlVersions) {
return new LintIssue(
lintId,
severity,
nodeName,
`You are using ${nodeName} via url to archive file. Please use ${nodeName} from npm. Invalid ${nodeName} include: ${auditResult.dependenciesWithArchiveUrlVersion.join(
', '
)}`
);
}

return null;
Expand Down
38 changes: 30 additions & 8 deletions src/validators/dependency-audit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,15 +344,30 @@ export const doVersContainGitRepository = (packageJsonData: PackageJson | any, n
return false;
};

export interface AuditDependenciesForArchiveUrlVersionResponse {
hasArchiveUrlVersions: boolean;
dependenciesWithArchiveUrlVersion: string[];
dependenciesWithoutArchiveUrlVersion: string[];
}

/**
* Determines whether or not dependency versions contains archive url
* @param {object} packageJsonData Valid JSON
* @param {string} nodeName Name of a node in the package.json file
* @param {object} config Rule configuration
* @return {boolean} True if the package contain archive url.
* @param packageJsonData Valid JSON
* @param nodeName Name of a node in the package.json file
* @param config Rule configuration
* @return True if the package contain archive url.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const doVersContainArchiveUrl = (packageJsonData: PackageJson | any, nodeName: string, config: any): boolean => {
export const auditDependenciesForArchiveUrlVersion = (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
packageJsonData: PackageJson | any,
nodeName: string,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
config: any
): AuditDependenciesForArchiveUrlVersionResponse => {
let hasArchiveUrlVersions = false;
const dependenciesWithArchiveUrlVersion = [];
const dependenciesWithoutArchiveUrlVersion = [];

// eslint-disable-next-line no-restricted-syntax
for (const dependencyName in packageJsonData[nodeName]) {
if (hasExceptions(config) && config.exceptions.includes(dependencyName)) {
Expand All @@ -363,11 +378,18 @@ export const doVersContainArchiveUrl = (packageJsonData: PackageJson | any, node
const dependencyVersion = packageJsonData[nodeName][dependencyName];

if (isArchiveUrl(dependencyVersion)) {
return true;
hasArchiveUrlVersions = true;
dependenciesWithArchiveUrlVersion.push(dependencyName);
} else {
dependenciesWithoutArchiveUrlVersion.push(dependencyName);
}
}

return false;
return {
hasArchiveUrlVersions,
dependenciesWithArchiveUrlVersion,
dependenciesWithoutArchiveUrlVersion,
};
};

export interface AuditDependenciesForFileUrlVersionResponse {
Expand Down
2 changes: 1 addition & 1 deletion src/validators/property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {PackageJson} from 'type-fest';
* @param nodeName Name of a node in the package.json file
* @return True if the node exists. False if it is not.
*/
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/no-explicit-any
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const exists = (packageJsonData: PackageJson | any, nodeName: string): boolean =>
packageJsonData.hasOwnProperty(nodeName);

Expand Down
12 changes: 6 additions & 6 deletions src/validators/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {PackageJson} from 'type-fest';
* @param nodeName Name of a node in the package.json file
* @return True if the node is an array or is missing. False if it is not.
*/
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/no-explicit-any
export const isArray = (packageJsonData: PackageJson | any, nodeName: string) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const isArray = (packageJsonData: PackageJson | any, nodeName: string): boolean => {
if (!packageJsonData.hasOwnProperty(nodeName)) {
return true;
}
Expand All @@ -24,8 +24,8 @@ export const isArray = (packageJsonData: PackageJson | any, nodeName: string) =>
* @param nodeName Name of a node in the package.json file
* @return True if the node is a boolean or is missing. False if it is not.
*/
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/no-explicit-any
export const isBoolean = (packageJsonData: PackageJson | any, nodeName: string) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const isBoolean = (packageJsonData: PackageJson | any, nodeName: string): boolean => {
if (!packageJsonData.hasOwnProperty(nodeName)) {
return true;
}
Expand All @@ -40,8 +40,8 @@ export const isBoolean = (packageJsonData: PackageJson | any, nodeName: string)
* @param nodeName Name of a node in the package.json file
* @return True if the node is an object or is missing. False if it is not.
*/
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/no-explicit-any
export const isObject = (packageJsonData: PackageJson | any, nodeName: string) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const isObject = (packageJsonData: PackageJson | any, nodeName: string): boolean => {
if (!packageJsonData.hasOwnProperty(nodeName)) {
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/rules/no-archive-dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('no-archive-dependencies Unit Tests', () => {
expect(response.severity).toStrictEqual('error');
expect(response.node).toStrictEqual('dependencies');
expect(response.lintMessage).toStrictEqual(
'You are using dependencies via url to archive file. Please use dependencies from npm.'
'You are using dependencies via url to archive file. Please use dependencies from npm. Invalid dependencies include: test-module'
);
});
});
Expand All @@ -40,7 +40,7 @@ describe('no-archive-dependencies Unit Tests', () => {
expect(response.severity).toStrictEqual('error');
expect(response.node).toStrictEqual('dependencies');
expect(response.lintMessage).toStrictEqual(
'You are using dependencies via url to archive file. Please use dependencies from npm.'
'You are using dependencies via url to archive file. Please use dependencies from npm. Invalid dependencies include: test-module'
);
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/unit/rules/no-archive-devDependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('no-archive-devDependencies Unit Tests', () => {
expect(response.severity).toStrictEqual('error');
expect(response.node).toStrictEqual('devDependencies');
expect(response.lintMessage).toStrictEqual(
'You are using devDependencies via url to archive file. Please use devDependencies from npm.'
'You are using devDependencies via url to archive file. Please use devDependencies from npm. Invalid devDependencies include: test-module'
);
});
});
Expand All @@ -40,7 +40,7 @@ describe('no-archive-devDependencies Unit Tests', () => {
expect(response.severity).toStrictEqual('error');
expect(response.node).toStrictEqual('devDependencies');
expect(response.lintMessage).toStrictEqual(
'You are using devDependencies via url to archive file. Please use devDependencies from npm.'
'You are using devDependencies via url to archive file. Please use devDependencies from npm. Invalid devDependencies include: test-module'
);
});
});
Expand Down
64 changes: 41 additions & 23 deletions test/unit/validators/dependency-audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,23 @@ describe('dependency-audit Unit Tests', () => {
expect(response).toBe(true);
});
});

describe('when the node exists in the package.json file, one absolute version but has expection', () => {
test('false should be returned', () => {
const packageJson = {
dependencies: {
'module-from-archive': 'https://github.com/miripiruni/repo/archive/v1.2.3.zip',
'grunt-npm-package-json-lint': '2.0.0',
'gulp-npm-package-json-lint': '^2.0.0',
},
};
const response = dependencyAudit.doVersContainGitRepository(packageJson, 'dependencies', {
exceptions: ['module-from-archive'],
});

expect(response).toBe(false);
});
});
});

describe('doVersContainNonAbsolute method', () => {
Expand Down Expand Up @@ -965,17 +982,21 @@ describe('dependency-audit Unit Tests', () => {
});
});

describe('doVersContainArchiveUrl method', () => {
describe('auditDependenciesForArchiveUrlVersion method', () => {
describe('when the node exists in the package.json file, some versions are archive url', () => {
test('with tar.gz dependency true should be returned', () => {
const packageJson = {
dependencies: {
'my-module': 'https://github.com/miripiruni/repo/archive/v1.2.3.tar.gz',
},
};
const response = dependencyAudit.doVersContainArchiveUrl(packageJson, 'dependencies', {});
const response = dependencyAudit.auditDependenciesForArchiveUrlVersion(packageJson, 'dependencies', {});

expect(response).toBe(true);
expect(response).toStrictEqual({
hasArchiveUrlVersions: true,
dependenciesWithArchiveUrlVersion: ['my-module'],
dependenciesWithoutArchiveUrlVersion: [],
});
});

test('with zip dependency true should be returned', () => {
Expand All @@ -984,9 +1005,13 @@ describe('dependency-audit Unit Tests', () => {
'my-module': 'https://github.com/miripiruni/repo/archive/v1.2.3.zip',
},
};
const response = dependencyAudit.doVersContainArchiveUrl(packageJson, 'dependencies', {});
const response = dependencyAudit.auditDependenciesForArchiveUrlVersion(packageJson, 'dependencies', {});

expect(response).toBe(true);
expect(response).toStrictEqual({
hasArchiveUrlVersions: true,
dependenciesWithArchiveUrlVersion: ['my-module'],
dependenciesWithoutArchiveUrlVersion: [],
});
});
});

Expand All @@ -1001,26 +1026,19 @@ describe('dependency-audit Unit Tests', () => {
'module-from-archive': 'https://github.com/user/repo.git',
},
};
const response = dependencyAudit.doVersContainArchiveUrl(packageJson, 'dependencies', {});
const response = dependencyAudit.auditDependenciesForArchiveUrlVersion(packageJson, 'dependencies', {});

expect(response).toBe(false);
});
});

describe('when the node exists in the package.json file, one absolute version but has expection', () => {
test('false should be returned', () => {
const packageJson = {
dependencies: {
'module-from-archive': 'https://github.com/miripiruni/repo/archive/v1.2.3.zip',
'grunt-npm-package-json-lint': '2.0.0',
'gulp-npm-package-json-lint': '^2.0.0',
},
};
const response = dependencyAudit.doVersContainGitRepository(packageJson, 'dependencies', {
exceptions: ['module-from-archive'],
expect(response).toStrictEqual({
hasArchiveUrlVersions: false,
dependenciesWithArchiveUrlVersion: [],
dependenciesWithoutArchiveUrlVersion: [
'npm-package-json-lint',
'grunt-npm-package-json-lint',
'gulp-npm-package-json-lint',
'module-from-local',
'module-from-archive',
],
});

expect(response).toBe(false);
});
});
});
Expand Down
1 change: 1 addition & 0 deletions website/docs/rules/dependencies/no-archive-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,5 @@ With exceptions

## History

* Improved messaging when an invalid configuration is detected in version 6.3.0
* Introduced in version 4.3.0
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,5 @@ With exceptions

## History

* Improved messaging when an invalid configuration is detected in version 6.3.0
* Introduced in version 4.3.0

0 comments on commit 85a8424

Please sign in to comment.