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

Address most reportCallIssue found by pyright #2333

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

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Jul 27, 2024

Address most reportCallIssue found by pyright. Everything left needs to be fixed in typeshed.

@@ -76,7 +76,7 @@ def Reset(self):
return self._oleobj_.Reset()

def Clone(self):
return self.__class__(self._oleobj_.Clone(), self.resultCLSID)
return self.__class__(self._oleobj_.Clone())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using the following code snippet:

import pythoncom
from win32com.client import util

catinf = pythoncom.CoCreateInstance(
    pythoncom.CLSID_StdComponentCategoriesMgr,
    None,
    pythoncom.CLSCTX_INPROC,
    pythoncom.IID_ICatInformation,
)
enum = util.Enumerator(catinf.EnumCategories())
enum.Clone()

Before:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Program Files\Python39\lib\site-packages\win32com\client\util.py", line 78, in Clone
    return self.__class__(self._oleobj_.Clone(), self.resultCLSID)
AttributeError: 'Enumerator' object has no attribute 'resultCLSID'

After:

<win32com.client.util.Enumerator object at 0x00000131BD4E1F40>

@Avasam Avasam force-pushed the fix-most-reportCallIssue branch from 28a7ac2 to 3db5527 Compare July 27, 2024 02:08
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.

I don't understand what this actually fixes? Why shouldn't we just tell pyright to not complain about all of this?

@@ -439,7 +441,7 @@ def ImportFile():
# meaning sys.modules can change as a side-effect of looking at
# module.__file__ - so we must take a copy (ie, list(items()))
for key, mod in sys.modules.items():
if getattr(mod, "__file__", None):
if hasattr(mod, "__file__") and mod.__file__:
Copy link
Owner

Choose a reason for hiding this comment

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

how is this an improvement?

Copy link
Collaborator Author

@Avasam Avasam Mar 10, 2025

Choose a reason for hiding this comment

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

Since dropping Python 3.7, this can be rewritten to still use getattr and still be statically safe. I've updated in the last push

@@ -226,7 +226,7 @@ def currentNumberToken(self):
# quote. consumes all tokens until the end of the string
def currentQuotedString(self):
# Handle quoted strings - pity shlex doesn't handle it.
assert self.token.startswith('"'), self.token
assert self.token and self.token.startswith('"'), self.token
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Improves the error message in the case where self.token is None (from an AttributeError to this AssertionError)

@@ -177,7 +177,7 @@ class RCParser:
dialogs: dict[str, list[list[str | int | None | tuple[str | int, ...]]]] = {}
_dialogs: dict[str, DialogDef] = {}
debugEnabled = False
token = ""
token: str | None = ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reveals issues in code that assume token can't be None

Comment on lines +480 to +501
@overload
def GetTextRange(
self,
start: int,
end: int,
decode: Literal[False],
) -> bytes: ...
@overload
def GetTextRange(
self,
start: int = 0,
end: int = -1,
*,
decode: Literal[False],
) -> bytes: ...
@overload
def GetTextRange(
self,
start: int = 0,
end: int = -1,
decode: Literal[True] = True,
) -> str: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This overload makes it so callers statically know whether the return type will be str or bytes based on the decode param. (this fixes typing issues elsewhere in the code).

@Avasam Avasam requested a review from mhammond March 12, 2025 03:17
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