From 719f6380fa82655aea5aaf95bab37b32b5fd3bda Mon Sep 17 00:00:00 2001
From: NanoSector <rick@nanosector.nl>
Date: Fri, 25 Apr 2025 13:32:32 +0200
Subject: [PATCH] Disallow more than 1 public method named with regex

Right now it is possible to define a class that has two public methods:
`exampleOne` and `exampleTwo`. If you do not give this class extra
methods and configure the ShouldHaveOnlyOnePublicMethodNamed rule to
look for `^example.+$` then it will report success, despite two methods
existing with this naming scheme.

This rule will only start failing after introducing a method which does
*not* follow the naming scheme, e.g. `foo`. This PR aims to fix this by
also failing in the `exampleOne` and `exampleTwo` scenario.

Note that we interpreted this rule as "can have several public methods,
but only one named ...", instead of "should have only one public method
*and* it is named ...", so this might not follow the original intention
of the rule.
---
 .../PublicMethodNamedExtractor.php            | 22 +++-----
 .../ClassWithTwoSimilarlyNamedMethods.php     | 18 +++++++
 .../ClassWithTwoUnrelatedNamedMethods.php     | 18 +++++++
 ...eThanOnePublicMethodNamedWithRegexTest.php | 49 ++++++++++++++++++
 ...lyNamedPublicMethodsNamedWithRegexTest.php | 50 +++++++++++++++++++
 5 files changed, 143 insertions(+), 14 deletions(-)
 create mode 100644 tests/fixtures/Special/ClassWithTwoSimilarlyNamedMethods.php
 create mode 100644 tests/fixtures/Special/ClassWithTwoUnrelatedNamedMethods.php
 create mode 100644 tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/MoreThanOnePublicMethodNamedWithRegexTest.php
 create mode 100644 tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/SimilarlyNamedPublicMethodsNamedWithRegexTest.php

diff --git a/src/Rule/Extractor/Declaration/PublicMethodNamedExtractor.php b/src/Rule/Extractor/Declaration/PublicMethodNamedExtractor.php
index b04f7193..8719df58 100644
--- a/src/Rule/Extractor/Declaration/PublicMethodNamedExtractor.php
+++ b/src/Rule/Extractor/Declaration/PublicMethodNamedExtractor.php
@@ -25,25 +25,19 @@ protected function meetsDeclaration(Node $node, Scope $scope, array $params = []
         $reflectionClass = $node->getClassReflection()->getNativeReflection();
         $methods = $reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC);
 
-        $methodsWithoutConstructor = array_filter(
+        $methodsMeetingDeclaration = array_filter(
             $methods,
-            fn ($method) => $method->getName() !== '__construct'
-        );
-
-        foreach ($methodsWithoutConstructor as $method) {
-            if ($params['isRegex'] === true) {
-                if (preg_match($params['name'], $method->getName()) !== 1) {
+            function ($method) use ($params) {
+                if ($method->getName() === '__construct') {
                     return false;
                 }
 
-                continue;
-            }
-
-            if ($method->getName() !== $params['name']) {
-                return false;
+                return $params['isRegex'] === true
+                    ? preg_match($params['name'], $method->getName()) === 1
+                    : $method->getName() === $params['name'];
             }
-        }
+        );
 
-        return true;
+        return count($methodsMeetingDeclaration) === 1;
     }
 }
