parse_version() should accept None and empty strings

[pombredanne]

In https://github.com/nexB/univers/blob/63bd5aec16ec95b5b811ede638ac225f3ab1f6c6/src/univers/versions.py#L286 we should let parse_version() accept None and empty strings, otherwise the calling code has too much work to do.

[Hritik14]

@pombredanne In the current structure this should translate to the build_value fn: https://github.com/nexB/univers/blob/a52ea9900d87ef6cd0ed989bf20a41a794cf39c4/src/univers/versions.py#L87

Also, now that we have is_valid, is this even required ? https://github.com/nexB/univers/blob/a52ea9900d87ef6cd0ed989bf20a41a794cf39c4/src/univers/versions.py#L70

[Hritik14]

To add, Version(None) throws error

[Subhraneel77]

Adding version("") should work , as assigning "" to a variable to initialize an empty string in Python. @pombredanne

[Hritik14]

Version(None) still throws an error.

>>> from univers.versions import Version
>>> Version(None)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-dece84ce917a> in <module>
----> 1 Version(None)

~/Contrib/univers/src/univers/versions.py in __init__(self, string, normalized_string, value)
      5     _inst_dict['normalized_string'] = normalized_string
      6     _inst_dict['value'] = value
----> 7     self.__attrs_post_init__()

~/Contrib/univers/src/univers/versions.py in __attrs_post_init__(self)
     59
     60     def __attrs_post_init__(self):
---> 61         normalized_string = self.normalize(self.string)
     62         if not self.is_valid(normalized_string):
     63             raise InvalidVersion(f"{self.string!r} is not a valid {self.__class__!r}")

~/Contrib/univers/src/univers/versions.py in normalize(cls, string)
     86         """
     87         # FIXME: Is removing spaces and strip v the right thing to do?
---> 88         return remove_spaces(string).rstrip("v")
     89
     90     @classmethod

~/Contrib/univers/src/univers/utils.py in remove_spaces(string)
      7
      8 def remove_spaces(string):
----> 9     return "".join(string.split())
     10
     11

AttributeError: 'NoneType' object has no attribute 'split'

[pombredanne]

Version(None) still throws an error.

IMHO that's a feature. When would a None version make sense?

[pombredanne]

Adding version("") should work , as assigning "" to a variable to initialize an empty string in Python. @pombredanne

Thanks, but None or empty would be the same... and I am not sure this would make any sense.

[Hritik14]

@pombredanne

When would a None version make sense?

Please see https://github.com/nexB/univers/issues/33

[janniclas]

This causes problems in vulnerablecode when the importers/improvers get incomplete data as it is currently the case for some of them. This should be handled in the calling code in vulnerablecode ( PR https://github.com/nexB/vulnerablecode/pull/1215 ). However, I think we could also make a case for handling this in this lib, as this crash came rather unexpected to be honest.

[pombredanne]

@janniclas Thanks! Now, what should we return in these cases? None?

[janniclas]

None is what I did in my workaround to fix the problem when I encountered it in vulnerablecode. However, I'm no python dev so I don't know if this is the idiomatic solution for this, but it works fine in the calling code. I expect that None would also not break anything in any client code as the current version throws the value error for these cases which would directly crash the calling code if not handled. TLDR; None would be finde for me.

[janniclas]

@pombredanne I just added a test case to the previously linked PR, maybe this is also helpful for you :)