src-namespace-prefix causes crash

[jasonw4331]

It appears to fail when building psr-4 plugins. src-namespace-prefix remains apart of the plugin manifest while the folder structure is in psr-0, triggering an error on PM server startup

[SOF3]

pharynx avoids tampering with the plugin.yml as its main purpose is to process source files. It doesn't make sense that plugins use src-namespace-prefix if they intend to use pharynx.

That said, pharynx could do the job of removing that field to help users adopt the tool if it is to be used in amore foolproof context (e.g. bootstrap-plugin-dev).

[SOF3]

As for explicit PSR-4 support, it is a non goal of pharynx. It has no problem with whatever convention the input sources use as source files are split automatically, but its output is always PSR-0 because the input classes may not all be using the same namespace.

[dktapps]

pharynx avoids tampering with the plugin.yml as its main purpose is to process source files. It doesn't make sense that plugins use src-namespace-prefix if they intend to use pharynx.

It also doesn't make sense to change the file locations.

The only reason you find this problematic is because you want to put all files (including libraries) inside src. This is not industry standard and is only useful due to PocketMine-MP limitations. In all normal cases, one would simply include 'vendor/autoload.php'; and everything would work normally. Granted, a mechanism to make this more convenient in PM would be useful (perhaps an autoload-files or autoload-directories option in pocketmine.yml would help fill this need better).

I'll also point out that moving classes around is liable to break stuff like relative include paths.

[SOF3]

@dktapps This is not true. Unprocessed PHP files with multiple classes may fail to autoload even with classmap due to how PHP always loads classes in the order they are defined in a single file. Per-file autoloading is required to allow PHP to load files like this:

<?php // file.php
echo "breakpoint 1";
final class Foo implements Bar {}
echo "breakpoint 2";
interface Bar {}

using classmap autoloading, we will end up like this:

autoloader: loadClass("Foo");
require "file.php";
breakpoint 1
autoloader: loadClass("Bar");
require "file.php";
breakpoint 1
fatal: cannot redeclare class `Foo`

pharynx is required to split them to multiple files so that PHP can load in the correct order.

[SOF3]

I'll also point out that moving classes around is liable to break stuff like relative include paths.

Great point, but in the case of PocketMine plugins, we expect static assets to be shipped in the resources folder instead of relative to __FILE__ anyway.

[dktapps]

pharynx is required to split them to multiple files so that PHP can load in the correct order.

The autoloader won't be called the second time, as the class will already exist. And even if they didn't, you could just use require_once.

[SOF3]

@dktapps

1. the composer autoloader uses include not include_once.
2. the autoloader is called the second time, but for a different class in the same file, which causes the autoloader to try to include the same file again.

[SOF3]

The example code I showed above cannot be loaded no matter what, even if you don't use an autoloader and write include statements manually.

[dktapps]

Works just fine for me.

main.php:

<?php

spl_autoload_register(function(string $class) : bool{
        require './classes.php';
        return true;
});                                                               
new Foo();

classes.php:

<?php // file.php
echo "breakpoint 1";
final class Foo implements Bar {}
echo "breakpoint 2";
interface Bar {}

Output:

~ $ php main.php
breakpoint 1breakpoint 2~ $

PHP:

PHP 8.0.11 (cli) (built: Sep 30 2021 21:00:49) ( ZTS )
Copyright (c) The PHP Group
Zend Engine v4.0.11, Copyright (c) Zend Technologies

[dktapps]

Some technical specifics:

When a file is require'd, all of the classes are committed to compiler_globals in an unlinked state, before attempting to resolve references to other classes. References are resolved once everything in the file has been surface-level evaluated. This means that the Bar class already exists, just in an incomplete state, which means that the autoloader won't be invoked a second time for Bar.

[SOF3]

well, I reproduced a similar issue in InfoAPI just last week that I ended up resorting to running pharynx before phpunit. I will see if I can reproduce it again later.