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

Add preliminary support for PHP 5.4 #43

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

LinusU
Copy link

@LinusU LinusU commented Jan 18, 2025

I'm using an absolute ancient version of PHP at work (got handed a 10+ year old project I now need to maintain 🙈) and are constantly running into problems with me using features that aren't implemented. So this linter is actually great for me!

This PR adds the ability to configure the php_version all the way down to "5.4".

There is still some rules missing in order to cover 100% of the features introduced between 5.4 and 7.3, but I think that this is a good start! 🐎

Please let me know if I'm doing anything wrong, which I probably am 😅

Here is a screenshot of it in action:

Screenshot 2025-01-18 at 16 50 34

Copy link
Member

@azjezz azjezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this approach, but I believe we should define the minimum PHP versions we officially support:

  • >= 7.4: This version is well-tested and reliable with Mago.
  • >= 5.4: This can be allowed, but with caution.

The reasoning is that PHP 7 and PHP 8 introduced significant changes, particularly around operator precedence and associativity (left, right, or none). While Mago ( esp. the parser ) might work with your codebase, it could potentially fail or behave unexpectedly with others.

impl Rule for FinallyFeatureRule {
fn get_definition(&self) -> RuleDefinition {
RuleDefinition::enabled("Finally Feature", Level::Error)
.with_maximum_supported_php_version(PHPVersion::PHP54)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.with_maximum_supported_php_version(PHPVersion::PHP54)
.with_maximum_supported_php_version(PHPVersion::PHP55)

this has became exlusive now.

.with_annotation(
Annotation::primary(param.span()).with_message("Variadic parameter used here."),
)
.with_note("Use `func_get_args()` if you need compatibility with older PHP versions.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.with_note("Use `func_get_args()` if you need compatibility with older PHP versions.");
.with_help("Use `func_get_args()` if you need compatibility with older PHP versions.");

notes are used to provide more information, help are used to provide a solution, or a workaround.

@azjezz
Copy link
Member

azjezz commented Jan 18, 2025

We could improve support for PHP 5.x to the point where no warnings would be necessary, but achieving this would require:

  • Compiling a comprehensive list of syntax changes between PHP 5.x and PHP 8.
  • Updating the lexer and parser to adapt based on the specified PHP version.

When I mention making the parser version-aware, I'm primarily referring to handling elements like operator precedence. The parser would still process features from future PHP versions, provided they don't conflict with syntax from earlier versions. For example, features like property hooks can be parsed safely even if the version is set to PHP 5.4.

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.

2 participants