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

Remove superfluous phpdoc tags #33151

Merged
merged 1 commit into from Aug 14, 2019

Conversation

@tigitz
Copy link

commented Aug 13, 2019

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ...
License MIT

As Symfony is moving towards type hinted code, this PR is attempting to remove the now superfluous annotations

See #33150 to make this cleaning as an automatic process

@tigitz tigitz requested review from dunglas, lyrixx and xabbuh as code owners Aug 13, 2019

@tigitz tigitz force-pushed the tigitz:remove_superfluous_phpdoc_tags branch from c2e2235 to 7efa0a7 Aug 13, 2019

@nicolas-grekas
Copy link
Member

left a comment

"mixed" would not be considered superfluous to me (to answer your question on the issue)

@@ -188,7 +188,6 @@ public function setAutocompleterCallback(callable $callback = null): self
/**
* Sets a validator for the question.
*
* @param callable|null $validator
*

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 13, 2019

Member

can extra blank lines be removed by the tool too?

This comment has been minimized.

Copy link
@tigitz

tigitz Aug 13, 2019

Author

hmm shouldn't fabbot.io catch those ?

I ran php-cs-fixer with 'no_superfluous_phpdoc_tags' => ['allow_mixed' => true], as the only rule to speed up things, so others rules from .php_cs.dist might handle these.

What's clear is that the no_superfluous_phpdoc_tags doesn't handle it itself after it's cleaning

I'll check my guess and see if there's even another rule we can add that can do this check if not in .php_cs.dist file already

* @param RawMessage $message
*
* {@inheritdoc}
*/

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 13, 2019

Member

why superfuous here?

This comment has been minimized.

Copy link
@tigitz

tigitz Aug 13, 2019

Author

I thought it was a typo as the annotated param here is $message, probably copy pasted from the function above, while the real param is $email here but with no indications on it's type it seems. So I decided to simply remove it.

I'll keep the inheritdoc though.

* @param \ReflectionClass $reflectionClass
* @param array|bool $allowedAttributes
* @param string|null $format
* @param string $class

This comment has been minimized.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

/cc @derrabus some things to borrow to close #32179 here :)

@tigitz

This comment has been minimized.

Copy link
Author

commented Aug 13, 2019

Cleaning of some files went a little bit too far and broke some php doc extraction tests 😅, will fix this

@nicolas-grekas nicolas-grekas modified the milestones: next, 5.0 Aug 14, 2019

@lyrixx

lyrixx approved these changes Aug 14, 2019

Copy link
Member

left a comment

👍 for the workflow part

@nicolas-grekas nicolas-grekas modified the milestones: 5.0, 3.4 Aug 14, 2019

@nicolas-grekas nicolas-grekas changed the base branch from master to 3.4 Aug 14, 2019

@nicolas-grekas nicolas-grekas force-pushed the tigitz:remove_superfluous_phpdoc_tags branch from 7efa0a7 to 608e23c Aug 14, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

Thank you @tigitz
I've also added the phpdoc_trim_consecutive_blank_line_separation rule and opened FriendsOfPHP/PHP-CS-Fixer#4504

@nicolas-grekas nicolas-grekas merged commit 608e23c into symfony:3.4 Aug 14, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Aug 14, 2019

minor #33151 Remove superfluous phpdoc tags (Philippe Segatori)
This PR was submitted for the master branch but it was merged into the 3.4 branch instead (closes #33151).

Discussion
----------

Remove superfluous phpdoc tags

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ...
| License       | MIT

As Symfony is moving towards type hinted code, this PR is attempting to remove the now superfluous annotations

See #33150 to make this cleaning as an automatic process

Commits
-------

608e23c Remove superfluous phpdoc tags
@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

This is now merged up to master 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.