Avoid usage of global functions

[alexander-schranz]

While I tried to use Twig inside scoped .phar I found out that twig registers function in the global namespace like here in the core extension:

Twig/src/Extension/CoreExtension.php

Line 305 in 237f178

namespace {

I think it would be great when also the twig functions live in a namespace this would make it possible to scope twig and avoid conflicts in global namespace and errors when scoping twig.

[fabpot]

All functions are prefixed with twig_, so there should be no conflicts.
In any case, that would be a BC break.

[alexander-schranz]

@fabpot I'm aware of the bc break it's more what could be rethinked when creating a new major version. The usecase as said is make it possible to scope twig away. So when using twig inside a phar application it does not conflict with the twig version used in the project. I'm currently using the box project and its php scoper but twig can currently not scoped away because of this global function requirements.

[clytras]

@alexander-schranz there is an amazing PHP tool you can use to namespace prefix all packages of any individual project that may conflict with others (like when building PHARs). PHP Scoper does an awesome job and you can also automate the whole process using composer scripts and hooks. I have used it to build Wordpress plugins and even Google uses it to prefix namespaces for their plugins as you can see here.

[alexander-schranz]

@clytras I'm coming here from using PHP Scoper 😄, linked the php scoper issue in the description. And as twig does not use namespaces for its function and declare function in the global namespace, it is not possible that php scoper can scope twig as twig requires that its function are in the global namespace. So it currently would not possible to scope twig away and application use another version of twig, then its global function conflicts.

[clytras]

@alexander-schranz oh I misread your posts here. That's interesting. I'm using Twig with PHP Scoper currently with no issues, but the truth is that I haven't encoutered an other plugin or code that also uses Twig yet to tell if it conflicts or not.

The only caviat I've noticed, is inside src/Node/ModuleNode.php:

Twig/src/Node/ModuleNode.php

Lines 141 to 155 in 393ea10

	if (!$this->getAttribute('index')) {
	$compiler
	->write("use Twig\Environment;\n")
	->write("use Twig\Error\LoaderError;\n")
	->write("use Twig\Error\RuntimeError;\n")
	->write("use Twig\Extension\SandboxExtension;\n")
	->write("use Twig\Markup;\n")
	->write("use Twig\Sandbox\SecurityError;\n")
	->write("use Twig\Sandbox\SecurityNotAllowedTagError;\n")
	->write("use Twig\Sandbox\SecurityNotAllowedFilterError;\n")
	->write("use Twig\Sandbox\SecurityNotAllowedFunctionError;\n")
	->write("use Twig\Source;\n")
	->write("use Twig\Template;\n\n")
	;
	}

where there are Twig namespace inside some strings for the generated cahced files and I'm currently having a composer post-install-cmd script to sed all these like:

"sed -i -e 's/use Twig\\\\/use MyNS\\\\\\\\Vendor\\\\\\\\Twig\\\\/g' vendor-patched/vendor/twig/twig/src/Node/ModuleNode.php"

[alexander-schranz]

@clytras are you compiling the templates into cache files? What scoper config do you use?

[clytras]

@alexander-schranz yes I'm using the cache for views even with auto_reload set to true and it works, like:

<?php

namespace MyNS\Templates;

use MyNS\Vendor\Twig\{
    Environment,
    Loader\FilesystemLoader,
};

...

public static function __construct()
    $this->loader = new FilesystemLoader();

    $this->loader->addPath(TEMPLATES_DIR, 'default');
    $this->loader->addPath(TEMPLATES_USER_DIR, 'user');

    $this->twig = new Environment($this->loader, [
        'cache' => TEMPLATES_CACHE_DIR,
        'auto_reload' => true,
    ]);
}

All the PHP Scoper auto-runs using composer scripts inside composer.json and I've chosen to have my own namespace name rather than auto-generated like:

"scripts": {
    "post-install-cmd": [
        "@prefix-dependencies"
    ],
    "post-update-cmd": [
        "@prefix-dependencies"
    ],
    "prefix-dependencies": [
        "php-scoper add-prefix --output-dir ./vendor-patched --force --quiet",
        "@composer dump-autoload --working-dir ./vendor-patched",
        "sed -i -e 's/use Twig\\\\/use MyNS\\\\\\\\Vendor\\\\\\\\Twig\\\\/g' vendor-patched/vendor/twig/twig/src/Node/ModuleNode.php"
    ]
}

and in scoper.inc.php:

