Skip to content

Commit

Permalink
Fix an issue with how config overrides get applied
Browse files Browse the repository at this point in the history
Please see #123 for discussion with @simison.
  • Loading branch information
tclindner committed Oct 19, 2019
1 parent 09d248e commit 3d2b703
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 28 deletions.
8 changes: 4 additions & 4 deletions package.json
@@ -1,6 +1,6 @@
{
"name": "npm-package-json-lint",
"version": "4.0.0",
"version": "4.0.1",
"description": "Configurable linter for package.json files.",
"keywords": [
"lint",
Expand Down Expand Up @@ -51,15 +51,15 @@
},
"devDependencies": {
"eslint": "^6.5.1",
"eslint-config-tc": "^8.0.1",
"eslint-config-tc": "^8.1.0",
"eslint-formatter-pretty": "^2.1.1",
"eslint-plugin-import": "^2.18.2",
"eslint-plugin-jest": "^22.17.0",
"eslint-plugin-jest": "^22.19.0",
"eslint-plugin-prettier": "^3.1.1",
"figures": "^3.0.0",
"jest": "^24.9.0",
"npm-package-json-lint-config-default": "^2.0.0",
"npm-package-json-lint-config-tc": "^2.2.0",
"npm-package-json-lint-config-tc": "^3.0.0",
"prettier": "^1.18.2"
},
"engines": {
Expand Down
13 changes: 8 additions & 5 deletions src/Config.js
Expand Up @@ -31,9 +31,6 @@ class Config {
this.configFile = configFile;
this.configBaseDirectory = configBaseDirectory;
this.rules = rules;
this.explorer = cosmiconfig('npmpackagejsonlint', {
transform: cosmicConfigTransformer.transform(cwd, configBaseDirectory)
});
}

/**
Expand All @@ -54,10 +51,14 @@ class Config {
debug(`User passed config is undefined.`);
if (this.configFile) {
debug(`Config file specified, loading it.`);
config = this.explorer.loadSync(this.configFile);
config = cosmiconfig('npmpackagejsonlint', {
transform: cosmicConfigTransformer.transform(this.cwd, this.configBaseDirectory, this.configFile)
}).loadSync(this.configFile);
} else {
debug(`Config file wasn't specified, searching for config.`);
config = this.explorer.searchSync(filePathToSearch);
config = cosmiconfig('npmpackagejsonlint', {
transform: cosmicConfigTransformer.transform(this.cwd, this.configBaseDirectory, filePathToSearch)
}).searchSync(filePathToSearch);
}
} else {
debug(`User passed config is set, using it.`);
Expand All @@ -78,6 +79,8 @@ class Config {
}

debug(`Overrides applied for ${filePath}`);
debug('Final Config');
debug(config);

configValidator.validateRules(config, 'cli', this.rules);

Expand Down
24 changes: 21 additions & 3 deletions src/config/cosmicConfigTransformer.js
@@ -1,21 +1,39 @@
const path = require('path');
const debug = require('debug')('npm-package-json-lint:cosmicConfigTransformer');
const applyExtendsIfSpecified = require('./applyExtendsIfSpecified');
const applyOverrides = require('./applyOverrides');

const transform = (cwd, configBaseDirectory) => {
const transform = (cwd, configBaseDirectory, filePathBeingLinted) => {
debug(`cwd: ${cwd}`);
debug(`configBaseDirectory`);
debug(configBaseDirectory);

return cosmiconfigResult => {
debug(`cosmiconfigResult`);
debug(cosmiconfigResult);

if (!cosmiconfigResult) {
return null;
}

const {config, filepath} = cosmiconfigResult;

debug(`cosmiconfigResult.config`);
debug(config);
debug(`cosmiconfigResult.filepath`);
debug(filepath);

/* eslint-disable no-unused-vars */
const configDir = configBaseDirectory || path.dirname(filepath || '');
const npmPackageJsonLintConfig = {...config};

const configAfterExtends = applyExtendsIfSpecified(npmPackageJsonLintConfig, filepath);
const configAfterOverrides = applyOverrides(cwd, filepath, configAfterExtends.rules, configAfterExtends.overrides);
const configAfterExtends = applyExtendsIfSpecified(npmPackageJsonLintConfig, filePathBeingLinted);
const configAfterOverrides = applyOverrides(
cwd,
filePathBeingLinted,
configAfterExtends.rules,
configAfterExtends.overrides
);

return configAfterOverrides;
};
Expand Down
2 changes: 1 addition & 1 deletion src/utils/getFileList.js
Expand Up @@ -43,7 +43,7 @@ const getFileList = (patterns, cwd) => {
files.push(filePath);
});

