fix: remove .csproj support, add packages detection to imporve version, and addd tests for packages folder detection (#27)

Author: aryehbeitz

lib/dependency.js

@@ -44,23 +44,6 @@ Dependency.from = {
     result.versionSpec = manifest.$.version;
     return result;
   },
-  csprojEntry: function (referenceItem) {
-    debug('Extracting by hintPath ' + referenceItem.HintPath[0]);
-    var sep = (
-      /\//.exec(referenceItem.HintPath[0]) ||
-      /\\/.exec(referenceItem.HintPath[0]))[0];
-    var depLocalPath = referenceItem.HintPath[0]
-      .split(sep).slice(2,3).join(sep);
-    var packageInfo = Dependency.extractFromDotVersionNotation(depLocalPath);
-    var name = packageInfo.name;
-    var version = packageInfo.version;
-    var result = new Dependency(
-      name,
-      version,
-      'unknown'
-    );
-    return result;
-  },
 };
 
 module.exports = Dependency;

lib/find-folder.js

@@ -11,6 +11,6 @@ function findFolder (dir) {
     }
     return false;
   }
-};
+}
 
 module.exports = findFolder;

lib/index.js

@@ -20,9 +20,6 @@ function determineManifestType (filename) {
     case /packages.config$/.test(filename): {
       return 'packages.config';
     }
-    case /.csproj$/.test(filename): {
-      return '.csproj';
-    }
     default: {
       throw new Error('Could not determine manifest type for ' + filename);
     }
@@ -117,57 +114,15 @@ module.exports = {
                 });
               resolve(installedPackages);
             }
-          })
-          break;
-        }
-        case '.csproj': {
-          debug('Trying to parse .csproj manifest');
-          parseXML(fileContent, function scanCsprojContent(err, result) {
-            if (err) {
-              reject(err);
-            } else {
-              (result.Project.ItemGroup || []).forEach(function (itemGroup) {
-                (itemGroup.Reference || []).forEach(function (referenceItem) {
-                  if (referenceItem.HintPath) {
-                    try {
-                      var installedDependency =
-                        Dependecy.from.csprojEntry(referenceItem);
-                      if (installedDependency.name.indexOf('System.') !== 0) {
-                        installedPackages.push(installedDependency);
-                      }
-                    }
-                    catch (err) {
-                      debug('Could not parse package name. Skipping');
-                      // gracefully continue
-                    }
-                  }
-                });
-              });
-            }
-            resolve(installedPackages);
           });
           break;
         }
       }
     }).then(function scanInstalled(installedPackages) {
       if (manifestType !== 'dotnet-core') {
         debug('Located ' + installedPackages.length + ' packages in manifest');
-        function injectPath(dep) {
-          dep.path = dep.localPath
-            ? path.resolve(
-              packagesFolder,
-              dep.localPath
-            )
-            : path.resolve(
-              packagesFolder,
-              dep.name + '.' + dep.version
-            );
-          if (dep.localPath) {
-            delete dep.localPath;
-          }
-        }
         installedPackages.forEach(function (entry) {
-          injectPath(entry);
+          injectPath(entry, packagesFolder);
           flattendPackageList[entry.name] =
             flattendPackageList[entry.name] || entry;
           debug('Entry: ' + entry.name + ' -> ' + entry.path);
@@ -178,15 +133,24 @@ module.exports = {
           packagesFolder);
           fs.readdirSync(packagesFolder)
             .filter(function (name) {
-              return name.slice(0, 7).toLowerCase() !== 'system.'
+              return name.slice(0, 7).toLowerCase() !== 'system.';
             })
             .map(function (folderName) {
               return Dependecy.from.folderName(folderName);
             })
             .forEach(function (dep) {
-              injectPath(dep);
-              flattendPackageList[dep.name] =
-                flattendPackageList[dep.name] || dep;
+              injectPath(dep, packagesFolder);
+              // only add a package from packages folder if version is different
+              if (flattendPackageList[dep.name] &&
+                flattendPackageList[dep.name].version !== dep.version) {
+                // prefer found from packages folder (dep) over existing
+                debug('For package ' + dep.name + ' the version ' +
+                  flattendPackageList[dep.name].version +
+                  ' was extracted from manifest file.' +
+                  '\nWe are overwriting it with version ' + dep.version +
+                  ' from the packages folder');
+                flattendPackageList[dep.name] = dep;
+              }
             });
         }
         catch (err) {
@@ -196,7 +160,7 @@ module.exports = {
       } else {
         debug('Located ' +
           Object.keys(tree.dependencies).length + 'packages in manifest');
-        var sorted = {}
+        var sorted = {};
         Object.keys(flattendPackageList).sort().forEach(function (key) {
           sorted[key] = flattendPackageList[key];
         });
@@ -245,7 +209,7 @@ module.exports = {
               requiredChild.version);
             transitiveDependency.versionSpec = requiredChild.version;
           }
-          transitiveDependency.from = node.from.concat()
+          transitiveDependency.from = node.from.concat();
           var transitiveChildren =
             (nuspecResolutions[transitiveDependency.name] &&
              nuspecResolutions[transitiveDependency.name].children) || [];
@@ -260,23 +224,33 @@ module.exports = {
       var _nugtKeyCount = Object.keys(nuspecResolutions).length;
       Object.keys(flattendPackageList).forEach(function (packageName) {
         tree.dependencies[packageName] =
-          flattendPackageList[packageName].cloneShallow()
-      })
+          flattendPackageList[packageName].cloneShallow();
+      });
       if (_nugtKeyCount > 0) {
         // local folders scanned, build list from .nuspec
         for (var key in nuspecResolutions) {
           var resolution = nuspecResolutions[key];
           var node = flattendPackageList[resolution.name].cloneShallow();
-          node.from = tree.from.concat()
+          node.from = tree.from.concat();
           buildTree(node, resolution.children, flattendPackageList);
           tree.dependencies[node.name] = node;
         }
       }
       return packageTree;
     })['catch'](function (err) {
       throw(err);
-    })
+    });
 
     return job;
   },
