Skip to content

Correctly determine Type of Node when PHP-Parser's namespaces are prefixed#441

Closed
maks-rafalko wants to merge 1 commit intonikic:masterfrom
maks-rafalko:patch-2
Closed

Correctly determine Type of Node when PHP-Parser's namespaces are prefixed#441
maks-rafalko wants to merge 1 commit intonikic:masterfrom
maks-rafalko:patch-2

Conversation

@maks-rafalko
Copy link
Copy Markdown
Contributor

@maks-rafalko maks-rafalko commented Nov 11, 2017

Hi there,

I'm working on mutation testing framework (Infection) that is distributed as a PHAR. One of its goal is to run target project's test suite against mutated code. Since we use reflection and load project's autoloader, we want to avoid potential conflicts between vendor files of Infection itself and the target project.

To avoid this issue, there is a project called PHP-Scoper. What it does is it prefixes all the namespaces of the library (including vendor folder) with some character(s), for example namespace Infection\Mutator\PublicVisibility is transformed to ScoperAbc123\Infection\Mutant\PublicVisibility.

But since it also prefixes vendor folder, PHP-Parser's classes are prefixed as well and NodeAbstract::getType() after this prefixing works incorrectly.

There is a hardcoded number 15 which means to remove 'PhpParser\Node' (length=15) substring from the FQCN.

Code:

// PHPParser\Node\Stmt\Declare_ -> Stmt_Declare

return strtr(substr(rtrim(get_class($this), '_'), 15), '\\', '_');

What I suggest is a little bit more dynamic solution, to correctly extract class name (type) from the prefixed FQCN:

ScoperAbc123\PHPParser\Node\Stmt\Declare_ -> Stmt_Declare

…fixed

Hi there,

I'm working on mutation testing framework ([Infection](https://github.com/infection/infection/)) that is distributed as a PHAR. One of this goal is to run target project's test suite against mutated code. Since we use reflection and load project's autoloader, we want to avoid potential conflicts between vendor files of Infection itself and the target project.

To avoid this issue, there is a project calld [PHP-Scoper](https://github.com/humbug/php-scoper). What it does is it prefixes all the namespaces of the library (including vendor folder) with some character(s), for example namespace `Infection\Mutator\PublicVisibility` is transformed to `ScoperAbc123\Infection\Mutant\PublicVisibility`. 

But since it also prefixes vendor folder, PHP-Parser's classes are prefixed as well and `NodeAbstract::getType()` after this prefixing works incorrectly.

There is a hardcoded number `15` which means to remove `'PhpParser\Node'` (length=15) substring from the FQCN.

Code:

```php
// PHPParser\Node\Stmt\Declare_ -> Stmt_Declare

return strtr(substr(rtrim(get_class($this), '_'), 15), '\\', '_');
```

What I suggest is a little be more dynamic solution, to correctly extract class name (type) from the ***prefixed*** FQCL:

`ScoperAbc123\PHPParser\Node\Stmt\Declare_` -> `Stmt_Declare`
@nikic
Copy link
Copy Markdown
Owner

nikic commented Nov 12, 2017

Merged bac91b4 into 3.x. For 4.0 I've generated explicit getType() methods instead: 1c11626

@nikic nikic closed this Nov 12, 2017
@maks-rafalko maks-rafalko deleted the patch-2 branch November 12, 2017 21:25
@maks-rafalko
Copy link
Copy Markdown
Contributor Author

maks-rafalko commented Nov 12, 2017

Great, thank you. Do you plan to release a new tag for 3.x?

@theofidry
Copy link
Copy Markdown
Contributor

@borNfreee you may also ran into the issue with the Lexer cf. https://github.com/humbug/php-scoper/blob/master/scoper.inc.php#L48. It would be great to fix it here at the core rather than those ugly brittle patches

nikic added a commit that referenced this pull request Nov 13, 2017
@nikic
Copy link
Copy Markdown
Owner

nikic commented Nov 13, 2017

@theofidry Done with 94ca9a7.

@theofidry
Copy link
Copy Markdown
Contributor

Awesome, thanks :)

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

Successfully merging this pull request may close these issues.

3 participants