Skip to content

Commit

Permalink
Fix handling */ on the same line as the last annotation
Browse files Browse the repository at this point in the history
In both `Annotation::remove()` and `PhpdocNoEmptyReturnFixer`
  • Loading branch information
dmvdbrugge committed Jul 9, 2018
1 parent f1631f0 commit f47f675
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 3 deletions.
20 changes: 18 additions & 2 deletions src/DocBlock/Annotation.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,23 @@ public function setTypes(array $types)
public function remove()
{
foreach ($this->lines as $line) {
$line->remove();
if ($line->isTheStart() && $line->isTheEnd()) {
// Single line doc block, remove entirely
$line->remove();
} elseif ($line->isTheStart()) {
// Multi line doc block, but start is on the same line as the first annotation, keep only the start
$content = Preg::replace('#(\s*/\*\*).*#', '$1', $line->getContent());

$line->setContent($content);
} elseif ($line->isTheEnd()) {
// Multi line doc block, but end is on the same line as the last annotation, keep only the end
$content = Preg::replace('#(\s*)\S.*(\*/.*)#', '$1$2', $line->getContent());

$line->setContent($content);
} else {
// Multi line doc block, neither start nor end on this line, can be removed safely
$line->remove();
}
}

$this->clearCache();
Expand Down Expand Up @@ -270,7 +286,7 @@ private function getTypesContent()
}

$matchingResult = Preg::match(
'{^(?:\s*\*|/\*\*)\s*@'.$name.'\s+'.self::REGEX_TYPES.'(?:[ \t].*)?$}sx',
'{^(?:\s*\*|/\*\*)\s*@'.$name.'\s+'.self::REGEX_TYPES.'(?:[* \t].*)?$}sx',
$this->lines[0]->getContent(),
$matches
);
Expand Down
2 changes: 1 addition & 1 deletion src/DocBlock/DocBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private function findAnnotationLength($start)
}

if (!$line->containsUsefulContent()) {
// if we next line is also non-useful, or contains a tag, then we're done here
// if the next line is also non-useful, or contains a tag, then we're done here
$next = $this->getLine($index + 1);
if (null === $next || !$next->containsUsefulContent() || $next->containsATag()) {
break;
Expand Down
40 changes: 40 additions & 0 deletions tests/DocBlock/AnnotationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,46 @@ public function provideRemoveCases()
return $cases;
}

/**
* @param string $content
* @param string $expected
*
* @dataProvider provideRemoveEdgeCasesCases
*/
public function testRemoveEdgeCases($content, $expected)
{
$doc = new DocBlock($content);
$annotation = $doc->getAnnotation(0);

$annotation->remove();
$this->assertSame($expected, $doc->getContent());
}

public function provideRemoveEdgeCasesCases()
{
return array(
// Single line
array('/** @return null*/', ''),
array('/** @return null */', ''),
array('/** @return null */', ''),

// Multi line, annotation on start line
array('/** @return null
*/', '/**
*/'),
array('/** @return null ' . '
*/', '/**
*/'),
// Multi line, annotation on end line
array('/**
* @return null*/', '/**
*/'),
array('/**
* @return null */', '/**
*/'),
);
}

/**
* @param string $input
* @param string[] $expected
Expand Down
38 changes: 38 additions & 0 deletions tests/Fixer/Phpdoc/PhpdocNoEmptyReturnFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,44 @@ public function testFixNull()
* @return null
*/
EOF;

$this->doTest($expected, $input);
}

public function testFixNullWithEndOnSameLine()
{
$expected = <<<'EOF'
<?php
/**
*/
EOF;

$input = <<<'EOF'
<?php
/**
* @return null */
EOF;

$this->doTest($expected, $input);
}

public function testFixNullWithEndOnSameLineNoSpace()
{
$expected = <<<'EOF'
<?php
/**
*/
EOF;

$input = <<<'EOF'
<?php
/**
* @return null*/
EOF;

$this->doTest($expected, $input);
Expand Down

0 comments on commit f47f675

Please sign in to comment.