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

Class name length rule #765

Merged
merged 42 commits into from Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4ca680f
Strings support class:
frankdekker May 4, 2020
c024596
Strings support class:
frankdekker May 4, 2020
e2d66e1
Update LongVariable rule with getStringProperty and string split/length
frankdekker May 4, 2020
b6ef2af
LongVariable Rule + Tests
frankdekker May 4, 2020
00d7cbd
Updated documentation
frankdekker May 4, 2020
205973b
ShortClassName rule + testcases
frankdekker May 4, 2020
0a180dd
Updated documentation for the short class name rule
frankdekker May 4, 2020
ad3ced9
Added example to exceptions list
frankdekker May 5, 2020
9d5fabc
Updated docblock of StringsTest
frankdekker May 5, 2020
ac08020
PHPUnit 4.8.3 expectException fix.
frankdekker May 5, 2020
083628e
Fixed threshold calculation.
frankdekker May 5, 2020
0e56d78
Move length calculation inside the if for consistency amongst rules
frankdekker May 5, 2020
1692729
Remove superfluous class aliases
ravage84 May 5, 2020
8ec7d42
Improve doc block wording
ravage84 May 5, 2020
cf4a9bb
Correct coverage annotation and FQCN
ravage84 May 5, 2020
a9b4caf
Improve test case doc blocks
ravage84 May 5, 2020
bb6c341
Correct test case name & test file
ravage84 May 5, 2020
a5b6b43
Correct test case name & test file
ravage84 May 5, 2020
43d0d5d
Rename Strings::length() to lengthWithoutSuffixes()
ravage84 May 5, 2020
26309d7
Use double quotes
ravage84 May 6, 2020
426e034
Rename Strings::split() to splitToList()
ravage84 May 6, 2020
0dff3ca
Use correct coverage annotation
ravage84 May 6, 2020
a2d0939
Correct doc block statement
ravage84 May 6, 2020
251d02c
Update test case names to changed method names
ravage84 May 6, 2020
f1a4fd5
Improve test doc blocks
ravage84 May 6, 2020
93f3ec9
Rename Support namespace to Utility
ravage84 May 6, 2020
99dfd2f
Improve wording
ravage84 May 6, 2020
ecb1b9f
Improve wording
ravage84 May 6, 2020
a2e8255
Get class or interface name only once
ravage84 May 6, 2020
feb6f15
CS
ravage84 May 6, 2020
c382d71
CS
ravage84 May 6, 2020
fb0bb33
Improve wording
ravage84 May 6, 2020
243de22
Improve wording
ravage84 May 6, 2020
ad1fcb4
Get class or interface name only once
ravage84 May 6, 2020
878a438
Correct coverage annotation and FQCN
ravage84 May 6, 2020
7d8212c
Clarify code through intermediate variables
ravage84 May 6, 2020
f29310d
Improve wording
ravage84 May 6, 2020
00c808b
Improve test case doc blocks
ravage84 May 6, 2020
b8e3cf7
Bump maximum threshold of LongClassName
ravage84 May 6, 2020
1487ff1
Suppress violation for long variable name
ravage84 May 6, 2020
ff6d928
Changed Short Class name example from `Id` to `Fo`
frankdekker May 6, 2020
b34aba2
Remove doubly "long" in example class name
ravage84 May 6, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/main/php/PHPMD/Rule/Naming/LongClassName.php
@@ -0,0 +1,68 @@
<?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/
*/

namespace PHPMD\Rule\Naming;

use PHPMD\AbstractNode;
use PHPMD\AbstractRule;
use PHPMD\Rule\ClassAware;
use PHPMD\Rule\InterfaceAware;
use PHPMD\Support\Strings;

