Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

[package-json-lint] Add rule for hard coded dependencies in module and devModule. #704

Merged
merged 19 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@
"url": "https://github.com/cerner/terra-toolkit/issues"
},
"package-json-lint": {
"extends": "./packages/package-json-lint-config-terra/package-json-lint.config.js"
"extends": "./packages/package-json-lint-config-terra/package-json-lint.config.js",
"projectType": "devModule",
"rules": {
"require-no-hard-coded-dependency-versions": {"severityType": "error", "allowList": ["eslint-plugin-react", "axe-core"] }
}
},
"eslintConfig": {
"extends": "./packages/eslint-config-terra/eslint.config.js",
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-config-terra/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Changed
* Updated hard coded dependency to compatible dependencies.

## 1.2.0 - (May 11, 2021)

* Added
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config-terra/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"@jest/reporters": "^25.3.0",
"babel-jest": "^26.6.3",
"identity-obj-proxy": "^3.0.0",
"jest-environment-jsdom": "26.6.2",
"jest-environment-jsdom": "^26.6.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not tell you why i locked this down, or if it was a mistake. 🤷‍♂️

"jest-mock": "^26.6.2",
"strip-ansi": "^6.0.0"
},
Expand Down
6 changes: 6 additions & 0 deletions packages/package-json-lint-config-terra/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

* Changed
* Updated `severity` to be an object with `severityType` and `allowList`.

* Added
* Added new rule `require-no-hard-coded-dependency-versions`.

## 1.0.0 - (August 13, 2021)

* Initial stable release
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module.exports = {
rules: {
'require-no-terra-base-peer-dependency-versions': 'warn',
'require-theme-context-versions': 'warn',
'require-no-hard-coded-dependency-versions': { severityType: 'error' },
'require-no-terra-base-peer-dependency-versions': { severityType: 'warn' },
'require-theme-context-versions': { severityType: 'warn' },
},
};
3 changes: 3 additions & 0 deletions packages/package-json-lint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Added
* Added new rule `require-no-hard-coded-dependency-versions`.

## 1.1.0 - (August 31, 2021)

* Added
Expand Down
2 changes: 2 additions & 0 deletions packages/package-json-lint/src/rules/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const requireThemeContextVersions = require('./require-theme-context-versions');
const requireNoTerraBasePeerDependencyVersions = require('./require-no-terra-base-peer-dependency-versions');
const requireNoHardCodedDependencyVersions = require('./require-no-hard-coded-dependency-versions');

