Skip to content

Commit

Permalink
Add errors and report duplicate or different tags.
Browse files Browse the repository at this point in the history
  • Loading branch information
yunosh committed Mar 23, 2017
1 parent 74bcb4a commit d9686f7
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 12 deletions.
3 changes: 3 additions & 0 deletions framework/Refactor/lib/Horde/Refactor/Cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ public static function main()
$rule->run();
if ($rule->warnings) {
$cli->writeln();
foreach ($rule->errors as $error) {
$cli->writeln($cli->color('red', $error));
}
foreach ($rule->warnings as $warning) {
$cli->writeln($cli->color('brown', $warning));
}
Expand Down
11 changes: 9 additions & 2 deletions framework/Refactor/lib/Horde/Refactor/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ abstract class Rule
*/
protected $_tokens;

/**
* A list of error messages.
*
* @var string[]
*/
protected $_errors = array();

/**
* A list of warning messages.
*
Expand Down Expand Up @@ -85,8 +92,8 @@ public function dump()
*/
public function __get($name)
{
if ($name == 'warnings') {
return $this->_warnings;
if ($name == 'errors' || $name == 'warnings') {
return $this->{'_' . $name};
}
}
}
50 changes: 49 additions & 1 deletion framework/Refactor/lib/Horde/Refactor/Rule/FileLevelDocBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,26 @@ protected function _addEmptyBlocks()
*/
protected function _checkDocBlocks()
{
// Checking for different tags.
$tags = array();
foreach ($this->_firstBlock->getTags() as $tag) {
if (!isset($tags[$tag->getName()])) {
$tags[$tag->getName()] = array();
}
$tags[$tag->getName()][] = $tag->getContent();
}
foreach ($tags as $name => $values) {
$secondTags = $this->_secondBlock->getTagsByName($name);
foreach ($secondTags as $tag) {
if (!in_array($tag->getContent(), $values)) {
$this->_errors[] = sprintf(
Translation::t("The DocBlocks contain different values for the @%s tag"),
$name
);
}
}
}

$this->_checkDocBlock('file');
$this->_checkDocBlock('class');
}
Expand Down Expand Up @@ -298,6 +318,34 @@ protected function _checkDocBlock($which)
}
}

// Checking for duplicate tags.
$tags = array();
foreach ($docblock->getTags() as $tag) {
if (!isset($tags[$tag->getName()])) {
$tags[$tag->getName()] = array();
}
$tags[$tag->getName()][] = $tag->getContent();
}
foreach ($tags as $name => &$values) {
if (count($values) != count(array_unique($values))) {
$this->_warnings[] = sprintf(
Translation::t("The %s DocBlock contains duplicate @%s tags"),
$warn, $name
);
$values = array_unique($values);
}
}
$newtags = array();
foreach ($tags as $name => $namedTags) {
foreach ($namedTags as $value) {
$newtags[] = TagFactory::create($name, $value);
}
}
if (count($newtags) != count($docblock->getTags())) {
$docblock = $this->_getDocBlock($docblock, $newtags);
$update = true;
}

