Skip to content

Squiz/FunctionDeclarationArgumentSpacing: handle asym modifiers for constructor property promotion #1121

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

Draft
wants to merge 1 commit into
base: php-8.4/feature/file-methods-support-asym-visibility
Choose a base branch
from

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Jun 6, 2025

Description

⚠️ This PR depends on PR #1116.


This commit adds handling of the spacing after asymmetric visibility modifiers used for constructor property promotion to this sniff.

The spacing requirements are aligned with the spacing expectations of the Squiz.WhiteSpace.ScopeKeywordSpacing sniff, so the sniffs should not conflict with each other.

Additionally, the new check has a dedicated error code, which means that - if there would be a conflict anywhere - the asym visibility spacing check within this sniff can easily be turned off.

Includes tests.

Suggested changelog entry

  • Added support for PHP 8.4 asymmetric visibility modifiers to the following sniffs:
    • Squiz.Functions.FunctionDeclarationArgumentSpacing

Related issues/external references

Follow up on #851
Follow up on #1116

Related to #734

…onstructor property promotion

This commit adds handling of the spacing after asymmetric visibility modifiers used for constructor property promotion to this sniff.

The spacing requirements are aligned with the spacing expectations of the `Squiz.WhiteSpace.ScopeKeywordSpacing` sniff, so the sniffs should not conflict with each other.

Additionally, the new check has a dedicated error code, which means that - if there would be a conflict anywhere - the asym visibility spacing check within this sniff can easily be turned off.

Includes tests.
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.

1 participant