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

Avoid incompatibility between Generic and PSR2 standards for function call indentation #38

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

fredden
Copy link
Member

@fredden fredden commented Nov 9, 2023

Description

This is a re-creation of squizlabs/PHP_CodeSniffer#3631

Generic.WhiteSpace.ScopeIndent and PSR2.Methods.FunctionCallSignature both try to fix indentation differently. This results in files not being able to be fixed automatically with phpcbf.

See squizlabs/PHP_CodeSniffer#2078 (comment) where @gsherwood says that Generic.WhiteSpace.ScopeIndent should win in these cases.

Suggested changelog entry

  • Avoid incompatibility between Generic and PSR2 standards for function call indentation

Related issues/external references

Fixes squizlabs/PHP_CodeSniffer#2078
Related to (and fixes one item from) #152

Types of changes

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

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.

@@ -135,7 +129,6 @@ public function getErrorList($testFile='FunctionCallSignatureUnitTest.inc')
567 => 1,
568 => 1,
573 => 1,
574 => 1,
Copy link
Member

Choose a reason for hiding this comment

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

All these errors no longer being thrown, gives me the impression you are breaking the PEAR sniff behaviour in favour of fixing something which affects only PSR2. That doesn't seem right....

What is the justification for this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've gone back through these changes and re-assessed each in turn. Below are my findings, grouped together by class / reasoning.

Lines 106 & 355

There were two errors being flagged for this line:

106 | ERROR | [x] Multi-line function call not indented correctly; expected 0 spaces but found 4 (PEAR.Functions.FunctionCallSignature.Indent)
106 | ERROR | [x] Closing parenthesis of a multi-line function call must be on a line by itself (PEAR.Functions.FunctionCallSignature.CloseBracketLine)

The first error is actually not correct (or is misleading). The parameter is correctly indented by 4 spaces. The closing parenthesis should be indented by zero spaces, but not on this line. Fixing the second error (which is still reported after the changes in this pull request) by moving the closing parenthesis to its own line makes this clear as the indents for the parameter and closing parenthesis are then checked/reported independently.

The error message when the line is incorrectly indented (eg with 5 spaces) is misleading as it reports "expected 0 spaces but found 5" whereas it should probably report "expected 4 spaces but found 5" for the parameter (and leave the closing parenthesis to report its own indentation anomalies when it's on its own line.). I'm not trying to fix that problem here; this is an improvement that could be made in the future.


Lines 378-380 & 386-388 (& 394-396)

This code snippet shows these errors:

377 | ERROR | [x] Opening statement of multi-line function call not indented correctly; expected 4 spaces but found 5 (PEAR.Functions.FunctionCallSignature.OpeningIndent)
378 | ERROR | [x] Multi-line function call not indented correctly; expected 9 spaces but found 8 (PEAR.Functions.FunctionCallSignature.Indent)
379 | ERROR | [x] Multi-line function call not indented correctly; expected 9 spaces but found 8 (PEAR.Functions.FunctionCallSignature.Indent)
380 | ERROR | [x] Multi-line function call not indented correctly; expected 5 spaces but found 4 (PEAR.Functions.FunctionCallSignature.Indent)

There is ambiguity in the sniff here in that it will accept any multiple of 4 spaces (including zero) on line 377. When line 377 is no longer being flagged as problematic, the following three lines are either correct (when 377 has four spaces) or off by some multiple of four (eg, when 377 has 12 leading spaces, the following three lines are each indented by 8 too few spaces).

The coding standard does say that "Parameters need to be indented 4 spaces compared to the level of the function call", so I can see why the following three lines are being reported as "wrong" by this sniff. However the same coding standard says to "Use an indent of 4 spaces", which makes me think that "expected 9 spaces" and "expected 5 spaces" are in violation of the coding standard.

Lines 394-396 don't show up in the delta for this pull request an error message is still shown for these lines, but the message is changing in this pull request. Previously the sniff would suggest an indent of 13/9 spaces, now the sniff suggests an indent of 12/8 spaces.


Line 574

This one is harder to justify. The sniff (now) accepts both 4 & 6 spaces indentation here as "correct." Should we be accepting only one of these numbers? I can't see any examples of "inline HTML" in the coding standard at https://pear.php.net/manual/en/standards.php

@fredden
Copy link
Member Author

fredden commented Jan 16, 2024

As suggested by @jrfnl, I have run all three sniffs mentioned / changed in this pull request on both the Joomla and WordPress codebases. This is to determine if any of the changes here produce different behaviour. For the Joomla codebase, all three combinations produced identical results. For the WordPress codebase, Generic.WhiteSpace.ScopeIndent produced identical results, but PEAR.Functions.FunctionCallSignature and PSR2.Methods.FunctionCallSignature produced different results.

Differences when running `PEAR.Functions.FunctionCallSignature`
--- pr-38.wordpress.master.PEAR.Functions.FunctionCallSignature.diff	2024-01-16 16:40:25.423993745 +0000
+++ pr-38.wordpress.whitespace-incompatibility.PEAR.Functions.FunctionCallSignature.diff	2024-01-16 16:55:55.348267329 +0000
@@ -85752,7 +85752,7 @@
  		}
  
 diff --git a/wp-admin/includes/class-pclzip.php b/wp-admin/includes/class-pclzip.php
