-
Notifications
You must be signed in to change notification settings - Fork 37
[package-json-lint] Add rule for hard coded dependencies in module and devModule. #704
Changes from 16 commits
e33f311
6c60eb4
cc6d1e0
841d4f9
ad34194
dfc30a5
b3446c5
2786855
80c4eb1
0095a1f
bc28dc4
1f1cc1b
2b4a8d3
f7e974f
d6206c3
a3783ca
d828bdd
bce14c4
74b9ac7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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' }, | ||
}, | ||
}; |
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,32 @@ | ||
const documentation = { | ||
ruleName: 'require-no-hard-coded-dependency-versions', | ||
defaultValue: 'error', | ||
description: "This rule doesn't allow any hard-coded dependencies to be passed in the package.json. Only applies for module and devModule.", | ||
}; | ||
|
||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @benbcai, If we are sure about it's going to be There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
}); | ||
} | ||
} | ||
}, | ||
}), | ||
documentation, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
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 |
---|---|---|
|
@@ -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", | ||
}, | ||
} | ||
`; | ||
|
||
|
@@ -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", | ||
}, | ||
} | ||
`; | ||
|
||
|
@@ -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 |
---|---|---|
@@ -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(); | ||
}); | ||
}); |
There was a problem hiding this comment.
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. 🤷♂️