Skip to content

Conversation

@aidandj
Copy link
Collaborator

@aidandj aidandj commented Dec 18, 2025

Pulling in the whole googleapis subtree, generating stubs, and running mypy on them.

Found a few things.

Fields named property and collections caused conflicts with @property and collections.abc.Iterable. In theory this means that fields called builtins, typing, or other built in names could cause issues.

To work around this I did 2 things. property -> builtins.property, and all the import names aliased.

import collections.abc-> import collections.abc.Iterable as _collections_abc

This is similar to how the first party .pyi generator does it (from collections.abc import Mapping as _Mapping), but matching that exactly would have required more architectural changes.

During this switch, I found out that flake8 does ast parsing on string names, so _typing does not get parsed as typing, this caused a few issues, but I added some noqa's to handle those. I figure most people aren't running flake8 on their generated stubs.

Overall the change seems large, but conceptually it's a very minor change. And we're now testing on over 13000 proto files!!!

image

It also turns out that even though mypy-protobuf reserves field numbers 1151-1154, google itself appears to not respect it. 😬

git-subtree-dir: third_party/googleapis
git-subtree-split: 6d6acd2e993c45a7a45fc6374d6c2fc987af1f77
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR integrates the googleapis repository as comprehensive test data for mypy-protobuf, significantly expanding test coverage from a few dozen to over 13,000 proto files. The changes address naming conflicts discovered during this integration where proto field names like property and collections clash with Python built-ins and standard library modules. The solution aliases all imported modules with underscore prefixes (e.g., import collections.abc as _collections_abc) and uses builtins.property for the @property decorator to avoid conflicts.

Key Changes:

  • Modified import aliasing strategy to prefix all standard library imports with underscores
  • Updated field name handling to use builtins.property to avoid conflicts with @property decorator
  • Added googleapis as a third-party test target
  • Extended test exclusions and cleanup procedures for the new test data
  • Added test proto fields (property, collections) to validate the fix

Reviewed changes

Copilot reviewed 77 out of 8800 changed files in this pull request and generated no comments.

File Description
run_test.sh Adds googleapis proto generation, mypy validation, and cleanup steps to test pipeline
pyproject.toml Excludes unittest proto stubs from type checking
proto/testproto/test.proto Adds property and collections fields to test naming conflict resolution
proto/google/protobuf/*.proto Adds Google protobuf test proto files for comprehensive testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
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.

1 participant