Updates array schema validation to work with prefer-property-order (#144)

tclindner · web-flow · commit 8725cbb3c212 · 2019-10-19T15:13:22.000-05:00
* Updates array schema validation to work with prefer-property-order Fixes #142 * Update prefer-property-order.test.js
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+## [4.0.2] - 2019-10-19
+### Fixed
+- Resolved an issue with config schema validation that caused `prefer-property-order` to fail with valid config. Addresses [#142](https://github.com/tclindner/npm-package-json-lint/issues/142) from @sakthivel-tw.
+
 ## [4.0.1] - 2019-10-19
 ### Fixed
 - Resolved an issue with applying override config. Please see [#123](https://github.com/tclindner/npm-package-json-lint/issues/123) for more details. Thank you @simison for reporting this issue.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "npm-package-json-lint",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "description": "Configurable linter for package.json files.",
   "keywords": [
     "lint",
diff --git a/src/config/ConfigSchema.js b/src/config/ConfigSchema.js
@@ -24,31 +24,33 @@ const standardRuleSchema = {
   }
 };
 
-const arrayRuleSchema = {
-  type: 'array',
-  items: [
-    standardRuleSchema,
-    {
-      type: 'array',
-      minItems: 1,
-      uniqueItems: true,
-      errorMessage: {
-        type: 'the second item in an array rule config must be an array.',
-        minItems: 'the second item in an array rule config must have at least 1 item.',
-        uniqueItems: 'the second item in an array rule config must have unique items.'
+const arrayRuleSchema = minItems => {
+  return {
+    type: 'array',
+    items: [
+      standardRuleSchema,
+      {
+        type: 'array',
+        minItems,
+        uniqueItems: true,
+        errorMessage: {
+          type: 'the second item in an array rule config must be an array.',
+          minItems: 'the second item in an array rule config must have at least 1 item.',
+          uniqueItems: 'the second item in an array rule config must have unique items.'
+        }
       }
+    ],
+    minItems: 2,
+    maxItems: 2,
+    additionalItems: false,
+    errorMessage: {
+      type: 'rule config must be an array, e.g. ["error", ["value1", "value2"]].',
+      minItems: 'array rules must have two items, severity and options array. e.g. ["error", ["value1", "value2"]].',
+      maxItems: 'array rules must have two items, severity and options array. e.g. ["error", ["value1", "value2"]].',
+      additionalItems:
+        'array rules are only allowed two items, severity and the list is values. e.g. ["error", ["value1", "value2"]].'
     }
-  ],
-  minItems: 2,
-  maxItems: 2,
-  additionalItems: false,
-  errorMessage: {
-    type: 'rule config must be an array, e.g. ["error", ["value1", "value2"]].',
-    minItems: 'array rules must have two items, severity and options array. e.g. ["error", ["value1", "value2"]].',
-    maxItems: 'array rules must have two items, severity and options array. e.g. ["error", ["value1", "value2"]].',
-    additionalItems:
-      'array rules are only allowed two items, severity and the list is values. e.g. ["error", ["value1", "value2"]].'
-  }
+  };
 };
 
 const objectRuleSchema = {
@@ -150,10 +152,11 @@ const isStandardRuleSchemaValid = ruleConfig => {
  * Validates array rules config.
  *
  * @param {Object} ruleConfig The ruleConfig object to validate.
+ * @param {number} minItems   Min number of items in the array
  * @returns {boolean} True if valid. Error if not.
  */
-const isArrayRuleSchemaValid = ruleConfig => {
-  const validate = ajv.compile(arrayRuleSchema);
+const isArrayRuleSchemaValid = (ruleConfig, minItems) => {
+  const validate = ajv.compile(arrayRuleSchema(minItems));
   const isValid = validate(ruleConfig);
 
   if (!isValid) {
diff --git a/src/config/ConfigValidator.js b/src/config/ConfigValidator.js
@@ -46,10 +46,11 @@ const isOptionalObjRuleConfigValid = ruleConfig => {
  * Validates array rule config
  *
  * @param  {Array}     ruleConfig  Array rule
+ * @param  {number}    minItems    Min number of items in the array
  * @return {Boolean}               True if config is valid, false if not
  * @static
  */
-const isArrayRuleConfigValid = ruleConfig => {
+const isArrayRuleConfigValid = (ruleConfig, minItems) => {
   if (typeof ruleConfig === 'string' && ruleConfig === 'off') {
     return true;
   }
@@ -58,7 +59,7 @@ const isArrayRuleConfigValid = ruleConfig => {
     throw new Error('\t- is an array type rule. It must be set to "off" if an array is not supplied.');
   }
 
-  return ConfigSchema.isArrayRuleSchemaValid(ruleConfig);
+  return ConfigSchema.isArrayRuleSchemaValid(ruleConfig, minItems);
 };
 
 /**
@@ -85,7 +86,7 @@ const validateRule = (ruleModule, ruleName, userConfig, source) => {
   if (ruleModule) {
     try {
       if (ruleModule.ruleType === 'array') {
-        isArrayRuleConfigValid(userConfig);
+        isArrayRuleConfigValid(userConfig, ruleModule.minItems);
       } else if (ruleModule.ruleType === 'object') {
         isObjectRuleConfigValid(userConfig);
       } else if (ruleModule.ruleType === 'optionalObject') {
diff --git a/src/rules/no-restricted-dependencies.js b/src/rules/no-restricted-dependencies.js
@@ -5,6 +5,7 @@ const lintId = 'no-restricted-dependencies';
 const nodeName = 'dependencies';
 const message = 'You are using a restricted dependency. Please remove it.';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, invalidDependencies) => {
   if (hasDependency(packageJsonData, nodeName, invalidDependencies)) {
@@ -16,5 +17,6 @@ const lint = (packageJsonData, severity, invalidDependencies) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/no-restricted-devDependencies.js b/src/rules/no-restricted-devDependencies.js
@@ -5,6 +5,7 @@ const lintId = 'no-restricted-devDependencies';
 const nodeName = 'devDependencies';
 const message = 'You are using a restricted dependency. Please remove it.';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, invalidDependencies) => {
   if (hasDependency(packageJsonData, nodeName, invalidDependencies)) {
@@ -16,5 +17,6 @@ const lint = (packageJsonData, severity, invalidDependencies) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/no-restricted-pre-release-dependencies.js b/src/rules/no-restricted-pre-release-dependencies.js
@@ -5,6 +5,7 @@ const lintId = 'no-restricted-pre-release-dependencies';
 const nodeName = 'dependencies';
 const message = 'You are using a restricted pre-release dependency. Please remove it.';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, invalidPreRelDeps) => {
   if (hasDepPrereleaseVers(packageJsonData, nodeName, invalidPreRelDeps)) {
@@ -16,5 +17,6 @@ const lint = (packageJsonData, severity, invalidPreRelDeps) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/no-restricted-pre-release-devDependencies.js b/src/rules/no-restricted-pre-release-devDependencies.js
@@ -5,6 +5,7 @@ const lintId = 'no-restricted-pre-release-devDependencies';
 const nodeName = 'devDependencies';
 const message = 'You are using a restricted pre-release dependency. Please remove it.';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, invalidPreRelDeps) => {
   if (hasDepPrereleaseVers(packageJsonData, nodeName, invalidPreRelDeps)) {
@@ -16,5 +17,6 @@ const lint = (packageJsonData, severity, invalidPreRelDeps) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/prefer-property-order.js b/src/rules/prefer-property-order.js
@@ -5,6 +5,7 @@ const lintId = 'prefer-property-order';
 const nodeName = '';
 const message = 'Your package.json properties are not in the desired order.';
 const ruleType = 'array';
+const minItems = 0;
 
 const lint = (packageJsonData, severity, preferredOrder) => {
   const result = isInPreferredOrder(packageJsonData, preferredOrder);
@@ -18,5 +19,6 @@ const lint = (packageJsonData, severity, preferredOrder) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-author.js b/src/rules/valid-values-author.js
@@ -7,6 +7,7 @@ const lintId = 'valid-values-author';
 const nodeName = 'author';
 const message = 'Invalid value for author';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, validValues) => {
   let value;
@@ -32,5 +33,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-engines.js b/src/rules/valid-values-engines.js
@@ -9,6 +9,7 @@ const lintId = 'valid-values-engines';
 const nodeName = 'engines';
 const message = 'Invalid value for engines';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, validValues) => {
   if (packageJsonData.hasOwnProperty(nodeName)) {
@@ -42,5 +43,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-license.js b/src/rules/valid-values-license.js
@@ -5,6 +5,7 @@ const lintId = 'valid-values-license';
 const nodeName = 'license';
 const message = 'Invalid value for license';
 const ruleType = 'array';
+const minItems = 1;
 
 /**
  * Lints package.json file to check for valid values in the license field
@@ -24,5 +25,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-name-scope.js b/src/rules/valid-values-name-scope.js
@@ -5,6 +5,7 @@ const lintId = 'valid-values-name-scope';
 const nodeName = 'name';
 const message = 'Invalid value for name scope';
 const ruleType = 'array';
+const minItems = 1;
 
 /**
  * Lints package.json file to check for valid scope values in the name field
@@ -26,5 +27,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-private.js b/src/rules/valid-values-private.js
@@ -5,6 +5,7 @@ const lintId = 'valid-values-private';
 const nodeName = 'private';
 const message = 'Invalid value for private';
 const ruleType = 'array';
+const minItems = 1;
 
 /**
  * [function description]
@@ -23,5 +24,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/src/rules/valid-values-publishConfig.js b/src/rules/valid-values-publishConfig.js
@@ -6,6 +6,7 @@ const lintId = 'valid-values-publishConfig';
 const nodeName = 'publishConfig';
 const message = 'Invalid value for publishConfig';
 const ruleType = 'array';
+const minItems = 1;
 
 const lint = (packageJsonData, severity, validValues) => {
   if (packageJsonData.hasOwnProperty(nodeName)) {
@@ -26,5 +27,6 @@ const lint = (packageJsonData, severity, validValues) => {
 
 module.exports = {
   lint,
-  ruleType
+  ruleType,
+  minItems
 };
diff --git a/test/fixtures/monorepo/.npmpackagejsonlintrc.json b/test/fixtures/monorepo/.npmpackagejsonlintrc.json
@@ -4,7 +4,8 @@
     {
       "patterns": ["packages/packageOne/**/package.json"],
       "rules": {
-        "license-type": "warning"
+        "license-type": "warning",
+        "prefer-property-order": ["error", []]
       }
     },
     {
diff --git a/test/integration/cli.test.js b/test/integration/cli.test.js
@@ -312,7 +312,8 @@ ${figures.cross} license-type - node: license - Type should be a string
 
 ./packages/packageOne/package.json
 ${figures.warning} license-type - node: license - Type should be a string
-0 errors
+${figures.cross} prefer-property-order - node:  - Your package.json properties are not in the desired order. Please move "author" after "license".
+1 error
 1 warning
 
 ./packages/packageTwo/package.json
@@ -322,7 +323,7 @@ ${figures.cross} valid-values-author - node: author - Invalid value for author
 1 warning
 
 Totals
-2 errors
+3 errors
 2 warnings
 0 files ignored
 `;
diff --git a/test/unit/config/ConfigValidator.test.js b/test/unit/config/ConfigValidator.test.js
@@ -133,6 +133,17 @@ describe('configValidator Unit Tests', () => {
         });
       });
 
+      describe('when a valid array rule config is passed (scenario 2)', () => {
+        test('true should be returned', () => {
+          const ruleConfig = {
+            'prefer-property-order': ['error', []]
+          };
+          const source = 'cli';
+
+          configValidator.validateRules(ruleConfig, source, rules);
+        });
+      });
+
       describe('when a valid array rule config is passed with a value of off', () => {
         test('true should be returned', () => {
           const ruleConfig = {
diff --git a/test/unit/rules/no-restricted-dependencies.test.js b/test/unit/rules/no-restricted-dependencies.test.js
@@ -1,6 +1,6 @@
 const ruleModule = require('./../../../src/rules/no-restricted-dependencies');
 
-const {lint, ruleType} = ruleModule;
+const {lint, ruleType, minItems} = ruleModule;
 
 describe('no-restricted-dependencies Unit Tests', () => {
   describe('a rule type value should be exported', () => {
@@ -9,6 +9,12 @@ describe('no-restricted-dependencies Unit Tests', () => {
     });
   });
 
+  describe('a minItems value should be exported', () => {
+    test('it should equal 1', () => {
+      expect(minItems).toStrictEqual(1);
+    });
+  });
+
   describe('when package.json has node with a restricted value', () => {
     test('LintIssue object should be returned', () => {
       const packageJsonData = {
diff --git a/test/unit/rules/no-restricted-devDependencies.test.js b/test/unit/rules/no-restricted-devDependencies.test.js
@@ -1,6 +1,6 @@
 const ruleModule = require('./../../../src/rules/no-restricted-devDependencies');
 
-const {lint, ruleType} = ruleModule;
+const {lint, ruleType, minItems} = ruleModule;
 
 describe('no-restricted-devDependencies Unit Tests', () => {
   describe('a rule type value should be exported', () => {
@@ -9,6 +9,12 @@ describe('no-restricted-devDependencies Unit Tests', () => {
     });
   });
 
+  describe('a minItems value should be exported', () => {
+    test('it should equal 1', () => {
+      expect(minItems).toStrictEqual(1);
+    });
+  });
+
   describe('when package.json has node with a restricted value', () => {
     test('LintIssue object should be returned', () => {
       const packageJsonData = {
diff --git a/test/unit/rules/no-restricted-pre-release-dependencies.test.js b/test/unit/rules/no-restricted-pre-release-dependencies.test.js
diff --git a/test/unit/rules/no-restricted-pre-release-devDependencies.test.js b/test/unit/rules/no-restricted-pre-release-devDependencies.test.js
diff --git a/test/unit/rules/prefer-property-order.test.js b/test/unit/rules/prefer-property-order.test.js
diff --git a/test/unit/rules/valid-values-author.test.js b/test/unit/rules/valid-values-author.test.js
diff --git a/test/unit/rules/valid-values-engines.test.js b/test/unit/rules/valid-values-engines.test.js
diff --git a/test/unit/rules/valid-values-license.test.js b/test/unit/rules/valid-values-license.test.js
diff --git a/test/unit/rules/valid-values-name-scope.test.js b/test/unit/rules/valid-values-name-scope.test.js
diff --git a/test/unit/rules/valid-values-private.test.js b/test/unit/rules/valid-values-private.test.js
diff --git a/test/unit/rules/valid-values-publishConfig.test.js b/test/unit/rules/valid-values-publishConfig.test.js

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "npm-package-json-lint",`
`3`		`- "version": "4.0.1",`
	`3`	`+ "version": "4.0.2",`
`4`	`4`	`"description": "Configurable linter for package.json files.",`
`5`	`5`	`"keywords": [`
`6`	`6`	`"lint",`