Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metadata/doctrine): Use Type of TypeInfo instead of PropertyInfo #6979

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Feb 21, 2025

Q A
Branch? main
Tickets
License MIT
Doc PR

In order to anticipate the deprecation of PropertyInfo's Type in favor of TypeInfo's one, this PR adds a new property phpType (the name can be challenged) that will aim to replace builtinTypes.

What has been done:

  • Add ApiProperty::$phpType property, ApiProperty::getPhpType(), and ApiProperty::withPhpType().
  • Add PropertyInfoToTypeInfoHelper::convertTypeToLegacyTypes to store legacy types in ApiProperty::$builtinTypes when setting ApiProperty::$phpType, this allows API Platform to work "the legacy way" while upgrading other packages.
  • Doesn't trigger deprecations yet in ApiProperty to allow other API Platform packages to upgrade first.
  • Update AbstractItemNormalizer to not be dependent on the type order (type order might not be the same with these changes)
  • Implement PropertyTypeExtractorInterface::getType() in DoctrineExtractor (as getTypes is deprecated)
  • Bump api-platform/metadata to ^4.1, symfony/property-info to ^7.1 in all packages
  • Require symfony/type-info:^7.2 in api-platform/metadata and api-platform/doctrine

@mtarld mtarld marked this pull request as draft February 21, 2025 14:54
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch 2 times, most recently from 9f1eab4 to 99cfee3 Compare February 21, 2025 15:43
@mtarld mtarld changed the title [Metadata] Use Type of TypeInfo instead of PropertyInfo feat(metadata): Use Type of TypeInfo instead of PropertyInfo Feb 21, 2025
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch from 99cfee3 to ffd86b8 Compare February 21, 2025 15:53
@mtarld mtarld changed the title feat(metadata): Use Type of TypeInfo instead of PropertyInfo feat(metadata/doctrine): Use Type of TypeInfo instead of PropertyInfo Feb 21, 2025
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch 9 times, most recently from d8618a7 to 6e47055 Compare February 22, 2025 10:20
@mtarld mtarld marked this pull request as ready for review February 22, 2025 10:40
@mtarld
Copy link
Contributor Author

mtarld commented Feb 24, 2025

For information, the code can become simpler if the following PRs get merged:

symfony/symfony#59844 will update

$typeIsResourceClass = function (Type $type) use (&$typeIsResourceClass): bool {
    return match (true) {
        $type instanceof CollectionType => $type->getCollectionValueType()->isSatisfiedBy($typeIsResourceClass),
        $type instanceof WrappingTypeInterface => $type->wrappedTypeIsSatisfiedBy($typeIsResourceClass),
        $type instanceof CompositeTypeInterface => $type->composedTypesAreSatisfiedBy($typeIsResourceClass),
        default => $type instanceof ObjectType && $this->isResourceClass($type->getClassName()),
    };
};

to

$typeIsResourceClass = fn (Type $type): bool => $type instanceof ObjectType && $this->isResourceClass($type->getClassName());

symfony/symfony#59845 will update

foreach ($type instanceof CompositeTypeInterface ? $type->getTypes() : [$type] as $t) {
    while ($t instanceof WrappingTypeInterface) {
        $t = $t->getWrappedType();
    }

    $typeStrings[] = (string) $t;
}

to

foreach ($type->traverse() as $t) {
    if ($t instanceof CompositeTypeInterface || $t instanceof WrappingTypeInterface) {
        continue;
    }

    $typeStrings[] = (string) $t;
}

@mtarld mtarld force-pushed the chore/type-info-type-metadata branch from f44914f to 6e47055 Compare February 24, 2025 13:00
@mtarld
Copy link
Contributor Author

mtarld commented Feb 26, 2025

Anyway, I don't think the deprecation can be triggered before January 2026 (before the EOL of PHP 8.1), so we have plenty of time 🙂

Edit: I was wrong about it, there is no need to wait, the deprecation could be merged in Symfony 7.3

@mtarld mtarld force-pushed the chore/type-info-type-metadata branch 2 times, most recently from aba898d to cc85008 Compare February 28, 2025 09:47
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch 4 times, most recently from 7ad53c9 to 4b6eaa7 Compare February 28, 2025 10:41
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch from 4b6eaa7 to b4ae0b4 Compare February 28, 2025 10:58
chalasr added a commit to symfony/symfony that referenced this pull request Mar 22, 2025
This PR was merged into the 7.3 branch.

Discussion
----------

[PropertyInfo] Deprecate `Type`

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Issues        |
| License       | MIT

A new attempt to #53160, now that `symfony/type-info` is not experimental anymore.

Deprecates:
- `Type` class in favor of the `Type` class of `symfony/type-info`
- `PropertyTypeExtractorInterface::getTypes()` in favor of the `PropertyTypeExtractorInterface::getType()` method
- `ConstructorArgumentTypeExtractorInterface::getTypesFromConstructor()` in favor of the `ConstructorArgumentTypeExtractorInterface::getTypeFromConstructor()` method

The work for upgrading dependent packages has begun already:
- api-platform/core#6979
- symfony/ux#2607

Commits
-------

4d2ccf4 Fix md formatting
f819aed [PropertyInfo] Deprecate Type
symfony-splitter pushed a commit to symfony/doctrine-bridge that referenced this pull request Mar 22, 2025
This PR was merged into the 7.3 branch.

Discussion
----------

[PropertyInfo] Deprecate `Type`

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Issues        |
| License       | MIT

A new attempt to symfony/symfony#53160, now that `symfony/type-info` is not experimental anymore.

Deprecates:
- `Type` class in favor of the `Type` class of `symfony/type-info`
- `PropertyTypeExtractorInterface::getTypes()` in favor of the `PropertyTypeExtractorInterface::getType()` method
- `ConstructorArgumentTypeExtractorInterface::getTypesFromConstructor()` in favor of the `ConstructorArgumentTypeExtractorInterface::getTypeFromConstructor()` method

The work for upgrading dependent packages has begun already:
- api-platform/core#6979
- symfony/ux#2607

Commits
-------

4d2ccf4ac94 Fix md formatting
f819aed8d13 [PropertyInfo] Deprecate Type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants