Merge pull request #82 from snyk/fix/support-or-statements

fix: Support 'or' operators in marker expressions

dev-requirements.txt

@@ -1,2 +1,3 @@
 pipenv
 virtualenv
+mock

package.json

@@ -9,6 +9,7 @@
   "main": "dist/index.js",
   "scripts": {
     "build": "tsc",
+    "watch": "tsc -w",
     "build-tests": "tsc -p tsconfig-test.json",
     "format:check": "prettier --check '{lib,test}/**/*.{js,ts}'",
     "format": "prettier --write '{lib,test}/**/*.{js,ts}'",

pysrc/pip_resolve.py

@@ -20,7 +20,7 @@
         raise ImportError(
             "Could not import pkg_resources; please install setuptools or pip.")
 
-PYTHON_MARKER_REGEX = re.compile(r'python_version\s*(?P<operator>==|<=|=>|>|<)\s*\'(?P<python_version>.+)\'')
+PYTHON_MARKER_REGEX = re.compile(r'python_version\s*(?P<operator>==|<=|=>|>|<)\s*[\'"](?P<python_version>.+?)[\'"]')
 SYSTEM_MARKER_REGEX = re.compile(r'sys_platform\s*==\s*[\'"](.+)[\'"]')
 
 def format_provenance_label(prov_tuple):
@@ -123,7 +123,7 @@ def create_package_as_root(package, dir_as_root):
             dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_tree
     return dir_as_root
 
-def satisfies_python_requirement(parsed_operator, py_version_str, sys=sys):
+def satisfies_python_requirement(parsed_operator, py_version_str):
     # TODO: use python semver library to compare versions
     operator_func = {
         ">": gt,
@@ -145,34 +145,35 @@ def satisfies_python_requirement(parsed_operator, py_version_str, sys=sys):
 
 def get_markers_text(requirement):
     if isinstance(requirement, pipfile.PipfileRequirement):
-        markers_text = requirement.markers
-    else:
-        markers_text = requirement.line
-    return markers_text
+        return requirement.markers
+    return requirement.line
 
 def matches_python_version(requirement):
     """Filter out requirements that should not be installed
     in this Python version.
     See: https://www.python.org/dev/peps/pep-0508/#environment-markers
     """
     markers_text = get_markers_text(requirement)
-    if not markers_text:
-        return True
-    if not 'python_version' in markers_text:
+    if not (markers_text and re.match(".*;.*python_version", markers_text)):
         return True
-    match = PYTHON_MARKER_REGEX.search(markers_text)
-    if not match:
-        return False
-    parsed_operator = match.group('operator')
-    parsed_python_version = match.group('python_version')
-
-    if not parsed_python_version or not parsed_operator:
-        return False
-
-    return satisfies_python_requirement(
-        parsed_operator,
-        parsed_python_version
-    )
+
+    cond_text = markers_text.split(";", 1)[1]
+
+    # Gloss over the 'and' case and return true on the first matching python version
+
+    for sub_exp in re.split("\s*(?:and|or)\s*", cond_text):
+        match = PYTHON_MARKER_REGEX.search(sub_exp)
+
+        if match:
+            match_dict = match.groupdict()
+
+            if len(match_dict) == 2 and satisfies_python_requirement(
+                    match_dict['operator'],
+                    match_dict['python_version']
+                ):
+                return True
+
+    return False
 
 
 def matches_environment(requirement):

pysrc/test_pip_resolve.py

@@ -1,24 +1,142 @@
 # run with:
-# cd pysrc; python3 pip_resolve_test.py; cd ..
+# cd pysrc; python3 test_pip_resolve.py; cd ..
 
-from pip_resolve import satisfies_python_requirement
+from pip_resolve import satisfies_python_requirement, \
+                        matches_python_version, \
+                        matches_environment
 from collections import namedtuple
 
 import unittest
 
-fake_sys = namedtuple('Sys', ['version_info'])
+try:
+    from mock import patch
+except:
+    from unittest.mock import patch
 
 class TestStringMethods(unittest.TestCase):
 
-    def test(self):
-        self.assertTrue(satisfies_python_requirement('>', '2.4', sys=fake_sys((2, 5))))
-        self.assertTrue(satisfies_python_requirement('==', '2.3', sys=fake_sys((2, 3))))
-        self.assertTrue(satisfies_python_requirement('<=', '2.3', sys=fake_sys((2, 3))))
-        self.assertFalse(satisfies_python_requirement('<', '2.3', sys=fake_sys((2, 3))))
-        self.assertTrue(satisfies_python_requirement('>', '3.1', sys=fake_sys((3, 5))))
-        self.assertFalse(satisfies_python_requirement('>', '3.1', sys=fake_sys((2, 8))))
-        self.assertTrue(satisfies_python_requirement('==', '2.*', sys=fake_sys((2, 6))))
-        self.assertTrue(satisfies_python_requirement('==', '3.*', sys=fake_sys((3, 6))))
+    def test_satisfies_python_requirement(self):
+
+        with patch('pip_resolve.sys') as mock_sys:
+            mock_sys.version_info = (2, 5)
+            self.assertTrue(satisfies_python_requirement('>', '2.4'))
+
+            mock_sys.version_info = (2, 3)
+            self.assertTrue(satisfies_python_requirement('==', '2.3'))
+            self.assertTrue(satisfies_python_requirement('<=', '2.3'))
+            self.assertFalse(satisfies_python_requirement('<', '2.3'))
+
+            mock_sys.version_info = (3, 5)
+            self.assertTrue(satisfies_python_requirement('>', '3.1'))
+
+            mock_sys.version_info = (2, 8)
+            self.assertFalse(satisfies_python_requirement('>', '3.1'))
+
+            mock_sys.version_info = (2, 6)
+            self.assertTrue(satisfies_python_requirement('==', '2.*'))
+
+            mock_sys.version_info = (3, 6)
+            self.assertTrue(satisfies_python_requirement('==', '3.*'))
+
+
+    def test_matches_python_version(self):
+
+        req = namedtuple('requirement', ['line'])
+
+        with patch('pip_resolve.sys') as mock_sys:
+
+            mock_sys.version_info = (2, 5)
+            req.line = "futures==3.2.0; python_version == '2.6'"
+            self.assertFalse(matches_python_version(req))
+
+            mock_sys.version_info = (2, 6)
+            req.line = "futures==3.2.0; python_version == '2.6'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 5)
+            req.line = "futures==3.2.0; python_version <= '2.6'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 5)
+            req.line = 'futures==3.2.0; python_version <= "2.6"'
+            self.assertTrue(matches_python_version(req))
+
+            # BUG: python_version is always expected on the left side
+            # mock_sys.version_info = (2, 5)
+            # req.line = 'futures==3.2.0; "2.6" >= python_version'
+            # self.assertTrue(matches_python_version(req))
+
+            # BUG: Double quotes are supported but allow illegal statements
+            mock_sys.version_info = (2, 5)
+            req.line = '''futures==3.2.0; python_version <= '2.6"'''
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 6)
+            req.line = "futures==3.2.0; python_version == '2.6' or python_version == '2.7'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 7)
+            req.line = "futures==3.2.0 ; python_version == '2.6' or python_version == '2.7'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 7)
+            req.line = "futures==3.2.0 ; python_version == '2.5' or python_version == '2.6'" \
+                " or python_version == '2.7'"
+            self.assertTrue(matches_python_version(req))
+
+            # BUG: Comments are not supported
+            #mock_sys.version_info = (2, 7)
+            #req.line = "futures==3.2.0 ; python_version == '2.6' # or python_version == '2.7'"
+            #self.assertFalse(matches_python_version(req))
+
+            # BUG: The 'and' case doesn't really make sesne but should be handled correctly
+            mock_sys.version_info = (2, 7)
+            req.line = "futures==3.2.0 ; python_version == '2.6' and python_version == '2.7'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 6)
+            req.line = "futures==3.2.0; python_version == '2.6' and sys_platform == 'linux2'"
+            self.assertTrue(matches_python_version(req))
+
+            mock_sys.version_info = (2, 7)
+            req.line = "futures==3.2.0; python_version == '2.6' and sys_platform == 'linux2'"
+            self.assertFalse(matches_python_version(req))
+
+
+    def test_matches_environment(self):
+
+        req = namedtuple('requirement', ['line'])
+
+        with patch('pip_resolve.sys') as mock_sys:
+
+            mock_sys.platform = "LInux2"
+            req.line = "futures==3.2.0; sys_platform == 'linux2'"
+            self.assertTrue(matches_environment(req))
+
+            # BUG: sys_platform is always expected on the left side
+            # mock_sys.platform = "win2000"
+            # req.line = "futures==3.2.0; 'linux2' == sys_platform"
+            # self.assertFalse(matches_environment(req))
+
+            mock_sys.platform = "linux2"
+            req.line = 'futures==3.2.0; sys_platform == "linux2"'
+            self.assertTrue(matches_environment(req))
+
+            mock_sys.platform = "win2000"
+            req.line = "futures==3.2.0; sys_platform == 'linux2'"
+            self.assertFalse    (matches_environment(req))
+
+            # BUG: Only == operator is supported in the moment
+            # mock_sys.platform = "linux2"
+            # req.line = "futures==3.2.0; sys_platform != 'linux2'"
+            # self.assertTrue(matches_environment(req))
+
+            # BUG: Expressions containing logical operators are not supported
+            # mock_sys.platform = "win2000"
+            # req.line = "futures==3.2.0; python_version == '2.6' and sys_platform == 'linux2'"
+            # self.assertTrue(matches_environment(req))
+
+
 
 if __name__ == '__main__':
     unittest.main()