diff --git a/src/Standards/Generic/Sniffs/Strings/UnnecessaryStringConcatSniff.php b/src/Standards/Generic/Sniffs/Strings/UnnecessaryStringConcatSniff.php index ffaa1db121..033a6e7b3f 100644 --- a/src/Standards/Generic/Sniffs/Strings/UnnecessaryStringConcatSniff.php +++ b/src/Standards/Generic/Sniffs/Strings/UnnecessaryStringConcatSniff.php @@ -70,54 +70,58 @@ public function register() */ public function process(File $phpcsFile, $stackPtr) { - // Work out which type of file this is for. $tokens = $phpcsFile->getTokens(); - if ($tokens[$stackPtr]['code'] === T_STRING_CONCAT) { - if ($phpcsFile->tokenizerType === 'JS') { - return; - } - } else { - if ($phpcsFile->tokenizerType === 'PHP') { - return; - } + + if ($tokens[$stackPtr]['code'] === T_STRING_CONCAT && $phpcsFile->tokenizerType === 'JS') { + // JS uses T_PLUS for string concatenation, not T_STRING_CONCAT. + return; + } else if ($tokens[$stackPtr]['code'] === T_PLUS && $phpcsFile->tokenizerType === 'PHP') { + // PHP uses T_STRING_CONCAT for string concatenation, not T_PLUS. + return; } $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); $next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true); - if ($prev === false || $next === false) { + if ($next === false) { + return; + } + + if (isset(Tokens::$stringTokens[$tokens[$prev]['code']]) === false + || isset(Tokens::$stringTokens[$tokens[$next]['code']]) === false + ) { + // Bow out as at least one of the two tokens being concatenated is not a string. return; } - if (isset(Tokens::$stringTokens[$tokens[$prev]['code']]) === true - && isset(Tokens::$stringTokens[$tokens[$next]['code']]) === true + if ($tokens[$prev]['content'][0] !== $tokens[$next]['content'][0]) { + // Bow out as the two strings are not of the same type. + return; + } + + // Before we throw an error for PHP, allow strings to be + // combined if they would have < and ? next to each other because + // this trick is sometimes required in PHP strings. + if ($phpcsFile->tokenizerType === 'PHP') { + $prevChar = substr($tokens[$prev]['content'], -2, 1); + $nextChar = $tokens[$next]['content'][1]; + $combined = $prevChar.$nextChar; + if ($combined === '?'.'>' || $combined === '<'.'?') { + return; + } + } + + if ($this->allowMultiline === true + && $tokens[$prev]['line'] !== $tokens[$next]['line'] ) { - if ($tokens[$prev]['content'][0] === $tokens[$next]['content'][0]) { - // Before we throw an error for PHP, allow strings to be - // combined if they would have < and ? next to each other because - // this trick is sometimes required in PHP strings. - if ($phpcsFile->tokenizerType === 'PHP') { - $prevChar = substr($tokens[$prev]['content'], -2, 1); - $nextChar = $tokens[$next]['content'][1]; - $combined = $prevChar.$nextChar; - if ($combined === '?'.'>' || $combined === '<'.'?') { - return; - } - } - - if ($this->allowMultiline === true - && $tokens[$prev]['line'] !== $tokens[$next]['line'] - ) { - return; - } - - $error = 'String concat is not required here; use a single string instead'; - if ($this->error === true) { - $phpcsFile->addError($error, $stackPtr, 'Found'); - } else { - $phpcsFile->addWarning($error, $stackPtr, 'Found'); - } - }//end if - }//end if + return; + } + + $error = 'String concat is not required here; use a single string instead'; + if ($this->error === true) { + $phpcsFile->addError($error, $stackPtr, 'Found'); + } else { + $phpcsFile->addWarning($error, $stackPtr, 'Found'); + } }//end process() diff --git a/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.1.inc b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.1.inc new file mode 100644 index 0000000000..d2ac790d14 --- /dev/null +++ b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.1.inc @@ -0,0 +1,34 @@ +'; +$code = '<'.'?php '; + +$string = 'This is a really long string. ' + . 'It is being used for errors. ' + . 'The message is not translated.'; + +$shouldBail = 1 + 1; + +$shouldNotTrigger = 'My' . /* comment */ 'string'; +$shouldNotTrigger = 'My' /* comment */ . 'string'; + +// phpcs:set Generic.Strings.UnnecessaryStringConcat allowMultiline true +$string = 'Multiline strings are allowed ' + . 'when setting is enabled.'; +// phpcs:set Generic.Strings.UnnecessaryStringConcat allowMultiline false + +// phpcs:set Generic.Strings.UnnecessaryStringConcat error false +$throwWarning = 'My' . 'string'; +// phpcs:set Generic.Strings.UnnecessaryStringConcat error true diff --git a/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.2.inc b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.2.inc new file mode 100644 index 0000000000..6a5fcba9a7 --- /dev/null +++ b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.2.inc @@ -0,0 +1,7 @@ +'; -$code = '<'.'?php '; - -$string = 'This is a really long string. ' - . 'It is being used for errors. ' - . 'The message is not translated.'; -?> diff --git a/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.php b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.php index c08918d15c..e657e48700 100644 --- a/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.php +++ b/src/Standards/Generic/Tests/Strings/UnnecessaryStringConcatUnitTest.php @@ -33,7 +33,7 @@ final class UnnecessaryStringConcatUnitTest extends AbstractSniffUnitTest public function getErrorList($testFile='') { switch ($testFile) { - case 'UnnecessaryStringConcatUnitTest.inc': + case 'UnnecessaryStringConcatUnitTest.1.inc': return [ 2 => 1, 6 => 1, @@ -65,11 +65,21 @@ public function getErrorList($testFile='') * The key of the array should represent the line number and the value * should represent the number of warnings that should occur on that line. * + * @param string $testFile The name of the file being tested. + * * @return array */ - public function getWarningList() + public function getWarningList($testFile='') { - return []; + switch ($testFile) { + case 'UnnecessaryStringConcatUnitTest.1.inc': + return [ + 33 => 1, + ]; + + default: + return []; + } }//end getWarningList()