diff --git a/README.md b/README.md index f1e3b0e0..2d4dd3e5 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ potentially very difficult to debug due to dissimilar or unsupported package ver - [Finders and paths](#finders-and-paths) - [Patchers](#patchers) - [Whitelist][whitelist] - - [Class Whitelisting](#class-whitelisting) + - [Class & Constant Whitelisting](#class-constant-whitelisting) - [Namespace Whitelisting](#namespace-whitelisting) - [Building A Scoped PHAR](#building-a-scoped-phar) - [With Box](#with-box) @@ -261,9 +261,9 @@ a PHPUnit PHAR with isolated code, you still want the PHAR to be able to understand the `PHPUnit\Framework\TestCase` class. -### Class whitelisting +### Class & Constant whitelisting -You can whitelist classes and interfaces like so: +You can whitelist classes, interfaces and constants like so like so: ```php [ 'PHPUnit\Framework\TestCase', + 'PHPUNIT_VERSION', ], ]; ``` -Note that only classes are whitelisted, this does not affect constants, -functions or traits. This whitelisting will actually not prevent the -scoping to operate, i.e. the class or interface will still be prefixed, -but a `class_alias()` statement will be registered pointing the prefixed -class to the non-prefixed one. +This will _not_ work on traits or functions. + +The class aliasing mechanism is done like follows: +- Prefix the class or interface as usual +- Append a `class_alias()` statement at the end of the class/interface declaration to link the prefixed symbol to the + non prefixed one +- Append a `class_exists()` statement right after the autoloader is registered to trigger the loading of the method + which will ensure the `class_alias()` statement is executed + +It is done this way to ensure prefixed and whitelisted classes can co-exist together without breaking the autoloading. + +The constant aliasing mechanism is done by transforming the constant declaration into a `define()` statement when this +is not already the case. Note that there is a difference here since `define()` defines a constant at runtime whereas +`const` defines it at compile time. You have a more details post regarding the differences +[here](https://stackoverflow.com/a/3193704/3902761) ### Namespace whitelisting diff --git a/specs/const/const-declaration.php b/specs/const/const-declaration.php new file mode 100644 index 00000000..c9e5786e --- /dev/null +++ b/specs/const/const-declaration.php @@ -0,0 +1,164 @@ +, + * Pádraic Brady + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +return [ + 'meta' => [ + 'title' => 'Global constant usage in the global scope', + // Default values. If not specified will be the one used + 'prefix' => 'Humbug', + 'whitelist' => [], + ], + + 'Constants declaration in the global namespace' => [ + 'payload' => <<<'PHP' + [ + 'whitelist' => ['*'], + 'payload' => <<<'PHP' + [ + 'whitelist' => ['FOO_CONST', 'BAR_CONST'], + 'payload' => <<<'PHP' + [ + 'payload' => <<<'PHP' + [ + 'whitelist' => ['Acme\*'], + 'payload' => <<<'PHP' + [ + 'whitelist' => ['Acme\FOO_CONST'], + 'payload' => <<<'PHP' + [], ], - // As it is extremely rare to use a `use const` statement for a built-in constant from the - // global scope, we can relatively safely assume it is a user-land declared constant which should - // be prefixed. - [ 'spec' => <<<'SPEC' Constant call imported with an aliased use statement: @@ -46,6 +42,31 @@ use const Humbug\DUMMY_CONST as FOO; \Humbug\DUMMY_CONST; +PHP + ], + + [ + 'spec' => <<<'SPEC' +Whitelisted constant call imported with an aliased use statement: +- add prefixed namespace +- transforms the call into a FQ call +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + [], ], - // As it is extremely rare to use a `use const` statement for a built-in constant from the - // global scope, we can relatively safely assume it is a user-land declared constant which should - // be prefixed. - [ 'spec' => <<<'SPEC' Constant call imported with a use statement: @@ -46,6 +42,31 @@ use const Humbug\DUMMY_CONST; \Humbug\DUMMY_CONST; +PHP + ], + + [ + 'spec' => <<<'SPEC' +Whitelisted constant call imported with a use statement: +- add prefixed namespace +- transforms the call into a FQ call +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' +Whitelisted constant call in the global namespace: +- add prefixed namespace +- transforms the call into a FQ call +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' -Constant call on an imported single-level namespace +Whitelisted onstant call on an imported single-level namespace - do not prefix the use statement (see tests related to single-level classes) -- prefix the constant call: the whitelist only works on classes - transform the call into a FQ call SPEC , @@ -140,11 +139,11 @@ class Foo } namespace Humbug\Foo; -const DUMMY_CONST = ''; +\define('Foo\\DUMMY_CONST', ''); namespace Humbug; use Humbug\Foo as A; -\Humbug\Foo\DUMMY_CONST; +\Foo\DUMMY_CONST; PHP ], diff --git a/specs/const/global-scope-single-part-namespaced-with-single-level-use.php b/specs/const/global-scope-single-part-namespaced-with-single-level-use.php index 70271302..773d4976 100644 --- a/specs/const/global-scope-single-part-namespaced-with-single-level-use.php +++ b/specs/const/global-scope-single-part-namespaced-with-single-level-use.php @@ -22,7 +22,8 @@ [ 'spec' => <<<'SPEC' -Constant call on an imported single-level namespace +Constant call on an imported single-level namespace: +- add prefixed namespace - do not prefix the use statement (see tests related to single-level classes) - prefix the constant call - transform the call into a FQ call @@ -94,7 +95,7 @@ class Foo [ 'spec' => <<<'SPEC' -Constant call on an imported single-level namespace +Whitelisted constant call on an imported single-level namespace - do not prefix the use statement (see tests related to single-level classes) - prefix the constant call: the whitelist only works on classes - transform the call into a FQ call @@ -127,11 +128,11 @@ class Foo } namespace Humbug\Foo; -const DUMMY_CONST = ''; +\define('Foo\\DUMMY_CONST', ''); namespace Humbug; use Humbug\Foo; -\Humbug\Foo\DUMMY_CONST; +\Foo\DUMMY_CONST; PHP ], diff --git a/specs/const/global-scope-single-part-namespaced.php b/specs/const/global-scope-single-part-namespaced.php index 3ddef878..42805f57 100644 --- a/specs/const/global-scope-single-part-namespaced.php +++ b/specs/const/global-scope-single-part-namespaced.php @@ -64,7 +64,7 @@ [ 'spec' => <<<'SPEC' Namespaced constant call on a whitelisted constant -- prefix the call: the whitelist only works for classes +- add prefixed namespace SPEC , 'whitelist' => ['PHPUnit\DUMMY_CONST'], @@ -77,7 +77,7 @@ namespace Humbug; -\Humbug\PHPUnit\DUMMY_CONST; +\PHPUnit\DUMMY_CONST; PHP ], diff --git a/specs/const/global-scope-two-parts-namespaced-with-single-level-use-and-alias.php b/specs/const/global-scope-two-parts-namespaced-with-single-level-use-and-alias.php index 4622034f..c38a6ad4 100644 --- a/specs/const/global-scope-two-parts-namespaced-with-single-level-use-and-alias.php +++ b/specs/const/global-scope-two-parts-namespaced-with-single-level-use-and-alias.php @@ -81,9 +81,9 @@ class Foo [ 'spec' => <<<'SPEC' -Namespaced constant call with namespace partially imported +Whitelisted namespaced constant call with namespace partially imported +- add prefixed namespace - do not prefix the use statement (cf. tests related to global classes) -- prefix the call - transform the call in a FQ call SPEC , @@ -105,7 +105,7 @@ class Foo { } use Humbug\Foo as A; -\Humbug\Foo\Bar\DUMMY_CONST; +\Foo\Bar\DUMMY_CONST; PHP ], diff --git a/specs/const/global-scope-two-parts-namespaced-with-single-level-use.php b/specs/const/global-scope-two-parts-namespaced-with-single-level-use.php index 8160220f..d0082007 100644 --- a/specs/const/global-scope-two-parts-namespaced-with-single-level-use.php +++ b/specs/const/global-scope-two-parts-namespaced-with-single-level-use.php @@ -81,9 +81,9 @@ class Foo [ 'spec' => <<<'SPEC' -Namespaced constant call with namespace partially imported +Whitelisted namespaced constant call with namespace partially imported +- add prefixed namespace - do not prefix the use statement (cf. tests related to global classes) -- prefix the call - transform the call in a FQ call SPEC , @@ -105,7 +105,7 @@ class Foo { } use Humbug\Foo; -\Humbug\Foo\Bar\DUMMY_CONST; +\Foo\Bar\DUMMY_CONST; PHP ], diff --git a/specs/const/global-scope-two-parts-namespaced.php b/specs/const/global-scope-two-parts-namespaced.php index 00a7c5ec..4aad6134 100644 --- a/specs/const/global-scope-two-parts-namespaced.php +++ b/specs/const/global-scope-two-parts-namespaced.php @@ -64,10 +64,10 @@ [ 'spec' => <<<'SPEC' Namespaced constant call on a whitelisted constant -- prefix the call: the whitelist only works for classes +- add prefixed namespace SPEC , - 'whitelist' => ['PHPUnit\DUMMY_CONST'], + 'whitelist' => ['PHPUnit\Command\DUMMY_CONST'], 'payload' => <<<'PHP' <<<'SPEC' +Whitelisted constant call imported with an aliased use statement: +- prefix the namespace +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' +Whitelisted FQ constant call imported with a use statement: +- prefix the namespace +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' +Whitelisted constant call in a namespace: +- prefix the namespace +- do nothing: the constant can either belong to the same namespace or the global namespace +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' +Whitelisted FQ constant call in a namespace: +- prefix the namespace +- prefix the constant call +SPEC + , + 'whitelist' => ['DUMMY_CONST'], + 'payload' => <<<'PHP' + <<<'SPEC' -Namespaced constant call on a whitelisted constant +Whitelisted namespaced constant call on a whitelisted constant - prefix the namespace - prefix the call: the whitelist only works for classes SPEC @@ -88,6 +88,30 @@ \Humbug\A\PHPUnit\DUMMY_CONST; +PHP + ], + + [ + 'spec' => <<<'SPEC' +Whitelisted FQ namespaced constant call on a whitelisted constant +- prefix the namespace +- prefix the call: the whitelist only works for classes +SPEC + , + 'whitelist' => ['PHPUnit\DUMMY_CONST'], + 'payload' => <<<'PHP' + ['X\Y'], + 'whitelist' => ['X\Y', 'BAR_CONST'], 'payload' => <<<'PHP' ['X\Y'], + 'whitelist' => ['X\Y', 'BAR_CONST'], 'payload' => <<<'PHP' <<<'SPEC' -Constant use statement for a namespaced constant which has been whitelisted: -- prefix the use statement: whitelist is only for classes -SPEC - , + 'Constant use statement for a namespaced constant which has been whitelisted: do nothing' => [ 'whitelist' => ['Foo\BAR'], 'payload' => <<<'PHP' <<<'SPEC' -Constant use statement for a namespaced constant which has been whitelisted: -- prefix the use statement: whitelist is only for classes -SPEC - , + 'Constant use statement for a namespaced constant which has been whitelisted: do nothing' => [ 'whitelist' => ['Foo\BAR'], 'payload' => <<<'PHP' , + * Pádraic Brady + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Humbug\PhpScoper\PhpParser\NodeVisitor; + +use Humbug\PhpScoper\PhpParser\NodeVisitor\Resolver\FullyQualifiedNameResolver; +use Humbug\PhpScoper\Whitelist; +use PhpParser\Node; +use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Name; +use PhpParser\Node\Name\FullyQualified; +use PhpParser\Node\Param; +use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Expression; +use PhpParser\NodeVisitorAbstract; + +/** + * Replaces const declaration by define. + * + * ``` + * const DUMMY_CONST = 'foo'; + * ``` + * + * => + * + * ``` + * define('DUMMY_CONST', 'foo'); + * ``` + * + * @private + */ +final class ConstStmtReplacer extends NodeVisitorAbstract +{ + private $whitelist; + private $nameResolver; + + public function __construct(Whitelist $whitelist, FullyQualifiedNameResolver $nameResolver) + { + $this->whitelist = $whitelist; + $this->nameResolver = $nameResolver; + } + + /** + * {@inheritdoc} + * + * @param Node\Stmt\Const_ $node + */ + public function enterNode(Node $node): Node + { + if (false === ($node instanceof Node\Stmt\Const_)) { + return $node; + } + + // TODO: check this: when can Node\Stmt\Const_ be empty or have more than one constant + /** @var Node\Const_ $constant */ + $constant = current($node->consts); + + $resolvedConstantName = $this->nameResolver->resolveName( + new Name( + (string) $constant->name, + $node->getAttributes() // Take the parent node attribute since no "parent" attribute is recorded in + // Node\Const_ + // TODO: check with nikic if this is expected + ) + )->getName(); + + if (false === $this->whitelist->isClassWhitelisted((string) $resolvedConstantName)) { + return $node; + } + + return new Expression( + new FuncCall( + new FullyQualified('define'), + [ + new String_((string) $resolvedConstantName), + $constant->value, + ] + ) + ); + } +} diff --git a/src/PhpParser/NodeVisitor/NameStmtPrefixer.php b/src/PhpParser/NodeVisitor/NameStmtPrefixer.php index 91445b9e..77802c17 100644 --- a/src/PhpParser/NodeVisitor/NameStmtPrefixer.php +++ b/src/PhpParser/NodeVisitor/NameStmtPrefixer.php @@ -158,13 +158,17 @@ private function prefixName(Name $name): Node } } - // Constants have a fallback autoloading so we cannot prefix them when the name is ambiguous - // See https://wiki.php.net/rfc/fallback-to-root-scope-deprecation if ($parentNode instanceof ConstFetch) { + if ($this->whitelist->isClassWhitelisted($resolvedName->toString())) { + return $resolvedName; + } + if ($this->reflector->isConstantInternal($resolvedName->toString())) { return new FullyQualified($resolvedName->toString(), $resolvedName->getAttributes()); } + // Constants have a fallback autoloading so we cannot prefix them when the name is ambiguous + // See https://wiki.php.net/rfc/fallback-to-root-scope-deprecation if (false === ($resolvedName instanceof FullyQualified)) { return $resolvedName; } diff --git a/src/PhpParser/NodeVisitor/UseStmt/UseStmtPrefixer.php b/src/PhpParser/NodeVisitor/UseStmt/UseStmtPrefixer.php index 272f8bf9..36fe5a19 100644 --- a/src/PhpParser/NodeVisitor/UseStmt/UseStmtPrefixer.php +++ b/src/PhpParser/NodeVisitor/UseStmt/UseStmtPrefixer.php @@ -72,7 +72,10 @@ private function shouldPrefixUseStmt(UseUse $use): bool } if (Use_::TYPE_CONSTANT === $useType) { - return false === $this->reflector->isConstantInternal((string) $use->name); + return + false === $this->whitelist->isClassWhitelisted((string) $use->name) + && false === $this->reflector->isConstantInternal((string) $use->name) + ; } return Use_::TYPE_NORMAL !== $useType || false === $this->reflector->isClassInternal((string) $use->name); diff --git a/src/PhpParser/TraverserFactory.php b/src/PhpParser/TraverserFactory.php index b5cfbaa7..d9002614 100644 --- a/src/PhpParser/TraverserFactory.php +++ b/src/PhpParser/TraverserFactory.php @@ -53,6 +53,7 @@ public function create(string $prefix, Whitelist $whitelist): NodeTraverserInter $traverser->addVisitor(new NodeVisitor\StringScalarPrefixer($prefix, $whitelist, $this->reflector)); $traverser->addVisitor(new NodeVisitor\WhitelistedClassAppender($whitelist)); + $traverser->addVisitor(new NodeVisitor\ConstStmtReplacer($whitelist, $nameResolver)); return $traverser; } diff --git a/src/Whitelist.php b/src/Whitelist.php index 8d5f3c92..238ef76c 100644 --- a/src/Whitelist.php +++ b/src/Whitelist.php @@ -16,6 +16,9 @@ use Countable; use InvalidArgumentException; +use function array_map; +use function array_merge; +use function array_unique; use function count; use function in_array; use function sprintf; @@ -55,7 +58,10 @@ public static function create(string ...$elements): self } } - return new self($classes, $namespaces); + return new self( + array_unique($classes), + array_unique($namespaces) + ); } /** @@ -91,4 +97,16 @@ public function count(): int { return count($this->classes) + count($this->namespaces); } + + public function toArray(): array + { + $namespaces = array_map( + function (string $namespace): string { + return '' === $namespace ? '*' : $namespace.'\*'; + }, + $this->namespaces + ); + + return array_merge($this->classes, $namespaces); + } } diff --git a/tests/Scoper/PhpScoperTest.php b/tests/Scoper/PhpScoperTest.php index f2c24a05..28747a1e 100644 --- a/tests/Scoper/PhpScoperTest.php +++ b/tests/Scoper/PhpScoperTest.php @@ -38,6 +38,8 @@ use Throwable; use function Humbug\PhpScoper\create_fake_patcher; use function Humbug\PhpScoper\create_parser; +use function implode; +use function sprintf; class PhpScoperTest extends TestCase { @@ -448,6 +450,11 @@ public function test_can_scope_valid_files(string $spec, string $contents, strin $actual = $this->scoper->scope($filePath, $contents, $prefix, $patchers, $whitelist); + $formatedWhitelist = 0 === count($whitelist) + ? '[]' + : sprintf('[%s]', implode(', ', $whitelist->toArray())) + ; + $titleSeparator = str_repeat( '=', min( @@ -467,6 +474,7 @@ public function test_can_scope_valid_files(string $spec, string $contents, strin $titleSeparator INPUT +whitelist: $formatedWhitelist $titleSeparator $contents @@ -497,6 +505,8 @@ public function provideValidFiles() $files = (new Finder())->files()->in(self::SPECS_PATH); } + $files->sortByName(); + foreach ($files as $file) { try { $fixtures = include $file; diff --git a/tests/WhitelistTest.php b/tests/WhitelistTest.php index f0e282b9..c0f00129 100644 --- a/tests/WhitelistTest.php +++ b/tests/WhitelistTest.php @@ -15,7 +15,6 @@ namespace Humbug\PhpScoper; use PHPUnit\Framework\TestCase; -use Reflection; use ReflectionClass; /** @@ -67,6 +66,16 @@ public function test_it_can_tell_if_a_namespace_is_whitelisted(Whitelist $whitel $this->assertSame($expected, $actual); } + /** + * @dataProvider provideWhitelistToConvert + */ + public function test_it_can_be_converted_back_into_an_array(Whitelist $whitelist, array $expected) + { + $actual = $whitelist->toArray(); + + $this->assertSame($expected, $actual); + } + public function provideWhitelists() { yield [[], [], []]; @@ -161,4 +170,52 @@ public function provideNamespaceWhitelists() true, ]; } + + public function provideWhitelistToConvert() + { + yield [ + Whitelist::create(), + [], + ]; + + yield [ + Whitelist::create('Acme\Foo'), + ['Acme\Foo'], + ]; + + yield [ + Whitelist::create('\Acme\Foo'), + ['Acme\Foo'], + ]; + + yield [ + Whitelist::create('Acme\Foo\*'), + ['Acme\Foo\*'], + ]; + + yield [ + Whitelist::create('\Acme\Foo\*'), + ['Acme\Foo\*'], + ]; + + yield [ + Whitelist::create('*'), + ['*'], + ]; + + yield [ + Whitelist::create('\*'), + ['*'], + ]; + + yield [ + Whitelist::create('Acme', 'Acme\Foo', 'Acme\Foo\*', '*'), + ['Acme', 'Acme\Foo', 'Acme\Foo\*', '*'], + ]; + + yield [ + Whitelist::create('Acme', 'Acme'), + ['Acme'], + ]; + } }