Resolve circular references in user code

Summary:
# What?
Tests failed due to circular references that prevented buck's ability to build python binaries.

# Why?
The use of the "self-import" to set `_fbthrift_current_module` in the various types modules was the trigger. While the root cause might be elsewhere, I didn't see a strong enough reason to determine if we could make the `_fbthrift_current_module` solution work when there was previously proved workable solution available.

# Solution?
The original problem to solve was {D66664174}.

Use a reserved identifier as an alias for the types in the modules and use this alias as the type annotation rather than the original type. This prevents the problem that user functions, which, when named the same as a type, redefine the type as a function, and render that name unusable as a type annotation.

(Side Note: This was a change in a previous iteration before the use of the current module import, which we considered less attractive than the use of the fully qualified name with the current module at that time. Now we have different constraints, which changes our answer.)

# Reject options
- Obtain the current module via `sys.modules[__name__]`.
  - Rejected because this works at run-time. It does not work at type-check time.
- Use `__fbthrift_` as the alias prefix.
   - Rejected because this leads to an undefined symbol due to the way that Python mangles the name.
```
class TestStruct(...): ...
__fbthrift_TestStruct = TestStruct

class TestFieldNameSameAsTypeName(...):
    @_fbthrift_property
    def FieldNameSameAsTypeName(def) -> __fbthrift_TestStruct: ...
```
The type-checker sees `__fbthrift_TestStruct` as `_TestFieldNameSameAsTypeName_fbthrift_TestStruct`, which is not defined.

Reviewed By: prakashgayasen

Differential Revision: D66932282

fbshipit-source-id: dd143ecd351ff3172a5b9efbac33affa6c57471d

thrift/compiler/generate/templates/python/private/alias_prefix.mustache

@@ -0,0 +1,77 @@
+{{!
+
+  Copyright (c) Meta Platforms, Inc. and affiliates.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+}}_fbthrift_{{!
+    Why this huge comment for an 10 character string?
+    It is to give someone pause before they decide it is unnecessary
+    without the knowledge of the reasons for this partial.
+
+    What is the purpose of the alias prefix?
+    This resolves the type check error below.
+        Annotation `TestFieldNameSameAsTypeName.SomeTypeName` is not defined as a type.
+
+    When a thrift field name (TestFieldNameSameAsTypeName.SomeTypeName in this example)
+    is exactly the same as a thrift type name (SomeTypeName in this example)
+    and the type name is used as the type for a field within the same type
+    that contains that field, then type check fails because the type name,
+    which is now bound to a function, is not considered a valid type annotation.
+
+    Why use a partial for the prefix, which has way more characters, than the
+    prefix directly?
+
+    The goal of partial was NOT to reduce the length of the identifier at the
+    sites that reference it. It is
+    1. to reveal intent
+    2. make it easier to find the places that use this prefix.
+    3. It is easier to explain the reason for the choice of the prefix in one
+    place, as you can see from the comment in this file.
+    4. In cases where the prefix is a common string and harder to search for
+    just the instances with this specific intent, like `_fbthrift`,
+    this makes it easier to change the name in all the sites with
+    just the change to the partial.
+    (
+        More detail: The prefix went through a few iterations of changes.
+        When I changed the name each time, especially, when it was a name like
+        `_fbthrift_`, which appears in places other than just as this alias_prefix
+        just a search and replace was not easily possible.
+        I had to rely on the tests to ensure I addressed
+        all the locations where I needed to make this change,
+        which increase the number of cycles before the change completed.
+    )
+
+    Why _fbthrift_ and not __fbthrift_, which would deter unintentional use of that symbol?
+    As you might already know, Python mangles the double underscore version
+    and it becomes more difficult to infer the correct name of the type hint,
+    if that is even possible, and brittle as well, is case Python changes how
+    it mangles that name.
+
+    The single underscore version, _fbthrift_, does not prevent use by
+    client-sites unless the module deletes this symbol within the context of
+    the module. The delete caused the call to typing.get_type_hints by the test for
+    property type hints in the abstract types test to fail because that call
+    did not have access to the deleted symbol.
+
+    I briefly considered _fbthrift_WILL_NOT_FIX_BREAKAGES_IF_YOU_USE_THIS_.
+    That makes the generated code hard to read.
+
+    I decided to stay simple to unblock this fix. I added the DO_BEFORE below
+    to revisit this issue.
+
+    DO_BEFORE(satishvk, 20241231): Determine if it is acceptable to accept
+    the risk that user code will use the _fbthrift_actual_symbol or
+    it is important enough to find a different approach to solve the problem
+    of fieldname matches the typename.
+}}

