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

NoBlankLinesAfterPhpdoc - Add T_NAMESPACE in array of forbidden successors #5791

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

paulbalandan
Copy link
Contributor

Fixes #5111

When used, no_blank_lines_after_phpdoc removes the whitespace between the file-level doc-block and the namespace declaration. This should not be the case as namespaces are not considered structural elements that can be documented with doc-blocks. Besides, the file-level doc-block belongs to the file and NOT on the namespace.

See more: https://docs.phpdoc.org/3.0/guide/getting-started/what-is-a-docblock.html

@SpacePossum
Copy link
Contributor

Hi and thanks for your PR!

Looking good already. Could you adjust the test to only test the single issue here, maybe like:

    public function testWhitespaceInDocBlockAboveNamespaceIsNotTouched()
    {
        $expected = '<?php
/**
 * This is a file-level docblock.
 */

namespace Foo\Bar\Baz;
';
        $this->doTest($expected);
    }

Thanks!

@SpacePossum SpacePossum added this to the 2.19.1 milestone Jul 28, 2021
@SpacePossum SpacePossum changed the title Fix: NoBlankLinesAfterPhpdoc - Add T_NAMESPACE in array of forbidden successors NoBlankLinesAfterPhpdoc - Add T_NAMESPACE in array of forbidden successors Jul 28, 2021
@SpacePossum SpacePossum added the RTM Ready To Merge label Jul 28, 2021
@keradus keradus added RTM Ready To Merge and removed RTM Ready To Merge labels Jul 29, 2021
@coveralls
Copy link

coveralls commented Jul 29, 2021

Coverage Status

Coverage remained the same at 91.561% when pulling e4dd5a6 on paulbalandan:no-blank-line-after-phpdoc into d7c4564 on FriendsOfPHP:2.19.

@keradus
Copy link
Member

keradus commented Jul 29, 2021

@paulbalandan CI is failing for proposed changes

@paulbalandan
Copy link
Contributor Author

@keradus or @SpacePossum , is it an acceptable fix to just remove the integration of the fixer against single_blank_line_after_namespace? Before this PR, no_blank_lines_after_phpdoc considers T_NAMESPACE as documentable that's why it gets an integration/priority check with single_blank_line_after_namespace. Now that this skips T_NAMESPACE, I think the integration/priority checks should be dropped too.

@keradus
Copy link
Member

keradus commented Jul 30, 2021

if there is no longer priority conflict between those two rules anymore, move test from Integration/priority/ to Integration/misc and drop tests/AutoReview/FixerFactoryTest.php:143: [$fixers['no_blank_lines_after_phpdoc'], $fixers['single_blank_line_before_namespace']] row

@paulbalandan
Copy link
Contributor Author

I have fixed the integration failure. Not sure if the removal of the priority comment in SingleBlankLineBeforeNamespace is warranted.

@keradus
Copy link
Member

keradus commented Jul 30, 2021

please avoid squashing the commits in future. now, I have no clue what are the newest changes and have to review from scratch :(

@keradus
Copy link
Member

keradus commented Jul 30, 2021

Not sure if the removal of the priority comment (...)

we have tests for it. if CI is happy, all good

@keradus
Copy link
Member

keradus commented Aug 2, 2021

Thank you @paulbalandan.

@keradus keradus merged commit 7b18f04 into PHP-CS-Fixer:2.19 Aug 2, 2021
@paulbalandan paulbalandan deleted the no-blank-line-after-phpdoc branch August 2, 2021 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants