Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unnecessary line wrapping with -s #124

Closed
3 tasks done
anomiex opened this issue Dec 4, 2023 · 1 comment · Fixed by #125
Closed
3 tasks done

Unnecessary line wrapping with -s #124

anomiex opened this issue Dec 4, 2023 · 1 comment · Fixed by #125
Milestone

Comments

@anomiex
Copy link
Contributor

anomiex commented Dec 4, 2023

Describe the bug

This is a re-creation of squizlabs/PHP_CodeSniffer#3924:

When using -s along with a large --report-width, some lines are still wrapped despite being much shorter than the selected width.

Code sample

<?php

$x=1;

Custom ruleset

N/A

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above.
  2. Run phpcs -s --report-width=100000 --standard=PSR12 test.php
  3. See output displayed
FILE: /tmp/test/test.php
---------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 3 | ERROR | [x] Expected at least 1 space before "="; 0 found
   |       |     (PSR12.Operators.OperatorSpacing.NoSpaceBefore)
 3 | ERROR | [x] Expected at least 1 space after "="; 0 found
   |       |     (PSR12.Operators.OperatorSpacing.NoSpaceAfter)
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------

Expected behavior

FILE: /tmp/test/test.php
---------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 3 | ERROR | [x] Expected at least 1 space before "="; 0 found (PSR12.Operators.OperatorSpacing.NoSpaceBefore)
 3 | ERROR | [x] Expected at least 1 space after "="; 0 found (PSR12.Operators.OperatorSpacing.NoSpaceAfter)
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------

Versions (please complete the following information)

Operating System Debian sid
PHP version 8.2.12
PHP_CodeSniffer version 3.7.2, master
Standard PSR12
Install type Composer local

Additional context

The problem seems to be that the four non-displaying characters (\033[0m) added at

$msgLine .= "\033[0m".' ('.$error['source'].')';
are not accounted for when calculating $maxErrorLength at
$length += (strlen($error['source']) + 3);
Or, alternatively, that those four characters aren't being ignored by PHP's wordwrap().

The simple fix would be to add 4 more at line 69, with the downsides that the lines of dashes would be longer than necessary and a --report-width that's just barely long enough (e.g. 111 for this example) would still unnecessarily wrap.

A more complex fix might be to wordwrap() only the message, and then manually calculate the length of the last line to decide whether to append the source directly or to append it as a new line.

Please confirm:

  • I have searched the issue list and am not opening a duplicate issue.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer
@jrfnl
Copy link
Member

jrfnl commented Dec 10, 2023

Bug has been tested and confirmed. This issue is further discussed in the PR which is open to fix it #125.

@jrfnl jrfnl added this to the 3.x Next milestone Dec 10, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants