Skip to content

Conversation

rjd22
Copy link

@rjd22 rjd22 commented Oct 25, 2024

From PHP8 on use statements are considered a single token. For this reason they cannot be splitted up to shorter namespaces.

PHPCS doesn't account for that and will throw line length errors and warnings for the use statements. This change ignores the line length for the use statements.

Ofcourse this changes the current behavior so I want to use this PR to further discuss what might be needed to implement this change.

RFC link: https://wiki.php.net/rfc/namespaced_names_as_token
Issue link: squizlabs/PHP_CodeSniffer#3606

Suggested changelog entry

Related issues/external references

Fixes squizlabs/PHP_CodeSniffer#3606

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

From PHP8 on use statements are considered a single token. For this
reason they cannot be splitted up to shorter namespaces.

PHPCS doesn't account for that and will throw line length errors
and warnings for the use statements. This change ignores the line
length for the use statements.

RFC link: https://wiki.php.net/rfc/namespaced_names_as_token
Issue link: squizlabs/PHP_CodeSniffer#3606
@rjd22 rjd22 force-pushed the fix/ignore-line-length-for-use-statements branch from 0f40736 to 31c6fd6 Compare October 25, 2024 13:01
@jrfnl
Copy link
Member

jrfnl commented Oct 25, 2024

@rjd22 Thanks for your interest in contributing to PHP_CodeSniffer.

I want to use this PR to further discuss what might be needed to implement this change.

As per the contributing guidelines, this should have been an issue to discuss the various options.

There are basically four options:

  1. The property as implemented in this PR.
  2. Having a separate error code for lines containing import use statements, which allows for selectively ignoring that specific error code.
  3. Blindly ignoring import use statement lines altogether.
  4. Toggling the behaviour of the sniff based on the value of the phpVersion config setting.

Adding a property is the worst of those options and not a solution I will accept.

Also keep in mind that the tokenization of names in import use statements will change in PHPCS 4.0, which may inform how the solution should be coded.

Looking at the code and the tests, it also appears this PR:

  • Does not take into account that the use keyword is also used for trait use and closure use.
  • Does not allow for multi-use statements.
  • Does not allow for group use statements.
  • Does not allow for import use statements where the name is already on a second line.
  • Will significantly (and I mean that!) decrease performance of the sniff due to the way the findPrevious() call is made.

All in all, this is not an acceptable PR. I'd be happy to discuss/review a proposal to fix squizlabs/PHP_CodeSniffer#3606, but this is not the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LineLength too strict for PHP8 namespaces

2 participants