-index 963f31178c..dce5bc1d03 100644
+index 963f31178c..15bbc36f5b 100644
 --- a/wp-admin/includes/class-pclzip.php
 +++ b/wp-admin/includes/class-pclzip.php
 @@ -27,7 +27,7 @@
@@ -86224,7 +86224,7 @@
 -                          strlen($p_header['stored_filename']),
 -						  $p_header['extra_len']);
 +        strlen($p_header['stored_filename']),
-+        $p_header['extra_len']
++						  $p_header['extra_len']
 +    );
  
      // ----- Write the first 148 bytes of the header in the archive
@@ -86249,7 +86249,7 @@
 -                          $p_header['disk'], $p_header['internal'],
 -						  $p_header['external'], $p_header['offset']);
 +        $p_header['disk'], $p_header['internal'],
-+        $p_header['external'], $p_header['offset']
++						  $p_header['external'], $p_header['offset']
 +    );
  
      // ----- Write the 42 bytes of the header in the zip file
@@ -86264,7 +86264,7 @@
 +    $v_binary_data = pack(
 +        "VvvvvVVv", 0x06054b50, 0, 0, $p_nb_entries,
 +        $p_nb_entries, $p_size,
-+        $p_offset, strlen($p_comment)
++						  $p_offset, strlen($p_comment)
 +    );
  
      // ----- Write the 22 bytes of the header in the zip file
@@ -86280,7 +86280,7 @@
 +                "Filename '".$v_header['stored_filename']."' is "
  				  	    	  	   ."compressed by an unsupported compression "
 -				  	    	  	   ."method (".$v_header['compression'].") ");
-+                ."method (".$v_header['compression'].") "
++				  	    	  	   ."method (".$v_header['compression'].") "
 +            );
  
                return PclZip::errorCode();
@@ -380845,7 +380845,7 @@
  /** WP_Http class */
  require_once ABSPATH . WPINC . '/class-wp-http.php';
 diff --git a/wp-includes/class-json.php b/wp-includes/class-json.php
-index 6091d567f5..91afc0649e 100644
+index 6091d567f5..017053439d 100644
 --- a/wp-includes/class-json.php
 +++ b/wp-includes/class-json.php
 @@ -1,7 +1,7 @@
@@ -380903,8 +380903,8 @@
 -                     . chr((0xC0 & (ord($utf8[0]) << 6))
 -                         | (0x3F & ord($utf8[1])));
 +                    . chr(
-+                        (0xC0 & (ord($utf8[0]) << 6))
-+                        | (0x3F & ord($utf8[1]))
++                         (0xC0 & (ord($utf8[0]) << 6))
++                         | (0x3F & ord($utf8[1]))
 +                    );
  
              case 3:
@@ -380919,8 +380919,8 @@
 +                    | (0x0F & (ord($utf8[1]) >> 2))
 +                )
 +                    . chr(
-+                        (0xC0 & (ord($utf8[1]) << 6))
-+                        | (0x7F & ord($utf8[2]))
++                         (0xC0 & (ord($utf8[1]) << 6))
++                         | (0x7F & ord($utf8[2]))
 +                    );
          }
  

