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

PHP 8.0 | Squiz.WhiteSpace.MemberVarSpacing: add support for attributes #3334

Closed
wants to merge 1 commit into from
Closed

PHP 8.0 | Squiz.WhiteSpace.MemberVarSpacing: add support for attributes #3334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 140 additions & 55 deletions src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,87 +55,172 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)

$endOfStatement = $phpcsFile->findNext(T_SEMICOLON, ($stackPtr + 1), null, false, null, true);

$ignore = $validPrefixes;
$ignore[T_WHITESPACE] = T_WHITESPACE;
$ignore = $validPrefixes;
$ignore[] = T_WHITESPACE;

$start = $startOfStatement;
for ($prev = ($startOfStatement - 1); $prev >= 0; $prev--) {
if (isset($ignore[$tokens[$prev]['code']]) === true) {
continue;
}
$prev = $phpcsFile->findPrevious($ignore, ($startOfStatement - 1), null, true);

if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
do {
if ($tokens[$start]['code'] === T_ATTRIBUTE_END) {
$nextContent = $tokens[$start]['attribute_opener'];
} else {
$nextContent = $start;
}

if ($tokens[$prev]['code'] === T_ATTRIBUTE_END
&& isset($tokens[$prev]['attribute_opener']) === true
) {
$prev = $tokens[$prev]['attribute_opener'];
continue;
}
if ($tokens[$prev]['line'] === $tokens[$nextContent]['line']) {
$error = 'Each attribute must be on a line by itself';
$fix = $phpcsFile->addFixableError($error, $prev, 'NoNewLineBeforeAttribute');
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

break;
}
/*
* Remove redundant whitespaces.
*/

$cur = $prev;
while (($cur = $phpcsFile->findNext(T_WHITESPACE, ($cur + 1), $nextContent)) !== false) {
$phpcsFile->fixer->replaceToken($cur, '');
}

/*
* Fix indent.
*/

$indent = '';
$cur = $prev;

do {
$cur = $phpcsFile->findPrevious(T_WHITESPACE, ($cur - 1));
if ($tokens[$cur]['line'] !== $tokens[$prev]['line']) {
break;
}

$indent = str_repeat(' ', $tokens[$cur]['length']);
} while (true);

$phpcsFile->fixer->addContentBefore($nextContent, $indent);

$phpcsFile->fixer->addNewlineBefore($nextContent);
$phpcsFile->fixer->endChangeset();
}//end if
}//end if

$foundLines = ($tokens[$nextContent]['line'] - $tokens[$prev]['line'] - 1);
if ($foundLines > 0) {
$error = 'Expected 0 blank lines after attribute; %s found';
$data = [$foundLines];
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterAttribute', $data);
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

for ($i = ($prev + 1); $i <= $nextContent; $i++) {
if ($tokens[$i]['line'] === $tokens[$nextContent]['line']) {
break;
}

$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->addNewline($prev);
$phpcsFile->fixer->endChangeset();
}
}

$start = $prev;
$prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['attribute_opener'] - 1), null, true);
} while ($tokens[$prev]['code'] === T_ATTRIBUTE_END);
}//end if

if (isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === true) {
// Assume the comment belongs to the member var if it is on a line by itself.
$prevContent = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true);
if ($tokens[$prevContent]['line'] !== $tokens[$prev]['line']) {
if ($tokens[$start]['code'] === T_ATTRIBUTE_END) {
$nextContent = $tokens[$start]['attribute_opener'];
} else {
$nextContent = $start;
}

// Check the spacing, but then skip it.
$foundLines = ($tokens[$startOfStatement]['line'] - $tokens[$prev]['line'] - 1);
$foundLines = ($tokens[$nextContent]['line'] - $tokens[$prev]['line'] - 1);
if ($foundLines > 0) {
for ($i = ($prev + 1); $i < $startOfStatement; $i++) {
if ($tokens[$i]['column'] !== 1) {
continue;
$error = 'Expected 0 blank lines after member var comment; %s found';
$data = [$foundLines];
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data);
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();
// Inline comments have the newline included in the content but
// docblock do not.
if ($tokens[$prev]['code'] === T_COMMENT) {
$phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content']));
}

if ($tokens[$i]['code'] === T_WHITESPACE
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line']
) {
$error = 'Expected 0 blank lines after member var comment; %s found';
$data = [$foundLines];
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data);
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();
// Inline comments have the newline included in the content but
// docblocks do not.
if ($tokens[$prev]['code'] === T_COMMENT) {
$phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content']));
}

for ($i = ($prev + 1); $i <= $startOfStatement; $i++) {
if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) {
break;
}

// Remove the newline after the docblock, and any entirely
// empty lines before the member var.
if ($tokens[$i]['code'] === T_WHITESPACE
&& $tokens[$i]['line'] === $tokens[$prev]['line']
|| ($tokens[$i]['column'] === 1
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line'])
) {
$phpcsFile->fixer->replaceToken($i, '');
}
}

$phpcsFile->fixer->addNewline($prev);
$phpcsFile->fixer->endChangeset();
}//end if
for ($i = ($prev + 1); $i <= $nextContent; $i++) {
if ($tokens[$i]['line'] === $tokens[$start]['line']) {
break;
}

break;
}//end if
}//end for
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->addNewline($prev);
$phpcsFile->fixer->endChangeset();
}
}//end if