return [
  'prefix' => 'MyNS\\Vendor',
  ...
  'exclude-namespaces' => [
    'MyNS',
    // 'Acme\Foo'                     // The Acme\Foo namespace (and sub-namespaces)
    // '~^PHPUnit\\\\Framework$~',    // The whole namespace PHPUnit\Framework (but not sub-namespaces)
    // '~^$~',                        // The root namespace only
    // '',                            // Any namespace
  ],
  ...
];

[alexander-schranz]

I'm additional loading allfiles to create the cache files in an init of my application like its done in symfony cache warmup:

$templateIterator = new \DirectoryIterator(dirname(__DIR__) . '/templates');
foreach ($templateIterator as $file) {
    if (str_ends_with($file->getFilename(), '.html.twig')) {
        $twig->load($file->getFilename());
    }
}

But run into problems with the built in filters when used then in the run from the cached files.

[alexander-schranz]

@clytras Do you have: // '~^$~', activated because this will not scope then the twig global filters. And that is what I'm also currently using but could make problems when the project has another twig version installed as used the scoped app.

[clytras]

@alexander-schranz no I don't have // '~^$~', exclude-namespaces activated. Every filter works inside the templates for me. It's all a matter of scoping, for classes and functions. If there are any functions or classes that PHP Scoper can't catch, like in the src/Node/ModuleNode.php I mentioned above, then those must be scoped manually. The tricky part with this approach is if you're using an auto generated namespace prefix, then you have to find a way to get that name each time PHP Scoper generates a new prefix and composer scripts must get it, so the sed commands can have it in the regexps. I'm using a fixed namespace prefix so I don't have to deal with a random prefix.

[alexander-schranz]

@clytras interesting how does the scoped version of this lines look for you:

Twig/src/Extension/CoreExtension.php

Lines 305 to 314 in 237f178

	namespace {
	use Twig\Environment;
	use Twig\Error\LoaderError;
	use Twig\Error\RuntimeError;
	use Twig\Extension\CoreExtension;
	use Twig\Extension\SandboxExtension;
	use Twig\Markup;
	use Twig\Source;
	use Twig\Template;
	use Twig\TemplateWrapper;

The tricky part with this approach is if you're using an auto generated namespace prefix,

You should configure a patcher instead of using sed in that case there you get the prefix and can work with it: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#patchers

[clytras]

@alexander-schranz Everything gets the namespace prefix inside CoreExtension.php like:

namespace MyNS\Vendor\Twig\Extension;

use MyNS\Vendor\Twig\ExpressionParser;
use MyNS\Vendor\Twig\Node\Expression\Binary\AddBinary;
use MyNS\Vendor\Twig\Node\Expression\Binary\AndBinary;
use MyNS\Vendor\Twig\Node\Expression\Binary\BitwiseAndBinary;
use MyNS\Vendor\Twig\Node\Expression\Binary\BitwiseOrBinary;
use MyNS\Vendor\Twig\Node\Expression\Binary\BitwiseXorBinary;

...

namespace MyNS\Vendor;

use MyNS\Vendor\Twig\Environment;
use MyNS\Vendor\Twig\Error\LoaderError;
use MyNS\Vendor\Twig\Error\RuntimeError;
use MyNS\Vendor\Twig\Extension\CoreExtension;
use MyNS\Vendor\Twig\Extension\SandboxExtension;
use MyNS\Vendor\Twig\Markup;
use MyNS\Vendor\Twig\Source;
use MyNS\Vendor\Twig\Template;
use MyNS\Vendor\Twig\TemplateWrapper;

and inside scoper-autoload.php I get all of the Twig function recreated from my namespace like:

if (!function_exists('twig_template_from_string')) {
    function twig_template_from_string() {
        return \MyNS\Vendor\twig_template_from_string(...func_get_args());
    }
}
if (!function_exists('twig_var_dump')) {
    function twig_var_dump() {
        return \MyNS\Vendor\twig_var_dump(...func_get_args());
    }
}
if (!function_exists('twig_cycle')) {
    function twig_cycle() {
        return \MyNS\Vendor\twig_cycle(...func_get_args());
    }
}

I know about the patcher. I just got the sed approach from another repo and I was too lazy to replace that with preg_replace after I read the documentation! I just don't know yet if the __NAMESPACE__ constant holds the auto-generated namespace there; it's on the documentation but it's not clear if that contains the actual prefixed namespace.

[alexander-schranz]

if (!function_exists('twig_var_dump')) {
    function twig_var_dump() {
        return \MyNS\Vendor\twig_var_dump(...func_get_args());
    }
}

That looks unexpected for me, this normally should only be done by the scoper if a configured to expose something, which I dont see in your configuration 🤔. Else the a already loaded twig version would be used, which was my problem.

[clytras]

That's a very clean environment. A stock Wordpress Docker container having just the plugin I'm developing and stock plugins, but again, this file is inside the vendor-patched directory that PHP Scoper produces each time it runs, so it has nothing to do with other plugins/code and it generates every time I install/uninstal a composer package.

Here is my entire scoper.inc.php:

<?php

declare(strict_types=1);

use Isolated\Symfony\Component\Finder\Finder;

// You can do your own things here, e.g. collecting symbols to expose dynamically
// or files to exclude.
// However beware that this file is executed by PHP-Scoper, hence if you are using
// the PHAR it will be loaded by the PHAR. So it is highly recommended to avoid
// to auto-load any code here: it can result in a conflict or even corrupt
// the PHP-Scoper analysis.

return [
  // The prefix configuration. If a non null value is be used, a random prefix
  // will be generated instead.
  //
  // For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#prefix
  'prefix' => 'MyNS\\Vendor',

  // By default when running php-scoper add-prefix, it will prefix all relevant code found in the current working
  // directory. You can however define which files should be scoped by defining a collection of Finders in the
  // following configuration key.
  //
  // This configuration entry is completely ignored when using Box.
  //
  // For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#finders-and-paths
  'finders' => [
    // Finder::create()->files()->in('lib'),
    Finder::create()
      ->files()
      ->ignoreVCS(true)
      ->notName('/LICENSE|.*\\.md|.*\\.dist|Makefile|composer\\.json|composer\\.lock/')
      ->exclude([
        'doc',
        'test',
        'test_old',
        'tests',
        'Tests',
        'vendor-bin',
      ])
      ->in('vendor'),
    Finder::create()->append([
      'composer.json',
    ]),
  ],

  // List of excluded files, i.e. files for which the content will be left untouched.
  // Paths are relative to the configuration file unless if they are already absolute
  //
  // For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#patchers
  'exclude-files' => [
    'src/a-whitelisted-file.php',
  ],

  // When scoping PHP files, there will be scenarios where some of the code being scoped indirectly references the
  // original namespace. These will include, for example, strings or string manipulations. PHP-Scoper has limited
  // support for prefixing such strings. To circumvent that, you can define patchers to manipulate the file to your
  // heart contents.
  //
  // For more see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#patchers
  'patchers' => [
    static function (string $filePath, string $prefix, string $contents): string {
      // Change the contents here.

      if (preg_match('/scssphp\/src\/.*?\\.php/', $filePath)) {
        // For comment definitions
        return preg_replace(
          '/( \\\\?)(ScssPhp\\\\)/',
          "$1{$prefix}\\\\$2",
          $contents
        );
      }

      return $contents;
    },
  ],

  // List of symbols to consider internal i.e. to leave untouched.
  //
  // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#excluded-symbols
  'exclude-namespaces' => [
    'MyNS',
    // 'Acme\Foo'                     // The Acme\Foo namespace (and sub-namespaces)
    // '~^PHPUnit\\\\Framework$~',    // The whole namespace PHPUnit\Framework (but not sub-namespaces)
    // '~^$~',                        // The root namespace only
    // '',                            // Any namespace
  ],
  'exclude-classes' => [
    // 'ReflectionClassConstant',
  ],
  'exclude-functions' => [
    // 'mb_str_split',
  ],
  'exclude-constants' => [
    // 'STDIN',
  ],

  // List of symbols to expose.
  //
  // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#exposed-symbols
  'expose-global-constants' => true,
  'expose-global-classes' => true,
  'expose-global-functions' => true,
  'expose-namespaces' => [
    // 'Acme\Foo'                     // The Acme\Foo namespace (and sub-namespaces)
    // '~^PHPUnit\\\\Framework$~',    // The whole namespace PHPUnit\Framework (but not sub-namespaces)
    // '~^$~',                        // The root namespace only
    // '',                            // Any namespace
  ],
  'expose-classes' => [],
  'expose-functions' => [],
  'expose-constants' => [],
];

and scoper-autoload.php has Twig functions everytime.

And of course there are some sed commads runnning on composer post-install-cmd that I need to move those replacements inside scoper patchers:

        "prefix-dependencies": [
            "php-scoper add-prefix --output-dir ./vendor-patched --force --quiet",
            "@composer dump-autoload --working-dir ./vendor-patched",
            "sed -i -e 's/use Twig\\\\/use MyNS\\\\\\\\Vendor\\\\\\\\Twig\\\\/g' vendor-patched/vendor/twig/twig/src/Node/ModuleNode.php"
        ]

[alexander-schranz]

Thx for sharing. I see, you are exposing global things via expose-global-* and that is what I wanted to avoid and why I created this issue.

In the generated templates the \MyNS\Vendor\twig_... functions are never called. They always will call the global twig_..., sure the scope autoloader will make sure they exist when using explode-global-function. But if they already there it will call unexpected twig version of that method, which could end in an error if there are breaking changes between the twig versions. Still twig is very stable and had never a lot of breaking changes so in most cases I think it will just work.

Really thank you for your insights, atleast we know at the end we need to expose the global function to get twig work, aslong as twig using global functions.

[clytras]

@alexander-schranz you're welcome.

I can now see your point and I agree with you, indeed there might be issues with these Twig global function when something else has included an other version of Twig. Though with some deeper thinking, I see that will be the case with all packages that expose global helper functions, not only with Twig, it's just that Twig is very popular and it's most likely for a conflict to occur.

I believe a solution from the PHP Scoper side dealing with all these kind of global functions would be more suitable to handle everything, because today is Twig but tomorrow there will be something else having the same issue.

[alexander-schranz]

@clytras definitely. But think it is today as I know very uncommon that a library is defining global function (Update: seems to have ignore Laravel completely here 🙈 ). I think php scoper is doing a good work currently as it scope global function into an own namespace, so it will not effect the application. This normally works great. But twig is here special case as it generates php files which the scoper can not take into its account. So it is a rare case only for libraries which generates php files which the php scoper can not take into account to scope global functions.

[rhelms]

Because those global functions are not namespaced, I'd be happy with an autoload.files entry in composer.json, so that those functions can be found by composer and static analysis tools like PHPStan.

However, because it's only for the benefit of static analysis, I can work around this by adding the file explicitly to my own autoload-dev.files entry.

[schakko]

Hi, I have the same problem with our WordPress plug-in. We use Strauss instead of PHP Scoper so that the Twig classes are packed into their own namespaces.
Unfortunately, Strauss cannot rewrite the global namespace in Twig's CoreExtension.php.

I could provide a simple patch for CoreExtension.php which would put each global function in its parent namespace - so make it working with Strauss, PHP Scope and keeping Twig's current functionality:

namespace {
    use <prefixed or vanilla namespaces...>;

// ...

if (!function_exists(__NAMESPACE__ . '\twig_cycle')) {
	function twig_cycle($values, $position)
	{
		// ...
	}
}

if (!function_exists(__NAMESPACE__ . '\twig_random')) {
	function twig_random(Environment $env, $values = null, $max = null)
	{
		// ...
	}
}

// ...

} // namespace

What do you guys think?

[schakko]

Hi, I have the same problem with our WordPress plug-in. We use Strauss instead of PHP Scoper so that the Twig classes are packed into their own namespaces. Unfortunately, Strauss cannot rewrite the global namespace in Twig's CoreExtension.php.

I could provide a simple patch for CoreExtension.php which would put each global function in its parent namespace - so make it working with Strauss, PHP Scope and keeping Twig's current functionality:

namespace {
    use <prefixed or vanilla namespaces...>;

// ...

if (!function_exists(__NAMESPACE__ . '\twig_cycle')) {
	function twig_cycle($values, $position)
	{
		// ...
	}
}

if (!function_exists(__NAMESPACE__ . '\twig_random')) {
	function twig_random(Environment $env, $values = null, $max = null)
	{
		// ...
	}
}

// ...

} // namespace

What do you guys think?

OK, that proposal does not work. I missed that the function arguments are type-hinted. In my case, I've written a simple patching utility which prepends each of twig_ functions with a specific namespace prefix.

[light-source]

Here is a workaround for Scoper for anyone who might be interested.

@fabpot Thank you for the fantastic Twig! It would be great if Twig v.4 wouldn't use global things, so it doesn't cause any issues while scoping.

[fabpot]

Closing as functions are gone in Twig 4.