Merge pull request #95 from bestit/feature/PHPCS-318-concat-security

PHPCS-318 Add Sniff against no-brackets-calcs

README.md

@@ -180,6 +180,7 @@ The base for the BestIt Standard is [PSR-12](https://github.com/php-fig/fig-stan
 | BestIt.Spacing.UseSpacing.IncorrectLinesCountBeforeFirstUse | There MUST be one line before the first usage. | no |
 | BestIt.Spacing.UseSpacing.IncorrectLinesCountBetweenDifferentTypeOfUse | There must be no line betweeen different usages. | no |
 | BestIt.Spacing.UseSpacing.IncorrectLinesCountBetweenSameTypeOfUse | The MUST be no line between same usages. | no |
+| BestIt.Strings.ConcatCalculationSniff.CalculationWithoutBrackets | You MUST encapsulate your calculation with brackets. | no |
 | BestIt.TypeHints.ExplicitAssertions.RequiredExplicitAssertion | Use assertion instead of inline documentation comment. | no |
 | BestIt.TypeHints.ReturnTypeDeclaration.MissingReturnTypeHint | Every function or method MUST have a type hint if the return annotation is valid. | yes |
 | Generic.Formatting.SpaceAfterCast | There MUST be a space after cast. |

src/Standards/BestIt/Sniffs/Strings/ConcatCalculationSniff.php

@@ -0,0 +1,150 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BestIt\Sniffs\Strings;
+
+use BestIt\CodeSniffer\CodeError;
+use BestIt\Sniffs\AbstractSniff;
+use const T_CLOSE_PARENTHESIS;
+use const T_MINUS;
+use const T_OPEN_CURLY_BRACKET;
+use const T_OPEN_PARENTHESIS;
+use const T_PLUS;
+use const T_SEMICOLON;
+use const T_STRING_CONCAT;
+
+/**
+ * You MUST encapsulate your calculation with brackets.
+ *
+ * @author blange <[email protected]>
+ * @package BestIt\Sniffs\Strings
+ */
+class ConcatCalculationSniff extends AbstractSniff
+{
+    /**
+     * You MUST encapsulate your calculation with brackets.
+     *
+     * @var string
+     */
+    public const CODE_CALCULATION_WITHOUT_BRACKETS = 'CalculationWithoutBrackets';
+
+    /**
+     * The error message for the error.
+     *
+     * @var string
+     */
+    private const MESSAGE_CALCULATION_WITHOUT_BRACKETS = 'You should encapsulate your calculation with brackets.';
+
+    /**
+     * Caching of the checks, already done.
+     *
+     * @var array
+     */
+    private static array $alreadyDoneChecks = [];
+
+    /**
+     * Checks for a possible wrong calculation after the token.
+     *
+     * @return void
+     * @throws CodeError
+     *
+     */
+    protected function checkForCalculationAfterToken(): void
+    {
+        $nextClosingPos = $this->file->findNext(
+            [T_OPEN_PARENTHESIS, T_SEMICOLON, T_STRING_CONCAT],
+            $this->getStackPos() + 1,
+        );
+
+        $nextMathPos = $this->file->findNext(
+            [T_MINUS, T_PLUS],
+            $this->getStackPos() + 1,
+            (int) $nextClosingPos,
+        );
+
+        if ($nextMathPos !== false) {
+            throw new CodeError(
+                static::CODE_CALCULATION_WITHOUT_BRACKETS,
+                self::MESSAGE_CALCULATION_WITHOUT_BRACKETS,
+                $nextMathPos,
+            );
+        }
+    }
+
+    /**
+     * Checks for a possible wrong calculation before the token.
+     *
+     * @return $this
+     * @throws CodeError
+     *
+     */
+    protected function checkForCalculationBeforeToken(): self
+    {
+        $prevClosingPos = $this->file->findPrevious(
+            [T_CLOSE_PARENTHESIS, T_OPEN_CURLY_BRACKET, T_SEMICOLON],
+            $this->getStackPos() + 1,
+        );
+
+        $prevMathPos = $this->file->findPrevious(
+            [T_MINUS, T_PLUS],
+            $this->getStackPos() + 1,
+            (int) $prevClosingPos,
+        );
+
+        if (($prevMathPos !== false) && !$this->isCheckAlreadyDone($prevMathPos)) {
+            throw new CodeError(
+                static::CODE_CALCULATION_WITHOUT_BRACKETS,
+                self::MESSAGE_CALCULATION_WITHOUT_BRACKETS,
+                $prevMathPos,
+            );
+        }
+
+        return $this;
+    }
+
+    /**
+     * Checks if the check for the given position in the actual file was already done.
+     *
+     * @param int $prevMathPos
+     * @param bool $withSave
+     *
+     * @return bool
+     */
+    private function isCheckAlreadyDone(int $prevMathPos, bool $withSave = true): bool
+    {
+        $checkMarker = $this->file->getFilename() . $prevMathPos;
+        $isDone = @self::$alreadyDoneChecks[$checkMarker];
+
+        if ($withSave) {
+            self::$alreadyDoneChecks[$checkMarker] = true;
+        }
+
+        return (bool) $isDone;
+    }
+
+    /**
+     * Processes the token.
+     *
+     * @return void
+     * @throws CodeError
+     *
+     */
+    protected function processToken(): void
+    {
+        // Who does 1 + 2 . 1 + 2 in reality? So I skipped it for now!
+        $this
+            ->checkForCalculationBeforeToken()
+            ->checkForCalculationAfterToken();
+    }
+
+    /**
+     * Registers the tokens on string concatinations.
+     *
+     * @return array
+     */
+    public function register(): array
+    {
+        return [T_STRING_CONCAT];
+    }
+}

tests/Sniffs/Strings/ConcatCalculationSniffTest.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BestIt\Sniffs\Strings;
+
+use BestIt\Sniffs\DefaultSniffIntegrationTestTrait;
+use BestIt\Sniffs\TestTokenRegistrationTrait;
+use BestIt\SniffTestCase;
+use BestIt\TestRequiredConstantsTrait;
+use const T_STRING_CONCAT;
+
+/**
+ * Test ConcatCalculationSniff.
+ *
+ * @author blange <[email protected]>
+ * @package BestIt\Sniffs\Strings
+ */
+class ConcatCalculationSniffTest extends SniffTestCase
+{
+    use DefaultSniffIntegrationTestTrait;
+    use TestRequiredConstantsTrait;
+    use TestTokenRegistrationTrait;
+
+    /**
+     * Checks the registration of the sniff.
+     *
+     * @return array
+     */
+    protected function getExpectedTokens(): array
+    {
+        return [T_STRING_CONCAT];
+    }
+
+    /**
+     * Checks the required constants.
+     *
+     * @return iterable
+     */
+    public function getRequiredConstantAsserts(): iterable
+    {
+        return [
+            'CODE_CALCULATION_WITHOUT_BRACKETS' => [
+                'CODE_CALCULATION_WITHOUT_BRACKETS',
+                'CalculationWithoutBrackets',
+            ],
+        ];
+    }
+
+    /**
+     * Sets up the test.
+     *
+     * @return void
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->fixture = new ConcatCalculationSniff();
+    }
+}

tests/Sniffs/Strings/Fixtures/ConcatCalculationSniff/correct/Correct.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BestIt\Sniffs\Strings\Fixtures\ConcatCalculationSniff\correct;
+
+class Correct
+{
+    public function __construct()
+    {
+        $b = 10;
+        echo 'This string is korrect + ' . (1 + $b) . ' und enthält mehrere Concats: ' . ($b -1);
+    }
+}

tests/Sniffs/Strings/Fixtures/ConcatCalculationSniff/with_errors/CalculationWithoutBrackets.12,13,15.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BestIt\Sniffs\Strings\Fixtures\ConcatCalculationSniff\with_errors;
+
+class Errors
+{
+    public function __construct()
+    {
+        $b = 10;
+        echo $b + 2 .
+            'This string is korrect + ' . 1 + $b . ' und enthält mehrere Concats: ' .
+            $b
+                -
+                1
+            ;
+    }
+}