diff --git a/changelog/@unreleased/pr-749.v2.yml b/changelog/@unreleased/pr-749.v2.yml new file mode 100644 index 000000000..435d2f7cb --- /dev/null +++ b/changelog/@unreleased/pr-749.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: order union properties to avoid type collision + links: + - https://github.com/palantir/conjure-python/pull/749 diff --git a/conjure-python-core/src/main/java/com/palantir/conjure/python/poet/UnionSnippet.java b/conjure-python-core/src/main/java/com/palantir/conjure/python/poet/UnionSnippet.java index 03b12f492..7d39deb36 100644 --- a/conjure-python-core/src/main/java/com/palantir/conjure/python/poet/UnionSnippet.java +++ b/conjure-python-core/src/main/java/com/palantir/conjure/python/poet/UnionSnippet.java @@ -178,22 +178,16 @@ default void emit(PythonPoetWriter poetWriter) { poetWriter.decreaseIndent(); // python @builtins.property for each member of the union - options().forEach(option -> { - poetWriter.writeLine(); - - poetWriter.writeIndentedLine("@builtins.property"); - poetWriter.writeIndentedLine( - String.format("def %s(self) -> Optional[%s]:", propertyName(option), option.myPyType())); + options().stream() + .filter(option -> !propertyName(option).equals("float")) + .forEach(option -> emitProperty(poetWriter, option)); - poetWriter.increaseIndent(); - option.docs().ifPresent(docs -> { - poetWriter.writeIndentedLine("\"\"\""); - poetWriter.writeIndentedLine(docs.get().trim()); - poetWriter.writeIndentedLine("\"\"\""); - }); - poetWriter.writeIndentedLine(String.format("return self.%s", fieldName(option))); - poetWriter.decreaseIndent(); - }); + // properties with builtin names must come after other properties + // can be removed once these names are properly sanitized but + // that will require a breaking change and major version bump + options().stream() + .filter(option -> propertyName(option).equals("float")) + .forEach(option -> emitProperty(poetWriter, option)); String visitorName = String.format("%sVisitor", className()); String definitionVisitorName = String.format("%sVisitor", definitionName()); @@ -246,6 +240,23 @@ default void emit(PythonPoetWriter poetWriter) { }); } + private static void emitProperty(PythonPoetWriter poetWriter, PythonField option) { + poetWriter.writeLine(); + + poetWriter.writeIndentedLine("@builtins.property"); + poetWriter.writeIndentedLine( + String.format("def %s(self) -> Optional[%s]:", propertyName(option), option.myPyType())); + + poetWriter.increaseIndent(); + option.docs().ifPresent(docs -> { + poetWriter.writeIndentedLine("\"\"\""); + poetWriter.writeIndentedLine(docs.get().trim()); + poetWriter.writeIndentedLine("\"\"\""); + }); + poetWriter.writeIndentedLine(String.format("return self.%s", fieldName(option))); + poetWriter.decreaseIndent(); + } + class Builder extends ImmutableUnionSnippet.Builder {} static Builder builder() { diff --git a/conjure-python-core/src/test/resources/types/example-types.yml b/conjure-python-core/src/test/resources/types/example-types.yml index aa4814fe4..8c89b4615 100644 --- a/conjure-python-core/src/test/resources/types/example-types.yml +++ b/conjure-python-core/src/test/resources/types/example-types.yml @@ -172,3 +172,9 @@ types: alias: RecursiveObjectAlias CollectionAliasExample: alias: map + UnionWithBuiltinVariantName: + union: + # the name of this variant is 'float' which conflicts with the python type 'float' + float: double + # the python type for this field is 'float' which conflicts with the variant name above + double: double diff --git a/conjure-python-core/src/test/resources/types/expected/package_name/_impl.py b/conjure-python-core/src/test/resources/types/expected/package_name/_impl.py index 239b3861d..8f2ed2eb5 100644 --- a/conjure-python-core/src/test/resources/types/expected/package_name/_impl.py +++ b/conjure-python-core/src/test/resources/types/expected/package_name/_impl.py @@ -1594,6 +1594,83 @@ def _property(self, property: int) -> Any: product_UnionTypeExampleVisitor.__module__ = "package_name.product" +class product_UnionWithBuiltinVariantName(ConjureUnionType): + _float: Optional[float] = None + _double: Optional[float] = None + + @builtins.classmethod + def _options(cls) -> Dict[str, ConjureFieldDefinition]: + return { + 'float': ConjureFieldDefinition('float', float), + 'double': ConjureFieldDefinition('double', float) + } + + def __init__( + self, + float: Optional[float] = None, + double: Optional[float] = None, + type_of_union: Optional[str] = None + ) -> None: + if type_of_union is None: + if (float is not None) + (double is not None) != 1: + raise ValueError('a union must contain a single member') + + if float is not None: + self._float = float + self._type = 'float' + if double is not None: + self._double = double + self._type = 'double' + + elif type_of_union == 'float': + if float is None: + raise ValueError('a union value must not be None') + self._float = float + self._type = 'float' + elif type_of_union == 'double': + if double is None: + raise ValueError('a union value must not be None') + self._double = double + self._type = 'double' + + @builtins.property + def double(self) -> Optional[float]: + return self._double + + @builtins.property + def float(self) -> Optional[float]: + return self._float + + def accept(self, visitor) -> Any: + if not isinstance(visitor, product_UnionWithBuiltinVariantNameVisitor): + raise ValueError('{} is not an instance of product_UnionWithBuiltinVariantNameVisitor'.format(visitor.__class__.__name__)) + if self._type == 'float' and self.float is not None: + return visitor._float(self.float) + if self._type == 'double' and self.double is not None: + return visitor._double(self.double) + + +product_UnionWithBuiltinVariantName.__name__ = "UnionWithBuiltinVariantName" +product_UnionWithBuiltinVariantName.__qualname__ = "UnionWithBuiltinVariantName" +product_UnionWithBuiltinVariantName.__module__ = "package_name.product" + + +class product_UnionWithBuiltinVariantNameVisitor: + + @abstractmethod + def _float(self, float: float) -> Any: + pass + + @abstractmethod + def _double(self, double: float) -> Any: + pass + + +product_UnionWithBuiltinVariantNameVisitor.__name__ = "UnionWithBuiltinVariantNameVisitor" +product_UnionWithBuiltinVariantNameVisitor.__qualname__ = "UnionWithBuiltinVariantNameVisitor" +product_UnionWithBuiltinVariantNameVisitor.__module__ = "package_name.product" + + class product_UuidExample(ConjureBeanType): @builtins.classmethod diff --git a/conjure-python-core/src/test/resources/types/expected/package_name/product/__init__.py b/conjure-python-core/src/test/resources/types/expected/package_name/product/__init__.py index d92acf564..31cbf3ffd 100644 --- a/conjure-python-core/src/test/resources/types/expected/package_name/product/__init__.py +++ b/conjure-python-core/src/test/resources/types/expected/package_name/product/__init__.py @@ -43,6 +43,8 @@ product_StringExample as StringExample, product_UnionTypeExample as UnionTypeExample, product_UnionTypeExampleVisitor as UnionTypeExampleVisitor, + product_UnionWithBuiltinVariantName as UnionWithBuiltinVariantName, + product_UnionWithBuiltinVariantNameVisitor as UnionWithBuiltinVariantNameVisitor, product_UuidAliasExample as UuidAliasExample, product_UuidExample as UuidExample, )