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

Document unofficial parameter in phpdoc #9752

Closed
wants to merge 1 commit into from

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented May 11, 2022

This is a follow-up for #9747 (comment)

Pros:

  • The DebugClassLoader of Symfony can emit a deprecation about that
  • The phpdoc no longer needs to be repeated in implementing classes

Cons:

  • PHP_CodeSniffer is unhappy about this.
  • PHPStan is unhappy about this.
  • Psalm does not take into account the inherited phpdoc.

@greg0ire greg0ire force-pushed the trigger-sf-deprecation branch from 833d6b9 to 1780134 Compare May 11, 2022 06:51
@greg0ire greg0ire marked this pull request as draft May 11, 2022 06:52
@greg0ire greg0ire force-pushed the trigger-sf-deprecation branch from 1780134 to 84fb4e7 Compare May 11, 2022 06:55
Pros:
- The DebugClassLoader of Symfony can emit a deprecation about that
- The phpdoc no longer needs to be repeated in implementing classes

Cons:
- PHP_CodeSniffer is unhappy about this.
- PHPStan is unhappy about this.
@greg0ire greg0ire force-pushed the trigger-sf-deprecation branch from 84fb4e7 to 4470268 Compare May 11, 2022 06:58
@greg0ire
Copy link
Member Author

@stof this does not look too great: we have to add ignore rules for 3 of our tools, to trigger a deprecation for a tool that isn't necessarily used by our users.

@greg0ire greg0ire requested a review from stof May 11, 2022 07:00
@greg0ire greg0ire marked this pull request as ready for review May 11, 2022 07:00
@SenseException
Copy link
Member

It seems like @param class-string $className The fully-qualified class name. had to be added to the classes instead of the interface. Maybe using the psalm- or phpstan annotations than the phpdoc one.

@greg0ire
Copy link
Member Author

@SenseException the goal of this is to trigger a deprecation at autoload time for implementing classes outside doctrine/orm, so the interface phpdoc has to be modified. That being said, given the cons I listed, I'm not convinced this is a great idea, so let's just close this.

@greg0ire greg0ire closed this May 11, 2022
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