// Checking for missing tags.
$tags = $docblock->getTags();
foreach ($this->_config->{$which . 'Tags'} as $tag => $value) {
Expand Down Expand Up @@ -331,7 +379,7 @@ protected function _checkDocBlock($which)
foreach ($docblock->getTags() as $tag) {
if (isset($this->_config->{$which . 'ForbiddenTags'}[$tag->getName()])) {
$test = $this->_config->{$which . 'ForbiddenTags'}[$tag->getName()];
if (($test instanceof Regexp && $test->match($tag->getValue())) ||
if (($test instanceof Regexp && $test->match($tag->getContent())) ||
$test) {
$this->_warnings[] = sprintf(
Translation::t("The %s DocBlock tags should not include: "),
Expand Down
2 changes: 2 additions & 0 deletions framework/Refactor/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
<file name="ClassLevelDocBlockWithFileLevelDocs.php" role="test" />
<file name="ClassLevelDocsInFileLevel.php" role="test" />
<file name="CorrectDocBlocks.php" role="test" />
<file name="DifferentTagContents.php" role="test" />
<file name="ExtractYearFixTagOrder.php" role="test" />
<file name="FileLevelDocsInClassLevel.php" role="test" />
<file name="IncorrectDocBlocks.php" role="test" />
Expand Down Expand Up @@ -342,6 +343,7 @@
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/ClassLevelDocBlockWithFileLevelDocs.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/ClassLevelDocBlockWithFileLevelDocs.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/ClassLevelDocsInFileLevel.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/ClassLevelDocsInFileLevel.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/CorrectDocBlocks.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/CorrectDocBlocks.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/DifferentTagContents.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/DifferentTagContents.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/ExtractYearFixTagOrder.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/ExtractYearFixTagOrder.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/FileLevelDocsInClassLevel.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/FileLevelDocsInClassLevel.php" />
<install as="Horde/Refactor/fixtures/FileLevelDocBlock/IncorrectDocBlocks.php" name="test/Horde/Refactor/fixtures/FileLevelDocBlock/IncorrectDocBlocks.php" />
Expand Down
37 changes: 28 additions & 9 deletions framework/Refactor/test/Horde/Refactor/FileLevelDocBlockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,37 +77,56 @@ public function testWarnings()
new Config\FileLevelDocBlock(array('year' => 2017))
);
$rule->run();
$this->assertCount(12, $rule->warnings);
$warnings = $rule->warnings;
$this->assertCount(13, $warnings);
$this->assertEquals(
'More than one @license tag.',
$rule->warnings[0]
array_shift($warnings)
);
$this->assertEquals(
'The file-level DocBlock summary is not valid',
$rule->warnings[1]
array_shift($warnings)
);
$this->assertEquals(
'The file-level DocBlock description is not valid',
$rule->warnings[2]
array_shift($warnings)
);
foreach (array(3 => 'author', 4 => 'category', 5 => 'license', 6 => 'package') as $warning => $tag) {
foreach (array('author', 'category', 'license', 'package') as $warning => $tag) {
$this->assertEquals(
'The file-level DocBlock tags should include: ' . $tag,
$rule->warnings[$warning]
array_shift($warnings)
);
}
$this->assertEquals(
'The file-level DocBlock tags should not include: copyright',
$rule->warnings[7]
array_shift($warnings)
);
$this->assertEquals(
'The class-level DocBlock contains duplicate @license tags',
array_shift($warnings)
);
foreach (array(8 => 'author', 9 => 'category', 10 => 'copyright', 11 => 'package') as $warning => $tag) {
foreach (array('author', 'category', 'copyright', 'package') as $warning => $tag) {
$this->assertEquals(
'The class-level DocBlock tags should include: ' . $tag,
$rule->warnings[$warning]
array_shift($warnings)
);
}
}

public function testErrors()
{
$rule = new Rule\FileLevelDocBlock(
__DIR__ . '/fixtures/FileLevelDocBlock/DifferentTagContents.php',
new Config\FileLevelDocBlock()
);
$rule->run();
$this->assertCount(1, $rule->errors);
$this->assertEquals(
'The DocBlocks contain different values for the @license tag',
$rule->errors[0]
);
}

public function getFileNames()
{
return array(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php
/**
* Copyright 2017 Horde LLC (http://www.horde.org/)
*
* See the enclosed file COPYING for license information (LGPL). If you did
* not receive this file, see http://www.horde.org/licenses/lgpl21.
*
* @author Some Author <author@acme.org>
* @category Horde
* @license http://www.horde.org/licenses/lgpl21 LGPL-2.1
* @package Auth
*/

/**
* This class does stuff.
*
* @author Some Author <author@acme.org>
* @category Horde
* @license http://www.horde.org/licenses/gpl GPL
* @copyright 2017 Horde LLC
* @package Auth
*/
class Foo
{
}

0 comments on commit d9686f7

Please sign in to comment.