From d1c18c84d0f6e831f0b544636fafad2f7e996328 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Mon, 22 Apr 2019 08:46:56 +0200
Subject: [PATCH] fix(pipenv): handle nested definitions better

Closes #3482
---
 lib/manager/pipenv/extract.js                 |  62 +++++------
 .../pipenv/__snapshots__/extract.spec.js.snap | 105 ++++++++++++++++--
 test/manager/pipenv/_fixtures/Pipfile4        |  19 ++++
 test/manager/pipenv/extract.spec.js           |  17 ++-
 4 files changed, 161 insertions(+), 42 deletions(-)
 create mode 100644 test/manager/pipenv/_fixtures/Pipfile4

diff --git a/lib/manager/pipenv/extract.js b/lib/manager/pipenv/extract.js
index 2a404a366a..dc0119d6da 100644
--- a/lib/manager/pipenv/extract.js
+++ b/lib/manager/pipenv/extract.js
@@ -46,56 +46,56 @@ function extractFromSection(pipfile, section, registryUrls) {
   const specifierRegex = new RegExp(`^${specifierPattern}$`);
   const pipfileSection = pipfile[section];
 
-  Object.keys(pipfileSection).forEach(key => {
-    if (is.object(pipfileSection[key]))
-      pipfileSection[key].version = pipfileSection[key].version || '*';
-  });
-
   const deps = Object.entries(pipfileSection)
     .map(x => {
       const [depName, requirements] = x;
       let currentValue;
       let pipenvNestedVersion;
-      if (requirements.version) {
+      let skipReason;
+      if (requirements.git) {
+        skipReason = 'git-dependency';
+      } else if (requirements.file) {
+        skipReason = 'file-dependency';
+      } else if (requirements.path) {
+        skipReason = 'local-dependency';
+      } else if (requirements.version) {
         currentValue = requirements.version;
         pipenvNestedVersion = true;
-      } else if (requirements.git) {
-        logger.debug('Skipping git dependency');
-        return null;
-      } else if (requirements.path) {
-        logger.debug('Skipping relative path dependency');
-        return null;
+      } else if (is.object(requirements)) {
+        skipReason = 'any-version';
       } else {
         currentValue = requirements;
-        pipenvNestedVersion = false;
-      }
-      const packageMatches = packageRegex.exec(depName);
-      const specifierMatches = specifierRegex.exec(currentValue);
-      let skipReason;
-      if (!packageMatches) {
-        logger.debug(
-          `Skipping dependency with malformed package name "${depName}".`
-        );
-        return null;
       }
       if (currentValue === '*') {
         skipReason = 'any-version';
-      } else if (!specifierMatches) {
-        logger.debug(
-          `Skipping dependency with malformed version specifier "${currentValue}".`
-        );
-        return null;
+      }
+      if (!skipReason) {
+        const packageMatches = packageRegex.exec(depName);
+        if (!packageMatches) {
+          logger.info(
+            `Skipping dependency with malformed package name "${depName}".`
+          );
+          skipReason = 'invalid-name';
+        }
+        const specifierMatches = specifierRegex.exec(currentValue);
+        if (!specifierMatches) {
+          logger.debug(
+            `Skipping dependency with malformed version specifier "${currentValue}".`
+          );
+          skipReason = 'invalid-version';
+        }
       }
       const dep = {
-        depName,
-        currentValue,
-        pipenvNestedVersion,
-        datasource: 'pypi',
         depType: section,
+        depName,
       };
+      if (currentValue) dep.currentValue = currentValue;
       if (skipReason) {
         dep.skipReason = skipReason;
+      } else {
+        dep.datasource = 'pypi';
       }
+      if (pipenvNestedVersion) dep.pipenvNestedVersion = pipenvNestedVersion;
       if (registryUrls) {
         dep.registryUrls = registryUrls;
       }
diff --git a/test/manager/pipenv/__snapshots__/extract.spec.js.snap b/test/manager/pipenv/__snapshots__/extract.spec.js.snap
index 40c46af618..5f82e838a3 100644
--- a/test/manager/pipenv/__snapshots__/extract.spec.js.snap
+++ b/test/manager/pipenv/__snapshots__/extract.spec.js.snap
@@ -7,7 +7,6 @@ Array [
     "datasource": "pypi",
     "depName": "some-package",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
       "http://example.com/private-pypi/",
@@ -18,12 +17,31 @@ Array [
     "datasource": "pypi",
     "depName": "some-other-package",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
       "http://example.com/private-pypi/",
     ],
   },
+  Object {
+    "currentValue": "==1.0.0",
+    "depName": "_invalid-package",
+    "depType": "packages",
+    "registryUrls": Array [
+      "https://pypi.org/simple",
+      "http://example.com/private-pypi/",
+    ],
+    "skipReason": "invalid-name",
+  },
+  Object {
+    "currentValue": "==0 0",
+    "depName": "invalid-version",
+    "depType": "packages",
+    "registryUrls": Array [
+      "https://pypi.org/simple",
+      "http://example.com/private-pypi/",
+    ],
+    "skipReason": "invalid-version",
+  },
   Object {
     "currentValue": "==1.0.0",
     "datasource": "pypi",
@@ -40,7 +58,6 @@ Array [
     "datasource": "pypi",
     "depName": "dev-package",
     "depType": "dev-packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
       "http://example.com/private-pypi/",
@@ -49,6 +66,83 @@ Array [
 ]
 `;
 
+exports[`lib/manager/pipenv/extract extractPackageFile() extracts example pipfile 1`] = `
+Object {
+  "deps": Array [
+    Object {
+      "depName": "requests",
+      "depType": "packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "any-version",
+    },
+    Object {
+      "currentValue": ">0.5.0",
+      "datasource": "pypi",
+      "depName": "records",
+      "depType": "packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+    },
+    Object {
+      "depName": "django",
+      "depType": "packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "git-dependency",
+    },
+    Object {
+      "depName": "e682b37",
+      "depType": "packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "file-dependency",
+    },
+    Object {
+      "depName": "e1839a8",
+      "depType": "packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "local-dependency",
+    },
+    Object {
+      "currentValue": "*",
+      "depName": "pywinusb",
+      "depType": "packages",
+      "pipenvNestedVersion": true,
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "any-version",
+    },
+    Object {
+      "currentValue": "*",
+      "depName": "nose",
+      "depType": "dev-packages",
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+      "skipReason": "any-version",
+    },
+    Object {
+      "currentValue": ">=1.0,<3.0",
+      "datasource": "pypi",
+      "depName": "unittest2",
+      "depType": "dev-packages",
+      "pipenvNestedVersion": true,
+      "registryUrls": Array [
+        "https://pypi.python.org/simple",
+      ],
+    },
+  ],
+}
+`;
+
 exports[`lib/manager/pipenv/extract extractPackageFile() extracts multiple dependencies 1`] = `
 Array [
   Object {
@@ -56,7 +150,6 @@ Array [
     "datasource": "pypi",
     "depName": "Django",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
     ],
@@ -66,7 +159,6 @@ Array [
     "datasource": "pypi",
     "depName": "distribute",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
     ],
@@ -76,7 +168,6 @@ Array [
     "datasource": "pypi",
     "depName": "dj-database-url",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
     ],
@@ -86,7 +177,6 @@ Array [
     "datasource": "pypi",
     "depName": "psycopg2",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
     ],
@@ -96,7 +186,6 @@ Array [
     "datasource": "pypi",
     "depName": "wsgiref",
     "depType": "packages",
-    "pipenvNestedVersion": false,
     "registryUrls": Array [
       "https://pypi.org/simple",
     ],
diff --git a/test/manager/pipenv/_fixtures/Pipfile4 b/test/manager/pipenv/_fixtures/Pipfile4
new file mode 100644
index 0000000000..7c47bf3426
--- /dev/null
+++ b/test/manager/pipenv/_fixtures/Pipfile4
@@ -0,0 +1,19 @@
+[[source]]
+url = 'https://pypi.python.org/simple'
+verify_ssl = true
+name = 'pypi'
+
+[requires]
+python_version = '2.7'
+
+[packages]
+requests = { extras = ['socks'] }
+records = '>0.5.0'
+django = { git = 'https://github.com/django/django.git', ref = '1.11.4', editable = true }
+"e682b37" = {file = "https://github.com/divio/django-cms/archive/release/3.4.x.zip"}
+"e1839a8" = {path = ".", editable = true}
+pywinusb = { version = "*", os_name = "=='nt'", index="pypi"}
+
+[dev-packages]
+nose = '*'
+unittest2 = {version = ">=1.0,<3.0", markers="python_version < '2.7.9' or (python_version >= '3.0' and python_version < '3.4')"}
\ No newline at end of file
diff --git a/test/manager/pipenv/extract.spec.js b/test/manager/pipenv/extract.spec.js
index ec888afa28..f85fcf2441 100644
--- a/test/manager/pipenv/extract.spec.js
+++ b/test/manager/pipenv/extract.spec.js
@@ -13,6 +13,10 @@ const pipfile3 = fs.readFileSync(
   'test/manager/pipenv/_fixtures/Pipfile3',
   'utf8'
 );
+const pipfile4 = fs.readFileSync(
+  'test/manager/pipenv/_fixtures/Pipfile4',
+  'utf8'
+);
 
 describe('lib/manager/pipenv/extract', () => {
   describe('extractPackageFile()', () => {
@@ -29,7 +33,8 @@ describe('lib/manager/pipenv/extract', () => {
     it('extracts dependencies', () => {
       const res = extractPackageFile(pipfile1, config).deps;
       expect(res).toMatchSnapshot();
-      expect(res).toHaveLength(4);
+      expect(res).toHaveLength(6);
+      expect(res.filter(dep => !dep.skipReason)).toHaveLength(4);
     });
     it('marks packages with "extras" as skipReason === any-version', () => {
       const res = extractPackageFile(pipfile3, {
@@ -55,7 +60,8 @@ describe('lib/manager/pipenv/extract', () => {
     it('ignores invalid package names', () => {
       const content = '[packages]\r\nfoo = "==1.0.0"\r\n_invalid = "==1.0.0"';
       const res = extractPackageFile(content, config).deps;
-      expect(res).toHaveLength(1);
+      expect(res).toHaveLength(2);
+      expect(res.filter(dep => !dep.skipReason)).toHaveLength(1);
     });
     it('ignores relative path dependencies', () => {
       const content = '[packages]\r\nfoo = "==1.0.0"\r\ntest = {path = "."}';
@@ -65,7 +71,8 @@ describe('lib/manager/pipenv/extract', () => {
     it('ignores invalid versions', () => {
       const content = '[packages]\r\nfoo = "==1.0.0"\r\nsome-package = "==0 0"';
       const res = extractPackageFile(content, config).deps;
-      expect(res).toHaveLength(1);
+      expect(res).toHaveLength(2);
+      expect(res.filter(dep => !dep.skipReason)).toHaveLength(1);
     });
     it('extracts all sources', () => {
       const content =
@@ -75,5 +82,9 @@ describe('lib/manager/pipenv/extract', () => {
       const res = extractPackageFile(content, config).deps;
       expect(res[0].registryUrls).toEqual(['source-url', 'other-source-url']);
     });
+    it('extracts example pipfile', () => {
+      const res = extractPackageFile(pipfile4, config);
+      expect(res).toMatchSnapshot();
+    });
   });
 });
-- 
GitLab