Differences when running `PSR2.Methods.FunctionCallSignature`
--- pr-38.wordpress.master.PSR2.Methods.FunctionCallSignature.diff	2024-01-16 16:36:52.090689591 +0000
+++ pr-38.wordpress.whitespace-incompatibility.PSR2.Methods.FunctionCallSignature.diff	2024-01-16 16:52:07.347124466 +0000
@@ -83536,7 +83536,7 @@
  		}
  
 diff --git a/wp-admin/includes/class-pclzip.php b/wp-admin/includes/class-pclzip.php
-index 963f31178c..108549d0f6 100644
+index 963f31178c..25379c1972 100644
 --- a/wp-admin/includes/class-pclzip.php
 +++ b/wp-admin/includes/class-pclzip.php
 @@ -27,7 +27,7 @@
@@ -83746,8 +83746,8 @@
 -		                       "Invalid number / type of arguments");
 +          PclZip::privErrorLog(
 +              PCLZIP_ERR_INVALID_PARAMETER,
-+              "Invalid number / type of arguments"
-+          );
++            "Invalid number / type of arguments"
++        );
            return 0;
          }
        }
@@ -83761,10 +83761,10 @@
 -                                               $v_supported_attributes);
 +      $v_result = $this->privFileDescrParseAtt(
 +          $v_entry,
-+          $v_filedescr_list[],
-+          $v_options,
-+          $v_supported_attributes
-+      );
++        $v_filedescr_list[],
++        $v_options,
++        $v_supported_attributes
++    );
        if ($v_result != 1) {
          return 0;
        }
@@ -83802,10 +83802,10 @@
 -                                               $v_supported_attributes);
 +      $v_result = $this->privFileDescrParseAtt(
 +          $v_entry,
-+          $v_filedescr_list[],
-+          $v_options,
-+          $v_supported_attributes
-+      );
++        $v_filedescr_list[],
++        $v_options,
++        $v_supported_attributes
++    );
        if ($v_result != 1) {
          return 0;
        }
@@ -83898,12 +83898,12 @@
 +          $v_arg_list,
 +          $v_size,
 +          $v_options,
-+          array (PCLZIP_OPT_BY_NAME => 'optional',
++        array (PCLZIP_OPT_BY_NAME => 'optional',
                                                 PCLZIP_OPT_BY_EREG => 'optional',
                                                 PCLZIP_OPT_BY_PREG => 'optional',
 -                                               PCLZIP_OPT_BY_INDEX => 'optional' ));
-+          PCLZIP_OPT_BY_INDEX => 'optional' )
-+      );
++        PCLZIP_OPT_BY_INDEX => 'optional' )
++    );
        if ($v_result != 1) {
            return 0;
        }
@@ -83948,9 +83948,9 @@
 -							   .$p_options_list[$i]."'");
 +          PclZip::privErrorLog(
 +              PCLZIP_ERR_INVALID_PARAMETER,
-+              "Unknown parameter '"
-+              .$p_options_list[$i]."'"
-+          );
++            "Unknown parameter '"
++            .$p_options_list[$i]."'"
++        );
  
            // ----- Return
            return PclZip::errorCode();
@@ -83962,8 +83962,8 @@
 -		                           "Unknown parameter '".$v_key."'");
 +          PclZip::privErrorLog(
 +              PCLZIP_ERR_INVALID_PARAMETER,
-+              "Unknown parameter '".$v_key."'"
-+          );
++            "Unknown parameter '".$v_key."'"
++        );
  
            // ----- Return
            return PclZip::errorCode();
@@ -84098,11 +84098,11 @@
 -			                       "Filename '".$v_header['stored_filename']."' is "
 +              PclZip::privErrorLog(
 +                  PCLZIP_ERR_UNSUPPORTED_COMPRESSION,
-+                  "Filename '".$v_header['stored_filename']."' is "
++                "Filename '".$v_header['stored_filename']."' is "
  				  	    	  	   ."compressed by an unsupported compression "
 -				  	    	  	   ."method (".$v_header['compression'].") ");
-+                  ."method (".$v_header['compression'].") "
-+              );
++                ."method (".$v_header['compression'].") "
++            );
  
                return PclZip::errorCode();
  		  }
@@ -84114,11 +84114,11 @@
 -			                       "Unsupported encryption for "
 +              PclZip::privErrorLog(
 +                  PCLZIP_ERR_UNSUPPORTED_ENCRYPTION,
-+                  "Unsupported encryption for "
++                "Unsupported encryption for "
  				  	    	  	   ." filename '".$v_header['stored_filename']
 -								   ."'");
