Merge pull request #68 from guywithnose/master

Fix the Unnecessary String Concat Sniff

Author: nubs

DWS/Sniffs/Strings/UnnecessaryStringConcatSniff.php

@@ -0,0 +1,80 @@
+<?php
+/**
+ * Checks that two strings are not concatenated together; suggests
+ * using one string instead.
+ *
+ * @package DWS
+ * @subpackage Sniffs
+ */
+
+/**
+ * Checks that two strings are not concatenated together; suggests
+ * using one string instead.
+ *
+ * @package DWS
+ * @subpackage Sniffs
+ */
+final class DWS_Sniffs_Strings_UnnecessaryStringConcatSniff implements PHP_CodeSniffer_Sniff
+{
+    /**
+     * The limit that the length of a line should not exceed.
+     *
+     * @var int
+     */
+    public $stringLimit = 144;
+
+    /**
+     * A list of tokenizers this sniff supports.
+     *
+     * @var array
+     */
+    public $supportedTokenizers = array('PHP', 'JS');
+
+    /**
+     * Returns an array of tokens this test wants to listen for.
+     *
+     * @return array
+     */
+    public function register()
+    {
+        return array(T_STRING_CONCAT, T_PLUS);
+    }
+
+    /**
+     * Processes this test, when one of its tokens is encountered.
+     *
+     * @param PHP_CodeSniffer_File $phpcsFile The file being scanned.
+     * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+     *
+     * @return void
+     */
+    public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
+    {
+        // Work out which type of file this is for.
+        $tokens = $phpcsFile->getTokens();
+        if ($tokens[$stackPtr]['code'] === T_STRING_CONCAT XOR $phpcsFile->tokenizerType === 'PHP') {
+            return;
+        }
+
+        //Find the surrounding non-whitespace characters
+        $prev = $phpcsFile->findPrevious(T_WHITESPACE, $stackPtr - 1, null, true);
+        $next = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true);
+
+        $stringTokens = PHP_CodeSniffer_Tokens::$stringTokens;
+        //See if the characters before and after the concatenation are quotes
+        if (
+            in_array($tokens[$prev]['code'], $stringTokens) !== true ||
+            in_array($tokens[$next]['code'], $stringTokens) !== true
+        ) {
+            return;
+        }
+
+        //Make sure the combined string would not be too long for one line.
+        if (strlen($tokens[$prev]['content']) + strlen($tokens[$next]['content']) + $tokens[$prev]['column'] > $this->stringLimit) {
+            return;
+        }
+
+        $error = 'String concat is not required here; use a single string instead';
+        $phpcsFile->addError($error, $stackPtr, 'Found');
+    }
+}

DWS/ruleset.xml

@@ -56,7 +56,6 @@
     <rule ref="Generic.PHP.NoSilencedErrors" />
     <rule ref="Squiz.Scope.StaticThisUsage" />
     <rule ref="Squiz.Strings.EchoedStrings" />
-    <rule ref="Generic.Strings.UnnecessaryStringConcat" />
     <rule ref="Squiz.WhiteSpace.CastSpacing" />
     <rule ref="Generic.WhiteSpace.DisallowTabIndent" />
     <rule ref="Squiz.WhiteSpace.FunctionOpeningBraceSpace" />

tests/DWS/Sniffs/Strings/UnnecessaryStringConcatSniffTest.inc

@@ -0,0 +1,55 @@
+<?php
+if ($foo === 3) {
+    foreach ($bar as $thing) {
+        $reallyLongVariableName = 'This is an error. We shoule not ' . 'concatenate if the string is not tooi long.';
+    }
+}
+
+if ($foo === 3) {
+    foreach ($bar as $thing) {
+        $reallyLongVariableName = 'This is an error because it is a line tha' . 't is over 144 characers.  We really should never do this!!!!!!!!!!!!!!';
+    }
+}
+
+if ($foo === 3) {
+    foreach ($bar as $thing) {
+        $reallyLongVariableName = 'This is ok.  The string is very long such' .
+        ' that if we kept it on one line it would be over 144 characters.  So there is no problem concatenating here.';
+    }
+}
+
+if ($foo === 3) {
+    foreach ($bar as $thing) {
+        $reallyLongVariableName = 3 . 'We can still concatenate with numbers.';
+    }
+}
+
+if ($foo === 3) {
+    foreach ($bar as $thing) {
+        $reallyLongVariableName = 'We should have no reason to add ' +
+        'strings in PHP, but since this is possible in javascript, it is a good idea yo test how PHPCS handles it.  It should do fine!';
+    }
+}
+
+$foo = 'abc' . "{$foo}bar";
+
+$foo = "{$foo}bar" . 'abc';
+
+$foo = 'abc' . "{$foo}bar";
+
+$foo = "{$foo}bar" .
+'abc and a bunch more characters to make this take up >144 when concatenated to the previous line';
+
+$foo = 'abc and a bunch more characters to make this take up >144 when concatenated to the next line' .
+"{$foo}bar";
+
+$foo = 'First line
+Second line' . 'additional';
+
+$foo = 'First line
+Second line a bunch of characters to make this line take up <144 characters but enough that when concatenated with the previous line it is >144' .
+'additional';
+
+$foo = 'First line
+Second line a bunch of characters to make this line take up >144 characters' .
+'additional';

tests/DWS/Sniffs/Strings/UnnecessaryStringConcatSniffTest.php

@@ -0,0 +1,18 @@
+<?php
+final class DWS_Sniffs_Strings_UnnecessaryStringConcatSniffTest extends AbstractSniffUnitTest
+{
+    public function getErrorList()
+    {
+        return array(4 => 1, 34 => 1, 36 => 1, 38 => 1, 40 => 1, 43 => 1, 47 => 1, 54 => 1);
+    }
+
+    public function getWarningList()
+    {
+        return array();
+    }
+
+    protected function _getSniffName()
+    {
+        return 'DWS_Sniffs_Strings_UnnecessaryStringConcatSniff';
+    }
+}