From 92c7677c6e61072cf4f728858c1e8444f5e3db55 Mon Sep 17 00:00:00 2001 From: nuno maduro Date: Tue, 4 Aug 2026 20:46:21 +0100 Subject: [PATCH] fix: don't allow invalid class names --- overrides/Runner/TestSuiteLoader.php | 4 +- src/Exceptions/AfterAllAlreadyExist.php | 2 +- src/Exceptions/AfterAllWithinDescribe.php | 2 +- src/Exceptions/AfterBeforeTestFunction.php | 2 +- src/Exceptions/BeforeAllAlreadyExist.php | 2 +- src/Exceptions/BeforeAllWithinDescribe.php | 2 +- src/Exceptions/DatasetAlreadyExists.php | 2 +- src/Exceptions/DatasetDoesNotExist.php | 2 +- src/Exceptions/FileOrFolderNotFound.php | 2 +- src/Exceptions/InvalidTestClassName.php | 41 ++++++++++++++ src/Exceptions/TestAlreadyExist.php | 2 +- .../TestCaseClassOrTraitNotFound.php | 2 +- src/Exceptions/TestDescriptionMissing.php | 2 +- src/Factories/TestCaseFactory.php | 11 ++++ src/Support/Str.php | 56 +++++++++++++++++++ tests/Features/DatasetProviderErrors.php | 4 +- tests/Features/DatasetsTests.php | 4 +- .../Suites/InvalidNames/2fa/login.test.php | 5 ++ .../ReservedKeyword/list.test.php | 5 ++ .../InvalidNames/ReservedType/int.test.php | 5 ++ .../StartsWithNumber/2fa.test.php | 5 ++ tests/Unit/TestSuite.php | 2 +- tests/Visual/Parallel.php | 2 +- 23 files changed, 147 insertions(+), 19 deletions(-) create mode 100644 src/Exceptions/InvalidTestClassName.php create mode 100644 tests/Fixtures/Suites/InvalidNames/2fa/login.test.php create mode 100644 tests/Fixtures/Suites/InvalidNames/ReservedKeyword/list.test.php create mode 100644 tests/Fixtures/Suites/InvalidNames/ReservedType/int.test.php create mode 100644 tests/Fixtures/Suites/InvalidNames/StartsWithNumber/2fa.test.php diff --git a/overrides/Runner/TestSuiteLoader.php b/overrides/Runner/TestSuiteLoader.php index 9006d08c..5fbde4e9 100644 --- a/overrides/Runner/TestSuiteLoader.php +++ b/overrides/Runner/TestSuiteLoader.php @@ -90,11 +90,11 @@ final class TestSuiteLoader (static function () use ($suiteClassFile) { try { include_once $suiteClassFile; + + TestSuite::getInstance()->tests->makeIfNeeded($suiteClassFile); } catch (Throwable $e) { Panic::with($e); } - - TestSuite::getInstance()->tests->makeIfNeeded($suiteClassFile); })(); $loadedClasses = array_values( diff --git a/src/Exceptions/AfterAllAlreadyExist.php b/src/Exceptions/AfterAllAlreadyExist.php index 5aea0c55..74d702bd 100644 --- a/src/Exceptions/AfterAllAlreadyExist.php +++ b/src/Exceptions/AfterAllAlreadyExist.php @@ -19,6 +19,6 @@ final class AfterAllAlreadyExist extends InvalidArgumentException implements Exc */ public function __construct(string $filename) { - parent::__construct(sprintf('The afterAll already exists in the filename `%s`.', $filename)); + parent::__construct(sprintf('The afterAll already exists in the filename [%s].', $filename)); } } diff --git a/src/Exceptions/AfterAllWithinDescribe.php b/src/Exceptions/AfterAllWithinDescribe.php index 4cdf2c2b..d7b58833 100644 --- a/src/Exceptions/AfterAllWithinDescribe.php +++ b/src/Exceptions/AfterAllWithinDescribe.php @@ -19,6 +19,6 @@ final class AfterAllWithinDescribe extends InvalidArgumentException implements E */ public function __construct(string $filename) { - parent::__construct(sprintf('The afterAll method can not be used within describe functions. Filename `%s`.', $filename)); + parent::__construct(sprintf('The afterAll method can not be used within describe functions. Filename [%s].', $filename)); } } diff --git a/src/Exceptions/AfterBeforeTestFunction.php b/src/Exceptions/AfterBeforeTestFunction.php index 7d18f701..019d4795 100644 --- a/src/Exceptions/AfterBeforeTestFunction.php +++ b/src/Exceptions/AfterBeforeTestFunction.php @@ -19,6 +19,6 @@ final class AfterBeforeTestFunction extends InvalidArgumentException implements */ public function __construct(string $filename) { - parent::__construct('After method cannot be used with before the [test|it] functions in the filename `['.$filename.']`.'); + parent::__construct('After method cannot be used with before the [test|it] functions in the filename ['.$filename.'].'); } } diff --git a/src/Exceptions/BeforeAllAlreadyExist.php b/src/Exceptions/BeforeAllAlreadyExist.php index a21389fe..baf1d1c2 100644 --- a/src/Exceptions/BeforeAllAlreadyExist.php +++ b/src/Exceptions/BeforeAllAlreadyExist.php @@ -19,6 +19,6 @@ final class BeforeAllAlreadyExist extends InvalidArgumentException implements Ex */ public function __construct(string $filename) { - parent::__construct(sprintf('The beforeAll already exists in the filename `%s`.', $filename)); + parent::__construct(sprintf('The beforeAll already exists in the filename [%s].', $filename)); } } diff --git a/src/Exceptions/BeforeAllWithinDescribe.php b/src/Exceptions/BeforeAllWithinDescribe.php index 31845284..8e4bb28b 100644 --- a/src/Exceptions/BeforeAllWithinDescribe.php +++ b/src/Exceptions/BeforeAllWithinDescribe.php @@ -19,6 +19,6 @@ final class BeforeAllWithinDescribe extends InvalidArgumentException implements */ public function __construct(string $filename) { - parent::__construct(sprintf('The beforeAll method can not be used within describe functions. Filename `%s`.', $filename)); + parent::__construct(sprintf('The beforeAll method can not be used within describe functions. Filename [%s].', $filename)); } } diff --git a/src/Exceptions/DatasetAlreadyExists.php b/src/Exceptions/DatasetAlreadyExists.php index bef2753d..ca57cd85 100644 --- a/src/Exceptions/DatasetAlreadyExists.php +++ b/src/Exceptions/DatasetAlreadyExists.php @@ -19,6 +19,6 @@ final class DatasetAlreadyExists extends InvalidArgumentException implements Exc */ public function __construct(string $name, string $scope) { - parent::__construct(sprintf('A dataset with the name `%s` already exists in scope [%s].', $name, $scope)); + parent::__construct(sprintf('A dataset with the name [%s] already exists in scope [%s].', $name, $scope)); } } diff --git a/src/Exceptions/DatasetDoesNotExist.php b/src/Exceptions/DatasetDoesNotExist.php index 72126fb8..fa8128c6 100644 --- a/src/Exceptions/DatasetDoesNotExist.php +++ b/src/Exceptions/DatasetDoesNotExist.php @@ -19,6 +19,6 @@ final class DatasetDoesNotExist extends InvalidArgumentException implements Exce */ public function __construct(string $name) { - parent::__construct(sprintf("A dataset with the name `%s` does not exist. You can create it using `dataset('%s', ['a', 'b']);`.", $name, $name)); + parent::__construct(sprintf("A dataset with the name [%s] does not exist. You can create it using `dataset('%s', ['a', 'b']);`.", $name, $name)); } } diff --git a/src/Exceptions/FileOrFolderNotFound.php b/src/Exceptions/FileOrFolderNotFound.php index 8e6a08d6..28735642 100644 --- a/src/Exceptions/FileOrFolderNotFound.php +++ b/src/Exceptions/FileOrFolderNotFound.php @@ -19,6 +19,6 @@ final class FileOrFolderNotFound extends InvalidArgumentException implements Exc */ public function __construct(string $filename) { - parent::__construct(sprintf('The file or folder with the name `%s` could not be found.', $filename)); + parent::__construct(sprintf('The file or folder with the name [%s] could not be found.', $filename)); } } diff --git a/src/Exceptions/InvalidTestClassName.php b/src/Exceptions/InvalidTestClassName.php new file mode 100644 index 00000000..319ffbd9 --- /dev/null +++ b/src/Exceptions/InvalidTestClassName.php @@ -0,0 +1,41 @@ +filename, $className); + } + + if ($this->namespace === null) { + foreach ($partsFQN as $partFQN) { + if (! Str::isValidIdentifier($partFQN)) { + throw InvalidTestClassName::fromNamespace($this->filename, $namespace, $partFQN); + } + } } $this->attributes = [ diff --git a/src/Support/Str.php b/src/Support/Str.php index 51cb6cc2..335432ee 100644 --- a/src/Support/Str.php +++ b/src/Support/Str.php @@ -17,6 +17,33 @@ final class Str private const string PREFIX = '__pest_evaluable_'; + /** + * The list of names PHP reserves, and therefore refuses, as class names. + * + * @see https://github.com/php/php-src/blob/master/Zend/zend_compile.c + * + * @var array + */ + private const array RESERVED_CLASS_NAMES = [ + 'array', + 'bool', + 'callable', + 'false', + 'float', + 'int', + 'iterable', + 'mixed', + 'never', + 'null', + 'object', + 'parent', + 'self', + 'static', + 'string', + 'true', + 'void', + ]; + /** * Create a (unsecure & non-cryptographically safe) random alpha-numeric * string value. @@ -64,6 +91,35 @@ final class Str return (string) preg_replace('/[^a-zA-Z0-9_\x80-\xff]/', '_', $code); } + /** + * Determine if the given name is a valid PHP identifier, and therefore may + * be used as a single namespace name. + */ + public static function isValidIdentifier(string $name): bool + { + return preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $name) === 1; + } + + /** + * Determine if the given name may be declared as a class name by an `eval`. + */ + public static function isValidClassName(string $name): bool + { + if (! self::isValidIdentifier($name)) { + return false; + } + + if (in_array(strtolower($name), self::RESERVED_CLASS_NAMES, true)) { + return false; + } + + $tokens = token_get_all(sprintf('toContain("A dataset with the name `missing` does not exist. You can create it using `dataset('missing', ['a', 'b']);`.") + ->toContain("A dataset with the name [missing] does not exist. You can create it using `dataset('missing', ['a', 'b']);`.") ->toContain('FAILED') ->toContain('Tests: 1 failed') ->and($result['code'])->not->toBe(0); @@ -31,7 +31,7 @@ test('reports missing datasets as errors alongside passing tests', function () u $result = $run('tests/Fixtures/Suites/IssueWithPassing.php'); expect($result['output']) - ->toContain("A dataset with the name `missing` does not exist. You can create it using `dataset('missing', ['a', 'b']);`.") + ->toContain("A dataset with the name [missing] does not exist. You can create it using `dataset('missing', ['a', 'b']);`.") ->toContain('1 passed') ->toContain('1 failed') ->and($result['code'])->not->toBe(0); diff --git a/tests/Features/DatasetsTests.php b/tests/Features/DatasetsTests.php index b7ac4811..3cf345a9 100644 --- a/tests/Features/DatasetsTests.php +++ b/tests/Features/DatasetsTests.php @@ -10,12 +10,12 @@ beforeEach(function (): void { }); it('throws exception if dataset does not exist', function (): void { - expect(fn () => DatasetsRepository::resolve(['first'], __FILE__))->toThrow(DatasetDoesNotExist::class, "A dataset with the name `first` does not exist. You can create it using `dataset('first', ['a', 'b']);`."); + expect(fn () => DatasetsRepository::resolve(['first'], __FILE__))->toThrow(DatasetDoesNotExist::class, "A dataset with the name [first] does not exist. You can create it using `dataset('first', ['a', 'b']);`."); }); it('throws exception if dataset already exist', function (): void { DatasetsRepository::set('second', [[]], __DIR__); - expect(fn () => DatasetsRepository::set('second', [[]], __DIR__))->toThrow(DatasetAlreadyExists::class, 'A dataset with the name `second` already exists in scope ['.__DIR__.'].'); + expect(fn () => DatasetsRepository::set('second', [[]], __DIR__))->toThrow(DatasetAlreadyExists::class, 'A dataset with the name [second] already exists in scope ['.__DIR__.'].'); }); it('sets closures', function (): void { diff --git a/tests/Fixtures/Suites/InvalidNames/2fa/login.test.php b/tests/Fixtures/Suites/InvalidNames/2fa/login.test.php new file mode 100644 index 00000000..f1d331eb --- /dev/null +++ b/tests/Fixtures/Suites/InvalidNames/2fa/login.test.php @@ -0,0 +1,5 @@ +assertTrue(true); diff --git a/tests/Fixtures/Suites/InvalidNames/ReservedKeyword/list.test.php b/tests/Fixtures/Suites/InvalidNames/ReservedKeyword/list.test.php new file mode 100644 index 00000000..a7747de1 --- /dev/null +++ b/tests/Fixtures/Suites/InvalidNames/ReservedKeyword/list.test.php @@ -0,0 +1,5 @@ +assertTrue(true); diff --git a/tests/Fixtures/Suites/InvalidNames/ReservedType/int.test.php b/tests/Fixtures/Suites/InvalidNames/ReservedType/int.test.php new file mode 100644 index 00000000..419a038a --- /dev/null +++ b/tests/Fixtures/Suites/InvalidNames/ReservedType/int.test.php @@ -0,0 +1,5 @@ +assertTrue(true); diff --git a/tests/Fixtures/Suites/InvalidNames/StartsWithNumber/2fa.test.php b/tests/Fixtures/Suites/InvalidNames/StartsWithNumber/2fa.test.php new file mode 100644 index 00000000..463d4aec --- /dev/null +++ b/tests/Fixtures/Suites/InvalidNames/StartsWithNumber/2fa.test.php @@ -0,0 +1,5 @@ +assertTrue(true); diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index 407ada1d..fe751465 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -15,7 +15,7 @@ it('does not allow to add the same test description twice', function (): void { $testSuite->tests->set($method); })->throws( TestAlreadyExist::class, - sprintf('A test with the description `%s` already exists in the filename `%s`.', 'bar', 'foo'), + sprintf('A test with the description [%s] already exists in the filename [%s].', 'bar', 'foo'), ); it('does not allow static closures', function (): void { diff --git a/tests/Visual/Parallel.php b/tests/Visual/Parallel.php index 16617ef3..3d7ddc4f 100644 --- a/tests/Visual/Parallel.php +++ b/tests/Visual/Parallel.php @@ -43,7 +43,7 @@ test('a parallel test can extend another test with same name', function () use ( test('parallel reports invalid datasets as failures', function () use ($run): void { expect($run('tests/Fixtures/Suites/ParallelInvalidDataset')) - ->toContain("A dataset with the name `missing.dataset` does not exist. You can create it using `dataset('missing.dataset', ['a', 'b']);`.") + ->toContain("A dataset with the name [missing.dataset] does not exist. You can create it using `dataset('missing.dataset', ['a', 'b']);`.") ->toContain('Tests: 1 failed, 1 passed (1 assertions)') ->toContain('Parallel: 3 processes'); })->skipOnWindows();