refactor(misc): cleanup & simplify (#131)

* refactor: PlatformJson

* refactor: CordovaCheck

* refactor(plist-helpers): rename underscore variable

* refactor(plist-helpers): rename pruneOBJECT function

* refactor(plist-helpers): formatting

* refactor(CordovaCheck): simplify isCordova w/ pwd

* refactor(plist-helpers): simplify array merge

* refactor(plist-helpers): simplify dict merge

* refactor(PlatformJson): use readJsonSync

Co-authored-by: エリス <erisu@users.noreply.github.com>

src/CordovaCheck.js

@@ -24,17 +24,14 @@ function isRootDir (dir) {
     if (fs.existsSync(path.join(dir, 'www'))) {
         if (fs.existsSync(path.join(dir, 'config.xml'))) {
             // For sure is.
-            if (fs.existsSync(path.join(dir, 'platforms'))) {
-                return 2;
-            } else {
-                return 1;
-            }
+            return fs.existsSync(path.join(dir, 'platforms')) ? 2 : 1;
         }
         // Might be (or may be under platforms/).
         if (fs.existsSync(path.join(dir, 'www', 'config.xml'))) {
             return 1;
         }
     }
+
     return 0;
 }
 
@@ -46,27 +43,33 @@ function isCordova (dir) {
         // Prefer PWD over cwd so that symlinked dirs within your PWD work correctly (CB-5687).
         const pwd = process.env.PWD;
         const cwd = process.cwd();
-        if (pwd && pwd !== cwd && pwd !== 'undefined') {
-            return isCordova(pwd) || isCordova(cwd);
-        }
-        return isCordova(cwd);
+        const hasPwd = pwd && pwd !== cwd && pwd !== 'undefined';
+
+        return (hasPwd && isCordova(pwd)) || isCordova(cwd);
     }
+
     let bestReturnValueSoFar = false;
+
     for (let i = 0; i < 1000; ++i) {
         const result = isRootDir(dir);
+
         if (result === 2) {
             return dir;
         }
+
         if (result === 1) {
             bestReturnValueSoFar = dir;
         }
+
         const parentDir = path.normalize(path.join(dir, '..'));
         // Detect fs root.
         if (parentDir === dir) {
             return bestReturnValueSoFar;
         }
+
         dir = parentDir;
     }
+
     console.error('Hit an unhandled case in CordovaCheck.isCordova');
     return false;
 }

src/PlatformJson.js