/**
* This rule class will check if a class name doesn't exceed the configured length excluding
* certain configured suffixes.
*/
class LongClassName extends AbstractRule implements ClassAware, InterfaceAware
{
/**
* Temporary cache of configured suffixes to subtract
*
* @var string[]|null
*/
private $subtractSuffixes;

/**
* This method checks if a class name exceeds the configured maximum length
* and emits a rule violation.
*
* @param \PHPMD\AbstractNode $node
* @return void
*/
public function apply(AbstractNode $node)
{
$threshold = $this->getIntProperty('maximum');
if (Strings::lengthWithoutSuffixes($node->getName(), $this->getSubtractSuffixList()) <= $threshold) {
return;
}
$this->addViolation($node, array($node->getName(), $threshold));
}

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

return $this->subtractSuffixes;
}
}
46 changes: 5 additions & 41 deletions src/main/php/PHPMD/Rule/Naming/LongVariable.php
Expand Up @@ -22,6 +22,7 @@
use PHPMD\Rule\ClassAware;
use PHPMD\Rule\FunctionAware;
use PHPMD\Rule\MethodAware;
use PHPMD\Support\Strings;

/**
* This rule class will detect variables, parameters and properties with really
Expand Down Expand Up @@ -105,7 +106,7 @@ protected function checkNodeImage(AbstractNode $node)
protected function checkMaximumLength(AbstractNode $node)
{
$threshold = $this->getIntProperty('maximum');
if ($threshold >= $this->getStringLength($node->getImage(), $this->getSubtractSuffixList()) - 1) {
if (Strings::lengthWithoutSuffixes($node->getImage(), $this->getSubtractSuffixList()) - 1 <= $threshold) {
return;
}
if ($this->isNameAllowedInContext($node)) {
Expand All @@ -126,29 +127,6 @@ private function isNameAllowedInContext(AbstractNode $node)
return $this->isChildOf($node, 'MemberPrimaryPrefix');
}

/**
* Returns the length of the variable name, excluding at most one suffix.
*
* @param string $variableName Variable name to calculate the length for.
* @param array $subtractSuffixes Optional list of suffixes to exclude from the calculated length.
* @return int The length of the string, without suffix, if applicable.
*/
private function getStringLength($variableName, array $subtractSuffixes)
{
$variableNameLength = strlen($variableName);

foreach ($subtractSuffixes as $suffix) {
$suffixLength = strlen($suffix);
if (substr($variableName, -$suffixLength) === $suffix) {
$variableName = substr($variableName, 0, $variableNameLength - $suffixLength);

return strlen($variableName);
}
}

return $variableNameLength;
}

/**
* Checks if the given node is a direct or indirect child of a node with
* the given type.
Expand Down Expand Up @@ -209,24 +187,10 @@ protected function isNotProcessed(AbstractNode $node)
*/
private function getSubtractSuffixList()
{
if ($this->subtractSuffixes !== null) {
return $this->subtractSuffixes;
if ($this->subtractSuffixes === null) {
$this->subtractSuffixes = Strings::splitToList($this->getStringProperty('subtract-suffixes', ''), ',');
}

try {
$suffixes = $this->getStringProperty('subtract-suffixes');
} catch (\OutOfBoundsException $e) {
return $this->subtractSuffixes = array();
}

return $this->subtractSuffixes = array_filter(
array_map(
'trim',
explode(',', $suffixes)
),
function ($value) {
return $value !== '';
}
);
return $this->subtractSuffixes;
}
}
73 changes: 73 additions & 0 deletions src/main/php/PHPMD/Rule/Naming/ShortClassName.php
@@ -0,0 +1,73 @@
<?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/
*/

namespace PHPMD\Rule\Naming;

use PHPMD\AbstractNode;
use PHPMD\AbstractRule;
use PHPMD\Rule\ClassAware;
use PHPMD\Rule\InterfaceAware;
use PHPMD\Support\Strings;

/**
* This rule class will detect classes and interfaces with short names.
*/
class ShortClassName extends AbstractRule implements ClassAware, InterfaceAware
{
/**
* Temporary cache of configured exceptions. Have name as key
*
* @var array<string, int>|null
*/
private $exceptions;

/**
* This method checks if a class or interface name is below the minimum configured length
* and emits a rule violation.
*
* @param \PHPMD\AbstractNode $node
* @return void
*/
public function apply(AbstractNode $node)
{
$threshold = $this->getIntProperty('minimum');
if (strlen($node->getName()) >= $threshold) {
return;
}

$exceptions = $this->getExceptionsList();
if (isset($exceptions[$node->getName()])) {
return;
}

$this->addViolation($node, array($node->getName(), $threshold));
}

/**
* Gets array of exceptions from property
*
* @return array<string, int>
*/
private function getExceptionsList()
{
if ($this->exceptions === null) {
$this->exceptions = array_flip(Strings::splitToList($this->getStringProperty('exceptions', ''), ','));
}

return $this->exceptions;
}
}
70 changes: 70 additions & 0 deletions src/main/php/PHPMD/Support/Strings.php
@@ -0,0 +1,70 @@
<?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/
*/