diff --git a/tests/fixtures/Special/ClassWithTwoSimilarlyNamedMethods.php b/tests/fixtures/Special/ClassWithTwoSimilarlyNamedMethods.php
new file mode 100644
index 00000000..2008bce5
--- /dev/null
+++ b/tests/fixtures/Special/ClassWithTwoSimilarlyNamedMethods.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\PHPat\fixtures\Special;
+
+final class ClassWithTwoSimilarlyNamedMethods
+{
+    public function exampleOne(): bool
+    {
+        return true;
+    }
+
+    public function exampleTwo(): bool
+    {
+        return true;
+    }
+}
\ No newline at end of file
diff --git a/tests/fixtures/Special/ClassWithTwoUnrelatedNamedMethods.php b/tests/fixtures/Special/ClassWithTwoUnrelatedNamedMethods.php
new file mode 100644
index 00000000..e925d322
--- /dev/null
+++ b/tests/fixtures/Special/ClassWithTwoUnrelatedNamedMethods.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\PHPat\fixtures\Special;
+
+final class ClassWithTwoUnrelatedNamedMethods
+{
+    public function bar(): bool
+    {
+        return true;
+    }
+
+    public function foo(): bool
+    {
+        return true;
+    }
+}
\ No newline at end of file
diff --git a/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/MoreThanOnePublicMethodNamedWithRegexTest.php b/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/MoreThanOnePublicMethodNamedWithRegexTest.php
new file mode 100644
index 00000000..19960266
--- /dev/null
+++ b/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/MoreThanOnePublicMethodNamedWithRegexTest.php
@@ -0,0 +1,49 @@
+<?php declare(strict_types=1);
+
+namespace Tests\PHPat\unit\rules\ShouldHaveOnlyOnePublicMethodNamed;
+
+use PHPat\Configuration;
+use PHPat\Rule\Assertion\Declaration\ShouldHaveOnlyOnePublicMethodNamed\HasOnlyOnePublicMethodNamedRule;
+use PHPat\Selector\Classname;
+use PHPat\Statement\Builder\StatementBuilderFactory;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPStan\Type\FileTypeMapper;
+use Tests\PHPat\fixtures\FixtureClass;
+use Tests\PHPat\fixtures\Special\ClassWithTwoUnrelatedNamedMethods;
+use Tests\PHPat\unit\FakeTestParser;
+
+/**
+ * @extends RuleTestCase<HasOnlyOnePublicMethodNamedRule>
+ * @internal
+ * @coversNothing
+ */
+class MoreThanOnePublicMethodNamedWithRegexTest extends RuleTestCase
+{
+    public const RULE_NAME = 'testFixtureClassShouldHaveOnlyOnePublicMethodNamed';
+
+    public function testRule(): void
+    {
+        // Should succeed as 'foo' is not named like 'ba*'
+        $this->analyse(['tests/fixtures/Special/ClassWithTwoUnrelatedNamedMethods.php'], []);
+    }
+
+    protected function getRule(): Rule
+    {
+        $testParser = FakeTestParser::create(
+            self::RULE_NAME,
+            ClassWithTwoUnrelatedNamedMethods::class,
+            [new Classname(FixtureClass::class, false)],
+            [],
+            [],
+            ['name' => '/^ba[a-zA-Z0-9]+/', 'isRegex' => true]
+        );
+
+        return new HasOnlyOnePublicMethodNamedRule(
+            new StatementBuilderFactory($testParser),
+            new Configuration(false, true, false),
+            self::createReflectionProvider(),
+            self::getContainer()->getByType(FileTypeMapper::class)
+        );
+    }
+}
diff --git a/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/SimilarlyNamedPublicMethodsNamedWithRegexTest.php b/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/SimilarlyNamedPublicMethodsNamedWithRegexTest.php
new file mode 100644
index 00000000..ce95cf6c
--- /dev/null
+++ b/tests/unit/rules/ShouldHaveOnlyOnePublicMethodNamed/SimilarlyNamedPublicMethodsNamedWithRegexTest.php
@@ -0,0 +1,50 @@
+<?php declare(strict_types=1);
+
+namespace Tests\PHPat\unit\rules\ShouldHaveOnlyOnePublicMethodNamed;
+
+use PHPat\Configuration;
+use PHPat\Rule\Assertion\Declaration\ShouldHaveOnlyOnePublicMethodNamed\HasOnlyOnePublicMethodNamedRule;
+use PHPat\Rule\Assertion\Declaration\ShouldHaveOnlyOnePublicMethodNamed\ShouldHaveOnlyOnePublicMethodNamed;
+use PHPat\Selector\Classname;
+use PHPat\Statement\Builder\StatementBuilderFactory;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPStan\Type\FileTypeMapper;
+use Tests\PHPat\fixtures\Special\ClassWithTwoSimilarlyNamedMethods;
+use Tests\PHPat\unit\FakeTestParser;
+
+/**
+ * @extends RuleTestCase<HasOnlyOnePublicMethodNamedRule>
+ * @internal
+ * @coversNothing
+ */
+class SimilarlyNamedPublicMethodsNamedWithRegexTest extends RuleTestCase
+{
+    public const RULE_NAME = 'testFixtureClassShouldHaveOnlyOnePublicMethodNamed';
+
+    public function testRule(): void
+    {
+        $this->analyse(['tests/fixtures/Special/ClassWithTwoSimilarlyNamedMethods.php'], [
+            [sprintf('%s should have only one public method named %s', ClassWithTwoSimilarlyNamedMethods::class, '/^example[a-zA-Z0-9]+/'), 7],
+        ]);
+    }
+
+    protected function getRule(): Rule
+    {
+        $testParser = FakeTestParser::create(
+            self::RULE_NAME,
+            ShouldHaveOnlyOnePublicMethodNamed::class,
+            [new Classname(ClassWithTwoSimilarlyNamedMethods::class, false)],
+            [],
+            [],
+            ['name' => '/^example[a-zA-Z0-9]+/', 'isRegex' => true]
+        );
+
+        return new HasOnlyOnePublicMethodNamedRule(
+            new StatementBuilderFactory($testParser),
+            new Configuration(false, true, false),
+            self::createReflectionProvider(),
+            self::getContainer()->getByType(FileTypeMapper::class)
+        );
+    }
+}