Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure disp_class cannot be None in DispatchWithEvents and WithEvents #2322

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Jul 25, 2024

If gencache.GetClassForProgID returns None, the error message is now a clearer TypeError instead of the previous generic AttributeError: 'NoneType' object has no attribute 'CLSID'

else:
disp_class = disp.__class__
if disp_class is None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! This could all still stay in that inner if block

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes, but neither mypy nor pyright seem to see that disp_class cannot be None if I raise after the try-except in the if branch.
Some likely relevant issues:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplicating code between DispatchWithEvents and WithEvents resolved this anyway

Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn! But fair enough I guess...

@Avasam Avasam requested a review from mhammond October 18, 2024 20:03
@Avasam Avasam changed the title Ensure disp_class cannot be None in DispatchWithEvents Ensure disp_class cannot be None in DispatchWithEvents and WithEvents Oct 18, 2024
@Avasam
Copy link
Collaborator Author

Avasam commented Oct 18, 2024

I noticed the code was near-identical in another function and that this improvement could apply to it as well. So I combined the code in a different method which also resolves the type-narrowing issue. Please let me know if this change suits you.

Comment on lines +20 to +22
from __future__ import annotations

mapCLSIDToClass: dict[str, type] = {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for this change. It just helped be make sense of, and validate the type narrowing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants