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.

Sign up for GitHub

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

Jump to bottom

PhpdocAnnotationWithoutDotFixer - add failing cases #4027

Closed
wants to merge 3 commits into from
Closed

PhpdocAnnotationWithoutDotFixer - add failing cases #4027

wants to merge 3 commits into from

Conversation

keradus
Copy link
Member

@keradus keradus commented Oct 11, 2018

@keradus keradus added this to the 2.12.4 milestone Oct 11, 2018
@keradus
Copy link
Member Author

keradus commented Oct 11, 2018

@kubawerlos , can you take a look on this one, please ?

This was referenced Oct 11, 2018
Merged
Closed
@kubawerlos
Copy link
Contributor

I see no simple solution for this - the actually broken PHPDoc (with missing *) handled in #3882 IMHO should not be handled in the fixer, but in DocBlock/Tag class - as all broken PHPDocs should, like the @return added in this PR (the content of tag - returned by Tag class - is the first line only, so here fixer behaves correctly).

What do you think about reverting #3882 for now, then updating DocBlock/Tag to allow missing * and then getting back to the cases with broken PHPDoc?

@keradus
Copy link
Member Author

keradus commented Oct 12, 2018

Changing DocBLock* has unpredictable consequences :(

@kubawerlos
Copy link
Contributor

We could give it a try... or assume that PHPDocs with gaps in asterisk are not handles as its have asterisk where missing => remove both test cases with missing asterisk and revert #3882 "fix".

@keradus
Copy link
Member Author

keradus commented Oct 14, 2018

indeed, when asterisks are missing, phpdoc is malformed and let not modify it

@keradus
Copy link
Member Author

keradus commented Oct 14, 2018

can you take a look, @kubawerlos ?

kubawerlos
kubawerlos approved these changes Oct 14, 2018
View reviewed changes
Copy link
Contributor

@kubawerlos kubawerlos left a comment •
edited

Choose a reason for hiding this comment

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

Looks good 👍

@keradus keradus removed the WIP label Oct 14, 2018
@SpacePossum SpacePossum added the RTM Ready To Merge label Oct 15, 2018
@SpacePossum
Copy link
Contributor

Thanks @keradus.

@SpacePossum SpacePossum removed the RTM Ready To Merge label Oct 18, 2018
@SpacePossum
Copy link
Contributor

(not sure why GH doens't state this PR as merged but as closed)

@keradus
Copy link
Member Author

keradus commented Oct 20, 2018

(as I see squashed commit went to target branch, but PR was not updated with squashed commit)

@keradus keradus deleted the 2.12_sf branch October 20, 2018 22:40
@keradus keradus mentioned this pull request Oct 20, 2018
Merged
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Oct 30, 2018
@nicolas-grekas
minor #28938 Revert "fixed CS" (keradus)
0c6f671 
This PR was merged into the 2.8 branch.

Discussion
----------

Revert "fixed CS"

This reverts commit d48a377.

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | n/a
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This PR reverts #28814 , that was caused as a bug of PHP CS Fixer fixed in PHP-CS-Fixer/PHP-CS-Fixer#4027

After fix on PHP CS Fixer side, the rule is passing now at Symfony's codebase.

This PR only reverts wrong chances done by PHP CS Fixer,
it does not apply new rule requested in #28817 ( PHP-CS-Fixer/PHP-CS-Fixer#4045 )

Commits
-------

6f83d9f Revert "fixed CS"
symfony-splitter pushed a commit to symfony/http-foundation that referenced this pull request Oct 30, 2018
@nicolas-grekas
minor #28938 Revert "fixed CS" (keradus)
0b6eb97 
This PR was merged into the 2.8 branch.

Discussion
----------

Revert "fixed CS"

This reverts commit d48a3776fe0b425d41e3fa9f3ef8b14315c02a1f.

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | n/a
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This PR reverts #28814 , that was caused as a bug of PHP CS Fixer fixed in PHP-CS-Fixer/PHP-CS-Fixer#4027

After fix on PHP CS Fixer side, the rule is passing now at Symfony's codebase.

This PR only reverts wrong chances done by PHP CS Fixer,
it does not apply new rule requested in #28817 ( PHP-CS-Fixer/PHP-CS-Fixer#4045 )

Commits
-------

6f83d9f9a3 Revert "fixed CS"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kubawerlos kubawerlos kubawerlos approved these changes

@SpacePossum SpacePossum SpacePossum approved these changes

Assignees
No one assigned
Labels
kind/bug
Projects
None yet
Milestone
2.12.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@keradus @kubawerlos @SpacePossum