diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml new file mode 100644 index 0000000..d3db5dd --- /dev/null +++ b/.github/workflows/phpunit.yml @@ -0,0 +1,40 @@ +name: PHPUnit + +on: + push: + branches: + - main + pull_request: + branches: + - main + +jobs: + phpunit: + name: Run PHPUnit + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.2' + extensions: json, curl + coverage: none + tools: composer:v2 + + - name: Cache Composer dependencies + uses: actions/cache@v5 + with: + path: vendor + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: | + ${{ runner.os }}-composer- + + - name: Install dependencies + run: composer install --prefer-dist --no-progress --no-interaction + + - name: Run PHPUnit + run: composer test diff --git a/.gitignore b/.gitignore index e20c424..e420734 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ composer.lock .phpunit.cache/ coverage-report/ .php-cs-fixer.cache +/htmlcov/ diff --git a/composer.json b/composer.json index 91af838..1fcdc0f 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,8 @@ "check": [ "@phpcs", "@phpstan", - "@rector" + "@rector", + "@test" ], "fix": [ "@phpcbf", @@ -62,8 +63,10 @@ "phpcbf": "phpcbf", "phpcs": "phpcs", "phpstan": "phpstan analyse -c phpstan.neon src", + "phpunit": "phpunit --testdox", + "phpunit-coverage": "phpunit --coverage-html htmlcov --coverage-text", "rector": "rector process --dry-run", "rector-fix": "rector process", - "test": "phpunit" + "test": "@phpunit" } } diff --git a/phpunit.xml b/phpunit.xml index 7a76898..91aa7bb 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -9,11 +9,15 @@ displayDetailsOnTestsThatTriggerNotices="true" displayDetailsOnTestsThatTriggerErrors="true" stderr="true" - beStrictAboutOutputDuringTests="false"> + beStrictAboutOutputDuringTests="false" + failOnWarning="false"> tests/Unit + + tests/Rules + diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..b0a206b --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,25 @@ + + + + + tests/Rules + + + + + + src + + + diff --git a/src/Rules/Module/ControllersMustReturnResponseRule.php b/src/Rules/Module/ControllersMustReturnResponseRule.php index 8a13969..8bc1c77 100644 --- a/src/Rules/Module/ControllersMustReturnResponseRule.php +++ b/src/Rules/Module/ControllersMustReturnResponseRule.php @@ -15,6 +15,7 @@ namespace OpenCoreEMR\PHPStan\Rules\Module; use PhpParser\Node; +use PhpParser\Node\Identifier; use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; @@ -72,26 +73,19 @@ public function processNode(Node $node, Scope $scope): array ]; } - // Check if return type is void - $function = $scope->getFunction(); - if ($function instanceof \PHPStan\Reflection\Php\PhpFunctionFromParserNodeReflection) { - $variants = $function->getVariants(); - if (count($variants) > 0) { - $returnType = $variants[0]->getReturnType(); - if ($returnType->isVoid()->yes()) { - return [ - RuleErrorBuilder::message( - sprintf( - 'Controller method %s() must return Response object, not void.', - $methodName - ) - ) - ->identifier('openemr.controllerMustReturnResponse') - ->tip('Controllers should return Response, JsonResponse, RedirectResponse, or BinaryFileResponse') - ->build() - ]; - } - } + // Check if return type is void (using AST to avoid reflection issues in tests) + if ($node->returnType instanceof Identifier && $node->returnType->name === 'void') { + return [ + RuleErrorBuilder::message( + sprintf( + 'Controller method %s() must return Response object, not void.', + $methodName + ) + ) + ->identifier('openemr.controllerMustReturnResponse') + ->tip('Controllers should return Response, JsonResponse, RedirectResponse, or BinaryFileResponse') + ->build() + ]; } return []; diff --git a/tests/RuleTestCase.php b/tests/RuleTestCase.php new file mode 100644 index 0000000..3f90dc2 --- /dev/null +++ b/tests/RuleTestCase.php @@ -0,0 +1,34 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests; + +use PHPStan\Testing\RuleTestCase as PHPStanRuleTestCase; + +/** + * Base test case that includes test-specific configuration. + * + * @template TRule of \PHPStan\Rules\Rule + * @extends PHPStanRuleTestCase + */ +abstract class RuleTestCase extends PHPStanRuleTestCase +{ + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/phpstan-tests.neon', + ]; + } +} diff --git a/tests/Rules/Database/ForbiddenClassesRuleTest.php b/tests/Rules/Database/ForbiddenClassesRuleTest.php new file mode 100644 index 0000000..702d6ed --- /dev/null +++ b/tests/Rules/Database/ForbiddenClassesRuleTest.php @@ -0,0 +1,55 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Database; + +use OpenCoreEMR\PHPStan\Rules\Database\ForbiddenClassesRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class ForbiddenClassesRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenClassesRule(); + } + + public function testForbiddenClasses(): void + { + $tip = 'See src/Common/Database/QueryUtils.php for modern database patterns'; + $this->analyse([__DIR__ . '/data/forbidden-classes.php'], [ + [ + 'Laminas-DB class "Laminas\Db\Adapter\Adapter" is deprecated. Use QueryUtils or DatabaseQueryTrait instead.', + 5, + $tip, + ], + [ + 'Laminas-DB class "Laminas\Db\Sql\Select" is deprecated. Use QueryUtils or DatabaseQueryTrait instead.', + 6, + $tip, + ], + [ + 'Laminas-DB class "Laminas\Db\TableGateway\TableGateway" is deprecated. Use QueryUtils or DatabaseQueryTrait instead.', + 7, + $tip, + ], + ]); + } + + public function testAllowedClasses(): void + { + $this->analyse([__DIR__ . '/data/allowed-classes.php'], []); + } +} diff --git a/tests/Rules/Database/ForbiddenFunctionsRuleTest.php b/tests/Rules/Database/ForbiddenFunctionsRuleTest.php new file mode 100644 index 0000000..cd4059f --- /dev/null +++ b/tests/Rules/Database/ForbiddenFunctionsRuleTest.php @@ -0,0 +1,75 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Database; + +use OpenCoreEMR\PHPStan\Rules\Database\ForbiddenFunctionsRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class ForbiddenFunctionsRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenFunctionsRule(); + } + + public function testForbiddenFunctions(): void + { + $tip = 'Or use DatabaseQueryTrait in your class'; + $this->analyse([__DIR__ . '/data/forbidden-functions.php'], [ + [ + 'Use QueryUtils::querySingleRow() or QueryUtils::fetchRecords() instead of sqlQuery().', + 5, + $tip, + ], + [ + 'Use QueryUtils::sqlStatementThrowException() or QueryUtils::fetchRecords() instead of sqlStatement().', + 6, + $tip, + ], + [ + 'Use QueryUtils::sqlInsert() instead of sqlInsert().', + 7, + $tip, + ], + [ + 'Use QueryUtils::fetchRecords() or QueryUtils::fetchArrayFromResultSet() instead of sqlFetchArray().', + 8, + $tip, + ], + [ + 'Use QueryUtils::startTransaction() instead of sqlBeginTrans().', + 9, + $tip, + ], + [ + 'Use QueryUtils::commitTransaction() instead of sqlCommitTrans().', + 10, + $tip, + ], + [ + 'Use QueryUtils::rollbackTransaction() instead of sqlRollbackTrans().', + 11, + $tip, + ], + ]); + } + + public function testAllowedFunctions(): void + { + $this->analyse([__DIR__ . '/data/allowed-functions.php'], []); + } +} diff --git a/tests/Rules/Database/data/allowed-classes.php b/tests/Rules/Database/data/allowed-classes.php new file mode 100644 index 0000000..84ac16d --- /dev/null +++ b/tests/Rules/Database/data/allowed-classes.php @@ -0,0 +1,7 @@ + $x * 2, [1, 2, 3]); +json_encode(['key' => 'value']); + +// Method calls are allowed (only global functions are forbidden) +$db->sqlQuery('SELECT * FROM users'); +$utils->sqlStatement('UPDATE users SET name = ?', ['John']); diff --git a/tests/Rules/Database/data/forbidden-classes.php b/tests/Rules/Database/data/forbidden-classes.php new file mode 100644 index 0000000..37ae8df --- /dev/null +++ b/tests/Rules/Database/data/forbidden-classes.php @@ -0,0 +1,7 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\ExceptionHandling; + +use OpenCoreEMR\PHPStan\Rules\ExceptionHandling\CatchThrowableNotExceptionRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class CatchThrowableNotExceptionRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new CatchThrowableNotExceptionRule(); + } + + public function testCatchingException(): void + { + $tip = 'Change catch (\Exception $e) to catch (\Throwable $e)'; + $this->analyse([__DIR__ . '/data/catch-exception.php'], [ + [ + 'Catch \Throwable instead of \Exception to handle both exceptions and errors (TypeError, ParseError, etc.).', + 7, + $tip, + ], + [ + 'Catch \Throwable instead of \Exception to handle both exceptions and errors (TypeError, ParseError, etc.).', + 13, + $tip, + ], + ]); + } + + public function testCatchingThrowable(): void + { + $this->analyse([__DIR__ . '/data/catch-throwable.php'], []); + } +} diff --git a/tests/Rules/ExceptionHandling/data/catch-exception.php b/tests/Rules/ExceptionHandling/data/catch-exception.php new file mode 100644 index 0000000..ad264d7 --- /dev/null +++ b/tests/Rules/ExceptionHandling/data/catch-exception.php @@ -0,0 +1,15 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Globals; + +use OpenCoreEMR\PHPStan\Rules\Globals\ForbiddenGlobalsAccessRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class ForbiddenGlobalsAccessRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenGlobalsAccessRule(); + } + + public function testForbiddenGlobalsAccess(): void + { + $tip = 'For encrypted values, OEGlobalsBag handles decryption automatically. See src/Core/OEGlobalsBag.php'; + $this->analyse([__DIR__ . '/data/forbidden-globals.php'], [ + [ + 'Direct access to $GLOBALS is forbidden. Use OEGlobalsBag::getInstance()->get() instead.', + 5, + $tip, + ], + [ + 'Direct access to $GLOBALS is forbidden. Use OEGlobalsBag::getInstance()->get() instead.', + 6, + $tip, + ], + [ + 'Direct access to $GLOBALS is forbidden. Use OEGlobalsBag::getInstance()->get() instead.', + 12, + $tip, + ], + ]); + } + + public function testAllowedGlobalsAccess(): void + { + $this->analyse([__DIR__ . '/data/allowed-globals.php'], []); + } +} diff --git a/tests/Rules/Globals/data/allowed-globals.php b/tests/Rules/Globals/data/allowed-globals.php new file mode 100644 index 0000000..17fa543 --- /dev/null +++ b/tests/Rules/Globals/data/allowed-globals.php @@ -0,0 +1,14 @@ + 'value']; +$value = $arr['key']; + +// Variable named GLOBALS (not superglobal) is allowed +$notGlobals = 'test'; diff --git a/tests/Rules/Globals/data/forbidden-globals.php b/tests/Rules/Globals/data/forbidden-globals.php new file mode 100644 index 0000000..b5c8ae5 --- /dev/null +++ b/tests/Rules/Globals/data/forbidden-globals.php @@ -0,0 +1,14 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Module; + +use OpenCoreEMR\PHPStan\Rules\Module\ControllersMustReturnResponseRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class ControllersMustReturnResponseRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ControllersMustReturnResponseRule(); + } + + public function testControllerWithoutReturnType(): void + { + $tip = 'Add return type: Response, JsonResponse, RedirectResponse, or BinaryFileResponse'; + $this->analyse([__DIR__ . '/data/controller-no-return-type.php'], [ + [ + 'Controller method index() must declare a return type (Response or subclass).', + 10, + $tip, + ], + ]); + } + + public function testControllerWithVoidReturn(): void + { + $tip = 'Controllers should return Response, JsonResponse, RedirectResponse, or BinaryFileResponse'; + $this->analyse([__DIR__ . '/data/controller-void-return.php'], [ + [ + 'Controller method index() must return Response object, not void.', + 10, + $tip, + ], + ]); + } + + public function testControllerWithProperReturn(): void + { + $this->analyse([__DIR__ . '/data/controller-proper-return.php'], []); + } +} diff --git a/tests/Rules/Module/NoLegacyResponseMethodsRuleTest.php b/tests/Rules/Module/NoLegacyResponseMethodsRuleTest.php new file mode 100644 index 0000000..f06c5d5 --- /dev/null +++ b/tests/Rules/Module/NoLegacyResponseMethodsRuleTest.php @@ -0,0 +1,66 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Module; + +use OpenCoreEMR\PHPStan\Rules\Module\NoLegacyResponseMethodsRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class NoLegacyResponseMethodsRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new NoLegacyResponseMethodsRule(); + } + + public function testLegacyResponseMethodsInController(): void + { + $tip = 'Controllers should return Response objects'; + $echoTip = 'Return new Response($content) or use JsonResponse'; + $this->analyse([__DIR__ . '/data/legacy-response-in-controller.php'], [ + [ + 'Function header() is forbidden in controllers. Use RedirectResponse, Response, or JsonResponse.', + 12, + $tip, + ], + [ + 'Function http_response_code() is forbidden in controllers. Use Response constructor with status code.', + 17, + $tip, + ], + [ + 'die/exit is forbidden in controllers. Throw an exception instead.', + 22, + $tip, + ], + [ + 'die/exit is forbidden in controllers. Throw an exception instead.', + 27, + $tip, + ], + [ + 'Direct echo is forbidden in controllers. Use Response objects or Twig templates.', + 32, + $echoTip, + ], + ]); + } + + public function testLegacyResponseMethodsOutsideController(): void + { + $this->analyse([__DIR__ . '/data/legacy-response-outside-controller.php'], []); + } +} diff --git a/tests/Rules/Module/NoSuperGlobalsInControllersRuleTest.php b/tests/Rules/Module/NoSuperGlobalsInControllersRuleTest.php new file mode 100644 index 0000000..4dfd364 --- /dev/null +++ b/tests/Rules/Module/NoSuperGlobalsInControllersRuleTest.php @@ -0,0 +1,60 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Module; + +use OpenCoreEMR\PHPStan\Rules\Module\NoSuperGlobalsInControllersRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class NoSuperGlobalsInControllersRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new NoSuperGlobalsInControllersRule(); + } + + public function testSuperGlobalsInController(): void + { + $tip = 'Use Symfony Request object: Request::createFromGlobals()'; + $this->analyse([__DIR__ . '/data/superglobals-in-controller.php'], [ + [ + 'Direct access to $_GET is forbidden in controllers. Use $request->query->get() instead.', + 12, + $tip, + ], + [ + 'Direct access to $_POST is forbidden in controllers. Use $request->request->get() instead.', + 13, + $tip, + ], + [ + 'Direct access to $_FILES is forbidden in controllers. Use $request->files->get() instead.', + 14, + $tip, + ], + [ + 'Direct access to $_SERVER is forbidden in controllers. Use $request->server->get() instead.', + 15, + $tip, + ], + ]); + } + + public function testSuperGlobalsOutsideController(): void + { + $this->analyse([__DIR__ . '/data/superglobals-outside-controller.php'], []); + } +} diff --git a/tests/Rules/Module/data/controller-no-return-type.php b/tests/Rules/Module/data/controller-no-return-type.php new file mode 100644 index 0000000..569af7f --- /dev/null +++ b/tests/Rules/Module/data/controller-no-return-type.php @@ -0,0 +1,14 @@ + 'ok']); + } + + /** + * Private methods are exempt from this rule. + */ + private function helper(): void + { + // Private methods can return void + } + + /** + * Magic methods are exempt from this rule. + */ + public function __construct() + { + // Constructor is exempt + } +} + +/** + * Non-controller class - no restrictions. + */ +class SomeService +{ + public function doSomething(): void + { + // Non-controllers can return void + } +} diff --git a/tests/Rules/Module/data/controller-void-return.php b/tests/Rules/Module/data/controller-void-return.php new file mode 100644 index 0000000..0d92a5b --- /dev/null +++ b/tests/Rules/Module/data/controller-void-return.php @@ -0,0 +1,14 @@ + 'ok']); + } +} + +/** + * Another non-controller class. + */ +class ErrorHandler +{ + public function handleFatal(): void + { + die('Fatal error'); + } +} diff --git a/tests/Rules/Module/data/superglobals-in-controller.php b/tests/Rules/Module/data/superglobals-in-controller.php new file mode 100644 index 0000000..1101322 --- /dev/null +++ b/tests/Rules/Module/data/superglobals-in-controller.php @@ -0,0 +1,17 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Testing; + +use OpenCoreEMR\PHPStan\Rules\Testing\NoCoversAnnotationOnClassRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class NoCoversAnnotationOnClassRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new NoCoversAnnotationOnClassRule(); + } + + public function testCoversAnnotationOnClass(): void + { + $this->analyse([__DIR__ . '/data/covers-on-class.php'], [ + [ + 'Please do not use the @covers annotation. It excludes transitively used code from coverage reports, resulting in incomplete coverage information.', + 10, + ], + ]); + } + + public function testNoCoversAnnotationOnClass(): void + { + $this->analyse([__DIR__ . '/data/no-covers-on-class.php'], []); + } +} diff --git a/tests/Rules/Testing/NoCoversAnnotationRuleTest.php b/tests/Rules/Testing/NoCoversAnnotationRuleTest.php new file mode 100644 index 0000000..c5006a9 --- /dev/null +++ b/tests/Rules/Testing/NoCoversAnnotationRuleTest.php @@ -0,0 +1,43 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +namespace OpenCoreEMR\PHPStan\Tests\Rules\Testing; + +use OpenCoreEMR\PHPStan\Rules\Testing\NoCoversAnnotationRule; +use PHPStan\Rules\Rule; +use PHPStan\Testing\RuleTestCase; + +/** + * @extends RuleTestCase + */ +class NoCoversAnnotationRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new NoCoversAnnotationRule(); + } + + public function testCoversAnnotationOnMethod(): void + { + $this->analyse([__DIR__ . '/data/covers-on-method.php'], [ + [ + 'Please do not use the @covers annotation. It excludes transitively used code from coverage reports, resulting in incomplete coverage information.', + 12, + ], + ]); + } + + public function testNoCoversAnnotation(): void + { + $this->analyse([__DIR__ . '/data/no-covers-annotation.php'], []); + } +} diff --git a/tests/Rules/Testing/data/covers-on-class.php b/tests/Rules/Testing/data/covers-on-class.php new file mode 100644 index 0000000..9be7a1b --- /dev/null +++ b/tests/Rules/Testing/data/covers-on-class.php @@ -0,0 +1,16 @@ + + * @copyright Copyright (c) 2026 OpenCoreEMR Inc + * @license https://github.com/openCoreEMR/openemr-phpstan-rules/blob/main/LICENSE GNU General Public License 3 + */ + +// Load Composer autoloader require_once __DIR__ . '/../vendor/autoload.php'; + +// Suppress error_log output during tests +ini_set('error_log', '/dev/null'); diff --git a/tests/phpstan-tests.neon b/tests/phpstan-tests.neon new file mode 100644 index 0000000..af04174 --- /dev/null +++ b/tests/phpstan-tests.neon @@ -0,0 +1,4 @@ +parameters: + tipsOfTheDay: false + scanDirectories: + - %currentWorkingDirectory%/tests/Rules