package-url / packageurl-python

Python implementation of the package url spec. This project is sponsored by NLnet project https://nlnet.nl/project/vulnerabilitydatabase/ , the Google Summer of Code, nexB and other generous sponsors.
69 stars 44 forks source link

Qualifiers typing #169

Open jhoward-lm opened 1 day ago

jhoward-lm commented 1 day ago

Examining PackageURL's __new__ and from_string methods, it looks like it isn't possible to provide encode=True when the qualifiers get normalized. In other words, an instantiated PackageURL object's qualifiers will always be a dict, unless I'm missing something. It appears that using encode only comes into play when calling to_dict.

The behavior is the same whether calling from_string:

Example ```python >>> import rich >>> from packageurl import PackageURL >>> purl = PackageURL.from_string("pkg:deb/ubuntu/ca-certificates@20230311?arch=all&distro=lunar") >>> rich.print(purl) PackageURL( type='deb', namespace='ubuntu', name='ca-certificates', version='20230311', qualifiers={'arch': 'all', 'distro': 'lunar'}, subpath=None ) >>> rich.print(f"{purl.qualifiers=} ({type(purl.qualifiers)})") purl.qualifiers={'arch': 'all', 'distro': 'lunar'} () ```

or calling the constructor directly, passing in qualifiers explicitly as a string:

Example ```python >>> purl = PackageURL( ... type="deb", ... namespace="ubuntu", ... name="ca-certificates", ... version="20230311", ... qualifiers="arch=all&distro=lunar", ... subpath=None, ... ) >>> rich.print(purl) PackageURL( type='deb', namespace='ubuntu', name='ca-certificates', version='20230311', qualifiers={'arch': 'all', 'distro': 'lunar'}, subpath=None ) >>> rich.print(f"{purl.qualifiers=} ({type(purl.qualifiers)})") purl.qualifiers={'arch': 'all', 'distro': 'lunar'} () ```

However, the type of that field is declared as Union[str, Dict[str, str], None], causing downstream code to have to write quite a few if isinstance(purl.qualifiers, dict) checks, or write custom type stubs to work around it. The workaround is less than ideal when having to maintain multiple copies of the same stubs across multiple projects.

Would it be possible to change the field declaration type to just Dict[str, str]? The signature of __new__ would not be affected. I'm happy to contribute the change if the maintainers are in agreement.

tdruez commented 21 hours ago

Hi @gruebel, since you started implementing type hints in this repo, I’d love to hear your thoughts on this issue. What do you think?

gruebel commented 20 hours ago

Hey @jhoward-lm, thanks for raising this. I also double checked the code and you are right. PackageURL itself is immutable, so there is no way to override the value after initialization. Definitely go ahead and adjust field type 💪

@tdruez I'm not sure, how you want to treat the change? For me this is a bugfix, but it is up to you.

tdruez commented 20 hours ago

@gruebel Thanks for your input! @jhoward-lm You're welcome to start a PR 👍