thrift/compiler/generate/templates/python/thrift_abstract_types.py.mustache

@@ -25,7 +25,6 @@ Generated Python classes for abstract thrift-python types.
 
 import abc as _abc
 import typing as _typing
-{{> types/set_current_module}}
 
 {{!
     If any type has a field named property, then the use of the built-in property will conflict
@@ -103,7 +102,8 @@ class {{> structs/unadapted_name}}({{!
 {{#struct:has_adapter?}}
 
 {{struct:py_name}} = {{adapter:type_hint}}{{#adapter:is_generic?}}[{{> structs/unadapted_name}}]{{/adapter:is_generic?}}
-{{/struct:has_adapter?}}{{!
+{{/struct:has_adapter?}}
+{{> private/alias_prefix}}{{struct:py_name}} = {{struct:py_name}}{{!
 }}{{/program:structs}}
 {{#program:typedefs?}}
 

thrift/compiler/generate/templates/python/types/import_enums.mustache

@@ -25,6 +25,7 @@ These are referenced in the generated pyi files and, for simplicity,
 need to be imported through the types modules.
 }}{{#program:enums}}
 from {{program:module_path}}.thrift_enums import _fbthrift_compatible_with_{{enum:name}}
+from {{program:module_path}}.thrift_enums import {{enum:name}} as {{> private/alias_prefix}}{{enum:name}}
 {{/program:enums}}
 
 from {{program:module_path}}.thrift_enums import *

thrift/compiler/generate/templates/python/types/python_type.mustache

@@ -27,8 +27,8 @@ in a different thrift file, or defined in the same thrift file.
 }}{{#type:string?}}str{{/type:string?}}{{!
 }}{{#type:binary?}}{{#type:iobuf?}}_fbthrift_iobuf.IOBuf{{/type:iobuf?}}{{^type:iobuf?}}bytes{{/type:iobuf?}}{{/type:binary?}}{{!
 }}{{#type:struct}}{{!
-  }}{{#type:need_module_path?}}{{type:module_mangle}}{{/type:need_module_path?}}{{!
-  }}{{^type:need_module_path?}}_fbthrift_current_module{{/type:need_module_path?}}.{{struct:py_name}}{{!
+  }}{{#type:need_module_path?}}{{type:module_mangle}}.{{/type:need_module_path?}}{{!
+  }}{{^type:need_module_path?}}{{> private/alias_prefix}}{{/type:need_module_path?}}{{struct:py_name}}{{!
 }}{{/type:struct}}{{!
 }}{{#type:list?}}{{!
   }}_fbthrift_python_types.ListTypeFactory({{!
@@ -46,7 +46,7 @@ in a different thrift file, or defined in the same thrift file.
     }}{{#type:value_type}}{{> types/typeinfo }}{{/type:value_type}}){{!
 }}{{/type:map?}}{{!
 }}{{#type:enum}}{{!
-  }}{{#type:need_module_path?}}{{type:module_mangle}}{{/type:need_module_path?}}{{!
-  }}{{^type:need_module_path?}}_fbthrift_current_module{{/type:need_module_path?}}.{{enum:name}}{{!
+  }}{{#type:need_module_path?}}{{type:module_mangle}}.{{/type:need_module_path?}}{{!
+  }}{{^type:need_module_path?}}{{> private/alias_prefix}}{{/type:need_module_path?}}{{enum:name}}{{!
 }}{{/type:enum}}{{!
 }}{{#type:void?}}None{{/type:void?}}

thrift/compiler/generate/templates/python/types/types.mustache

@@ -20,7 +20,6 @@ Generated Python classes for Thrift types
 {{> common/auto_generated_py}}
 
 import folly.iobuf as _fbthrift_iobuf
-{{> types/set_current_module}}
 
 {{#program:enable_abstract_types?}}
 from abc import ABCMeta as _fbthrift_ABCMeta
@@ -190,14 +189,15 @@ _fbthrift_ABCMeta.register(_fbthrift_abstract_types.{{> structs/unadapted_name}}
 
 
 {{struct:py_name}} = {{adapter:type_hint}}{{#adapter:is_generic?}}[{{> structs/unadapted_name}}]{{/adapter:is_generic?}}
-{{/struct:has_adapter?}}{{!
+{{/struct:has_adapter?}}
+{{> private/alias_prefix}}{{struct:py_name}} = {{struct:py_name}}{{!
 }}{{/program:structs}}
 
 {{#program:generate_immutable_types}}
 # This unfortunately has to be down here to prevent circular imports
 import {{program:module_path}}.thrift_metadata
 {{/program:generate_immutable_types}}
-from {{program:module_path}}.thrift_enums import *
+{{> types/import_enums}}
 
 {{!
     The symbol below is only required for AnyRegistry.register_module.

thrift/compiler/generate/templates/python/types/types_pyi.mustache

@@ -20,7 +20,6 @@ Generated Python typestubs for Thrift types
 {{> common/auto_generated_py}}
 
 import typing as _typing
-{{> types/set_current_module}}
 
 {{#program:unions?}}
 import enum
@@ -137,7 +136,8 @@ class {{> structs/unadapted_name}}({{!
     {{/struct:legacy_api?}}
 {{#struct:has_adapter?}}
 {{struct:py_name}} = {{adapter:type_hint}}{{#adapter:is_generic?}}[{{> structs/unadapted_name}}]{{/adapter:is_generic?}}
-{{/struct:has_adapter?}}{{!
+{{/struct:has_adapter?}}
+{{> private/alias_prefix}}{{struct:py_name}} = {{struct:py_name}}{{!
 }}{{/program:structs}}
 {{#program:constants?}}
 

thrift/compiler/generate/templates/python/types/unadapted_pep484_type.mustache

@@ -31,8 +31,8 @@ thrift-python specific container types.
 }}{{#type:string?}}str{{/type:string?}}{{!
 }}{{#type:binary?}}{{#type:iobuf?}}_fbthrift_iobuf.IOBuf{{/type:iobuf?}}{{^type:iobuf?}}bytes{{/type:iobuf?}}{{/type:binary?}}{{!
 }}{{#type:struct}}{{!
-    }}{{#type:need_module_path?}}{{type:module_mangle}}{{/type:need_module_path?}}{{!
-  }}{{^type:need_module_path?}}_fbthrift_current_module{{/type:need_module_path?}}.{{struct:py_name}}{{!
+    }}{{#type:need_module_path?}}{{type:module_mangle}}.{{/type:need_module_path?}}{{!
+  }}{{^type:need_module_path?}}{{> private/alias_prefix}}{{/type:need_module_path?}}{{struct:py_name}}{{!
 }}{{/type:struct}}{{!
 }}{{#type:list?}}{{> types/sequence }}[{{!
     }}{{#type:list_elem_type}}{{> types/pep484_type}}{{/type:list_elem_type}}{{!
@@ -46,6 +46,6 @@ thrift-python specific container types.
 }}]{{/type:map?}}{{!
 }}{{#type:void?}}None{{/type:void?}}{{!
 }}{{#type:enum}}{{!
-  }}{{#type:need_module_path?}}{{type:module_mangle}}{{/type:need_module_path?}}{{!
-  }}{{^type:need_module_path?}}_fbthrift_current_module{{/type:need_module_path?}}.{{enum:name}}{{!
+  }}{{#type:need_module_path?}}{{type:module_mangle}}.{{/type:need_module_path?}}{{!
+  }}{{^type:need_module_path?}}{{> private/alias_prefix}}{{/type:need_module_path?}}{{enum:name}}{{!
 }}{{/type:enum}}

thrift/compiler/test/fixtures/adapter/out/python_a/gen-python/a/thrift_types.py

@@ -9,7 +9,6 @@
 
 import folly.iobuf as _fbthrift_iobuf
 
-import a.thrift_types as _fbthrift_current_module
 import thrift.python.types as _fbthrift_python_types
 import thrift.python.exceptions as _fbthrift_python_exceptions
 
@@ -91,11 +90,12 @@ def _to_py_deprecated(self):
         except ModuleNotFoundError:
             py_asyncio_types = importlib.import_module("a.ttypes")
             return thrift.util.converter.to_py_struct(py_asyncio_types.MyStruct, self)
+_fbthrift_MyStruct = MyStruct
 
 # This unfortunately has to be down here to prevent circular imports
 import a.thrift_metadata
-from a.thrift_enums import *
 
+from a.thrift_enums import *
 _fbthrift_all_enums = [
 ]
 

thrift/compiler/test/fixtures/adapter/out/python_a/gen-python/a/thrift_types.pyi

@@ -9,7 +9,6 @@ from __future__ import annotations
 
 import typing as _typing
 
-import a.thrift_types as _fbthrift_current_module
 import folly.iobuf as _fbthrift_iobuf
 import thrift.python.types as _fbthrift_python_types
 import thrift.python.exceptions as _fbthrift_python_exceptions
@@ -49,6 +48,7 @@ class MyStruct(_fbthrift_python_types.Struct, _fbthrift_compatible_with_MyStruct
     def _to_python(self) -> _typing.Self: ...
     def _to_py3(self) -> "a.types.MyStruct": ...  # type: ignore
     def _to_py_deprecated(self) -> "a.ttypes.MyStruct": ...  # type: ignore
+_fbthrift_MyStruct = MyStruct
 
 
 class _fbthrift_MyService_adapted_return_args(_fbthrift_python_types.Struct):

thrift/compiler/test/fixtures/adapter/out/python_with_containers/gen-python/with_containers/thrift_types.py

@@ -9,7 +9,6 @@
 
 import folly.iobuf as _fbthrift_iobuf
 
-import with_containers.thrift_types as _fbthrift_current_module
 import thrift.python.types as _fbthrift_python_types
 import thrift.python.exceptions as _fbthrift_python_exceptions
 
@@ -75,7 +74,7 @@ def _to_py_deprecated(self):
 
 
 AnnotationWithContainers = my.AdaptedType[_fbthrift_unadapted_AnnotationWithContainers]
-
+_fbthrift_AnnotationWithContainers = AnnotationWithContainers
 
 class _fbthrift_unadapted_MyStruct(metaclass=_fbthrift_python_types.StructMeta):
     _fbthrift_SPEC = (
@@ -125,11 +124,12 @@ def _to_py_deprecated(self):
 
 
 MyStruct = my.AdaptedType[_fbthrift_unadapted_MyStruct]
+_fbthrift_MyStruct = MyStruct
 
 # This unfortunately has to be down here to prevent circular imports
 import with_containers.thrift_metadata
-from with_containers.thrift_enums import *
 
+from with_containers.thrift_enums import *
 _fbthrift_all_enums = [
 ]
 

thrift/compiler/test/fixtures/adapter/out/python_with_containers/gen-python/with_containers/thrift_types.pyi

@@ -9,7 +9,6 @@ from __future__ import annotations
 
 import typing as _typing
 
-import with_containers.thrift_types as _fbthrift_current_module
 import folly.iobuf as _fbthrift_iobuf
 import thrift.python.types as _fbthrift_python_types
 import thrift.python.exceptions as _fbthrift_python_exceptions
@@ -41,7 +40,7 @@ class _fbthrift_unadapted_AnnotationWithContainers(_fbthrift_python_types.Struct
     def _to_py3(self) -> "with_containers.types._fbthrift_unadapted_AnnotationWithContainers": ...  # type: ignore
     def _to_py_deprecated(self) -> "with_containers.ttypes._fbthrift_unadapted_AnnotationWithContainers": ...  # type: ignore
 AnnotationWithContainers = my.AdaptedType[_fbthrift_unadapted_AnnotationWithContainers]
-
+_fbthrift_AnnotationWithContainers = AnnotationWithContainers
 
 class _fbthrift_compatible_with__fbthrift_unadapted_MyStruct:
     pass
@@ -63,3 +62,4 @@ class _fbthrift_unadapted_MyStruct(_fbthrift_python_types.Struct, _fbthrift_comp
     def _to_py3(self) -> "with_containers.types._fbthrift_unadapted_MyStruct": ...  # type: ignore
     def _to_py_deprecated(self) -> "with_containers.ttypes._fbthrift_unadapted_MyStruct": ...  # type: ignore
 MyStruct = my.AdaptedType[_fbthrift_unadapted_MyStruct]
+_fbthrift_MyStruct = MyStruct