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.

Sign up for GitHub

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

Jump to bottom

Suggestion: PHP Linting for Supported Versions #1709

Open
rsynnest opened this issue Nov 8, 2021 · 2 comments · May be fixed by #1710
Open

Suggestion: PHP Linting for Supported Versions #1709

rsynnest opened this issue Nov 8, 2021 · 2 comments · May be fixed by #1710

Comments

@rsynnest
Copy link

rsynnest commented Nov 8, 2021 •
edited

I hate adding cruft to OSS, so take it or leave it, but if it's helpful here's a very simple + performant one-liner you can add to the build process to syntax check all files using a given PHP version. It prints syntax errors to stdout and returns exit code 1 if any syntax errors are found. (one-liner pulled from discussions here )

Syntax check all files in current directory (recursive):

find . -type f -name '*.php' -print0 \
  | xargs -0 -n1 -P$(nproc) php5.3 -l -n \
  | (! grep -v "^No syntax errors detected" )

If you don't have a certain php version installed, you can use Docker. This bash script can be used in place of the php binary for a given version (though it will be much slower - a few seconds). v5.3 is used as an example:

#! /usr/bin/env bash
docker run -t --rm -v "$PWD:$PWD" -w $PWD php:5.3-cli php -l -n "$@"

Here's example output for the error fixed in PR #1706 :

> time find . -type f -name '*.php' -print0  \
  | xargs -0 -n1 -P$(nproc) php5.3 -l -n  \
  | (! grep -v "^No syntax errors detected" )

Parse error: syntax error, unexpected '[' in ./phpseclib/Net/SSH2.php on line 3558
Errors parsing ./phpseclib/Net/SSH2.php
xargs: php: exited with status 255; aborting

real    0m0.030s
user    0m0.072s
sys     0m0.016s

> echo $?
1
@terrafrost
Copy link
Member

Feel free to submit a PR!

I have Docker containers for PHP versions going back all the way to 4.4 available at https://github.com/phpseclib/docker-php (they're available as different branches). I chiefly use them for benchmarking purposes but certainly linting would be an option to. The PHP official docker containers only go back to 5.3 whereas mine to 4.4. Mine also install optional extensions useful for benchmarking that aren't available by default (gmp, bcmath, mcrypt, ssh2).

Maybe I'll do it myself at some point but it isn't a particularly high priority. I mean, if someone finds an issue with phpseclib 1.0 on PHP 4.4 I'll fix it but if PHP 4.4 support got broken, at some point, three years ago, I'm not gonna lose any sleep over it lol.

That said, what'd be better than doing php -l would be doing full on unit tests but idk if PHPUnit ever supported PHP 4.4 and I feel like any time I spent trying to make unit tests run on PHP 4.4 could probably be better spent doing other things.

@rsynnest
Copy link
Author

rsynnest commented Nov 9, 2021

100% agree, older version support is a nice favor to legacy users, but shouldn't be a priority for developer time.

I'm happy to add this, but not sure where. I'm assuming releases are manual via git tag/push? Where would be a good place for you? Maybe as a phing job with target name "lint"?

PHPUnit depends on a specific PHP version, so yeah, maintaining 3 different test suites of old PHPUnit versions is a special kind of hell.

Nice to see a trustworthy source for old PHP Docker builds, I'll likely be reaching for those, thanks for sharing!

@rsynnest rsynnest linked a pull request Nov 9, 2021 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add linter task to phing for checking old PHP versions rsynnest/phpseclib
2 participants
@terrafrost @rsynnest