coenjacobs / mozart

Developers tool for WordPress plugins: Wraps all your projects dependencies in your own namespace, in order to prevent conflicts with other plugins loading the same dependencies in different versions.
https://coenjacobs.me/projects/mozart/
MIT License
425 stars 53 forks source link

Bug: Prefixing function argument type which matches namespace #124

Open BrianHenryIE opened 3 years ago

BrianHenryIE commented 3 years ago

I'm using Mpdf and its namespace and main class name are the same.

With:

{
  "require": {
    "mpdf/mpdf": "8.0.10"
  },
  "minimum-stability": "dev",
  "require-dev": {
    "coenjacobs/mozart": "dev-master#3b1243ca8505fa6436569800dc34269178930f39"
  },
  "extra": {
    "mozart": {
      "dep_namespace": "Mozart\\",
      "dep_directory": "/mozart/dep/",
      "classmap_directory": "/mozart/classes/",
      "classmap_prefix": "Mozart_"
    }
  }
}

In mozart/dep/Mpdf/ServiceFactory.php:36:

class ServiceFactory
{

    public function getServices(
        Mozart\Mpdf $mpdf,
        LoggerInterface $logger,
        $config,
        $restrictColorSpace,
...

We're in the correct namespace already so the prefixing there is unnecessary (and PHP crashes when the code is run).

I mentioned this in a comment on #84 but reverting #84 would introduce its own problem.

Any advice on what negative/positive look-ahead/look-behind might help would be great. It is a valid place to do a replacement.

Failing test for NamespaceReplacerTest.php:

/**
 * @test
 */
public function it_doesnt_prefix_function_types_that_happen_to_match_the_namespace() {

    $namespace = 'Mpdf';
    $prefix = "Mozart\\";

    $replacer = self::createReplacer($namespace, $prefix);

    $contents = 'public function getServices( Mpdf $mpdf, LoggerInterface $logger, $config, )';
    $expected = 'public function getServices( Mpdf $mpdf, LoggerInterface $logger, $config, )';

    $this->assertEquals($expected, $replacer->replace($contents));
}
BrianHenryIE commented 3 years ago

Fixed in #125

adrianstaffen commented 3 years ago

Hi @BrianHenryIE, I'm using Mozart 0.7.1 and also having issues using it with mPDF: Fatal error: Uncaught Error: Argument 1 passed to Dep\Mpdf\ServiceFactory::getServices() must be an instance of Dep\Mpdf\Dep\Mpdf, instance of Dep\Mpdf\Mpdf given, called in /Plugin/mozart/dependencies/Mpdf/Mpdf.php on line 1063 in /Plugin/mozart/dependencies/Mpdf/ServiceFactory.php on line 30

I tried applying the changes of the commits 26e4f9d and 29ccf74 in #125 to the file NamespaceReplacer.php but this is causing another issue: PHP Warning: require(/Plugin/mozart/dependencies/Mpdf/../data/upperCase.php): failed to open stream: No such file or directory in /Plugin/mozart/dependencies/Mpdf/Mpdf.php on line 1117 This file only exists in vendor/mpdf/mpdf/data and is not copied to the mozart/dependencies directory.

I appreciate any help!

BrianHenryIE commented 3 years ago

You need a couple of lines in your composer post-install-cmd script to copy the files in. Something like:

"mkdir -p ./Plugin/mozart/dependencies/Mpdf/data; cp -R vendor/mpdf/mpdf/data ./Plugin/mozart/dependencies/Mpdf/",
"mkdir -p ./Plugin/mozart/dependencies/Mpdf/ttfonts; cp -R fonts/*.ttf ./Plugin/mozart/dependencies/Mpdf/"

I've stopped using Mozart though. I forked it to https://github.com/BrianHenryIE/strauss. I think there are further MPDF issues that I needed to fix.