debug('files');
debug('Final file list from `getFileList`');
debug(files);

return files;
Expand Down
18 changes: 18 additions & 0 deletions test/fixtures/monorepo/.npmpackagejsonlintrc.json
@@ -0,0 +1,18 @@
{
"extends": "npm-package-json-lint-config-default",
"overrides": [
{
"patterns": ["packages/packageOne/**/package.json"],
"rules": {
"license-type": "warning"
}
},
{
"patterns": ["packages/packageTwo/**/package.json"],
"rules": {
"license-type": "warning",
"valid-values-author": ["error", ["TC"]]
}
}
]
}
18 changes: 18 additions & 0 deletions test/fixtures/monorepo/package.json
@@ -0,0 +1,18 @@
{
"name": "npm-package-json-lint-errors",
"version": "0.1.0",
"description": "CLI app for linting package.json files.",
"keywords": [
"lint"
],
"homepage": "https://github.com/tclindner/npm-package-json-lint",
"author": "Thomas Lindner",
"repository": {
"type": "git",
"url": "https://github.com/tclindner/npm-package-json-lint"
},
"devDependencies": {
"mocha": "^2.4.5"
},
"license": false
}
18 changes: 18 additions & 0 deletions test/fixtures/monorepo/packages/packageOne/package.json
@@ -0,0 +1,18 @@
{
"name": "npm-package-json-lint-errors",
"version": "0.1.0",
"description": "CLI app for linting package.json files.",
"keywords": [
"lint"
],
"homepage": "https://github.com/tclindner/npm-package-json-lint",
"author": "Thomas Lindner",
"repository": {
"type": "git",
"url": "https://github.com/tclindner/npm-package-json-lint"
},
"devDependencies": {
"mocha": "^2.4.5"
},
"license": false
}
18 changes: 18 additions & 0 deletions test/fixtures/monorepo/packages/packageTwo/package.json
@@ -0,0 +1,18 @@
{
"name": "npm-package-json-lint-errors",
"version": "0.1.0",
"description": "CLI app for linting package.json files.",
"keywords": [
"lint"
],
"homepage": "https://github.com/tclindner/npm-package-json-lint",
"author": "Thomas Lindner",
"repository": {
"type": "git",
"url": "https://github.com/tclindner/npm-package-json-lint"
},
"devDependencies": {
"mocha": "^2.4.5"
},
"license": false
}
35 changes: 35 additions & 0 deletions test/integration/cli.test.js
Expand Up @@ -290,6 +290,41 @@ Totals
4 errors
4 warnings
0 files ignored
`;

expect(cli.stdout.toString()).toStrictEqual(expected);
expect(cli.stderr.toString()).toStrictEqual('');
expect(cli.status).toStrictEqual(twoLintErrorsDetected);
});
});

describe('when the cli is run against a monorepo with overrides', () => {
test('each file results and totals will be output', () => {
const cli = spawnSync('../../../src/cli.js', [`**/package.json`], {
env,
cwd: './test/fixtures/monorepo'
});
const expected = `
./package.json
${figures.cross} license-type - node: license - Type should be a string
1 error
0 warnings
./packages/packageOne/package.json
${figures.warning} license-type - node: license - Type should be a string
0 errors
1 warning
./packages/packageTwo/package.json
${figures.warning} license-type - node: license - Type should be a string
${figures.cross} valid-values-author - node: author - Invalid value for author
1 error
1 warning
Totals
2 errors
2 warnings
0 files ignored
`;

expect(cli.stdout.toString()).toStrictEqual(expected);
Expand Down
17 changes: 10 additions & 7 deletions test/unit/config/cosmicConfigTransformer.test.js
Expand Up @@ -13,8 +13,9 @@ describe('cosmicConfigTransformer Unit Tests', () => {
test('null should be returned', () => {
const cwd = 'cwd';
const configBaseDirectory = 'configBaseDirectory';
const filePathBeingLinted = 'myLintedFilePath';

const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory);
const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory, filePathBeingLinted);
const actual = transformer(null);
expect(actual).toBeNull();
});
Expand All @@ -31,8 +32,9 @@ describe('cosmicConfigTransformer Unit Tests', () => {

const cwd = 'cwd';
const configBaseDirectory = 'configBaseDirectory';
const filePathBeingLinted = 'myLintedFilePath';

const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory);
const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory, filePathBeingLinted);
const cosmiconfigResult = {
config: {
property: 'value'
Expand All @@ -44,9 +46,9 @@ describe('cosmicConfigTransformer Unit Tests', () => {

expect(path.dirname).toHaveBeenCalledTimes(0);
expect(applyExtendsIfSpecified).toHaveBeenCalledTimes(1);
expect(applyExtendsIfSpecified).toHaveBeenCalledWith(cosmiconfigResult.config, cosmiconfigResult.filepath);
expect(applyExtendsIfSpecified).toHaveBeenCalledWith(cosmiconfigResult.config, filePathBeingLinted);
expect(applyOverrides).toHaveBeenCalledTimes(1);
expect(applyOverrides).toHaveBeenCalledWith(cwd, cosmiconfigResult.filepath, 'rules', 'overrides');
expect(applyOverrides).toHaveBeenCalledWith(cwd, filePathBeingLinted, 'rules', 'overrides');
});
});

Expand All @@ -61,8 +63,9 @@ describe('cosmicConfigTransformer Unit Tests', () => {

const cwd = 'cwd';
const configBaseDirectory = null;
const filePathBeingLinted = 'myLintedFilePath';

const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory);
const transformer = cosmicConfigTransformer.transform(cwd, configBaseDirectory, filePathBeingLinted);
const cosmiconfigResult = {
config: {
property: 'value'
Expand All @@ -75,9 +78,9 @@ describe('cosmicConfigTransformer Unit Tests', () => {
expect(path.dirname).toHaveBeenCalledTimes(1);
expect(path.dirname).toHaveBeenCalledWith(cosmiconfigResult.filepath);
expect(applyExtendsIfSpecified).toHaveBeenCalledTimes(1);
expect(applyExtendsIfSpecified).toHaveBeenCalledWith(cosmiconfigResult.config, cosmiconfigResult.filepath);
expect(applyExtendsIfSpecified).toHaveBeenCalledWith(cosmiconfigResult.config, filePathBeingLinted);
expect(applyOverrides).toHaveBeenCalledTimes(1);
expect(applyOverrides).toHaveBeenCalledWith(cwd, cosmiconfigResult.filepath, 'rules', 'overrides');
expect(applyOverrides).toHaveBeenCalledWith(cwd, filePathBeingLinted, 'rules', 'overrides');
});
});
});
Expand Down
17 changes: 9 additions & 8 deletions test/unit/utils/getFileList.test.js
Expand Up @@ -14,13 +14,14 @@ describe('getFileList Unit Tests', () => {
expect(results[3]).toContain('/test/fixtures/hierarchyWithoutRoot/package.json');
expect(results[4]).toContain('/test/fixtures/ignorePath/package.json');
expect(results[5]).toContain('/test/fixtures/invalidConfig/package.json');
expect(results[6]).toContain('/test/fixtures/npmPackageJsonLintIgnore/package.json');
expect(results[7]).toContain('/test/fixtures/overrides/package.json');
expect(results[8]).toContain('/test/fixtures/packageJsonProperty/package.json');
expect(results[9]).toContain('/test/fixtures/valid/package.json');
expect(results[10]).toContain('/test/fixtures/warnings/package.json');
expect(results[11]).toContain('/test/fixtures/hierarchyWithoutRoot/subdirectory/package.json');
expect(results[12]).toContain('/test/fixtures/ignorePath/ignoredDirectory/package.json');
expect(results[13]).toContain('/test/fixtures/npmPackageJsonLintIgnore/ignoredDirectory/package.json');
expect(results[6]).toContain('/test/fixtures/monorepo/package.json')
expect(results[7]).toContain('/test/fixtures/npmPackageJsonLintIgnore/package.json');
expect(results[8]).toContain('/test/fixtures/overrides/package.json');
expect(results[9]).toContain('/test/fixtures/packageJsonProperty/package.json');
expect(results[10]).toContain('/test/fixtures/valid/package.json');
expect(results[11]).toContain('/test/fixtures/warnings/package.json');
expect(results[12]).toContain('/test/fixtures/hierarchyWithoutRoot/subdirectory/package.json');
expect(results[13]).toContain('/test/fixtures/ignorePath/ignoredDirectory/package.json');
expect(results[14]).toContain('/test/fixtures/npmPackageJsonLintIgnore/ignoredDirectory/package.json');
});
});

0 comments on commit 3d2b703

Please sign in to comment.