-+                  ."'"
-+              );
++                ."'"
++            );
  
                return PclZip::errorCode();
  		  }
@@ -84130,8 +84130,8 @@
 -		                                        $p_file_list[$v_nb_extracted++]);
 +          $v_result = $this->privConvertHeader2FileInfo(
 +              $v_header,
-+              $p_file_list[$v_nb_extracted++]
-+          );
++            $p_file_list[$v_nb_extracted++]
++        );
            if ($v_result != 1) {
                $this->privCloseFd();
                $this->privSwapBackMagicQuotes();
@@ -84145,11 +84145,11 @@
 -											  $p_options);
 +          $v_result1 = $this->privExtractFile(
 +              $v_header,
-+              $p_path,
++            $p_path,
 +              $p_remove_path,
-+              $p_remove_all_path,
-+              $p_options
-+          );
++            $p_remove_all_path,
++            $p_options
++        );
            if ($v_result1 < 1) {
              $this->privCloseFd();
              $this->privSwapBackMagicQuotes();
@@ -84161,8 +84161,8 @@
 -                                $p_entry['filename']);
 +      = PclZipUtilPathInclusion(
 +          $p_options[PCLZIP_OPT_EXTRACT_DIR_RESTRICTION],
-+          $p_entry['filename']
-+      );
++        $p_entry['filename']
++    );
        if ($v_inclusion == 0) {
  
 -        PclZip::privErrorLog(PCLZIP_ERR_DIRECTORY_RESTRICTION,
@@ -84230,9 +84230,9 @@
 -						   .' Some trailing bytes exists after the archive.');
 +      PclZip::privErrorLog(
 +          PCLZIP_ERR_BAD_FORMAT,
-+          'The central dir is not at the end of the archive.'
-+          .' Some trailing bytes exists after the archive.'
-+      );
++        'The central dir is not at the end of the archive.'
++        .' Some trailing bytes exists after the archive.'
++    );
  
        // ----- Return
        return PclZip::errorCode();
@@ -320781,7 +320781,7 @@
  	}
  
 diff --git a/wp-includes/ID3/module.audio-video.matroska.php b/wp-includes/ID3/module.audio-video.matroska.php
-index eb5febf433..fd447fc344 100644
+index eb5febf433..b4481b8878 100644
 --- a/wp-includes/ID3/module.audio-video.matroska.php
 +++ b/wp-includes/ID3/module.audio-video.matroska.php
 @@ -1047,7 +1047,8 @@ class getid3_matroska extends getid3_handler
@@ -320789,7 +320789,7 @@
  															$attachedfile_entry['FileName'],
  															$attachedfile_entry['data_offset'],
 -															$attachedfile_entry['data_length']);
-+                                                            $attachedfile_entry['data_length']
++															$attachedfile_entry['data_length']
 +                                                        );
  
  														$this->current_offset = $sub_subelement['end'];
@@ -320844,7 +320844,7 @@
  				case 3: // 14-bit little-endian
  					$fhBS .= substr(getid3_lib::BigEndian2Bin(strrev(substr($DTSheader, $word_offset, 2))), 2, 14);
 diff --git a/wp-includes/ID3/module.audio.flac.php b/wp-includes/ID3/module.audio.flac.php
-index 014061da94..d1b96ea4df 100644
+index 014061da94..f507d625f3 100644
 --- a/wp-includes/ID3/module.audio.flac.php
 +++ b/wp-includes/ID3/module.audio.flac.php
 @@ -423,7 +423,8 @@ class getid3_flac extends getid3_handler
@@ -320852,13 +320852,13 @@
  				$this->ftell(),
  				$picture['datalength'],
 -				$picture['image_mime']);
-+                $picture['image_mime']
++				$picture['image_mime']
 +            );
  		}
  
  		$info['flac']['PICTURE'][] = $picture;
 diff --git a/wp-includes/ID3/module.audio.mp3.php b/wp-includes/ID3/module.audio.mp3.php
-index 0d8fee3e5d..bca1016aa2 100644
+index 0d8fee3e5d..a1873fd917 100644
 --- a/wp-includes/ID3/module.audio.mp3.php
 +++ b/wp-includes/ID3/module.audio.mp3.php
 @@ -1358,7 +1358,8 @@ class getid3_mp3 extends getid3_handler
