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

Refactored PHPMD rules and improved readability #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions src/Project/Common/InstanceResolvingRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function getDescription(): string
/**
* @var string
*/
protected const INSTANCE_PATTERN = '([\w]+(Repository|EntityManager|QueryContainer|Facade|DependencyProvider|Client|Service)$)';
protected const INSTANCE_PATTERN = '/\w+(Repository|EntityManager|QueryContainer|Facade|DependencyProvider|Client|Service)$/';

/**
* @param \PHPMD\AbstractNode $node
Expand All @@ -55,15 +55,21 @@ public function apply(AbstractNode $node): void

$referenceName = trim($reference->getName(), '\\');

if (preg_match(static::INSTANCE_PATTERN, $referenceName)) {
$message = sprintf(
'Entity `%s` is initialized in method `%s`. %s',
$referenceName,
$methodName,
static::RULE,
);
$this->addViolation($methodNode, [$message]);
if (preg_match(static::INSTANCE_PATTERN, $referenceName) !== 1) {
continue;
}

$this->addViolation(
$methodNode,
[
sprintf(
'Entity `%s` is initialized in method `%s`. %s',
$referenceName,
$methodName,
static::RULE,
),
],
);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/Project/Common/LayerAccessRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,13 @@ protected function applyPatterns(AbstractNode $node, array $patterns)
*/
protected function collectPatterns(ClassNode $class): array
{
$patterns = [];
foreach ($this->patterns as [$srcPattern, $targetPattern, $message]) {
if (preg_match($srcPattern, $class->getNamespaceName())) {
$patterns[] = [$srcPattern, $targetPattern, $message];
return array_reduce($this->patterns, function ($collectedPatterns, $pattern) use ($class) {
[$srcPattern] = $pattern;
if (preg_match($srcPattern, $class->getNamespaceName()) === 1) {
$collectedPatterns[] = $pattern;
}
}

return $patterns;
return $collectedPatterns;
}, []);
}
}
16 changes: 4 additions & 12 deletions src/Project/Common/LocatorInDependencyProviderOnlyRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ public function getDescription(): string
*/
public function apply(AbstractNode $node): void
{
if ($this->isClassAllowedToUseLocator($node) === true) {
if ($this->isClassAllowedToUseLocator($node)) {
return;
}

foreach ($node->findChildrenOfType('MethodPostfix') as $classUsage) {
$methodName = $classUsage->getNode()->getImage();

if ($this->isLocator($methodName) === false) {
if (!$this->isLocator($methodName)) {
continue;
}

Expand All @@ -73,11 +73,7 @@ public function apply(AbstractNode $node): void
*/
protected function isLocator(string $methodName): bool
{
if (preg_match(static::LOCATOR_METHOD_NAMES, $methodName)) {
return true;
}

return false;
return preg_match(static::LOCATOR_METHOD_NAMES, $methodName) === 1;
}

/**
Expand All @@ -87,10 +83,6 @@ protected function isLocator(string $methodName): bool
*/
protected function isClassAllowedToUseLocator(AbstractNode $node): bool
{
if (preg_match(static::CLASSES_ALLOWED_TO_USE_LOCATOR, $node->getParentName())) {
return true;
}

return false;
return preg_match(static::CLASSES_ALLOWED_TO_USE_LOCATOR, $node->getParentName()) === 1;
}
}
66 changes: 52 additions & 14 deletions src/Project/Common/ProjectNoBridgeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ class ProjectNoBridgeRule extends AbstractRule implements ClassAware
*/
public const RULE = 'Project should not use and depend on Bridge pattern.';

/**
* @var string
*/
public const REGEX_BRIDGE = '/\w+Bridge$/';

/**
* @return string
*/
Expand All @@ -33,28 +38,61 @@ public function getDescription(): string
*/
public function apply(AbstractNode $node): void
{
if (preg_match('([\w]+Bridge$)', $node->getFullQualifiedName()) !== 0) {
$this->addViolation(
$node,
[
sprintf('Project should not use bridges: %s.', $node->getFullQualifiedName()),
],
);
$this->applyNotUseBridge($node);
$this->applyNotDependOnBridges($node);
}

/**
* @param \PHPMD\AbstractRule $node
*
* @return void
*/
protected function applyNotUseBridge(AbstractNode $node): void
{
if (!$this->isBridge($node->getFullQualifiedName())) {
return;
}

$this->addViolation(
$node,
[
sprintf('Project should not use bridges: %s.', $node->getFullQualifiedName()),
],
);
}

/**
* @param \PHPMD\AbstractNode $node
*
* @return void
*/
protected function applyNotDependOnBridges(AbstractNode $node): void
{
foreach ($node->getMethods() as $method) {
foreach ($method->getDependencies() as $dependency) {
$targetQName = sprintf('%s\\%s', $dependency->getNamespaceName(), $dependency->getName());

if (preg_match('([\w]+Bridge$)', $targetQName) !== 0) {
$this->addViolation(
$method,
[
sprintf('Project should not depend on bridges: %s.', $targetQName),
],
);
if (!$this->isBridge($targetQName)) {
continue;
}

$this->addViolation(
$method,
[
sprintf('Project should not depend on bridges: %s.', $targetQName),
],
);
}
}
}

/**
* @param string $nodeName
*
* @return bool
*/
protected function isBridge(string $nodeName): bool
{
return preg_match(static::REGEX_BRIDGE, $nodeName) === 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@ protected function isDependencyProvider(AbstractNode $node): bool
$parent = $node->getNode()->getParent();
$className = $parent->getNamespaceName() . '\\' . $parent->getName();

if (preg_match('/\\\\' . '(?:Client|Yves|Glue|Zed|Service)' . '\\\\.*\\\\\w+DependencyProvider$/', $className)) {
return true;
}

return false;
return preg_match('/\\\\' . '(?:Client|Yves|Glue|Zed|Service)' . '\\\\.*\\\\\w+DependencyProvider$/', $className) === 1;
}

/**
Expand All @@ -89,11 +85,7 @@ protected function isDependencyProvider(AbstractNode $node): bool
*/
protected function isGetInstance(string $methodName): bool
{
if (preg_match(static::GET_INSTANCE_METHOD_NAME, $methodName)) {
return true;
}

return false;
return preg_match(static::GET_INSTANCE_METHOD_NAME, $methodName) === 1;
}

/**
Expand All @@ -103,10 +95,6 @@ protected function isGetInstance(string $methodName): bool
*/
protected function isStaticCall(string $callSymbol): bool
{
if (preg_match('::', $callSymbol)) {
return true;
}

return false;
return preg_match('::', $callSymbol) === 1;
}
}
26 changes: 16 additions & 10 deletions src/Project/Common/TooManyPublicMethodsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PHPMD\AbstractNode;
use PHPMD\AbstractRule;
use PHPMD\Node\AbstractTypeNode;
use PHPMD\Node\MethodNode;
use PHPMD\Rule\ClassAware;

class TooManyPublicMethodsRule extends AbstractRule implements ClassAware
Expand Down Expand Up @@ -41,9 +42,7 @@ public function apply(AbstractNode $node): void
{
$ignoreClassRegexp = $this->getStringProperty('ignoreclasspattern');

$fullClassName = $node->getFullQualifiedName();

if (preg_match($ignoreClassRegexp, $fullClassName)) {
if (preg_match($ignoreClassRegexp, $node->getFullQualifiedName())) {
return;
}

Expand Down Expand Up @@ -78,14 +77,21 @@ public function apply(AbstractNode $node): void
*/
protected function countMethods(AbstractTypeNode $node): int
{
$count = 0;
foreach ($node->getMethods() as $method) {
if ($method->getNode()->isPublic() && !$this->isIgnoredMethodName($method->getName())) {
++$count;
}
}
return array_reduce(
$node->getMethods(),
fn (int $count, MethodNode $method) => $this->isMethodCountable($method) ? ++$count : $count,
0,
);
}

return $count;
/**
* @param \PHPMD\Node\MethodNode $method
*
* @return bool
*/
protected function isMethodCountable(MethodNode $method): bool
{
return $method->getNode()->isPublic() && !$this->isIgnoredMethodName($method->getName());
}

/**
Expand Down
14 changes: 12 additions & 2 deletions src/Project/Common/WeirdModuleNameRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function getDescription(): string
/**
* @var string
*/
protected const WEIRD_MODULE_NAME_PATTER = '(^[\w]+\\\\(Zed|Client|Yves|Glue|Service|Shared)\\\\[\w]*(?i:test|dummy|example|antelope)[\w]*\\\\.+)';
protected const WEIRD_MODULE_NAME_PATTERN = '/^[\w]+\\\\(Zed|Client|Yves|Glue|Service|Shared)\\\\[\w]*(?i:test|dummy|example|antelope)[\w]*\\\\.+/';

/**
* @param \PHPMD\AbstractNode $node
Expand All @@ -40,7 +40,7 @@ public function apply(AbstractNode $node): void
{
$classFullName = $node->getFullQualifiedName();

if (preg_match(static::WEIRD_MODULE_NAME_PATTER, $classFullName) === 0) {
if (!$this->isWeirdClassName($classFullName)) {
return;
}

Expand All @@ -51,4 +51,14 @@ public function apply(AbstractNode $node): void
],
);
}

/**
* @param string $classFullName
*
* @return bool
*/
protected function isWeirdClassName(string $classFullName): bool
{
return preg_match(static::WEIRD_MODULE_NAME_PATTERN, $classFullName) === 1;
}
}