Fix list configuration value parsing (#1676)

Resolves #1674.

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

docs/changelog/1674.bugfix.rst

@@ -0,0 +1 @@
+Fix list configuration value parsing from config file or environment variable - by :user:`gaborbernat`.

src/virtualenv/config/cli/parser.py

@@ -2,6 +2,8 @@
 
 from argparse import SUPPRESS, ArgumentDefaultsHelpFormatter, ArgumentParser
 
+from virtualenv.config.convert import get_type
+
 from ..env_var import get_env_var
 from ..ini import IniConfig
 
@@ -35,7 +37,7 @@ def _fix_defaults(self):
 
     def _fix_default(self, action):
         if hasattr(action, "default") and hasattr(action, "dest") and action.default != SUPPRESS:
-            as_type = type(action.default)
+            as_type = get_type(action)
             outcome = get_env_var(action.dest, as_type)
             if outcome is None and self.file_config:
                 outcome = self.file_config.get(action.dest, as_type)

src/virtualenv/config/convert.py

@@ -1,69 +1,81 @@
 from __future__ import absolute_import, unicode_literals
 
 import logging
+import os
 
-BOOLEAN_STATES = {
-    "1": True,
-    "yes": True,
-    "true": True,
-    "on": True,
-    "0": False,
-    "no": False,
-    "false": False,
-    "off": False,
-}
 
+class TypeData(object):
+    def __init__(self, default_type, as_type):
+        self.default_type = default_type
+        self.as_type = as_type
 
-def _convert_to_boolean(value):
-    if value.lower() not in BOOLEAN_STATES:
-        raise ValueError("Not a boolean: %s" % value)
-    return BOOLEAN_STATES[value.lower()]
+    def __repr__(self):
+        return "{}(base={}, as={})".format(self.__class__.__name__, self.default_type, self.as_type)
 
+    def convert(self, value):
+        return self.default_type(value)
 
-def _expand_to_list(value):
-    if isinstance(value, (str, bytes)):
-        value = filter(None, [x.strip() for x in value.splitlines()])
-    return list(value)
 
+class BoolType(TypeData):
+    BOOLEAN_STATES = {
+        "1": True,
+        "yes": True,
+        "true": True,
+        "on": True,
+        "0": False,
+        "no": False,
+        "false": False,
+        "off": False,
+    }
 
-def _as_list(value, flatten=True):
-    values = _expand_to_list(value)
-    if not flatten:
-        return values  # pragma: no cover
-    result = []
-    for value in values:
-        sub_values = value.split()
-        result.extend(sub_values)
-    return result
+    def convert(self, value):
+        if value.lower() not in self.BOOLEAN_STATES:
+            raise ValueError("Not a boolean: %s" % value)
+        return self.BOOLEAN_STATES[value.lower()]
 
 
-def _as_none(value):
-    if not value:
-        return None
-    return str(value)
+class NoneType(TypeData):
+    def convert(self, value):
+        if not value:
+            return None
+        return str(value)
 
 
-CONVERT = {bool: _convert_to_boolean, list: _as_list, type(None): _as_none}
+class ListType(TypeData):
+    def _validate(self):
+        """"""
 
-
-def _get_converter(as_type):
-    for of_type, func in CONVERT.items():
-        if issubclass(as_type, of_type):
-            getter = func
-            break
-    else:
-        getter = as_type
-    return getter
+    def convert(self, value, flatten=True):
+        if isinstance(value, (str, bytes)):
+            value = filter(None, [x.strip() for x in value.splitlines()])
+        values = list(value)
+        result = []
+        for value in values:
+            sub_values = value.split(os.pathsep)
+            result.extend(sub_values)
+        converted = [self.as_type(i) for i in result]
+        return converted
 
 
 def convert(value, as_type, source):
     """Convert the value as a given type where the value comes from the given source"""
-    getter = _get_converter(as_type)
     try:
-        return getter(value)
+        return as_type.convert(value)
     except Exception as exception:
-        logging.warning("%s failed to convert %r as %r because %r", source, value, getter, exception)
+        logging.warning("%s failed to convert %r as %r because %r", source, value, as_type, exception)
         raise
 
 
-__all__ = ("convert",)
+_CONVERT = {bool: BoolType, type(None): NoneType, list: ListType}
+
+
+def get_type(action):
+    default_type = type(action.default)
+    as_type = default_type if action.type is None else action.type
+    return _CONVERT.get(default_type, TypeData)(default_type, as_type)
+
+
+__all__ = (
+    "convert",
+    "get_type",
+)

tests/unit/config/test_env_var.py

@@ -1,11 +1,13 @@
 from __future__ import absolute_import, unicode_literals
 
+import os
 from argparse import Namespace
 
 import pytest
 
 from virtualenv.config.ini import IniConfig
 from virtualenv.run import session_via_cli
+from virtualenv.util.path import Path
 
 
 def parse_cli(args):
@@ -33,3 +35,14 @@ def test_value_bad(monkeypatch, caplog, empty_conf):
     assert len(caplog.messages) == 1
     assert "env var VIRTUALENV_VERBOSE failed to convert" in caplog.messages[0]
     assert "invalid literal" in caplog.messages[0]
+
+
+def test_extra_search_dir(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    value = "a{}0{}b{}c".format(os.linesep, os.linesep, os.pathsep)
+    monkeypatch.setenv("VIRTUALENV_EXTRA_SEARCH_DIR", value)
+    (tmp_path / "a").mkdir()
+    (tmp_path / "b").mkdir()
+    (tmp_path / "c").mkdir()
+    result = parse_cli(["venv"])
+    assert result.seeder.extra_search_dir == [Path("a").resolve(), Path("b").resolve(), Path("c").resolve()]