Skip to content

Commit

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

const lintId = 'no-file-dependencies';
const nodeName = 'dependencies';
const message = 'You are using dependencies via url to local 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) && doVersContainFileUrl(packageJsonData, nodeName, config)) {
return new LintIssue(lintId, severity, nodeName, message);
const auditResult = auditDependenciesForFileUrlVersion(packageJsonData, nodeName, config);

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

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

const lintId = 'no-file-devDependencies';
const nodeName = 'devDependencies';
const message = 'You are using devDependencies via url to local 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) && doVersContainFileUrl(packageJsonData, nodeName, config)) {
return new LintIssue(lintId, severity, nodeName, message);
const auditResult = auditDependenciesForFileUrlVersion(packageJsonData, nodeName, config);

if (packageJsonData.hasOwnProperty(nodeName) && auditResult.hasFileUrlVersions) {
return new LintIssue(
lintId,
severity,
nodeName,
`You are using ${nodeName} via url to local file. Please use ${nodeName} from npm. Invalid ${nodeName} include: ${auditResult.dependenciesWithFileUrlVersion.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 @@ -370,15 +370,30 @@ export const doVersContainArchiveUrl = (packageJsonData: PackageJson | any, node
return false;
};

export interface AuditDependenciesForFileUrlVersionResponse {
hasFileUrlVersions: boolean;
dependenciesWithFileUrlVersion: string[];
dependenciesWithoutFileUrlVersion: string[];
}

/**
* Determines whether or not dependency versions contains file 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 file 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 file url.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const doVersContainFileUrl = (packageJsonData: PackageJson | any, nodeName: string, config: any): boolean => {
export const auditDependenciesForFileUrlVersion = (
// 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
): AuditDependenciesForFileUrlVersionResponse => {
let hasFileUrlVersions = false;
const dependenciesWithFileUrlVersion = [];
const dependenciesWithoutFileUrlVersion = [];

// eslint-disable-next-line no-restricted-syntax
for (const dependencyName in packageJsonData[nodeName]) {
if (hasExceptions(config) && config.exceptions.includes(dependencyName)) {
Expand All @@ -389,9 +404,16 @@ export const doVersContainFileUrl = (packageJsonData: PackageJson | any, nodeNam
const dependencyVersion = packageJsonData[nodeName][dependencyName];

if (dependencyVersion.startsWith('file:')) {
return true;
hasFileUrlVersions = true;
dependenciesWithFileUrlVersion.push(dependencyName);
} else {
dependenciesWithoutFileUrlVersion.push(dependencyName);
}
}

return false;
return {
hasFileUrlVersions,
dependenciesWithFileUrlVersion,
dependenciesWithoutFileUrlVersion,
};
};
2 changes: 1 addition & 1 deletion test/unit/rules/no-file-dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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 local file. Please use dependencies from npm.'
'You are using dependencies via url to local file. Please use dependencies from npm. Invalid dependencies include: test-module'
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/rules/no-file-devDependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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 local file. Please use devDependencies from npm.'
'You are using devDependencies via url to local file. Please use devDependencies from npm. Invalid devDependencies include: test-module'
);
});
});
Expand Down
32 changes: 25 additions & 7 deletions test/unit/validators/dependency-audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1025,17 +1025,21 @@ describe('dependency-audit Unit Tests', () => {
});
});

describe('doVersContainFileUrl method', () => {
describe('auditDependenciesForFileUrlVersion method', () => {
describe('when the node exists in the package.json file, some versions are url to file', () => {
test('with github dependency true should be returned', () => {
const packageJson = {
dependencies: {
'my-module': 'file:local-module',
},
};
const response = dependencyAudit.doVersContainFileUrl(packageJson, 'dependencies', {});
const response = dependencyAudit.auditDependenciesForFileUrlVersion(packageJson, 'dependencies', {});

expect(response).toBe(true);
expect(response).toStrictEqual({
hasFileUrlVersions: true,
dependenciesWithFileUrlVersion: ['my-module'],
dependenciesWithoutFileUrlVersion: [],
});
});
});

Expand All @@ -1050,9 +1054,19 @@ describe('dependency-audit Unit Tests', () => {
'module-from-archive': 'https://github.com/user/repo/archive/v1.2.3.tar.gz',
},
};
const response = dependencyAudit.doVersContainFileUrl(packageJson, 'dependencies', {});
const response = dependencyAudit.auditDependenciesForFileUrlVersion(packageJson, 'dependencies', {});

expect(response).toBe(false);
expect(response).toStrictEqual({
hasFileUrlVersions: false,
dependenciesWithFileUrlVersion: [],
dependenciesWithoutFileUrlVersion: [
'npm-package-json-lint',
'grunt-npm-package-json-lint',
'gulp-npm-package-json-lint',
'module-from-git',
'module-from-archive',
],
});
});
});

Expand All @@ -1065,11 +1079,15 @@ describe('dependency-audit Unit Tests', () => {
'gulp-npm-package-json-lint': '^2.0.0',
},
};
const response = dependencyAudit.doVersContainFileUrl(packageJson, 'dependencies', {
const response = dependencyAudit.auditDependenciesForFileUrlVersion(packageJson, 'dependencies', {
exceptions: ['module-from-file'],
});

expect(response).toBe(false);
expect(response).toStrictEqual({
hasFileUrlVersions: false,
dependenciesWithFileUrlVersion: [],
dependenciesWithoutFileUrlVersion: ['grunt-npm-package-json-lint', 'gulp-npm-package-json-lint'],
});
});
});
});
Expand Down
1 change: 1 addition & 0 deletions website/docs/rules/dependencies/no-file-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ With exceptions

## History

* Improved messaging when an invalid configuration is detected in version 6.2.0
* Introduced in version 4.3.0
1 change: 1 addition & 0 deletions website/docs/rules/dependencies/no-file-devDependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ With exceptions

## History

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

0 comments on commit 5c33064

Please sign in to comment.