Fix an issue with how config overrides get applied (#143)

* Fix an issue with how config overrides get applied

Please see #123 for discussion with @simison.

* Update getFileList.test.js

Author: tclindner

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",
@@ -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": {

src/Config.js

@@ -31,9 +31,6 @@ class Config {
     this.configFile = configFile;
     this.configBaseDirectory = configBaseDirectory;
     this.rules = rules;
-    this.explorer = cosmiconfig('npmpackagejsonlint', {
-      transform: cosmicConfigTransformer.transform(cwd, configBaseDirectory)
-    });
   }
 
   /**
@@ -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.`);
@@ -78,6 +79,8 @@ class Config {
     }
 
     debug(`Overrides applied for ${filePath}`);
+    debug('Final Config');
+    debug(config);
 
     configValidator.validateRules(config, 'cli', this.rules);
 

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;
   };

src/utils/getFileList.js

@@ -43,7 +43,7 @@ const getFileList = (patterns, cwd) => {
     files.push(filePath);
   });
 
-  debug('files');
+  debug('Final file list from `getFileList`');
   debug(files);
 
   return files;

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"]]
+      }
+    }
+  ]
+}

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
+}

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
+}

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
+}

test/integration/cli.test.js

@@ -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);

test/unit/config/cosmicConfigTransformer.test.js

@@ -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();
       });
@@ -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'
@@ -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');
       });
     });
 
@@ -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'
@@ -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');
       });
     });
   });