Skip to content

Commit

Permalink
Do not generate abstract property for invariant types.
Browse files Browse the repository at this point in the history
Summary:
# What?
Do not generate `fbthrift_current_value` if any field of the union is an invariant type.

# Why?
`fbthrift_current_value` returns a `typing.Union` of the types of the fields in the union plus `None`. If any of the fields have an invariant type, then the type-check will report that the `fbthrift_current_value` in the derived class in `thrift_mutable_types` overrides the attribute defined in the `thrift_abstract_types` base class inconsistently. `thrift_types` doesn't flag this same error due to some unrelated error that has a separate task to fix it.

# Build fixtures
`buck2 run fbcode//thrift/compiler/test:build_fixtures`

# Internal
# What was the canary?
Instagram pyre check flagged this lone error below (P1711855823). You could say this was a needle-in-the-haystack as well.
```
distillery/igthrift/gen/apache/thrift/protocol/detail/protocol_detail/thrift_mutable_types.pyi:172:4 Inconsistent override [15]: `fbthrift_current_value` overrides attribute defined in `_fbthrift_python_abstract_types.Value` inconsistently. Type `_typing.Union[None, _fbthrift_python_mutable_containers.MutableList[Value], _fbthrift_python_mutable_containers.MutableMap[Value, Value], _fbthrift_python_mutable_containers.MutableSet[Value], Object, _fbthrift_iobuf.IOBuf, bool, bytes, float, int]` is not a subtype of the overridden attribute `_typing.Union[None, _typing.AbstractSet[_fbthrift_python_abstract_types.Value], _typing.Mapping[_fbthrift_python_abstract_types.Value, _fbthrift_python_abstract_types.Value], _typing.Sequence[_fbthrift_python_abstract_types.Value], _fbthrift_iobuf.IOBuf, _fbthrift_python_abstract_types.Object, bool, bytes, float, int]`.
```

# Task to fix `thrift_types.pyi` type checks
T212883284 [thrift-python] Fix thrift_types.pyi type-checks

Reviewed By: yoney

Differential Revision: D68270113

fbshipit-source-id: dde2d437330741239c573ecf64eadccfb44f5b86
  • Loading branch information
Satish Kumar authored and facebook-github-bot committed Jan 17, 2025
1 parent b289796 commit b7b0c97
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ class {{> structs/unadapted_name}}({{!
used in immutable thrift-python, which is the name
of the Union (sigh!)
}}FbThriftUnionFieldEnum.__name__ = "{{struct:py_name}}"
{{^struct:has_invariant_field?}}
@_fbthrift_property
{{> fields/maybe_abstract_method_annotation}}
def fbthrift_current_value(self) -> {{> types/field_value_pep484_union_type}}: ...
{{/struct:has_invariant_field?}}
@_fbthrift_property
{{> fields/maybe_abstract_method_annotation}}
def fbthrift_current_field(self) -> FbThriftUnionFieldEnum: ...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ class FbThriftUnionFieldEnum(_enum.Enum):
FbThriftUnionFieldEnum.__name__ = "InvariantTypes"
@_fbthrift_property
@_abc.abstractmethod
def fbthrift_current_value(self) -> _typing.Union[None, _typing.Mapping[_fbthrift_StructForInvariantTypes, int], _typing.Mapping[_fbthrift_UnionForInvariantTypes, int]]: ...
@_fbthrift_property
@_abc.abstractmethod
def fbthrift_current_field(self) -> FbThriftUnionFieldEnum: ...

_fbthrift_InvariantTypes = InvariantTypes

0 comments on commit b7b0c97

Please sign in to comment.