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 8 times, most recently from 6d4ac13 to d8618a7 Compare February 22, 2025 10:04
@mtarld mtarld force-pushed the chore/type-info-type-metadata branch 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
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.

1 participant