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

Generic/DisallowTabIndent: do not auto-fix heredoc/nowdoc closer indent #533

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Jul 4, 2024

Description

Follow up on squizlabs/PHP_CodeSniffer#3640, which added the T_END_HEREDOC and T_END_NOWDOC tokens to the list of tokens to examine and was included in the 3.7.2 release.

When a flexible heredoc/nowdoc is used, the indentation type of the heredoc/nowdoc content and the indentation type of the heredoc/nowdoc closer must be consistent, so auto-fixing the indentation of the closer, without also changing the indentation of the contents (from tabs to spaces) would cause a parse error. See: https://3v4l.org/7OF3M

I do believe indentation tabs should still be flagged, but the auto-fixer should be disabled.

To allow people to disregard tab indentation for heredoc/nowdoc closers completely, the error now also has a more specific error code - TabsUsedHeredocCloser -, which allows for excluding it in a ruleset.

While the change in the error code could be considered a breaking change, the fact that the potential for a parse error hasn't been reported as a bug so far, leads me to believe this is not a frequently encountered situation, so the impact of the higher specificity for the error code should be minimal.

Suggested changelog entry

Generic.WhiteSpace.DisallowTabIndent: tab indentation for heredoc/nowdoc closers will no longer be auto-fixed to prevent parse errors. They issue will still be reported.
The error code for heredoc/nowdoc indentation using tabs has also been made more specific - TabsUsedHeredocCloser - to allow for selectively excluding the indentation check for heredoc/nowdoc closers.

Related issues/external references

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Follow up on squizlabs/PHP_CodeSniffer 3640, which added the `T_END_HEREDOC` and `T_END_NOWDOC` tokens to the list of tokens to examine and was included in the 3.7.2 release.

When a flexible heredoc/nowdoc is used, the indentation type of the heredoc/nowdoc content and the indentation type of the heredoc/nowdoc closer must be consistent, so auto-fixing the indentation of the closer, without also changing the indentation of the contents (from tabs to spaces) would cause a parse error.
See: https://3v4l.org/7OF3M

I do believe indentation tabs should still be flagged, but the auto-fixer should be disabled.

To allow people to disregard tab indentation for heredoc/nowdoc closers completely, the error now also has a more specific error code - ` TabsUsedHeredocCloser` -, which allows for excluding it in a ruleset.

While the change in the error code _could_ be considered a breaking change, the fact that the potential for a parse error hasn't been reported as a bug so far, leads me to believe this is not a frequently encountered situation, so the impact of the higher specificity for the error code should be minimal.
@jrfnl jrfnl force-pushed the feature/generic-disallowtabindent-dont-autofix-heredocnowdoc-indent branch from 9127ff1 to 01b4cec Compare July 5, 2024 00:04
@jrfnl jrfnl modified the milestones: 3.11.0, 3.10.x Next Jul 5, 2024
@jrfnl jrfnl merged commit 1c4aa60 into master Jul 9, 2024
48 checks passed
@jrfnl jrfnl deleted the feature/generic-disallowtabindent-dont-autofix-heredocnowdoc-indent branch July 9, 2024 05:07
spaze added a commit to spaze/coding-standard that referenced this pull request Aug 7, 2024
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