Refactor rule loader for rule types (#59)

* Bump version and increase branch coverage requirements * Update standard rules to have a ruleType of standard * Update array rules to have a ruleType of array * Update keywords rule to have a ruleType of standard. Also fix unit tests so it imports the current source file! * Closes #56 - Update config validation to static rules Update npm package json lint to call validate when rule is being processed * Update changeling with details of 2.9.0 release * Remove unused var and add missing semicolon * Add max-statements override for ESLint * Remove destructuring since node 4 and 5 don't support it
tclindner · Aug 30, 2017 · be7ea2e · be7ea2e
1 parent 1b0a91f
commit be7ea2e
Show file tree

Hide file tree

Showing 157 changed files with 851 additions and 268 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 
+## [2.9.0] - 2017-08-29
+### Changed
+- Update all rules to export the type of rule they are. Current valid values are "standard" and "array". The rules loader has been updated to references the ruleType export rather than trying to maintain a separate list of array style rules. This change closes [issue #56](https://github.com/tclindner/npm-package-json-lint/issues/56) and should prevent the issue discussed in [issue #53](https://github.com/tclindner/npm-package-json-lint/issues/53) from occurring again.
+
 ## [2.8.2] - 2017-08-23
 ### Fixed
 - Rule loader so it recognized [prefer-property-order](https://github.com/tclindner/npm-package-json-lint/wiki/prefer-property-order) as an array type rule.

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "npm-package-json-lint",
-  "version": "2.8.2",
+  "version": "2.9.0",
   "description": "CLI app for linting package.json files.",
   "keywords": [
     "lint",
@@ -31,7 +31,7 @@
     "eslint": "eslint . --format=node_modules/eslint-formatter-pretty",
     "lint": "npm run eslint",
     "test": "mocha tests/unit --recursive",
-    "coverage": "nyc --extension .js --check-coverage --lines 99 --branches 97 --functions 96 npm test"
+    "coverage": "nyc --extension .js --check-coverage --lines 99 --branches 98 --functions 96 npm test"
   },
   "dependencies": {
     "chalk": "^2.1.0",

diff --git a/src/Config.js b/src/Config.js
@@ -3,7 +3,6 @@
 /* eslint class-methods-use-this: 'off' */
 
 const fs = require('fs');
-const inArray = require('in-array');
 const Parser = require('./Parser');
 const path = require('path');
 const userHome = require('user-home');
@@ -40,8 +39,6 @@ class Config {
   get() {
     const userConfig = this._getUserConfig();
 
-    this._validateConfig(userConfig);
-
     let extendsConfig = {};
 
     if (userConfig.hasOwnProperty('extends')) {
@@ -125,8 +122,6 @@ class Config {
 
     const configObj = this._getExtendsConfigModule(adjustedModuleName);
 
-    this._validateConfig(configObj);
-
     return configObj.rules;
   }
 
@@ -192,50 +187,51 @@ class Config {
   }
 
   /**
-   * Validates config object
-   * @param  {Object} rcFileObj   Object version of .npmpackagejsonlintrc file
-   * @return {boolean}            True if validate config is successful
+   * Validates array rule config
+   * @param  {String}    ruleName    Name of the rule
+   * @param  {Array}     ruleConfig  Array rule
+   * @return {Boolean}               True if config is valid, false if not
+   * @static
    */
-  _validateConfig(rcFileObj) {
-    if (rcFileObj.hasOwnProperty('rules')) {
-      this._validateRulesConfig(rcFileObj.rules);
+  static isArrayRuleConfigValid(ruleName, ruleConfig) {
+    if (typeof ruleConfig === 'string' && ruleConfig === 'off') {
+      return true;
+    } else if (typeof ruleConfig === 'string' && ruleConfig !== 'off') {
+      throw new Error(`${ruleName} - is an array type rule. It must be set to "off" if an array is not supplied.`);
+    } else if (typeof ruleConfig[0] !== 'string' || this._isSeverityInvalid(ruleConfig[0])) {
+      throw new Error(`${ruleName} - first key must be set to "error", "warning", or "off". Currently set to "${ruleConfig[0]}".`);
+    }
+
+    if (!Array.isArray(ruleConfig[1])) {
+      throw new Error(`${ruleName} - second key must be set an array. Currently set to "${ruleConfig[1]}".`);
     }
 
     return true;
   }
 
   /**
-   * Validates rules object
-   * @param  {Object}     rulesObj   Object version of .npmpackagejsonlintrc file
-   * @return {boolean}               True if validate config is successful
+   * Validates standard rule config
+   * @param  {String}     ruleName     Name of the rule
+   * @param  {Object}     ruleConfig   Value for standard rule config
+   * @return {Boolean}                 True if config is valid, false if not
+   * @static
    */
-  _validateRulesConfig(rulesObj) {
-    for (const rule in rulesObj) {
-      const ruleConfig = rulesObj[rule];
-
-      if (Array.isArray(ruleConfig) && inArray(this.arrayRules, rule)) {
-        if (typeof ruleConfig[0] !== 'string' || this._isRuleValid(ruleConfig[0])) {
-          throw new Error(`${rule} - first key must be set to "error", "warning", or "off". Currently set to ${ruleConfig[0]}`);
-        }
-
-        if (!Array.isArray(ruleConfig[1])) {
-          throw new Error(`${rule} - second key must be set an array. Currently set to ${ruleConfig[1]}`);
-        }
-      } else if (typeof ruleConfig !== 'string' || this._isRuleValid(ruleConfig)) {
-        throw new Error(`${rule} - must be set to "error", "warning", or "off". Currently set to ${ruleConfig}`);
-      }
+  static isStandardRuleConfigValid(ruleName, ruleConfig) {
+    if (this._isSeverityInvalid(ruleConfig)) {
+      throw new Error(`${ruleName} - must be set to "error", "warning", or "off". Currently set to "${ruleConfig}".`);
     }
 
     return true;
   }
 
   /**
-   * Validates the first key of an array type rule
-   * @param  {String}  key Error type of the rule
-   * @return {Boolean}     True if the rule is valid. False if the rule is invalid.
+   * Validates if the severity config is set correctly
+   * @param  {String}  severity Severity the rule is set to
+   * @return {Boolean}          True if the severity is valid. False if the severity is invalid.
+   * @static
    */
-  _isRuleValid(key) {
-    return typeof key === 'string' && key !== 'error' && key !== 'warning' && key !== 'off';
+  static _isSeverityInvalid(severity) {
+    return typeof severity !== 'string' || (typeof severity === 'string' && severity !== 'error' && severity !== 'warning' && severity !== 'off');
   }
 
 }

diff --git a/src/NpmPackageJsonLint.js b/src/NpmPackageJsonLint.js
@@ -1,9 +1,8 @@
 'use strict';
 
-/* eslint class-methods-use-this: 'off' */
+/* eslint class-methods-use-this: 'off', max-statements: 'off' */
 
 const Config = require('./Config');
-const inArray = require('in-array');
 const Rules = require('./Rules');
 
 class NpmPackageJsonLint {
@@ -17,12 +16,6 @@ class NpmPackageJsonLint {
   constructor(packageJsonData, config, options) {
     this.packageJsonData = packageJsonData;
     this.ignoreWarnings = options.ignoreWarnings;
-    this.arrayRuleTypes = [
-      'valid-values',
-      'no-restricted-dependencies',
-      'no-restricted-pre-release-dependencies',
-      'property-order'
-    ];
     this.errors = [];
     this.warnings = [];
 
@@ -41,7 +34,8 @@ class NpmPackageJsonLint {
     for (const rule in configObj) {
       const ruleModule = this.rules.get(rule);
 
-      if (inArray(this.arrayRuleTypes, ruleModule.ruleType)) {
+      if (ruleModule.ruleType === 'array') {
+        Config.isArrayRuleConfigValid(rule, configObj[rule]);
         const errorWarningOffSetting = typeof configObj[rule] === 'string' && configObj[rule] === 'off' ? configObj[rule] : configObj[rule][0];
         const ruleConfigArray = configObj[rule][1];
 
@@ -50,6 +44,7 @@ class NpmPackageJsonLint {
         }
 
       } else {
+        Config.isStandardRuleConfigValid(rule, configObj[rule]);
         const errorWarningOffSetting = configObj[rule];
 
         if (errorWarningOffSetting !== 'off') {

diff --git a/src/rules/bin-type.js b/src/rules/bin-type.js
@@ -6,7 +6,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'bin-type';
 const nodeName = 'bin';
 const message = 'Type should be either a string or an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName) && !isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/bundledDependencies-type.js b/src/rules/bundledDependencies-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'bundledDependencies-type';
 const nodeName = 'bundledDependencies';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/config-type.js b/src/rules/config-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'config-type';
 const nodeName = 'config';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/cpu-type.js b/src/rules/cpu-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'os-type';
 const nodeName = 'os';
 const message = 'Type should be an array';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isArray(packageJsonData, nodeName)) {

diff --git a/src/rules/dependencies-type.js b/src/rules/dependencies-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'dependencies-type';
 const nodeName = 'dependencies';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/description-type.js b/src/rules/description-type.js
@@ -5,7 +5,7 @@ const isString = require('./../validators/type').isString;
 const lintId = 'description-type';
 const nodeName = 'description';
 const message = 'Type should be a string';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName)) {

diff --git a/src/rules/devDependencies-type.js b/src/rules/devDependencies-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'devDependencies-type';
 const nodeName = 'devDependencies';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/directories-type.js b/src/rules/directories-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'directories-type';
 const nodeName = 'directories';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/engines-type.js b/src/rules/engines-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'engines-type';
 const nodeName = 'engines';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/files-type.js b/src/rules/files-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'files-type';
 const nodeName = 'files';
 const message = 'Type should be an Array';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isArray(packageJsonData, nodeName)) {

diff --git a/src/rules/homepage-type.js b/src/rules/homepage-type.js
@@ -5,7 +5,7 @@ const isString = require('./../validators/type').isString;
 const lintId = 'homepage-type';
 const nodeName = 'homepage';
 const message = 'Type should be a string';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName)) {

diff --git a/src/rules/keywords-type.js b/src/rules/keywords-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'keywords-type';
 const nodeName = 'keywords';
 const message = 'Type should be an Array';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isArray(packageJsonData, nodeName)) {

diff --git a/src/rules/license-type.js b/src/rules/license-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'license-type';
 const nodeName = 'license';
 const message = 'Type should be a string';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName)) {

diff --git a/src/rules/main-type.js b/src/rules/main-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'main-type';
 const nodeName = 'main';
 const message = 'Type should be a string';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName)) {

diff --git a/src/rules/man-type.js b/src/rules/man-type.js
@@ -6,7 +6,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'man-type';
 const nodeName = 'man';
 const message = 'Type should be either a string or an array';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isArray(packageJsonData, nodeName) && !isString(packageJsonData, nodeName)) {

diff --git a/src/rules/name-format.js b/src/rules/name-format.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'name-format';
 const nodeName = 'name';
 const message = 'Format should be all lowercase';
-const ruleType = 'format';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isLowercase(packageJsonData, nodeName)) {

diff --git a/src/rules/name-type.js b/src/rules/name-type.js
@@ -5,7 +5,7 @@ const isString = require('./../validators/type').isString;
 const lintId = 'name-type';
 const nodeName = 'name';
 const message = 'Type should be a string';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isString(packageJsonData, nodeName)) {

diff --git a/src/rules/no-restricted-dependencies.js b/src/rules/no-restricted-dependencies.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'no-restricted-dependencies';
 const nodeName = 'dependencies';
 const message = 'You are using a restricted dependency. Please remove it.';
-const ruleType = 'no-restricted-dependencies';
+const ruleType = 'array';
 
 const lint = function(packageJsonData, lintType, invalidDependencies) {
   if (hasDependency(packageJsonData, nodeName, invalidDependencies)) {

diff --git a/src/rules/no-restricted-devDependencies.js b/src/rules/no-restricted-devDependencies.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'no-restricted-devDependencies';
 const nodeName = 'devDependencies';
 const message = 'You are using a restricted dependency. Please remove it.';
-const ruleType = 'no-restricted-dependencies';
+const ruleType = 'array';
 
 const lint = function(packageJsonData, lintType, invalidDependencies) {
   if (hasDependency(packageJsonData, nodeName, invalidDependencies)) {

diff --git a/src/rules/no-restricted-pre-release-dependencies.js b/src/rules/no-restricted-pre-release-dependencies.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 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 = 'no-restricted-pre-release-dependencies';
+const ruleType = 'array';
 
 const lint = function(packageJsonData, lintType, invalidPreRelDeps) {
   if (hasDepPrereleaseVers(packageJsonData, nodeName, invalidPreRelDeps)) {

diff --git a/src/rules/no-restricted-pre-release-devDependencies.js b/src/rules/no-restricted-pre-release-devDependencies.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 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 = 'no-restricted-pre-release-dependencies';
+const ruleType = 'array';
 
 const lint = function(packageJsonData, lintType, invalidPreRelDeps) {
   if (hasDepPrereleaseVers(packageJsonData, nodeName, invalidPreRelDeps)) {

diff --git a/src/rules/optionalDependencies-type.js b/src/rules/optionalDependencies-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'optionalDependencies-type';
 const nodeName = 'optionalDependencies';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {

diff --git a/src/rules/os-type.js b/src/rules/os-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'os-type';
 const nodeName = 'os';
 const message = 'Type should be an array';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isArray(packageJsonData, nodeName)) {

diff --git a/src/rules/peerDependencies-type.js b/src/rules/peerDependencies-type.js
@@ -5,7 +5,7 @@ const LintIssue = require('./../LintIssue');
 const lintId = 'peerDependencies-type';
 const nodeName = 'peerDependencies';
 const message = 'Type should be an Object';
-const ruleType = 'type';
+const ruleType = 'standard';
 
 const lint = function(packageJsonData, lintType) {
   if (!isObject(packageJsonData, nodeName)) {