namespace PHPMD\Support;

use InvalidArgumentException;

/**
* Utility class to provide string checks and manipulations
*/
class Strings
{
/**
* Returns the length of the given string, excluding at most one suffix
*
* @param string $stringName String to calculate the length for.
* @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 lengthWithoutSuffixes($stringName, array $subtractSuffixes)
{
$stringLength = strlen($stringName);

foreach ($subtractSuffixes as $suffix) {
$suffixLength = strlen($suffix);
if (substr($stringName, -$suffixLength) === $suffix) {
$stringLength -= $suffixLength;
break;
}
}

return $stringLength;
}

/**
* Split a string with the given separator, trim whitespaces around the parts and remove any empty strings
*
* @param string $listAsString The string to split.
* @param string $separator The separator to split the string with, similar to explode.
* @return array The list of trimmed and filtered parts of the string.
* @throws InvalidArgumentException When the separator is an empty string.
*/
public static function splitToList($listAsString, $separator)
{
if ($separator === '') {
throw new InvalidArgumentException("Separator can't be empty string");
}

return array_filter(
array_map('trim', explode($separator, $listAsString)),
function ($value) {
return $value !== '';
}
);
}
}
52 changes: 52 additions & 0 deletions src/main/resources/rulesets/naming.xml
Expand Up @@ -8,6 +8,58 @@
The Naming Ruleset contains a collection of rules about names - too long, too short, and so forth.
</description>

<rule name="LongClassName"
since="2.9"
message="Avoid excessively long class names like {0}. Keep class name length under {1}."
class="PHPMD\Rule\Naming\LongClassName"
externalInfoUrl="https://phpmd.org/rules/naming.html#longclassname">
<description>
Detects when classes or interfaces are declared with excessively long names.
</description>
<priority>3</priority>
<properties>
<property name="maximum" description="The class name length reporting threshold" value="20"/>
ravage84 marked this conversation as resolved.
Show resolved Hide resolved
<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>
<![CDATA[
class AReallyLongClassName {

}

interface AReallyLongInterfaceName {

}
]]>
</example>
</rule>

<rule name="ShortClassName"
since="2.9"
message="Avoid classes with short names like {0}. Configured minimum length is {1}."
class="PHPMD\Rule\Naming\ShortClassName"
externalInfoUrl="https://phpmd.org/rules/naming.html#shortclassname">
<description>
Detects when classes or interfaces have a very short name.
</description>
<priority>3</priority>
<properties>
<property name="minimum" description="The class name length reporting threshold" value="3"/>
<property name="exceptions" description="Comma-separated list of exceptions. Example: Log,URL,FTP" value=""/>
</properties>
<example>
<![CDATA[
class Id {
frankdekker marked this conversation as resolved.
Show resolved Hide resolved

}

interface Id {

}
]]>
</example>
</rule>

<rule name="ShortVariable"
since="0.2"
message="Avoid variables with short names like {0}. Configured minimum length is {1}."
Expand Down
2 changes: 2 additions & 0 deletions src/site/rst/rules/index.rst
Expand Up @@ -64,6 +64,8 @@ many bugs, especially when the loop manipulates an array, as count happens on ea
Naming Rules
============

- `LongClassName <naming.html#longclassname>`_: Detects when classes or interfaces are declared with excessively long names.
- `ShortClassName <naming.html#shortclassname>`_: Detects when classes or interfaces have a very short name.
- `ShortVariable <naming.html#shortvariable>`_: Detects when a field, local, or parameter has a very short name.
- `LongVariable <naming.html#longvariable>`_: Detects when a field, formal or local variable is declared with a long name.
- `ShortMethodName <naming.html#shortmethodname>`_: Detects when very short method names are used.
Expand Down