@@ -320866,7 +320866,7 @@
  						$LongMPEGlayerLookup[$head4],
  						$LongMPEGpaddingLookup[$head4],
 -						$LongMPEGfrequencyLookup[$head4]);
-+                        $LongMPEGfrequencyLookup[$head4]
++						$LongMPEGfrequencyLookup[$head4]
 +                    );
  				}
  				if ($MPEGaudioHeaderLengthCache[$head4] > 4) {
@@ -322619,7 +322619,7 @@
  		return true;
  	}
 diff --git a/wp-includes/atomlib.php b/wp-includes/atomlib.php
-index 90f6282243..0dc60416bd 100644
+index 90f6282243..281145681d 100644
 --- a/wp-includes/atomlib.php
 +++ b/wp-includes/atomlib.php
 @@ -147,8 +147,8 @@ class AtomParser {
@@ -322652,13 +322652,12 @@
      function xml_escape($content)
      {
 -             return str_replace(array('&','"',"'",'<','>'),
--                array('&amp;','&quot;','&apos;','&lt;','&gt;'),
--                $content );
 +             return str_replace(
 +                 array('&','"',"'",'<','>'),
-+                 array('&amp;','&quot;','&apos;','&lt;','&gt;'),
-+                 $content 
-+             );
+                 array('&amp;','&quot;','&apos;','&lt;','&gt;'),
+-                $content );
++                $content 
++            );
      }
  }
 diff --git a/wp-includes/author-template.php b/wp-includes/author-template.php
@@ -484676,7 +484675,7 @@
 -unset( $filter, $action );
 +unset($filter, $action);
 diff --git a/wp-includes/deprecated.php b/wp-includes/deprecated.php
-index e63708f91b..c0496fd5e8 100644
+index e63708f91b..7344caf2a9 100644
 --- a/wp-includes/deprecated.php
 +++ b/wp-includes/deprecated.php
 @@ -23,7 +23,7 @@
@@ -484908,7 +484907,7 @@
 +        'hide_empty',
 +        'use_desc_for_title',
 +        'children',
-+        'child_of',
++		'child_of',
 +        'categories',
 +        'recurse',
 +        'feed',
@@ -576135,7 +576134,7 @@
 +_deprecated_file(basename(__FILE__), '2.1.0', WPINC . '/rss.php');
  require_once ABSPATH . WPINC . '/rss.php';
 diff --git a/wp-includes/rss.php b/wp-includes/rss.php
-index 6d8941ab38..f09972ba58 100644
+index 6d8941ab38..b83c195d54 100644
 --- a/wp-includes/rss.php
 +++ b/wp-includes/rss.php
 @@ -16,7 +16,7 @@
@@ -576292,28 +576291,28 @@
  			if ( $this->initem ) {
  				$this->concat(
 -					$this->current_item[ $this->current_namespace ][ $el ], $text);
-+                    $this->current_item[ $this->current_namespace ][ $el ],
++					$this->current_item[ $this->current_namespace ][ $el ],
 +                    $text
 +                );
  			}
  			elseif ($this->inchannel) {
  				$this->concat(
 -					$this->channel[ $this->current_namespace][ $el ], $text );
-+                    $this->channel[ $this->current_namespace][ $el ],
++					$this->channel[ $this->current_namespace][ $el ],
 +                    $text 
 +                );
  			}
  			elseif ($this->intextinput) {
  				$this->concat(
 -					$this->textinput[ $this->current_namespace][ $el ], $text );
-+                    $this->textinput[ $this->current_namespace][ $el ],
++					$this->textinput[ $this->current_namespace][ $el ],
 +                    $text 
 +                );
  			}
  			elseif ($this->inimage) {
  				$this->concat(
 -					$this->image[ $this->current_namespace ][ $el ], $text );
-+                    $this->image[ $this->current_namespace ][ $el ],
++					$this->image[ $this->current_namespace ][ $el ],
 +                    $text 
 +                );
  			}
