humanmade / S3-Uploads

The WordPress Plugin to Store Uploads on Amazon S3
1.92k stars 389 forks source link

Fixes: Fatal Error when running PHP lint command #648

Closed michaelbragg closed 8 months ago

michaelbragg commented 1 year ago

This PR fixes a linting error when running PHP's lint across the Imagick stub file.

When scanning our projects, including the S3 Uploads plugin, it was identified when running the lint command on new PHP versions.

System:

Error:

Fatal error: Namespace declaration statement has to be the very first statement or after any declare call in the script in psalm/stubs/imagick.php on line 11

Recreate: php -l psalm/stubs/imagick.php

Expect: No syntax errors detected in psalm/stubs/imagick.php

Another option would be to exclude the ./psalm directory from being included in the release via the .gitattributes file.

Chee-Ng commented 11 months ago

Can we get this merge in? I am having the same issue with my pipeline.

michaelbragg commented 8 months ago

Is there any update on a review of this pull request? Thanks.

tomjn commented 8 months ago

I'd wrangle it myself but I'm unfamiliar with this project, and it doesn't look like @joehoyle has had the opportunity to look at it, nor has it affected agency clients.

Looking at the actual change, I'm curious why the solution is a namespace { } and not to move the WP CLI stub causing the problem out of the imagick stub and into a separate WP CLI file?

michaelbragg commented 8 months ago

Thanks @tomjn.

Looking at the actual change, I'm curious why the solution is a namespace { } and not to move the WP CLI stub causing the problem out of the imagick stub and into a separate WP CLI file?

That would be an equally valid fix. My initial idea was to make the least changes 😄. Happy to rework the PR. Does that help in getting it moving?

joehoyle commented 8 months ago

Sorry for the delay, I think this makes sense!

michaelbragg commented 8 months ago

Thanks @joehoyle, any chance of a cheeky bugfix release for those of us who use composer?