@@ -28,10 +28,10 @@ class PlatformJson {
 
     static load (plugins_dir, platform) {
         const filePath = path.join(plugins_dir, `${platform}.json`);
-        let root = null;
-        if (fs.existsSync(filePath)) {
-            root = JSON.parse(fs.readFileSync(filePath, 'utf-8'));
-        }
+        const root = fs.existsSync(filePath)
+            ? fs.readJsonSync(filePath)
+            : null;
+
         return new PlatformJson(filePath, platform, root);
     }
 
@@ -91,16 +91,13 @@ class PlatformJson {
      */
     addPluginMetadata (pluginInfo) {
         const installedModules = this.root.modules || [];
-
         const installedPaths = installedModules.map(m => m.file);
-
         const modulesToInstall = pluginInfo.getJsModules(this.platform)
             .map(module => new ModuleMetadata(pluginInfo.id, module))
             // Filter out modules which are already added to metadata
             .filter(metadata => !installedPaths.includes(metadata.file));
 
         this.root.modules = installedModules.concat(modulesToInstall);
-
         this.root.plugin_metadata = this.root.plugin_metadata || {};
         this.root.plugin_metadata[pluginInfo.id] = pluginInfo.version;
 
@@ -127,12 +124,12 @@ class PlatformJson {
      * @returns {this} Current PlatformJson instance to allow calls chaining
      */
     removePluginMetadata (pluginInfo) {
+        const installedModules = this.root.modules || [];
         const modulesToRemove = pluginInfo.getJsModules(this.platform)
             .map(jsModule => ['plugins', pluginInfo.id, jsModule.src].join('/'));
 
-        const installedModules = this.root.modules || [];
+        // Leave only those metadatas which 'file' is not in removed modules
         this.root.modules = installedModules
-            // Leave only those metadatas which 'file' is not in removed modules
             .filter(m => !modulesToRemove.includes(m.file));
 
         if (this.root.plugin_metadata) {
@@ -159,10 +156,12 @@ class PlatformJson {
      */
     makeTopLevel (pluginId) {
         const plugin = this.root.dependent_plugins[pluginId];
+
         if (plugin) {
             delete this.root.dependent_plugins[pluginId];
             this.root.installed_plugins[pluginId] = plugin;
         }
+
         return this;
     }
 
@@ -206,6 +205,7 @@ function fix_munge (root) {
     const munge = root.config_munge;
     if (!munge.files) {
         const new_munge = { files: {} };
+
         for (const file in munge) {
             for (const selector in munge[file]) {
                 for (const xml_child in munge[file][selector]) {
@@ -216,17 +216,18 @@ function fix_munge (root) {
                 }
             }
         }
+
         root.config_munge = new_munge;
     }
 
     return root;
 }
 
-/**
- * Run-time representation of a module entry in 'cordova_plugins.js'
- */
 class ModuleMetadata {
     /**
+     * Creates a ModuleMetadata object that represents module entry in 'cordova_plugins.js'
+     * file at run time
+     *
      * @param {String}  pluginId  Plugin id where this module installed from
      * @param (JsModule|Object)  jsModule  A js-module entry from PluginInfo class to generate metadata for
      */
@@ -241,9 +242,11 @@ class ModuleMetadata {
         if (jsModule.clobbers && jsModule.clobbers.length > 0) {
             this.clobbers = jsModule.clobbers.map(o => o.target);
         }
+
         if (jsModule.merges && jsModule.merges.length > 0) {
             this.merges = jsModule.merges.map(o => o.target);
         }
+
         if (jsModule.runs) {
             this.runs = true;
         }

src/util/plist-helpers.js

@@ -18,29 +18,24 @@
 */
 
 // contains PLIST utility functions
-const __ = require('underscore');
+const _ = require('underscore');
 const plist = require('plist');
 
 // adds node to doc at selector
 module.exports.graftPLIST = graftPLIST;
 function graftPLIST (doc, xml, selector) {
     const obj = plist.parse(`<plist>${xml}</plist>`);
+    const node = doc[selector];
 
-    let node = doc[selector];
     if (node && Array.isArray(node) && Array.isArray(obj)) {
-        node = node.concat(obj);
-        for (let i = 0; i < node.length; i++) {
-            for (let j = i + 1; j < node.length; ++j) {
-                if (nodeEqual(node[i], node[j])) { node.splice(j--, 1); }
-            }
-        }
-        doc[selector] = node;
+        const isNew = item => !node.some(nodeChild => nodeEqual(item, nodeChild));
+        doc[selector] = node.concat(obj.filter(isNew));
     } else {
         // plist uses objects for <dict>. If we have two dicts we merge them instead of
         // overriding the old one. See CB-6472
-        if (node && __.isObject(node) && __.isObject(obj) && !__.isDate(node) && !__.isDate(obj)) { // arrays checked above
-            __.extend(obj, node);
-        }
+        const isDict = o => _.isObject(o) && !_.isDate(o); // arrays checked above
+        if (isDict(node) && isDict(obj)) _.extend(obj, node);
+
         doc[selector] = obj;
     }
 
@@ -52,19 +47,21 @@ module.exports.prunePLIST = prunePLIST;
 function prunePLIST (doc, xml, selector) {
     const obj = plist.parse(`<plist>${xml}</plist>`);
 
-    pruneOBJECT(doc, selector, obj);
+    pruneObject(doc, selector, obj);
 
     return true;
 }
 
-function pruneOBJECT (doc, selector, fragment) {
+function pruneObject (doc, selector, fragment) {
     if (Array.isArray(fragment) && Array.isArray(doc[selector])) {
         let empty = true;
+
         for (const i in fragment) {
             for (const j in doc[selector]) {
-                empty = pruneOBJECT(doc[selector], j, fragment[i]) && empty;
+                empty = pruneObject(doc[selector], j, fragment[i]) && empty;
             }
         }
+
         if (empty) {
             delete doc[selector];
             return true;
@@ -78,13 +75,16 @@ function pruneOBJECT (doc, selector, fragment) {
 }
 
 function nodeEqual (node1, node2) {
-    if (typeof node1 !== typeof node2) { return false; } else if (typeof node1 === 'string') {
+    if (typeof node1 !== typeof node2) {
+        return false;
+    } else if (typeof node1 === 'string') {
         node2 = escapeRE(node2).replace(/\\\$\(\S+\)/gm, '(.*?)');
         return new RegExp(`^${node2}$`).test(node1);
     } else {
         for (const key in node2) {
             if (!nodeEqual(node1[key], node2[key])) return false;
         }
+
         return true;
     }
 }