jaraco / keyring

MIT License
1.24k stars 152 forks source link

Type checking error due to casting a subclass as `backend.KeyringBackend` base class #645

Open lmmx opened 1 year ago

lmmx commented 1 year ago

Describe the bug

I am attempting to contribute an implementation using this library to a type checked project and mypy is failing the PR.

I think the cause of the failure is that the backend returned from keyring.get_keyring() is typed as the wrong class. It's being cast to keyring.backend.Backend but it is in fact a subclass.

>>> import keyring
>>> kr = keyring.get_keyring()
>>> type(kr)
<class 'keyring.backends.SecretService.Keyring'>
>>> kr = keyring.get_keyring()
>>> type(kr)
<class 'keyring.backends.SecretService.Keyring'>
>>> type(kr).mro()
[<class 'keyring.backends.SecretService.Keyring'>, <class 'keyring.backend.SchemeSelectable'>, <class 'keyring.backend.KeyringBackend'>, <class 'object'>]

https://github.com/jaraco/keyring/blob/af7233958ece36da8d493509a6388484f5534dfc/keyring/core.py#L30-L34

Type checkers reject this incorrect casting whenever a method is used that's unique to a subclass, such as get_preferred_collection() which exists on keyring.backends.SecretService.Keyring but not keyring.backend.KeyringBackend.

pydantic_settings/sources.py:666: error: Incompatible types in assignment (expression has type "KeyringBackend", variable has type "Keyring")  [assignment]
pydantic_settings/sources.py:676: error: Call to untyped function "get_preferred_collection" in typed context  [no-untyped-call]
Found 2 errors in 1 file (checked 5 source files)

due to this code at line 666 :imp: :

663     def _read_keyring(self, case_sensitive: bool) -> Mapping[str, str | None]:
664         keyring_backend = self.keyring_backend
665         if keyring_backend is None:
666             kr: SecretServiceKeyring = keyring.core.get_keyring()
667         else:
668             all_keyrings: Iterator[SecretServiceKeyring] = keyring.backend.get_all_keyring()

and further on at 676:

675         keyring_vars: dict[str, str | None] = {}
676         kr_collection: SecretServiceCollection = kr.get_preferred_collection()
677         kr_items = kr_collection.get_all_items()
678         keyring_vars.update({item.get_attributes()['service']: item.get_secret().decode() for item in kr_items})
679         return keyring_vars

It looks tricky but maybe I can help make some progress on these type annotations, maybe the cast helps identify a place to start?

I can't figure out if there is already a mypy workflow in use in this project (is pytest-mypy called somewhere?)

jaraco commented 9 months ago

Thanks for the report and the offer to help. Yes, mypy checks are run as part of the test suite (via pytest-mypy). Simply run tox and it will execute the test suite including any mypy checks. Run tox -- -k mypy to run only the mypy tests. I just ran them and they are in fact failing in main, so probably the failing tests need to be addressed first.

Sbennett99 commented 6 months ago

I assume there are plans to fix this issue . Is there an eta?

jaraco commented 6 months ago

No ETA. I welcome anyone to address it. I'm actively working on getting the test suite passing again.