Skip to content

Squiz.WhiteSpace.MemberVarSpacing false positives when attributes used without docblock#3440

Merged
gsherwood merged 1 commit intosquizlabs:masterfrom
javer:fix-property-spacing-with-attributes
Dec 3, 2021
Merged

Squiz.WhiteSpace.MemberVarSpacing false positives when attributes used without docblock#3440
gsherwood merged 1 commit intosquizlabs:masterfrom
javer:fix-property-spacing-with-attributes

Conversation

@javer
Copy link
Copy Markdown
Contributor

@javer javer commented Sep 24, 2021

Fixes false positives when class properties are declared with attributes and without DocComment.
Without this fix it's impossible to use only attributes for properties without DocComments, because this sniff forces you to insert blank line between the property and the attribute:

Expected 1 blank line(s) before member var; 0 found (Squiz.WhiteSpace.MemberVarSpacing.Incorrect)

Actual result:

--- src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed
+++ PHP_CodeSniffer
@@ -363,12 +363,14 @@
{
     protected $propertyDouble;
     #[ThirdAttribute]
+
     protected $propertyWithoutSpacing;
 }

Expected result:

--- src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed
+++ PHP_CodeSniffer
@@ -363,12 +363,14 @@
{
     protected $propertyDouble;
+
     #[ThirdAttribute]
     protected $propertyWithoutSpacing;
 }

@javer
Copy link
Copy Markdown
Contributor Author

javer commented Sep 28, 2021

@gsherwood Could you please take a look on this simple fix, which allows everybody to use property attributes without doc-comment?

@gsherwood gsherwood added this to the 3.6.2 milestone Sep 29, 2021
@jrfnl
Copy link
Copy Markdown
Contributor

jrfnl commented Oct 10, 2021

This is a more selective fix compared to PR #3334, which is open for the same and has an associated discussion open in #3419.

@jrfnl jrfnl mentioned this pull request Oct 10, 2021
Closed
@javer javer force-pushed the fix-property-spacing-with-attributes branch from a247461 to 7f7a2cc Compare November 28, 2021 18:41
@javer
Copy link
Copy Markdown
Contributor Author

javer commented Nov 28, 2021

Rebased on the latest master to make the build green.

@gsherwood gsherwood changed the title PHP 8.0 | Squiz.WhiteSpace.MemberVarSpacing: fix support for attributes Squiz.WhiteSpace.MemberVarSpacing false positives when attributes used without docblock Dec 3, 2021
gsherwood added a commit that referenced this pull request Dec 3, 2021
@gsherwood
Changelog for #3440
8884d16
@gsherwood gsherwood merged commit 7f7a2cc into squizlabs:master Dec 3, 2021
@gsherwood
Copy link
Copy Markdown
Member

gsherwood commented Dec 3, 2021

Thanks for finding, reporting, and fixing this.

@javer javer deleted the fix-property-spacing-with-attributes branch December 3, 2021 08:29
@pattisahusiwa pattisahusiwa mentioned this pull request May 4, 2022
Closed
@jrfnl jrfnl mentioned this pull request May 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Type: bug

Projects

None yet

Milestone

3.6.2

Development

Successfully merging this pull request may close these issues.

3 participants

@javer @jrfnl @gsherwood