Closes #57 and #58 (#60)

* Closes #57 and #58

This change gives better recommendations for what change is required by
the user to resolve the lint issue. It also no longer throws an error
when a property exists in the package.json file that doesn't exist in
the preferred property order array. Thanks @moshest for the input.

* Remove unused variable

Author: tclindner

CHANGELOG.md

@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 
+## [2.10.0] - 2017-09-02
+### Changed
+- Addressed issues, from @moshest, [#57](https://github.com/tclindner/npm-package-json-lint/issues/57) and [#58](https://github.com/tclindner/npm-package-json-lint/issues/58). This change gives better recommendations for what change is required by the user to resolve the lint issue. It also no longer throws an error when a property exists in the package.json file that doesn't exist in the preferred property order array. Thanks @moshest.
+
 ## [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.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "npm-package-json-lint",
-  "version": "2.9.0",
+  "version": "2.10.0",
   "description": "CLI app for linting package.json files.",
   "keywords": [
     "lint",
@@ -42,17 +42,17 @@
     "plur": "^2.1.2",
     "semver": "^5.4.1",
     "user-home": "^2.0.0",
-    "validator": "^8.0.0"
+    "validator": "^8.1.0"
   },
   "devDependencies": {
-    "chai": "^4.1.1",
+    "chai": "^4.1.2",
     "eslint": "^4.4.1",
     "eslint-config-tc": "^2.1.0",
     "eslint-formatter-pretty": "^1.1.0",
     "figures": "^2.0.0",
     "mocha": "^3.5.0",
     "nyc": "^11.1.0",
-    "sinon": "^3.1.0"
+    "sinon": "^3.2.1"
   },
   "engines": {
     "node": ">=4.2.0",

src/validators/property-order.js

@@ -4,6 +4,7 @@
 
 const notFound = -1;
 const empty = 0;
+const one = 1;
 const increment = 1;
 const defaultPreferredNodeOrder = [
   'name',
@@ -62,29 +63,26 @@ const defaultPreferredNodeOrder = [
 const isInPreferredOrder = function(packageJsonData, userPreferredNodeOrder) {
   let isValid = true;
   let msg = null;
-  const actualNodeList = Object.keys(packageJsonData);
   const preferredNodeOrder = userPreferredNodeOrder.length === empty ? Array.from(defaultPreferredNodeOrder) : Array.from(userPreferredNodeOrder);
-  const preferredNodeOrderCopy = Array.from(preferredNodeOrder);
+  const fltrdPreferredNodeOrder = preferredNodeOrder.filter((property) => packageJsonData.hasOwnProperty(property));
+  const actualNodeList = Object.keys(packageJsonData);
+  const fltrdActualNodeList = actualNodeList.filter((property) => preferredNodeOrder.indexOf(property) !== notFound);
+  const filteredPreferredOrderMap = new Map();
 
-  for (let keyIndex = 0;keyIndex < actualNodeList.length;keyIndex += increment) {
-    let preferredNodeOrderItem = null;
+  fltrdPreferredNodeOrder.forEach((property, index) => {
+    filteredPreferredOrderMap.set(property, index);
+  });
 
-    if (preferredNodeOrder.indexOf(actualNodeList[keyIndex]) === notFound) {
-      isValid = false;
-      msg = `${actualNodeList[keyIndex]} is not in the preferred property list.`;
-      break;
-    }
+  for (let keyIndex = 0;keyIndex < fltrdActualNodeList.length;keyIndex += increment) {
+    const currentPkgJsonProperty = fltrdActualNodeList[keyIndex];
+
+    const preferredOrderPosition = filteredPreferredOrderMap.get(currentPkgJsonProperty);
 
-    if (preferredNodeOrderCopy.indexOf(actualNodeList[keyIndex]) === notFound) {
+    if (preferredOrderPosition !== keyIndex) {
       isValid = false;
-      msg = `Please move ${actualNodeList[keyIndex]} before ${actualNodeList[keyIndex - increment]}.`;
+      msg = `Please move "${currentPkgJsonProperty}" after "${fltrdPreferredNodeOrder[preferredOrderPosition - one]}".`;
       break;
     }
-
-    do {
-      preferredNodeOrderItem = preferredNodeOrderCopy.shift();
-
-    } while (actualNodeList[keyIndex] !== preferredNodeOrderItem);
   }
 
   return {

tests/unit/rules/prefer-property-order.test.js

@@ -66,7 +66,7 @@ describe('prefer-property-order Unit Tests', function() {
       response.lintId.should.equal('prefer-property-order');
       response.lintType.should.equal('error');
       response.node.should.equal('');
-      response.lintMessage.should.equal('Your package.json properties are not in the desired order. Please move version before description.');
+      response.lintMessage.should.equal('Your package.json properties are not in the desired order. Please move "description" after "version".');
     });
   });
 });

tests/unit/validators/property-order.test.js

@@ -42,7 +42,7 @@ describe('property-order Unit Tests', function() {
     });
 
     context('when the actual node list does not have the same number of nodes as the desired list', function() {
-      it('false should be returned', function() {
+      it('true should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0'
@@ -74,7 +74,7 @@ describe('property-order Unit Tests', function() {
         const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
 
         response.status.should.be.false;
-        response.msg.should.equal('Please move version before description.');
+        response.msg.should.equal('Please move "description" after "version".');
       });
     });
 
@@ -93,12 +93,12 @@ describe('property-order Unit Tests', function() {
         const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
 
         response.status.should.be.false;
-        response.msg.should.equal('Please move name before version.');
+        response.msg.should.equal('Please move "version" after "name".');
       });
     });
 
     context('when the actual node list is in a different order than desired', function() {
-      it('false should be returned', function() {
+      it('true should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0',
@@ -118,7 +118,7 @@ describe('property-order Unit Tests', function() {
     });
 
     context('when the actual node list is in a different order than desired', function() {
-      it('false should be returned', function() {
+      it('true should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0',
@@ -163,8 +163,34 @@ describe('property-order Unit Tests', function() {
       });
     });
 
+    context('when the actual node list is in correct order, but has extra values in preferred order', function() {
+      it('false should be returned', function() {
+        const packageJson = {
+          name: 'awesome-module',
+          version: '1.0.0',
+          description: 'description',
+          homepage: 'https://github.com/tclindner/npm-package-json-lint',
+          license: 'MIT',
+          keywords: ['awesome']
+        };
+        const preferredOrder = [
+          'name',
+          'version',
+          'description',
+          'scripts',
+          'bin',
+          'keywords',
+          'homepage'
+        ];
+        const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
+
+        response.status.should.be.false;
+        response.msg.should.equal('Please move "homepage" after "keywords".');
+      });
+    });
+
     context('when the actual node list is not in correct order and also has extra values in preferred order', function() {
-      it('true should be returned', function() {
+      it('false should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0',
@@ -184,12 +210,12 @@ describe('property-order Unit Tests', function() {
         const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
 
         response.status.should.be.false;
-        response.msg.should.equal('Please move keywords before homepage.');
+        response.msg.should.equal('Please move "homepage" after "keywords".');
       });
     });
 
     context('when node is not in the preferred node list', function() {
-      it('false should be returned', function() {
+      it('true should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0',
@@ -204,13 +230,13 @@ describe('property-order Unit Tests', function() {
         ];
         const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
 
-        response.status.should.be.false;
-        response.msg.should.equal('description is not in the preferred property list.');
+        response.status.should.be.true;
+        (response.msg === null).should.be.true;
       });
     });
 
     context('when node is not in the preferred node list', function() {
-      it('false should be returned', function() {
+      it('true should be returned', function() {
         const packageJson = {
           name: 'awesome-module',
           version: '1.0.0',
@@ -224,8 +250,8 @@ describe('property-order Unit Tests', function() {
         ];
         const response = propertyOrder.isInPreferredOrder(packageJson, preferredOrder);
 
-        response.status.should.be.false;
-        response.msg.should.equal('name is not in the preferred property list.');
+        response.status.should.be.true;
+        (response.msg === null).should.be.true;
       });
     });
   });