Skip to content

Commit

Permalink
Do not generate the property for invariant container
Browse files Browse the repository at this point in the history
Summary:
Suppress generating properties for invariant container types.

`Mapping` is invariant in its key type, example below will emit error `Signature of "my_set" incompatible with supertype "Abstract_B"  [override]` in mypy.

```
from typing import Mapping

class Abstract_A:
    pass

class A(Abstract_A):
    pass

class Abstract_B:
    property
    def my_set(self) -> Mapping[Abstract_A, int]:
        return {}

class B(Abstract_B):
    property
    def my_set(self) -> Mapping[A, int]:
        return {}
```

Reviewed By: ahilger, createdbysk

Differential Revision: D67674205

fbshipit-source-id: 0d5faeeaf404a9bbf1c62844bb9f6c55eb31d3c8
  • Loading branch information
yoney authored and facebook-github-bot committed Dec 28, 2024
1 parent 55b13fa commit 1411e8e
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,8 @@ class python_mstch_field : public mstch_field {
&python_mstch_field::user_default_value},
{"field:has_adapter?", &python_mstch_field::adapter},
{"field:is_container_type", &python_mstch_field::is_container_type},
{"field:is_invariant_container_type?",
&python_mstch_field::is_invariant_container_type},
});
}

Expand Down Expand Up @@ -933,6 +935,19 @@ class python_mstch_field : public mstch_field {
type->get_true_type()->is_map() || type->get_true_type()->is_set();
}

mstch::node is_invariant_container_type() {
const auto* type = field_->get_type()->get_true_type();
if (type->is_map()) {
// Mapping is invariant in its key type
const auto* key_type =
dynamic_cast<const t_map*>(type)->get_key_type()->get_true_type();
return key_type->is_struct() || key_type->is_union() ||
key_type->is_exception() || key_type->is_container();
}

return false;
}

private:
const std::string py_name_;
const t_const* adapter_annotation_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ class {{> structs/unadapted_name}}({{!
}}{{^struct:exception?}}_abc.ABC{{/struct:exception?}}{{!
}}{{#struct:exception?}}_fbthrift_python_abstract_types.AbstractGeneratedError{{/struct:exception?}}):
{{#struct:fields_ordered_by_id}}
{{^field:is_invariant_container_type?}}
@_fbthrift_property
{{!
This (mis-)alignment is necessary to ensure that the abstract method annotation aligns with the def on the next line
and produces no output in the cases where it should not exist (currently for exceptions).
}}
{{> fields/maybe_abstract_method_annotation}}
def {{field:py_name}}(self) -> {{> fields/field_type_pyi}}: ...
{{/field:is_invariant_container_type?}}
{{/struct:fields_ordered_by_id}}
{{#struct:legacy_api?}}
{{> fields/maybe_abstract_method_annotation}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@ def sString(self) -> _typing.AbstractSet[str]: ...
@_fbthrift_property
@_abc.abstractmethod
def sByte(self) -> _typing.AbstractSet[int]: ...
@_fbthrift_property
@_abc.abstractmethod
def mListList(self) -> _typing.Mapping[_typing.Sequence[int], _typing.Sequence[int]]: ...
@_abc.abstractmethod
def _to_mutable_python(self) -> "module.thrift_mutable_types.MyStruct": ... # type: ignore
@_abc.abstractmethod
Expand Down Expand Up @@ -402,40 +399,16 @@ def setOfSetOfsetOfLong(self) -> _typing.AbstractSet[_typing.AbstractSet[_typing
def mapStructListOfListOfLong(self) -> _typing.Mapping[int, _typing.Sequence[_typing.Sequence[_fbthrift_MyStruct]]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mKeyStructValInt(self) -> _typing.Mapping[_fbthrift_MyStruct, int]: ...
@_fbthrift_property
@_abc.abstractmethod
def listOfMapKeyIntValInt(self) -> _typing.Sequence[_typing.Mapping[int, int]]: ...
@_fbthrift_property
@_abc.abstractmethod
def listOfMapKeyStrValList(self) -> _typing.Sequence[_typing.Mapping[str, _typing.Sequence[_fbthrift_MyStruct]]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeySetValLong(self) -> _typing.Mapping[_typing.AbstractSet[int], int]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeyListValLong(self) -> _typing.Mapping[_typing.Sequence[str], int]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeyMapValMap(self) -> _typing.Mapping[_typing.Mapping[int, str], _typing.Mapping[int, str]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeySetValMap(self) -> _typing.Mapping[_typing.AbstractSet[_typing.Sequence[int]], _typing.Mapping[_typing.Sequence[_typing.AbstractSet[str]], str]]: ...
@_fbthrift_property
@_abc.abstractmethod
def NestedMaps(self) -> _typing.Mapping[_typing.Mapping[_typing.Mapping[int, str], str], _typing.Mapping[int, str]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeyIntValList(self) -> _typing.Mapping[int, _typing.Sequence[_fbthrift_MyStruct]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeyIntValSet(self) -> _typing.Mapping[int, _typing.AbstractSet[bool]]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeySetValInt(self) -> _typing.Mapping[_typing.AbstractSet[bool], _fbthrift_MyEnum]: ...
@_fbthrift_property
@_abc.abstractmethod
def mapKeyListValSet(self) -> _typing.Mapping[_typing.Sequence[int], _typing.AbstractSet[_typing.Mapping[float, str]]]: ...
@_abc.abstractmethod
def _to_mutable_python(self) -> "module.thrift_mutable_types.ComplexNestedStruct": ... # type: ignore
@_abc.abstractmethod
Expand Down Expand Up @@ -532,12 +505,6 @@ def union_set(self) -> _typing.AbstractSet[_fbthrift_MyUnion]: ...
def enum_set(self) -> _typing.AbstractSet[_fbthrift_MyEnum]: ...
@_fbthrift_property
@_abc.abstractmethod
def struct_map(self) -> _typing.Mapping[_fbthrift_MyStruct, int]: ...
@_fbthrift_property
@_abc.abstractmethod
def union_map(self) -> _typing.Mapping[_fbthrift_MyUnion, int]: ...
@_fbthrift_property
@_abc.abstractmethod
def enum_map(self) -> _typing.Mapping[_fbthrift_MyEnum, int]: ...
@_fbthrift_property
@_abc.abstractmethod
Expand All @@ -550,21 +517,12 @@ def union_map_2(self) -> _typing.Mapping[int, _fbthrift_MyUnion]: ...
def enum_map_2(self) -> _typing.Mapping[int, _fbthrift_MyEnum]: ...
@_fbthrift_property
@_abc.abstractmethod
def list_map(self) -> _typing.Mapping[_typing.Sequence[int], int]: ...
@_fbthrift_property
@_abc.abstractmethod
def list_map_2(self) -> _typing.Mapping[int, _typing.Sequence[int]]: ...
@_fbthrift_property
@_abc.abstractmethod
def set_map(self) -> _typing.Mapping[_typing.AbstractSet[int], int]: ...
@_fbthrift_property
@_abc.abstractmethod
def set_map_2(self) -> _typing.Mapping[int, _typing.AbstractSet[int]]: ...
@_fbthrift_property
@_abc.abstractmethod
def map_map(self) -> _typing.Mapping[_typing.Mapping[int, int], int]: ...
@_fbthrift_property
@_abc.abstractmethod
def map_map_2(self) -> _typing.Mapping[int, _typing.Mapping[int, int]]: ...
@_abc.abstractmethod
def _to_mutable_python(self) -> "module.thrift_mutable_types.Containers": ... # type: ignore
Expand Down

0 comments on commit 1411e8e

Please sign in to comment.