Skip to content

Commit

Permalink
Add prefix removal for class names and variable names
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaBehrens committed Jun 16, 2022
1 parent b627127 commit 07048f7
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 9 deletions.
29 changes: 27 additions & 2 deletions src/main/php/PHPMD/Rule/Naming/LongClassName.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,17 @@
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
{
/**
* Temporary cache of configured prefixes to subtract
*
* @var string[]|null
*/
protected $subtractPrefixes;

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

if (Strings::lengthWithoutPrefixesAndSuffixes($classOrInterfaceName, $this->getSubtractPrefixList(), $this->getSubtractSuffixList()) <= $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 @@ -30,6 +30,13 @@
*/
class LongVariable extends AbstractRule implements ClassAware, MethodAware, FunctionAware
{
/**
* Temporary cache of configured prefixes to subtract
*
* @var string[]|null
*/
protected $subtractPrefixes;

/**
* Temporary cache of configured suffixes to subtract
*
Expand Down Expand Up @@ -108,7 +115,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, '$'),
$this->getSubtractPrefixList(),
$this->getSubtractSuffixList()
);
if ($lengthWithoutDollarSign <= $threshold) {
return;
}
Expand Down Expand Up @@ -183,6 +194,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
23 changes: 23 additions & 0 deletions src/main/php/PHPMD/Utility/Strings.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ class Strings
* @return int The length of the string, without suffix, if applicable.
*/
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);

Expand All @@ -43,6 +56,16 @@ public static function lengthWithoutSuffixes($stringName, array $subtractSuffixe
}
}

rsort($subtractPrefixes, SORT_STRING);

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 longest, 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 longest, 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 longest, 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 longest, first matching |
| | | suffix 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 @@ -121,4 +121,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());
}
}
32 changes: 26 additions & 6 deletions src/test/php/PHPMD/Utility/StringsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class StringsTest extends AbstractTest
*
* @return void
*/
public function testLengthWithoutSuffixesEmptyString()
public function testlengthWithoutSuffixesEmptyString()
{
static::assertSame(0, Strings::lengthWithoutSuffixes('', array()));
}
Expand All @@ -42,7 +42,7 @@ public function testLengthWithoutSuffixesEmptyString()
*
* @return void
*/
public function testLengthWithoutSuffixesEmptyStringWithConfiguredSubtractSuffix()
public function testlengthWithoutSuffixesEmptyStringWithConfiguredSubtractSuffix()
{
static::assertSame(0, Strings::lengthWithoutSuffixes('', array('Foo', 'Bar')));
}
Expand All @@ -52,7 +52,7 @@ public function testLengthWithoutSuffixesEmptyStringWithConfiguredSubtractSuffix
*
* @return void
*/
public function testLengthWithoutSuffixesStringWithoutSubtractSuffixMatch()
public function testlengthWithoutSuffixesStringWithoutSubtractSuffixMatch()
{
static::assertSame(8, Strings::lengthWithoutSuffixes('UnitTest', array('Foo', 'Bar')));
}
Expand All @@ -62,7 +62,7 @@ public function testLengthWithoutSuffixesStringWithoutSubtractSuffixMatch()
*
* @return void
*/
public function testLengthWithoutSuffixesStringWithSubtractSuffixMatch()
public function testlengthWithoutSuffixesStringWithSubtractSuffixMatch()
{
static::assertSame(4, Strings::lengthWithoutSuffixes('UnitBar', array('Foo', 'Bar')));
}
Expand All @@ -72,7 +72,7 @@ public function testLengthWithoutSuffixesStringWithSubtractSuffixMatch()
*
* @return void
*/
public function testLengthWithoutSuffixesStringWithDoubleSuffixMatchSubtractOnce()
public function testlengthWithoutSuffixesStringWithDoubleSuffixMatchSubtractOnce()
{
static::assertSame(7, Strings::lengthWithoutSuffixes('UnitFooBar', array('Foo', 'Bar')));
}
Expand All @@ -82,11 +82,31 @@ public function testLengthWithoutSuffixesStringWithDoubleSuffixMatchSubtractOnce
*
* @return void
*/
public function testLengthWithoutSuffixesStringWithPrefixMatchShouldNotSubtract()
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(8, Strings::lengthWithoutSuffixes('FooUnitTest', array('Foo', 'Bar')));
}

/**
* Tests the lengthWithoutPrefixesAndSuffixes() method that a Prefix should not be matched in the best order
*
* @return void
*/
public function testlengthWithPrefixesAndSuffixesStringWithPrefixesMatchShouldSubtractInBestOrder()
{
static::assertSame(4, Strings::lengthWithoutPrefixesAndSuffixes('FooUnitTest', array('Foo', 'FooUnit'), array('Foo', 'Bar')));
}

/**
* 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;
}

0 comments on commit 07048f7

Please sign in to comment.