Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Prefixing function argument type which matches namespace #124

Open
BrianHenryIE opened this issue Feb 17, 2021 · 3 comments
Open

Bug: Prefixing function argument type which matches namespace #124

BrianHenryIE opened this issue Feb 17, 2021 · 3 comments

Comments

@BrianHenryIE
Copy link
Contributor

BrianHenryIE commented Feb 17, 2021

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 referenced this issue in BrianHenryIE/strauss Feb 18, 2021
Instead of looking for characters: \ | ; and space, it now looks for phrases ";", "\", "|" and " as"

Fix for #124
@BrianHenryIE
Copy link
Contributor Author

Fixed in #125

@adrianstaffen
Copy link

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants