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

fix: add missing trigger_deprecation calls to deprecated classes, methods #2265

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

DominicLuidold
Copy link
Contributor

Q A
Bug fix? no
New feature? no
Deprecations? yes
Issues /

Adds missing deprecation triggers to classes and methods that have been deprecated with a @deprecated annotation.

@DominicLuidold DominicLuidold changed the title chore: Add missing trigger_deprecation calls to deprecated classes, methods fix: Add missing trigger_deprecation calls to deprecated classes, methods Apr 13, 2024
Comment on lines 30 to 46
if (null === $schema) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.15.0',
'Passing null for the $schema parameter of "PropertyDescriberInterface::describe()" is deprecated. In future versions, the $schema parameter will be made non-nullable',
);
}

if (null !== $groups) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.17.0',
'Using the $groups parameter of "PropertyDescriberInterface::describe()" is deprecated and will be removed in a future version. Pass groups via $context[\'groups\']',
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this way of checking the parameter be better for triggering deprecations?

Suggested change
if (null === $schema) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.15.0',
'Passing null for the $schema parameter of "PropertyDescriberInterface::describe()" is deprecated. In future versions, the $schema parameter will be made non-nullable',
);
}
if (null !== $groups) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.17.0',
'Using the $groups parameter of "PropertyDescriberInterface::describe()" is deprecated and will be removed in a future version. Pass groups via $context[\'groups\']',
);
}
if (null !== $groups || func_num_args() >= 3) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.17.0',
'Using the $groups parameter of "%s" is deprecated and will be removed in a future version. Pass groups via $context[\'groups\'].', __METHOD__
);
}
if (\func_num_args() < 4 || null === $schema) {
trigger_deprecation(
'nelmio/api-doc-bundle',
'4.15.0',
'The "%s()" method will have a new "OA\Schema $schema" arument not defining it is deprecated.', __METHOD__
);
$schema = null;
} else {
$schema = func_get_arg(3);
}

Copy link
Contributor Author

@DominicLuidold DominicLuidold Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DjordyKoert I'm not entirely sure that relying on the func_num_args count is an approach we should take here. Mainly because we deprecate not defining one argument while also deprecating if another is defined.

For example, with func_num_args() >= 3 we wouldn't know if someone calls the method like this, which would be totally valid:

$describer->describe(
    types: $types,
    property: $property,
    schema: $schema,
);

While func_num_args() < 4 would not work if someone calls the method like this, even though $schema is not defined/null:

$describer->describe(
    types: $types,
    property: $property,
    groups: $groups,
    context: $context,
);

@DominicLuidold DominicLuidold changed the title fix: Add missing trigger_deprecation calls to deprecated classes, methods fix: add missing trigger_deprecation calls to deprecated classes, methods Jun 16, 2024
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.

None yet

2 participants