$start = $prev;
}//end if

if ($tokens[$prevContent]['code'] === T_ATTRIBUTE_END) {
$error = 'Member var comment must be before attributes';
$fix = $phpcsFile->addFixableError($error, $prev, 'AttributeAfterComment');
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

/*
* Fix indent.
*/

$indent = '';
$cur = $tokens[$prevContent]['attribute_opener'];

do {
$cur = $phpcsFile->findPrevious(T_WHITESPACE, ($cur - 1));

if ($tokens[$cur]['line'] !== $tokens[$tokens[$prevContent]['attribute_opener']]['line']) {
break;
}

$indent = str_repeat(' ', $tokens[$cur]['length']);
} while (true);

$phpcsFile->fixer->addContentBefore($tokens[$prevContent]['attribute_opener'], $indent);

$commentContent = '';
for ($i = $tokens[$prev]['comment_opener']; $i <= $prev; $i++) {
$commentContent .= $phpcsFile->fixer->getTokenContent($i);
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->addNewlineBefore($tokens[$prevContent]['attribute_opener']);
$phpcsFile->fixer->addContentBefore($tokens[$prevContent]['attribute_opener'], $commentContent);

$phpcsFile->fixer->endChangeset();
}//end if
}//end if
}//end if

// There needs to be n blank lines before the var, not counting comments.
// There needs to be n blank lines before the var, not counting comments and attributes.
if ($start === $startOfStatement) {
// No comment found.
$first = $phpcsFile->findFirstOnLine(Tokens::$emptyTokens, $start, true);
if ($first === false) {
$first = $start;
}
} else if ($tokens[$start]['code'] === T_ATTRIBUTE_END) {
$first = $tokens[$start]['attribute_opener'];
} else if ($tokens[$start]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
$first = $tokens[$start]['comment_opener'];
} else {
Expand Down
61 changes: 59 additions & 2 deletions src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class HasAttributes

#[ORM\Id]#[ORM\Column("integer")]

private $id;
private $var1;


/**
Expand All @@ -354,5 +354,62 @@ class HasAttributes
#[ORM\GeneratedValue]

#[ORM\Column(ORM\Column::T_INTEGER)]
protected $height;
protected $var2;

#[Attribute1]
public $var3 = 'value';

#[Attribute1]


#[Attribute2]

public string $var4 = 'value';

/**
* Description.
*/

#[Attribute1]
protected $var5 = 'value';

/**
* Description.
*/

#[Attribute1]

#[Attribute2]


private $var6 = 'value';

#[Attribute1]

#[Attribute2]

/**
* Description.
*/
private $var7 = 'value';


/**
* Description.
*/
#[Attribute1] #[Attribute2] private $var8 = 'value';

/**
* Description.
*/
#[Attribute1]
#[Attribute2]private $var9 = 'value';

/**
* Description.
*/
#[Attribute1]#[MultiLineAttribute(
multi: 'line1',
line: 'line2'
)]private $var10 = 'value';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just some suggestions:

  • What about a test case where the attribute is before the docblock ?
  • What about a test case with one attribute on the same line as the property ?
  • What about a test case with two attributes on the same line as the property ?

Would be good to verify that the sniff will handle those cases correctly as well.

Copy link
Author

Choose a reason for hiding this comment

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

Despite there is no PSR for Attributes for now, I suggest to always keep phpDoc before attributes.

58 changes: 55 additions & 3 deletions src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,9 @@ class HasAttributes
*
* @var array
*/
#[ORM\Id]#[ORM\Column("integer")]
private $id;
#[ORM\Id]
#[ORM\Column("integer")]
private $var1;

/**
* Short description of the member variable.
Expand All @@ -338,5 +339,56 @@ class HasAttributes
*/
#[ORM\GeneratedValue]
#[ORM\Column(ORM\Column::T_INTEGER)]
protected $height;
protected $var2;

#[Attribute1]
public $var3 = 'value';

#[Attribute1]
#[Attribute2]
public string $var4 = 'value';

/**
* Description.
*/
#[Attribute1]
protected $var5 = 'value';

/**
* Description.
*/
#[Attribute1]
#[Attribute2]
private $var6 = 'value';

/**
* Description.
*/
#[Attribute1]
#[Attribute2]
private $var7 = 'value';

/**
* Description.
*/
#[Attribute1]
#[Attribute2]
private $var8 = 'value';

/**
* Description.
*/
#[Attribute1]
#[Attribute2]
private $var9 = 'value';

/**
* Description.
*/
#[Attribute1]
#[MultiLineAttribute(
multi: 'line1',
line: 'line2'
)]
private $var10 = 'value';
}
14 changes: 13 additions & 1 deletion src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,21 @@ public function getErrorList()
292 => 1,
333 => 1,
342 => 1,
344 => 2,
346 => 1,
353 => 1,
354 => 1,
357 => 1,
362 => 1,
365 => 1,
371 => 1,
378 => 1,
380 => 1,
382 => 1,
393 => 1,
400 => 3,
406 => 1,
411 => 1,
414 => 1,
];

}//end getErrorList()
Expand Down