Slight improvements + changelog for bug #1347

squizlabs · Feb 17, 2017 · eff63ec · eff63ec
1 parent dca8343
commit eff63ec
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 10 deletions.
diff --git a/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php b/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php
@@ -263,13 +263,25 @@ public function processSingleLineCall(PHP_CodeSniffer_File $phpcsFile, $stackPtr
             $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceBeforeCloseBracket', $data);
             if ($fix === true) {
                 $padding = str_repeat(' ', $this->requiredSpacesBeforeClose);
+
                 if ($spaceBeforeClose === 0) {
                     $phpcsFile->fixer->addContentBefore($closer, $padding);
-                } else {
+                } else if ($spaceBeforeClose === 'newline') {
                     $phpcsFile->fixer->beginChangeset();
-                    $prev = ($closer - 1);
 
-                    // We want to jump over any whitespace or inline comment and move the `)` after any other token.
+                    $closingContent = ')';
+
+                    $next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), null, true);
+                    if ($tokens[$next]['code'] === T_SEMICOLON) {
+                        $closingContent .= ';';
+                        for ($i = ($closer + 1); $i <= $next; $i++) {
+                            $phpcsFile->fixer->replaceToken($i, '');
+                        }
+                    }
+
+                    // We want to jump over any whitespace or inline comment and
+                    // move the closing parenthesis after any other token.
+                    $prev = ($closer - 1);
                     while (isset(PHP_CodeSniffer_Tokens::$emptyTokens[$tokens[$prev]['code']]) === true) {
                         if (($tokens[$prev]['code'] === T_COMMENT)
                             && (strpos($tokens[$prev]['content'], '*/') !== false)
@@ -280,14 +292,16 @@ public function processSingleLineCall(PHP_CodeSniffer_File $phpcsFile, $stackPtr
                         $prev--;
                     }
 
-                    $phpcsFile->fixer->addContent($prev, $padding.')');
+                    $phpcsFile->fixer->addContent($prev, $padding.$closingContent);
 
                     $prevNonWhitespace = $phpcsFile->findPrevious(T_WHITESPACE, ($closer - 1), null, true);
                     for ($i = ($prevNonWhitespace + 1); $i <= $closer; $i++) {
                         $phpcsFile->fixer->replaceToken($i, '');
                     }
 
                     $phpcsFile->fixer->endChangeset();
+                } else {
+                    $phpcsFile->fixer->replaceToken(($closer - 1), $padding);
                 }//end if
             }//end if
         }//end if

diff --git a/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.inc b/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.inc
@@ -141,4 +141,39 @@ foo('Testing
 // . $text2
  */
  );
-// @codingStandardsChangeSetting PSR2.Methods.FunctionCallSignature requiredSpacesBeforeClose 0
+// @codingStandardsChangeSetting PSR2.Methods.FunctionCallSignature requiredSpacesBeforeClose 0
+
+foo('Testing
+    multiline text'
+);
+
+foo('Testing
+    multiline text'
+ );
+
+foo('Testing
+    multiline text' // hello
+);
+
+foo('Testing
+    multiline text' /* hello */
+);
+
+foo('Testing
+    multiline text'
+    // hello
+);
+
+foo('Testing
+    multiline text'
+    /* hello */
+);
+
+$var = foo('Testing
+    multiline'
+    // hi
+    ) + foo('Testing
+    multiline'
+    // hi
+    )
+;
diff --git a/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.inc.fixed b/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.inc.fixed
@@ -134,20 +134,50 @@ foo('Testing
     multiline text' );
 
 foo('Testing
-    multiline text: ' ) // . $text
-;
+    multiline text: ' ); // . $text
+
 
 foo('Testing
     multiline text: ' /* . $text */ );
 
 foo('Testing
-    multiline text: ' /* . $text */ )
+    multiline text: ' /* . $text */ );
     // . $other_text
-;
+
 
 foo('Testing
     multiline text: ' /*
  . $text
 // . $text2
  */ );
-// @codingStandardsChangeSetting PSR2.Methods.FunctionCallSignature requiredSpacesBeforeClose 0
+// @codingStandardsChangeSetting PSR2.Methods.FunctionCallSignature requiredSpacesBeforeClose 0
+
+foo('Testing
+    multiline text');
+
+foo('Testing
+    multiline text');
+
+foo('Testing
+    multiline text'); // hello
+
+
+foo('Testing
+    multiline text' /* hello */);
+
+foo('Testing
+    multiline text');
+    // hello
+
+
+foo('Testing
+    multiline text'
+    /* hello */);
+
+$var = foo('Testing
+    multiline')
+    // hi
+ + foo('Testing
+    multiline');
+    // hi
+
diff --git a/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.php b/CodeSniffer/Standards/PSR2/Tests/Methods/FunctionCallSignatureUnitTest.php
@@ -57,6 +57,15 @@ public function getErrorList()
                 129 => 1,
                 133 => 1,
                 138 => 1,
+                146 => 1,
+                150 => 1,
+                154 => 1,
+                158 => 1,
+                162 => 1,
+                167 => 1,
+                172 => 1,
+                175 => 1,
+                178 => 1,
                );
 
     }//end getErrorList()

diff --git a/package.xml b/package.xml
@@ -34,6 +34,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
   - Fixed bug #1340 : STDIN file contents not being populated in some cases
     -- Thanks to David Biňovec for the patch
   - Fixed bug #1344 : PEAR.Functions.FunctionCallSignatureSniff throws error for blank comment lines
+  - Fixed bug #1347 : PSR2.Methods.FunctionCallSignature strips some comments during fixing
+    -- Thanks to Algirdas Gurevicius for the patch
   - Fixed bug #1349 : Squiz.Strings.DoubleQuoteUsage.NotRequired message is badly formatted when string contains a CR newline char
     -- Thanks to Algirdas Gurevicius for the patch
   - Fixed bug #1350 : Invalid Squiz.Formatting.OperatorBracket error when using namespaces