githubkit icon indicating copy to clipboard operation
githubkit copied to clipboard

Prevent `list` method from shadowing `list` in type annotations

Open sterliakov opened this issue 10 months ago • 5 comments
trafficstars

Currently some features do not support type checking with mypy due to invalid type hints in the library. For example, the following:

issues = gh.rest.issues.list_for_repo("python", "mypy", state="open", sort="created").parsed_data
for i in issues: ...

will produce mypy errors. A small repro to explain the problem:

class Foo:
    def list(self) -> None: ...
    
    def method(self) -> list[str]:  # E: Function "__main__.Foo.list" is not valid as a type  [valid-type]
        return [""]

Foo().method().append("")  # E: "list?[builtins.str]" has no attribute "append"  [attr-defined]

The most common solution for this problem is to use builtins.list in such cases, which I do in this PR. Unfortunately, the schema was recently updated, so there are unrelated diffs withs moved/added classes.

To prevent similar issues from appearing in future, I suggest to add a mypy (and probably pyright) check to the project CI. I will submit a PR to do so once this is merged (those changes are smaller but still overlap with codegen, because we need a few ignore comments until https://peps.python.org/pep-0747/ is approved and implemented). There are a few type errors to correct, but those are mostly simple. In particular, #176 introduced a bug: QueryParamTypes and HeaderTypes must stick to dict and list because upstream httpx declares so and explicitly checks with isinstance, so passing some other Mapping or Sequence will either result in a crash or produce unexpected results.

sterliakov avatar Jan 23 '25 20:01 sterliakov