appwrite / appwrite/sdk-generator
Alias model names that collide with enum names in generated services
- Dominant language
- Twig
- Stars
- 325
- Forks
- 212
- Avg merge
- 7h 36m
- Merged PRs (30d)
- 91
Description
## 🔖 Feature description
A spec is free to name an enum after a model. When it does, the Python generator emits two imports of the same name into one service module and the second silently wins:
```python
from ..models.addon import Addon # response model
from ..enums.addon import Addon # enum for get_addon_price — shadows the model
def get_addon(self, organization_id: str, addon_id: str) -> Addon:
...
return self._parse_response(response, model=Addon)
```
Four methods in the Console Python SDK's `organizations` service then raised `AttributeError: type object 'Addon' has no attribute 'model_validate'` — `get_addon`, `create_baa_addon`, `create_premium_geo_db_addon`, `confirm_addon_payment`. It generated, imported, compiled and passed `black --check`; only calling it failed.
That instance was fixed in the spec by renaming the enum (`Addon` → `AddonKey`), which is the right fix for that collision. This issue is about the generator not depending on specs avoiding the name.
The templates already have the mechanism: a model whose name collides with **its own service** is imported under a `Model` suffix, e.g. `from ..models.project import Project as ProjectModel` in `services/project.py`. The proposal is to make that a general rule — alias a model when its name collides with the service **or with any enum the same module imports** — and to resolve it in one place.
## 🎤 Pitch
Today the collision check is an inline template comparison duplicated across ten sites in the Python templates: the three import branches, the return annotation, the generic `[T]` annotation, the docstring `Returns` block, the `model=` argument in `api.twig` and `file.twig`, and the `Union[...]` members. Adding the enum case means editing all ten and keeping them in step, which is why the enum case is currently missing from all ten at once. A single filter — `model | getServiceModelName(service, spec)` — would return the local name for every one of those sites, so the rule is stated once.
Worth deciding as part of this:
- **Which side gets renamed.** Aliasing the model keeps enum names stable for callers, matching the existing service-name rule. Aliasing the enum instead would be a bigger break, since enums appear in user call sites.
- **Which languages need it.** Python is the sharp case: two module-level imports of one name shadow silently. TypeScript, Dart, Go and Kotlin either namespace models (`Models.Addon`, `models.Addon`) or fail at compile time, so they surface it immediately. A shared helper on `Language` with per-language opt-in would suit the ones that need it.
- **Whether the generator should also fail loudly.** A validation pass over the spec — "enum X collides with model X" — would catch the shape at generation time in every language, including any where aliasing is not the right answer.
Prior art in this repo: the `Model` suffix for a model matching its service name; the `as {{ enumRef }}` alias already emitted by `getServiceEnumName` when two enums in a service share a name.
## 👀 Have you spent some time to check if this issue has been raised before?
Yes.
## 🏢 Have you read the Code of Conduct?
Yes.
Contributor guide
Research direction
Start by locating the Python templates and the ten inline model-name comparisons described in the issue, including api.twig and file.twig. Read the existing Model suffix behavior for service-name collisions and getServiceEnumName, then trace how a shared getServiceModelName(service, spec) filter would cover imports, annotations, docstrings, model arguments, and Union members; done means the collision rule is resolved consistently across those sites.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100