@@ -576322,28 +576321,28 @@
  			if ( $this->initem ) {
  				$this->concat(
 -					$this->current_item[ $el ], $text);
-+                    $this->current_item[ $el ],
++					$this->current_item[ $el ],
 +                    $text
 +                );
  			}
  			elseif ($this->intextinput) {
  				$this->concat(
 -					$this->textinput[ $el ], $text );
-+                    $this->textinput[ $el ],
++					$this->textinput[ $el ],
 +                    $text 
 +                );
  			}
  			elseif ($this->inimage) {
  				$this->concat(
 -					$this->image[ $el ], $text );
-+                    $this->image[ $el ],
++					$this->image[ $el ],
 +                    $text 
 +                );
  			}
  			elseif ($this->inchannel) {
  				$this->concat(
 -					$this->channel[ $el ], $text );
-+                    $this->channel[ $el ],
++					$this->channel[ $el ],
 +                    $text 
 +                );
  			}

I'm reviewing these differences now. I expect to post an update with my findings in the coming days.

@fredden
Copy link
Member Author

fredden commented Jan 17, 2024

After reviewing the differences, I've identified a bug. The bug was that the PEAR (and PSR2 by inheritance) sniff didn't cope well when lines were indented with tabs, which is explicitly forbidden by the standard. I've fixed this bug (and added more tests) in e98cbb5. I'm not completely happy with some of the "fixed" lines in these tests, but I have verified that running the whole standard (not just the one sniff) produces expected results.

In order to get the test-suite to pass, I needed to merge in the main branch. I opted to do this instead of re-basing to make reviewing this pull request easier. @jrfnl please squash-merge this pull request when the time comes.

I've re-run the three sniffs over both Joomla and WordPress again. As before, I'm getting identical results for all three sniffs on the Joomla codebase. Also as before, the Generic.WhiteSpace.ScopeIndent sniff behaves the same on the WordPress codebase.

Also as before, for the WordPress codebase, Generic.WhiteSpace.ScopeIndent produced identical results, but PEAR.Functions.FunctionCallSignature and PSR2.Methods.FunctionCallSignature produced different results. Full details below as before.

(GitHub won't let me put these into this comment as they're too big.)
pear.txt
psr2.txt

From what I can tell, these all produce a better result than before.

@jrfnl
Copy link
Member

jrfnl commented Jan 17, 2024

@fredden Just checking for the codebases on which you tested which use tab indents (like WordPress), did you set the --tab-width=4 option when running your tests ? If not, the test results should be considered invalidated and will need to be redone... 🙈

@fredden
Copy link
Member Author

fredden commented Jan 18, 2024

No, I did not use any --tab-width flags when running the sniffs. There don't seem to be any test files which use (exclusively) tabs as indentation. The PEAR and PSR-2 standards both explicitly forbid using tabs as indentation. If you think that these sniffs should work properly when tabs are used for indentation, let's get relevant tests added in a different pull request. We can then review what impact those tests have here and adjust this pull request accordingly. (Adding tests to cover a whole new use-case seems out of scope here; this pull request is specifically dealing with a conflict between sniffs.)

@jrfnl
Copy link
Member

jrfnl commented Jan 19, 2024

There don't seem to be any test files which use (exclusively) tabs as indentation.

There shouldn't need to be any as - as long as standards which are tab-based use the tab-width setting - the behaviour should be fine with both tabs as well as spaces.

The tab-width setting makes sure that tabs in the 'content' key of the token array are replaced with the appropriate amount of spaces. If the 'content' key then contains a tab-replaced value, the token array will also contain an 'orig_content' key containing the original content, which can be used by sniffs which handle tabs vs spaces.

This means that all sniffs can presume a space-based token stream and work based on that. Yes, this will mean that fixers will make space-based replacements, but that is fine as a tab-based standard should always include the Generic.WhiteSpace.DisallowSpaceIndent sniff, which would fix that up in the next fixer round.

@fredden
Copy link
Member Author

fredden commented Jan 19, 2024

I have re-run the phpcbf checks with --tab-width set to 1 and all the way up to 12. For the --tab-width=4 case, the results are similar / the same as already posted above. For the other cases, the results are still better in the case of this branch.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

As discussed in our call, we cannot do this.

Yes, the PSR2 sniff was originally written for PSR2 use and explicitly via the ruleset exclude the OpeningIndent errorcode.

However, the sniff has now been in the PHPCS codebase for over 10 years and people may be including it in their ruleset stand-alone (i.e. not as part of PSR2, but as a single sniff).

Those people will be relying on the OpeningIndent error to be in the sniff and work as expected, so removing that error completely from the sniff is a breaking change we cannot make.

Can you think of another way to solve this ?

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.

Incorrect indent for closures as method argument.
2 participants