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

google.protobuf.any_pb2.global__Any is not marked as TypeAlias #600

Closed
Avasam opened this issue Apr 16, 2024 · 5 comments · Fixed by #607
Closed

google.protobuf.any_pb2.global__Any is not marked as TypeAlias #600

Avasam opened this issue Apr 16, 2024 · 5 comments · Fixed by #607

Comments

@Avasam
Copy link
Contributor

Avasam commented Apr 16, 2024

#321 added the TypeAlias annotation for class.V, but using stubs generated by typeshed, there's still a single Y026 (https://github.com/PyCQA/flake8-pyi/blob/main/ERRORCODES.md / https://docs.astral.sh/ruff/rules/type-alias-without-annotation/) violation in https://github.com/python/typeshed/blob/7d56cd9a6cf6e0a4ea89c68d0397e197aff32cbe/stubs/protobuf/google/protobuf/any_pb2.pyi#L177

@nipunn1313
Copy link
Owner

yep! Would accept a fix here!
I hadn't seen this lint error before. Is it relatively new?

@nipunn1313
Copy link
Owner

I am curious why this line

global___SimpleProto3 = SimpleProto3

does not cause this lint to fire? It seems like it should also cause the lint to fire - and we would have caught that.

@Avasam
Copy link
Contributor Author

Avasam commented Apr 23, 2024

I hadn't seen this lint error before. Is it relatively new?

It's existence in Flake8-pyi not new and has been enabled by default in early 2022: PyCQA/flake8-pyi#101
It's been added to Ruff inv0.0.279 last year astral-sh/ruff#5844

If you mean that it is raised in typeshed, also not new (same timeline as mentioned above), Y026 is just currently disabled in _pb2.pyi files (as with other non-autofixable lint issues for autogenerated files)1

After bumping mypy-protobuf to v3.6.0 in typeshed I just noticed that in typeshed's usage, that was the last violation of that rule left.


I am curious why this line [...] does not cause this lint to fire? It seems like it should also cause the lint to fire - and we would have caught that.

This should hopefully answer that question: https://github.com/PyCQA/flake8-pyi/blob/f60d7e3cdc833dd358b493d2ca80fd187cc38cfb/pyi.py#L1202-L1216

Footnotes

  1. Ruff supports autofixes for PYI026, but I believe typeshed is waiting on https://github.com/python/mypy/issues/16581 or https://github.com/plinss/flake8-noqa/pull/30 to avoid conflicting error codes.

@nipunn1313
Copy link
Owner

this seems relevant

        The only exceptions are the type aliases `X = (typing.)Any`
        (special-cased, because it is so common in a stub),
        and `X = None` (special-cased because it is so special).

I would argue that its actually false-positive in https://github.com/python/typeshed/blob/7d56cd9a6cf6e0a4ea89c68d0397e197aff32cbe/stubs/protobuf/google/protobuf/any_pb2.pyi#L177 - since that Any is not typing.Any, but rather a class Any.

But in any case, we can still aim to fix. Adding a TypeAlias annotation seems wholly reasonable.

@Avasam
Copy link
Contributor Author

Avasam commented Apr 24, 2024

[...] - since that Any is not typing.Any, but rather a class Any.

You're right, I didn't look further to see it's not typing.Any. I guess the question is then, are global___* meant to be Type Alias or Variable Alias (you seem to be saying they should be type aliases anyway).

# global prefix globals are generated, but aren't used at runtime
.pb2.global__.

Output Format
[...]

  • Only mangle-alias top-level identifiers w/ global___ to avoid conflict w/ fields of same name [previously was mangling inner messages as well]

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 a pull request may close this issue.

2 participants