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

PHP baseline contains 3 extra spaces per start of the line #8952

Closed
ruudk opened this issue Feb 23, 2023 · 7 comments · Fixed by phpstan/phpstan-src#2254
Closed

PHP baseline contains 3 extra spaces per start of the line #8952

ruudk opened this issue Feb 23, 2023 · 7 comments · Fixed by phpstan/phpstan-src#2254

Comments

@ruudk
Copy link
Contributor

ruudk commented Feb 23, 2023

Bug report

PHPStan 1.10.2

I just generated the baseline as PHP file and noticed that every line is prepended with 3 spaces.

Code snippet that reproduces the problem

<?php declare(strict_types = 1);

$ignoreErrors = [];
   $ignoreErrors[] = [
	'message' => '#^Cannot call method isGivenKind\\(\\) on PhpCsFixer\\\\Tokenizer\\\\Token\\|null\\.$#',
	'count' => 1,
	'path' => __DIR__ . '/src-dev/CodeStyle/SimpleBusCommandBusFixer.php',
];
   $ignoreErrors[] = [
	'message' => '#^Cannot call method isGivenKind\\(\\) on PhpCsFixer\\\\Tokenizer\\\\Token\\|null\\.$#',
	'count' => 1,
	'path' => __DIR__ . '/src-dev/CodeStyle/SimpleBusCommandSeeFixer.php',
];

Expected output

<?php declare(strict_types = 1);

$ignoreErrors = [];
$ignoreErrors[] = [
	'message' => '#^Cannot call method isGivenKind\\(\\) on PhpCsFixer\\\\Tokenizer\\\\Token\\|null\\.$#',
	'count' => 1,
	'path' => __DIR__ . '/src-dev/CodeStyle/SimpleBusCommandBusFixer.php',
];
$ignoreErrors[] = [
	'message' => '#^Cannot call method isGivenKind\\(\\) on PhpCsFixer\\\\Tokenizer\\\\Token\\|null\\.$#',
	'count' => 1,
	'path' => __DIR__ . '/src-dev/CodeStyle/SimpleBusCommandSeeFixer.php',
];

The weird thing is, this is not reflected in the phpstan-src:
https://github.com/phpstan/phpstan-src/blob/8260032bc6b3641ec80a4b11526869b53d250ac6/src/Command/ErrorFormatter/BaselinePhpErrorFormatter.php#L62-L69

But it is in the generated Phar file:

            foreach ($fileErrorsCounts as $message => $count) {
                $template = <<<'PHP'
   $ignoreErrors[] = [
	'message' => %s,
	'count' => %d,
	'path' => __DIR__ . %s,
];
PHP;

So something goes wrong with the conversion to Phar.

Did PHPStan help you today? Did it make you happy in any way?

As always, one of the best things that happened to the PHP ecosystem!

@mergeable
Copy link

mergeable bot commented Feb 23, 2023

This bug report is missing a link to reproduction at phpstan.org/try.

It will most likely be closed after manual review.

@ruudk ruudk changed the title PHP baseline contains 3 extra spaces per start of hte line PHP baseline contains 3 extra spaces per start of the line Feb 23, 2023
@ruudk
Copy link
Contributor Author

ruudk commented Feb 23, 2023

Also, instead of sprintf-ing , wouldn't it be faster to use simple string interpolation? Especially on a baseline that hits ten's of thousands of lines.

@ondrejmirtes
Copy link
Member

I'm not going to debug that 😊 In the transformation there's Box, PHP-Scoper, and Rector involved. Very complex pipeline. I'd say it doesn't matter...

About the sprintf - unless we can see a notable difference in a benchmark that's not something worth changing either.

@ruudk
Copy link
Contributor Author

ruudk commented Feb 23, 2023

Oke, the annoying thing is, is that I want to modify the baseline in a script, and now have to take these 3 spaces into account (to not make the diff bigger). Would it be OK to convert the HEREDOC to a regular string? I think the problem comes from that.

PS: I could make that PR.

@ruudk
Copy link
Contributor Author

ruudk commented Feb 23, 2023

Btw, that $template variable, is defined inside the foreach, but that should be outside the foreach, right? Or without $template variable and directly inlined into the sprintf call.

@ondrejmirtes
Copy link
Member

Yeah, feel free to try anything to improve it 😊 As a CI artifact in a PR here you get the compiled PHAR so you could easily debug the end result this way.

ruudk added a commit to ruudk/phpstan-src that referenced this issue Feb 24, 2023
Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.
ruudk added a commit to ruudk/phpstan-src that referenced this issue Feb 24, 2023
Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.
ruudk added a commit to ruudk/phpstan-src that referenced this issue Feb 26, 2023
Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.
ondrejmirtes pushed a commit to phpstan/phpstan-src that referenced this issue Feb 26, 2023
Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants