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

Add prefix removal for class names and variable names #962

Merged
35 changes: 33 additions & 2 deletions src/main/php/PHPMD/Rule/Naming/LongClassName.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,18 @@
use PHPMD\Utility\Strings;

/**
* This rule checks if an interface or class name exceeds the configured length excluding certain configured suffixes
* This rule checks if an interface or class name exceeds the configured length
* excluding certain configured prefixes and suffixes
*/
class LongClassName extends AbstractRule implements ClassAware, InterfaceAware, TraitAware, EnumAware
{
/**
* Temporary cache of configured prefixes to subtract
*
* @var string[]|null
*/
protected $subtractPrefixes;

/**
* Temporary cache of configured suffixes to subtract
*
Expand All @@ -47,12 +55,35 @@ public function apply(AbstractNode $node)
{
$threshold = $this->getIntProperty('maximum');
$classOrInterfaceName = $node->getName();
if (Strings::lengthWithoutSuffixes($classOrInterfaceName, $this->getSubtractSuffixList()) <= $threshold) {
$length = Strings::lengthWithoutPrefixesAndSuffixes(
$classOrInterfaceName,
$this->getSubtractPrefixList(),
$this->getSubtractSuffixList()
);

if ($length <= $threshold) {
return;
}
$this->addViolation($node, array($classOrInterfaceName, $threshold));
}

/**
* Gets array of prefixes from property
*
* @return string[]
*/
protected function getSubtractPrefixList()
{
if ($this->subtractPrefixes === null) {
$this->subtractPrefixes = Strings::splitToList(
$this->getStringProperty('subtract-prefixes', ''),
','
);
}

return $this->subtractPrefixes;
}

/**
* Gets array of suffixes from property
*
Expand Down
27 changes: 26 additions & 1 deletion src/main/php/PHPMD/Rule/Naming/LongVariable.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@
*/
class LongVariable extends AbstractRule implements ClassAware, MethodAware, FunctionAware, TraitAware
{
/**
* Temporary cache of configured prefixes to subtract
*
* @var string[]|null
*/
protected $subtractPrefixes;

/**
* Temporary cache of configured suffixes to subtract
*
Expand Down Expand Up @@ -109,7 +116,11 @@ protected function checkMaximumLength(AbstractNode $node)
{
$threshold = $this->getIntProperty('maximum');
$variableName = $node->getImage();
$lengthWithoutDollarSign = Strings::lengthWithoutSuffixes($variableName, $this->getSubtractSuffixList()) - 1;
$lengthWithoutDollarSign = Strings::lengthWithoutPrefixesAndSuffixes(
\ltrim($variableName, '$'),
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I dont' think we normally name space functions

Suggested change
\ltrim($variableName, '$'),
ltrim($variableName, '$'),

$this->getSubtractPrefixList(),
$this->getSubtractSuffixList()
);
if ($lengthWithoutDollarSign <= $threshold) {
return;
}
Expand Down Expand Up @@ -184,6 +195,20 @@ protected function isNotProcessed(AbstractNode $node)
return !isset($this->processedVariables[$node->getImage()]);
}

/**
* Gets array of suffixes from property
*
* @return string[]
*/
protected function getSubtractPrefixList()
{
if ($this->subtractPrefixes === null) {
$this->subtractPrefixes = Strings::splitToList($this->getStringProperty('subtract-prefixes', ''), ',');
}

return $this->subtractPrefixes;
}

/**
* Gets array of suffixes from property
*
Expand Down
25 changes: 25 additions & 0 deletions src/main/php/PHPMD/Utility/Strings.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@ class Strings
*/
public static function lengthWithoutSuffixes($stringName, array $subtractSuffixes)
{
return static::lengthWithoutPrefixesAndSuffixes($stringName, array(), $subtractSuffixes);
}

/**
* Returns the length of the given string, excluding at most one suffix
*
* @param string $stringName String to calculate the length for.
* @param array $subtractPrefixes List of prefixes to exclude from the calculated length.
* @param array $subtractSuffixes List of suffixes to exclude from the calculated length.
* @return int The length of the string, without suffix, if applicable.
*/
public static function lengthWithoutPrefixesAndSuffixes(
$stringName,
array $subtractPrefixes,
array $subtractSuffixes
) {

$stringLength = strlen($stringName);

foreach ($subtractSuffixes as $suffix) {
Expand All @@ -43,6 +60,14 @@ public static function lengthWithoutSuffixes($stringName, array $subtractSuffixe
}
}

foreach ($subtractPrefixes as $prefix) {
$prefixLength = strlen($prefix);
if (strncmp($stringName, $prefix, $prefixLength) === 0) {
$stringLength -= $prefixLength;
break;
}
}

return $stringLength;
}

Expand Down
7 changes: 7 additions & 0 deletions src/main/resources/rulesets/naming.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The Naming Ruleset contains a collection of rules about names - too long, too sh
<priority>3</priority>
<properties>
<property name="maximum" description="The class name length reporting threshold" value="40"/>
<property name="subtract-prefixes" description="Comma-separated list of prefixes that will not count in the length of the class name. Only the first matching prefix will be subtracted." value=""/>
<property name="subtract-suffixes" description="Comma-separated list of suffixes that will not count in the length of the class name. Only the first matching suffix will be subtracted." value=""/>
</properties>
<example>
Expand All @@ -29,6 +30,10 @@ class ATooLongClassNameThatHintsAtADesignProblem {

interface ATooLongInterfaceNameThatHintsAtADesignProblem {

}

class ClassGroupPrefixesThatIsUsedForGrouping {

}
]]>
</example>
Expand Down Expand Up @@ -99,12 +104,14 @@ Detects when a field, formal or local variable is declared with a long name.
<priority>3</priority>
<properties>
<property name="maximum" description="The variable length reporting threshold" value="20"/>
<property name="subtract-prefixes" description="Comma-separated list of prefixes that will not count in the length of the variable name. Only the first matching prefix will be subtracted." value=""/>
<property name="subtract-suffixes" description="Comma-separated list of suffixes that will not count in the length of the variable name. Only the first matching suffix will be subtracted." value=""/>
</properties>
<example>
<![CDATA[
class Something {
protected $reallyLongIntName = -3; // VIOLATION - Field
protected $hungarianUintArrOptions = []; // VIOLATION - Field
public static function main( array $interestingArgumentsList[] ) { // VIOLATION - Formal
$otherReallyLongName = -5; // VIOLATION - Local
for ($interestingIntIndex = 0; // VIOLATION - For
Expand Down
14 changes: 14 additions & 0 deletions src/site/rst/rules/naming.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,21 @@ Example: ::

}

class ClassGroupPrefixesThatIsUsedForGrouping {

}

This rule has the following properties:

+-----------------------------------+---------------+------------------------------------------------------------+
| Name | Default Value | Description |
+===================================+===============+============================================================+
| maximum | 40 | The class name length reporting threshold. |
+-----------------------------------+---------------+------------------------------------------------------------+
| subtract-prefixes | | Comma-separated list of prefixes that will not count in |
| | | the length of the class name. Only the first matching |
| | | prefix will be subtracted. |
+-----------------------------------+---------------+------------------------------------------------------------+
| subtract-suffixes | | Comma-separated list of suffixes that will not count in |
| | | the length of the class name. Only the first matching |
| | | suffix will be subtracted. |
Expand Down Expand Up @@ -101,6 +109,7 @@ Example: ::

class Something {
protected $reallyLongIntName = -3; // VIOLATION - Field
protected $hungarianUintArrOptions = []; // VIOLATION - Field
public static function main( array $interestingArgumentsList[] ) { // VIOLATION - Formal
$otherReallyLongName = -5; // VIOLATION - Local
for ($interestingIntIndex = 0; // VIOLATION - For
Expand All @@ -117,6 +126,11 @@ This rule has the following properties:
+===================================+===============+===========================================+
| maximum | 20 | The variable length reporting threshold |
+-----------------------------------+---------------+-------------------------------------------+
| subtract-prefixes | | Comma-separated list of prefixes that will|
| | | not count in the length of the variable |
| | | name. Only the first matching prefix will |
| | | be subtracted. |
+-----------------------------------+---------------+-------------------------------------------+
| subtract-suffixes | | Comma-separated list of suffixes that will|
| | | not count in the length of the variable |
| | | name. Only the first matching suffix will |
Expand Down
15 changes: 15 additions & 0 deletions src/test/php/PHPMD/Rule/Naming/LongClassNameTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,19 @@ public function testRuleAppliesToClassNameLengthWithoutSuffixSubtracted()
$rule->setReport($this->getReportWithOneViolation());
$rule->apply($this->getClass());
}

/**
* Tests that the rule applies to class name length (43) below threshold (40)
* not matching configured prefix length (15)
*
* @return void
*/
public function testRuleAppliesToClassNameWithPrefixMatched()
{
$rule = new LongClassName();
$rule->addProperty('maximum', 45);
$rule->addProperty('subtract-prefixes', 'testRule,testRuleApplies');
$rule->setReport($this->getReportWithNoViolation());
$rule->apply($this->getClass());
}
}
14 changes: 14 additions & 0 deletions src/test/php/PHPMD/Rule/Naming/LongVariableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,18 @@ public function testRuleAppliesToVariableNameWithEmptySubtractSuffixes()
$rule->setReport($this->getReportWithOneViolation());
$rule->apply($this->getClass());
}

/**
* testRuleAppliesToVariableNameFollowingHungarianNotation
*
* @return void
*/
public function testRuleAppliesToVariableNameFollowingHungarianNotation()
{
$rule = new LongVariable();
$rule->addProperty('maximum', 12);
$rule->addProperty('subtract-prefixes', 'arru8');
$rule->setReport($this->getReportWithNoViolation());
$rule->apply($this->getClass());
}
}
24 changes: 24 additions & 0 deletions src/test/php/PHPMD/Utility/StringsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,30 @@ public function testLengthWithoutSuffixesStringWithPrefixMatchShouldNotSubtract(
static::assertSame(11, Strings::lengthWithoutSuffixes('FooUnitTest', array('Foo', 'Bar')));
}

/**
* Tests the lengthWithoutSuffixes() method that a Prefix should be matched
*
* @return void
*/
public function testlengthWithPrefixesAndSuffixesStringWithPrefixMatchShouldSubtract()
{
static::assertSame(11, Strings::lengthWithoutSuffixes('FooUnitTest', array('Foo', 'Bar')));
static::assertSame(8, Strings::lengthWithoutSuffixes('UnitTestFoo', array('Foo', 'Bar')));
}

/**
* Tests the lengthWithoutPrefixesAndSuffixes() method that a Prefix should not be matched in order
*
* @return void
*/
public function testlengthWithPrefixesAndSuffixesStringWithPrefixesMatchShouldSubtractInOrder()
{
$prefixes = array('Foo', 'Bar');
$suffixes = array('Foo', 'FooUnit');
$length = Strings::lengthWithoutPrefixesAndSuffixes('FooUnitTest', $suffixes, $prefixes);
static::assertSame(8, $length);
}

/**
* Tests the splitToList() method with an empty separator
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* This file is part of PHP Mess Detector.
*
* Copyright (c) Manuel Pichler <mapi@phpmd.org>.
* All rights reserved.
*
* Licensed under BSD License
* For full copyright and license information, please see the LICENSE file.
* Redistributions of files must retain the above copyright notice.
*
* @author Manuel Pichler <mapi@phpmd.org>
* @copyright Manuel Pichler. All rights reserved.
* @license https://opensource.org/licenses/bsd-license.php BSD License
* @link http://phpmd.org/
*/

/**
* Class name length: 43
*/
class testRuleAppliesToClassNameWithPrefixMatched
{

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php
/**
* This file is part of PHP Mess Detector.
*
* Copyright (c) Manuel Pichler <mapi@phpmd.org>.
* All rights reserved.
*
* Licensed under BSD License
* For full copyright and license information, please see the LICENSE file.
* Redistributions of files must retain the above copyright notice.
*
* @author Manuel Pichler <mapi@phpmd.org>
* @copyright Manuel Pichler. All rights reserved.
* @license https://opensource.org/licenses/bsd-license.php BSD License
* @link http://phpmd.org/
*/

class testRuleAppliesToVariableNameFollowingHungarianNotation
{
private $arru8NumberList;
}