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

Replace the WordPress.WhiteSpace.PrecisionAlignment sniff #2129

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 7, 2022

... with a similar sniff as merged upstream in PHPCSExtra as Universal.WhiteSpace.PrecisionAlignment.

The upstream sniff has various improvements, including:

  • Detecting precision alignment in the leading whitespace for PHP 7.3 flexible heredoc/nowdoc closing identifiers (was false negative).
  • Improved handling of (trailing) whitespace on otherwise blank lines, both in code as well as in comments and HTML. This will now be ignored by default. (was false positive).
  • An additional option to report on precision alignment for whitespace found on otherwise blank lines (not relevant for WP as the Squiz.WhiteSpace.SuperfluousWhitespace is used).
  • And it also includes a fixer. Note: the fixer works based on "best guess" and may not always result in the desired indentation.

Ref:

... with a similar sniff as merged upstream in PHPCSExtra as `Universal.WhiteSpace.PrecisionAlignment`.

The upstream sniff has various improvements, including:
* Detecting precision alignment in the leading whitespace for PHP 7.3 flexible heredoc/nowdoc closing identifiers (was false negative).
* Improved handling of (trailing) whitespace on otherwise blank lines, both in code as well as in comments and HTML. This will now be ignored by default. (was false positive).
* An additional option to report on precision alignment for whitespace found on otherwise blank lines (not relevant for WP as the `Squiz.WhiteSpace.SuperfluousWhitespace` is used).
* And it also includes a fixer.
    Note: the fixer works based on "best guess" and may not always result in the desired indentation.

Ref:
* PHPCSStandards/PHPCSExtra 119
* PHPCSStandards/PHPCSExtra 122
* PHPCSStandards/PHPCSExtra 123
* PHPCSStandards/PHPCSExtra 124
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

✅ Approved, but I'm wondering how WPCS can be sure that the Universal sniff defaults match what is wanted for WP, both now and in the future (when the Universal sniff may change) (other than the fact you're the developer of the Universal sniff...)

@GaryJones GaryJones merged commit acbca75 into develop Dec 7, 2022
@GaryJones GaryJones deleted the feature/precisionalignment-replace-sniff branch December 7, 2022 20:12
@jrfnl
Copy link
Member Author

jrfnl commented Dec 7, 2022

I'm wondering how WPCS can be sure that the Universal sniff defaults match what is wanted for WP, both now and in the future (when the Universal sniff may change) (other than the fact you're the developer of the Universal sniff...)

Well, to start, the default public property values are "sensible" (minimal) values, so I see no reason why these would ever need to be changed for this sniff.

Secondly, any change to the default values of the public properties would be a BC break and can only be made in a major release and has to be clearly annotated in the changelog (semver).

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.

3 participants