-};
+};
+
+function injectPath(dep, packagesFolder) {
+  dep.path =
+    dep.localPath ?
+    path.resolve(packagesFolder, dep.localPath)
+    : path.resolve(packagesFolder, dep.name + '.' + dep.version);
+  if (dep.localPath) {
+    delete dep.localPath;
+  }
+}

lib/json-manifest-parser.js

@@ -18,7 +18,7 @@ function scanForDependencies(obj, deps) {
           version = version.toString();
         }
         deps[depName] = version;
-      })
+      });
     } else {
       scanForDependencies(obj[key], deps);
     }
@@ -32,8 +32,8 @@ module.exports = {
     var result = {};
     result.dependencies = scanForDependencies(rawContent, {});
     if (typeof rawContent.project === 'object') {
-      var pData = rawContent.project
-      var name = (pData.restore && pData.restore.projectName)
+      var pData = rawContent.project;
+      var name = (pData.restore && pData.restore.projectName);
       result.project = {
         version: pData.version || '0.0.0',
       };

lib/nuspec-parser.js

@@ -8,7 +8,7 @@ var Dependency = require('./dependency');
 
 function parseNuspec(library, sep) {
   var P = new Promise(function (resolve, reject) {
-    var pathSep = sep || '.'
+    var pathSep = sep || '.';
     var nuspecPath = path.resolve(
       library.path,
       library.name + pathSep + library.version + '.nupkg');
@@ -35,18 +35,18 @@ function parseNuspec(library, sep) {
           (metadata.dependencies || []).forEach(function (rawDependency) {
             (rawDependency.group || []).forEach(function (group) {
               (group.dependency || []).forEach(function (dep) {
-                var transitiveDependency = new Dependency(dep.$.id, dep.$.version) // jscs:ignore
-                transitiveDependency.versionSpec = dep.$.versionSpec
+                var transitiveDependency = new Dependency(dep.$.id, dep.$.version); // jscs:ignore
+                transitiveDependency.versionSpec = dep.$.versionSpec;
                 ownDependencies.push(transitiveDependency);
-              })
+              });
             });
             (rawDependency.dependency || []).forEach(function (dep) {
               var transitiveDependency =
                 new Dependency(dep.$.id, dep.$.version, null);
               transitiveDependency.versionSpec = dep.$.version;
               ownDependencies.push(transitiveDependency);
             });
-          })
+          });
         });
         resolve({
           name: library.name,

test/packages_dir.test.js

@@ -0,0 +1,91 @@
+var path = require('path');
+var test = require('tap').test;
+var plugin = require('../lib/index');
+var projectPath = './test/stubs/packages_dir';
+
+var app1Path = projectPath + '/only_jquery/';
+var app1ManifestFile = 'packages.config';
+var app1ExpectedTree = require(path.resolve(app1Path, 'expected.json'));
+
+var app2Path = projectPath + '/only_jquery_but_wrong_version/';
+var app2ManifestFile = 'packages.config';
+var app2ExpectedTree = require(path.resolve(app2Path, 'expected.json'));
+
+var app3Path = projectPath + '/only_momentjs/';
+var app3ManifestFile = 'packages.config';
+var app3ExpectedTree = require(path.resolve(app3Path, 'expected.json'));
+
+test('packages contains many deps: only jquery', function (t) {
+  plugin.inspect(
+    app1Path,
+    app1ManifestFile,
+    {
+      packagesFolder: projectPath + '/packages',
+    })
+    .then(function (result) {
+      t.ok(result.package.dependencies.jQuery,
+        'jQuery should be found because specified');
+      t.notOk(result.package.dependencies['Moment.js'],
+        'Moment.js should not be found, even though exists in packages');
+      t.deepEqual(
+        result,
+        app1ExpectedTree,
+        'expects project data to be correct'
+      );
+      t.end();
+    })
+    .catch(function (error) {
+      t.fail('Error was thrown: ' + err);
+    });
+});
+
+
+test('packages contains many deps: only moment', function (t) {
+  plugin.inspect(
+    app3Path,
+    app3ManifestFile,
+    {
+      packagesFolder: projectPath + '/packages',
+    })
+    .then(function (result) {
+      t.ok(result.package.dependencies['Moment.js'],
+        'Moment.js should be found because specified');
+      t.notOk(result.package.dependencies.jQuery,
+        'jQuery should not be found, even though exists in packages');
+      t.deepEqual(
+        result,
+        app3ExpectedTree,
+        'expects project data to be correct'
+      );
+      t.end();
+    })
+    .catch(function (error) {
+      t.fail('Error was thrown: ' + err);
+    });
+});
+
+
+test('packages contains many deps: different jquery version', function (t) {
+  plugin.inspect(
+    app2Path,
+    app2ManifestFile,
+    {
+      packagesFolder: projectPath + '/packages',
+    })
+    .then(function (result) {
+      t.ok(result.package.dependencies.jQuery,
+        'jQuery should be found because specified');
+      t.ok(result.package.dependencies.jQuery.version == '3.2.1',
+        'should find version as in packages folder 3.2.1, but found ' +
+        result.package.dependencies.jQuery.version);
+      t.deepEqual(
+        result,
+        app2ExpectedTree,
+        'expects project data to be correct'
+      );
+      t.end();
+    })
+    .catch(function (error) {
+      t.fail('Error was thrown: ' + err);
+    });
+});

test/parse-with-traverse.test.js

@@ -7,8 +7,6 @@ var targetPackagesConfigFile =
   targetProjectJsonFile + 'dummy_project_1/packages.config';
 var alternatePackagesFolder =
   targetProjectJsonFile + 'alternate_packages';
-var targetCSProjFile =
-  targetProjectJsonFile + 'dummy_project_1/WebApplication1.csproj';
 var targetJSONManifestData =
   require('./stubs/_2_project.json');
 
@@ -44,35 +42,6 @@ test('parse project.assets.json - like and traverse packages', function (t) {
   });
 });
 
-test('parse .csproj and traverse packages', function (t) {
-  var expectedTreeFile =
-    fs.readFileSync(
-      targetProjectJsonFile + 'dummy_project_1/expected_csproj.json');
-  var expectedTree = JSON.parse(expectedTreeFile.toString());
-
-  plugin.inspect(null, targetCSProjFile, null)
-  .then(function (result) {
-    t.test('traversing', function (t) {
-      t.deepEqual(
-        result.package.dependencies,
-        expectedTree.package.dependencies,
-        'expects dependency tree to be correct');
-      t.ok(result.plugin, 'plugin details exists in result');
-      t.equal(result.plugin.name, 'snyk-nuget-plugin',
-        'plugin\'s name is snyk-nuget-plugin');
-      t.equal(result.plugin.targetFile, targetCSProjFile,
-        'plugin shows correct targetFile');
-      t.end();
-    });
-    return result;
-  })
-  .then(function (result) {
-    if (result) {
-      t.end();
-    }
-  });
-});
-
 test('parse packages.config and traverse packages', function (t) {
   var expectedTreeFile =
     fs.readFileSync(