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

Remove 3 leading spaces in PHAR #2254

Merged
merged 2 commits into from
Feb 26, 2023
Merged

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Feb 24, 2023

Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.

@ruudk
Copy link
Contributor Author

ruudk commented Feb 24, 2023

As a CI artifact in a PR here you get the compiled PHAR so you could easily debug the end result this way.

@ondrejmirtes I must be blind but I don't see the artifact.

@staabm
Copy link
Contributor

staabm commented Feb 24, 2023

in the "compile phar" action

grafik

at the bottom

https://github.com/phpstan/phpstan-src/actions/runs/4260429654

grafik

sorry for the huge screenshots 😅

@ruudk
Copy link
Contributor Author

ruudk commented Feb 24, 2023

@staabm Thanks!

@ruudk
Copy link
Contributor Author

ruudk commented Feb 24, 2023

Oke, the generated PHAR now looks like this:

            ksort($fileErrorsCounts, SORT_STRING);
            foreach ($fileErrorsCounts as $message => $count) {
                $php .= '$ignoreErrors[] = [';
                $php .= "\n";
                $php .= "\t'message' => " . var_export(Helpers::escape('#^' . preg_quote($message, '#') . '$#'), \true) . ',';
                $php .= "\n";
                $php .= "\t'count' => " . $count . ',';
                $php .= "\n";
                $php .= "\t'path' => __DIR__ . " . var_export(Helpers::escape($file), \true) . ',';
                $php .= "\n";
                $php .= '];';
                $php .= "\n";
            }

I think this looks good.

I also noticed that the generated baseline did not have a line ending, so I added that.
Screenshot 2023-02-24 at 09 40 40@2x

This is ready.

@ruudk ruudk marked this pull request as ready for review February 24, 2023 08:40
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes
Copy link
Member

@mvorisek
Copy link
Contributor

When PHP-CS-Fixer is present in the project, what about invoking it and keeping the phpstan PHP file CS always fixed?

@ruudk
Copy link
Contributor Author

ruudk commented Feb 26, 2023

Feels like a waste of resources especially on large baselines with 15k lines. PHP CS Fixer is not the fastest....

I will fix the CS in this PR so that we can merge the actual fix.

Fixes phpstan/phpstan#8952

I think the problem comes from the nowdoc related to Box/Rector.
Ideally, all files should end with a newline
@ruudk
Copy link
Contributor Author

ruudk commented Feb 26, 2023

@ondrejmirtes CS is fixed. And I verified that the new PHAR generates the baseline correctly.

@ondrejmirtes ondrejmirtes merged commit c9b5060 into phpstan:1.10.x Feb 26, 2023
@ondrejmirtes
Copy link
Member

Thank you!

@ruudk ruudk deleted the fix-space branch February 27, 2023 07:50
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.

PHP baseline contains 3 extra spaces per start of the line
5 participants