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

Problem with how phpcs handles ignored files when running in parallel #3317

Closed
emil-nasso opened this issue Apr 22, 2021 · 1 comment · Fixed by #3318
Closed

Problem with how phpcs handles ignored files when running in parallel #3317

emil-nasso opened this issue Apr 22, 2021 · 1 comment · Fixed by #3318

Comments

@emil-nasso
Copy link
Contributor

emil-nasso commented Apr 22, 2021

I ran into a very confusing issue where I had my phpcs-checks fail on the build server but when I ran the exact same command on the exact same file locally, I didn't get any failures at all. This got worse when my co-workers also got the same error as the build-server....

I checked versions of phpcs and php and everything matched up. I started looking into it and noticed the I started receiving the expected error when i ran phpcs with --parallel=1 or -vv. I added more formatting errors in the files in the same directory as the file I had issues in and it seemed like the entire directory was being ignored by phpcs.

In the parent directory of the file we had a php-file with // phpcs:ignoreFile on top. This was a file that used php 8+ features quite heavily and phpcs didn't have php8 support at the time.

I started looking into how the runner was processing files and why my files were being ignored, even though they were not in the exclude-path in the phpcs.xml-file or hade ignore directives in them.

When the runner splits up the checks into separate processes it divides the files up into "batches" and runs each batch in a separate process. As part of this process, it checks if files are ignored. When I added some debugging there, I saw something weird.

image
image

It seemed like my file that contained the ignore-directive was being processed multiple times. At that time the problem became clear, after reading and understanding the code. :)

image

The process will loop through all files until $endAt and process each of them. At the very end of the loop, $todo->next() is called to move the pointer to the next file in the list.

However. When $file->ignored === true the script will just continue without moving the pointer forward. This will cause the ignored file to continue to be the file being processed and continuously being ignored.

In practice, this will make phpcs ignore all files that appear in the same batch as an ignored file but are scheduled to be processed after the ignored file.

By adding a $todo->next() just before the continue; i started seeing the report i was looking for, even when phpcs was run in parallel.

A PR will be provided shortly.

@emil-nasso emil-nasso changed the title Problem with how the phpcs handles ignored files when running in parallel Problem with how phpcs handles ignored files when running in parallel Apr 22, 2021
@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Apr 23, 2021
@gsherwood gsherwood added this to the 3.6.1 milestone Apr 23, 2021
gsherwood pushed a commit that referenced this issue Apr 23, 2021
gsherwood added a commit that referenced this issue Apr 23, 2021
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Apr 23, 2021
@gsherwood
Copy link
Member

Thanks for sending in such a detail bug report, and for the PR. The report made the solution obvious, and easy to test.

The fix will be released in 3.6.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PHPCS v3 Development
Ready for Release
2 participants