module.exports = {
'require-no-terra-base-peer-dependency-versions': requireNoTerraBasePeerDependencyVersions,
'require-theme-context-versions': requireThemeContextVersions,
'require-no-hard-coded-dependency-versions': requireNoHardCodedDependencyVersions,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
module.exports = {
create: ({ ruleConfig, projectType, report }) => ({
dependencies: (dependencies) => {
const compatibleVersionRegexList = ['dev', 'latest', 'main', 'master', /(\^|<|<=|>|>=|~)\s{0,1}\d/, /^x\.|\.x\.|\.x$/, /[\d.]{0,2}\d\s{0,1}-\s{0,1}[\d.]{0,2}\d/];
if (projectType === 'module' || projectType === 'devModule') {
const messageString = 'require-no-hard-coded-dependency-versions';
const currentProblems = Object.keys(dependencies).map(dependencyName => {
const dependencyVersion = dependencies[dependencyName];
const isCompatibleVersion = compatibleVersionRegexList.map(versionRegex => (!!dependencyVersion.match(versionRegex))).includes(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a little digging with semver. I wonder if we can say:

const isCompatibleVersion = semver.clean(version) === null;

Copy link
Contributor

@benbcai benbcai Sep 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like semver.clean can handle ranges so syntax like ">= 2.3.0-dev.0 <2.3.0-dev.x" or even just 2.3.0-dev.x would not work so we may still need to handle those separately.

Copy link
Contributor

@ryanthemanuel ryanthemanuel Sep 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’ll handle them fine. We want them to return undefined or null. Basically anything but a solitary version should return null or undefined by the clean function. Thus they will be accepted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benbcai,
semver.clean works for >= 2.3.0-dev.0 <2.3.0-dev.x. And it will work if it has < > symbols.
It only doesn't work for 2.3.0-dev.x as this seems to be a fixed version.

If we are sure about it's going to be dev.x or branch.x, we can add it as an exception and have a check for it using regex.
What do you think? @ryanthemanuel @benbcai

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.3.0-dev.x will only resolve to a version that is released as 2.3.0-dev.x so this is effectively hard coded. I think it's fine to not allow this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. 1.0.0-rc.x does not actually resolve to 1.0.0-rc.1

From https://semver.npmjs.com/

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confused myself at first but after thinking thru this again, it all makes sense now.

if (!isCompatibleVersion && !(ruleConfig.severity.allowList && ruleConfig.severity.allowList.includes(dependencyName))) {
return `${dependencyName}@${dependencyVersion} does not satisfy requirement for the ${messageString} rule.`;
}
return undefined;
}).filter(problem => !!problem);

if (currentProblems.length) {
const lintMessage = `The dependencies for this project have hard-coded versions that violates the ${messageString} rule:\n ${currentProblems.join('\n ')}`;
report({
lintId: messageString, severity: ruleConfig.severity.severityType, lintMessage, projectType,
});
}
}
},
}),
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module.exports = ({
}) => {
const currentProblems = versionSet.map(({ name, versionRange }) => {
const dependencyVersion = dependencies[name];
if (dependencyVersion && !semver.intersects(dependencyVersion, versionRange)) {
if (dependencyVersion && !semver.intersects(dependencyVersion, versionRange) && !(ruleConfig.severity.allowList && ruleConfig.severity.allowList.includes(name))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we add more rules will this if continue to grow? Is there a way we could clean this up?

return `${name}@${dependencyVersion} does not satisfy range requirement for ${messageString}: ${name}@${versionRange}`;
}
return undefined;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`require-no-hard-coded-dependency-versions succeeds when hardcoded dependency is passed in the allowList 1`] = `undefined`;

exports[`require-no-hard-coded-dependency-versions when projectType is application succeeds when there are hardcoded dependencies 1`] = `undefined`;

exports[`require-no-hard-coded-dependency-versions when projectType is application succeeds when there are no hardcoded dependencies 1`] = `undefined`;

exports[`require-no-hard-coded-dependency-versions when projectType is devModule fails when there are hardcoded dependencies 1`] = `
Object {
"lintId": "require-no-hard-coded-dependency-versions",
"lintMessage": "The dependencies for this project have hard-coded versions that violates the require-no-hard-coded-dependency-versions rule:
[email protected] does not satisfy requirement for the require-no-hard-coded-dependency-versions rule.",
"projectType": "devModule",
"severity": "error",
}
`;

exports[`require-no-hard-coded-dependency-versions when projectType is devModule succeeds when there are no hardcoded dependencies 1`] = `undefined`;

exports[`require-no-hard-coded-dependency-versions when projectType is module fails when there are hardcoded dependencies 1`] = `
Object {
"lintId": "require-no-hard-coded-dependency-versions",
"lintMessage": "The dependencies for this project have hard-coded versions that violates the require-no-hard-coded-dependency-versions rule:
[email protected] does not satisfy requirement for the require-no-hard-coded-dependency-versions rule.",
"projectType": "module",
"severity": "error",
}
`;

exports[`require-no-hard-coded-dependency-versions when projectType is module succeeds when there are no hardcoded dependencies 1`] = `undefined`;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ Object {
"lintMessage": "The dependencies for this project do not have the minimum versions required for no terra base peer dependencies:
terra-action-footer@^1.0.0 does not satisfy range requirement for no terra base peer dependencies: terra-action-footer@>2.5.0",
"projectType": "module",
"severity": "warning",
"severity": Object {
"severityType": "warning",
},
}
`;

Expand Down Expand Up @@ -88,7 +90,9 @@ Object {
[email protected] does not satisfy range requirement for no terra base peer dependencies: terra-toggle@>3.5.0
[email protected] does not satisfy range requirement for no terra base peer dependencies: terra-visually-hidden-text@>2.4.0",
"projectType": "devModule",
"severity": "error",
"severity": Object {
"severityType": "error",
},
}
`;

Expand All @@ -97,3 +101,5 @@ exports[`require-no-terra-base-peer-dependency-versions passes when all versions
exports[`require-no-terra-base-peer-dependency-versions succeeds when there are dependencies not in the list to check 1`] = `undefined`;

exports[`require-no-terra-base-peer-dependency-versions succeeds when there are no dependencies 1`] = `undefined`;

exports[`require-no-terra-base-peer-dependency-versions succeeds when versions do not meet the required version but package is passed in allowList 1`] = `undefined`;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ Object {
"lintMessage": "The dependencies for this project do not have the minimum versions required for theming context:
terra-action-footer@^1.0.0 does not satisfy range requirement for theming context: terra-action-footer@>=2.42.0",
"projectType": "module",
"severity": "warning",
"severity": Object {
"severityType": "warning",
},
}
`;

Expand Down Expand Up @@ -87,7 +89,9 @@ Object {
terra-time-input@<4.29.0 does not satisfy range requirement for theming context: terra-time-input@>=4.29.0
terra-toolbar@<1.8.0 does not satisfy range requirement for theming context: terra-toolbar@>=1.8.0",
"projectType": "module",
"severity": "error",
"severity": Object {
"severityType": "error",
},
}
`;

Expand All @@ -96,3 +100,5 @@ exports[`require-theme-context-versions passes when all versions meet the requir
exports[`require-theme-context-versions succeeds when there are dependencies not in the list to check 1`] = `undefined`;

exports[`require-theme-context-versions succeeds when there are no dependencies 1`] = `undefined`;

exports[`require-theme-context-versions succeeds when versions do not meet the required version but package is passed in allowList 1`] = `undefined`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
const requireNoHardCodedDependencyVersions = require('../../../src/rules/require-no-hard-coded-dependency-versions');

describe('require-no-hard-coded-dependency-versions', () => {
describe('when projectType is application', () => {
it('succeeds when there are hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'application',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '1.0.0',
});
expect(results).toMatchSnapshot();
});
it('succeeds when there are no hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'application',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '^1.0.0',
});
expect(results).toMatchSnapshot();
});
});

describe('when projectType is module', () => {
it('fails when there are hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'module',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '1.0.0',
});
expect(results).toMatchSnapshot();
});
it('succeeds when there are no hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'module',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '^1.0.0',
b: '1.0.0 - 2.9999.9999',
c: '>=1.0.2 <2.1.2',
d: '>1.0.2 <=2.3.4',
e: '<1.0.0 || >=2.3.1 <2.4.5 || >=2.5.2 <3.0.0',
f: '~1.2',
g: 'latest',
h: '~1.2.3',
i: '2.x',
j: '3.3.x',
});
expect(results).toMatchSnapshot();
});
});

describe('when projectType is devModule', () => {
it('fails when there are hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'devModule',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '1.0.0',
});
expect(results).toMatchSnapshot();
});
it('succeeds when there are no hardcoded dependencies', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
},
},
projectType: 'devModule',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '^1.0.0',
b: '1.0.0 - 2.9999.9999',
c: '>=1.0.2 <2.1.2',
d: '>1.0.2 <=2.3.4',
e: '<1.0.0 || >=2.3.1 <2.4.5 || >=2.5.2 <3.0.0',
f: '~1.2',
g: 'latest',
h: '~1.2.3',
i: '2.x',
j: '3.3.x',
});
expect(results).toMatchSnapshot();
});
});

it('succeeds when hardcoded dependency is passed in the allowList', () => {
let results;
requireNoHardCodedDependencyVersions.create({
ruleConfig: {
severity: {
severityType: 'error',
allowList: ['a'],
},
},
projectType: 'devModule',
report: (issues) => {
results = issues;
},
}).dependencies({
a: '1.0.0',
});
expect(results).toMatchSnapshot();
});
});
Loading