incompatible-binary-operands

vimeo · May 22, 2024 · c27f215 · c27f215
1 parent 0294324
commit c27f215
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 17 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.x-dev@a46a07c5ca8bd7b54d1ca814c1f5b5ed38a0ba90">
+<files psalm-version="5.x-dev@f372770ac0475e5aeca3b3bccba9a65e716d1cc6">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -457,8 +457,6 @@
       <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name]]></code>
-      <code><![CDATA[$first_var_name]]></code>
-      <code><![CDATA[$first_var_name]]></code>
       <code><![CDATA[$first_var_name_in_array_argument]]></code>
       <code><![CDATA[$get_debug_type_position]]></code>
       <code><![CDATA[$get_debug_type_position]]></code>
@@ -935,7 +933,6 @@
       <code><![CDATA[$dim_var_id]]></code>
       <code><![CDATA[$dim_var_id]]></code>
       <code><![CDATA[$extended_var_id]]></code>
-      <code><![CDATA[$extended_var_id]]></code>
       <code><![CDATA[$keyed_array_var_id]]></code>
       <code><![CDATA[$keyed_array_var_id]]></code>
     </RiskyTruthyFalsyComparison>

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -2,6 +2,7 @@
 
 namespace Psalm\Internal\Analyzer\Statements\Expression;
 
+use DateTimeInterface;
 use PhpParser;
 use Psalm\CodeLocation;
 use Psalm\Context;
@@ -17,6 +18,7 @@
 use Psalm\Internal\Codebase\VariableUseGraph;
 use Psalm\Internal\DataFlow\DataFlowNode;
 use Psalm\Internal\MethodIdentifier;
+use Psalm\Internal\Type\Comparator\UnionTypeComparator;
 use Psalm\Issue\DocblockTypeContradiction;
 use Psalm\Issue\ImpureMethodCall;
 use Psalm\Issue\InvalidOperand;
@@ -33,6 +35,7 @@
 use UnexpectedValueException;
 
 use function in_array;
+use function sprintf;
 use function strlen;
 
 /**
@@ -47,6 +50,8 @@ public static function analyze(
         int $nesting = 0,
         bool $from_stmt = false
     ): bool {
+        $codebase = $statements_analyzer->getCodebase();
+
         if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat && $nesting > 100) {
             $statements_analyzer->node_data->setType($stmt, Type::getString());
 
@@ -163,7 +168,6 @@ public static function analyze(
                     $new_parent_node->id => $new_parent_node,
                 ]);
 
-                $codebase = $statements_analyzer->getCodebase();
                 $event = new AddRemoveTaintsEvent($stmt, $context, $statements_analyzer, $codebase);
 
                 $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
@@ -255,6 +259,41 @@ public static function analyze(
                 );
             }
 
+            if (($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal
+                    || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotEqual
+                    || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Greater
+                    || $stmt instanceof PhpParser\Node\Expr\BinaryOp\GreaterOrEqual
+                    || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Smaller
+                    || $stmt instanceof PhpParser\Node\Expr\BinaryOp\SmallerOrEqual)
+                && $statements_analyzer->getCodebase()->config->strict_binary_operands
+                && $stmt_left_type
+                && $stmt_right_type
+                && ($stmt_left_type->hasObjectType() || $stmt_right_type->hasObjectType())
+                && (!UnionTypeComparator::isContainedBy($codebase, $stmt_left_type, $stmt_right_type)
+                    || !UnionTypeComparator::isContainedBy($codebase, $stmt_right_type, $stmt_left_type))
+                // It is okay if both sides implement \DateTimeInterface
+                && !(
+                    UnionTypeComparator::isContainedBy(
+                        $codebase,
+                        $stmt_left_type,
+                        new Union([new TNamedObject(DateTimeInterface::class)]),
+                    )
+                    && UnionTypeComparator::isContainedBy(
+                        $codebase,
+                        $stmt_right_type,
+                        new Union([new TNamedObject(DateTimeInterface::class)]),
+                    )
+                )
+            ) {
+                IssueBuffer::maybeAdd(
+                    new InvalidOperand(
+                        sprintf('Cannot compare %s to %s.', $stmt_left_type->getId(), $stmt_right_type->getId()),
+                        new CodeLocation($statements_analyzer, $stmt),
+                    ),
+                    $statements_analyzer->getSuppressedIssues(),
+                );
+            }
+
             if (($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal
                     || $stmt instanceof PhpParser\Node\Expr\BinaryOp\NotEqual
                     || $stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical

diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php
@@ -5,6 +5,7 @@
 use Psalm\Config;
 use Psalm\Context;
 use Psalm\Exception\CodeException;
+use Psalm\Issue\InvalidOperand;
 use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait;
 use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait;
 
@@ -316,6 +317,19 @@ function takesI(I $i): void
     public function providerValidCodeParse(): iterable
     {
         return [
+            'strict_binary_operands: objects of different shape' => [
+                'code' =><<<'PHP'
+                    <?php
+                    new \DateTime() > new \DateTime();
+                    new \DateTimeImmutable() > new \DateTimeImmutable();
+                    // special case for DateTimeInterface
+                    new \DateTime() > new \DateTimeImmutable();
+                    PHP,
+                'assertions' => [],
+                'ignored_issues' => [],
+                'php_version' => null,
+                'config_options' => ['strict_binary_operands' => true],
+            ],
             'regularAddition' => [
                 'code' => '<?php
                     $a = 5 + 4;',
@@ -1089,6 +1103,26 @@ function toPositiveInt(int $i): int
     public function providerInvalidCodeParse(): iterable
     {
         return [
+            'strict_binary_operands: different objects' => [
+                'code' =><<<'PHP'
+                    <?php
+                    new \stdClass() > new \DateTimeImmutable();
+                    PHP,
+                'error_message' => InvalidOperand::getIssueType(),
+                'error_levels' => [],
+                'php_version' => null,
+                'config_options' => ['strict_binary_operands' => true],
+            ],
+            'strict_binary_operands: different object shapes' => [
+                'code' =><<<'PHP'
+                    <?php
+                    ((object) ['a' => 0, 'b' => 1]) > ((object) ['a' => 0, 'c' => 1]);
+                    PHP,
+                'error_message' => InvalidOperand::getIssueType(),
+                'error_levels' => [],
+                'php_version' => null,
+                'config_options' => ['strict_binary_operands' => true],
+            ],
             'badAddition' => [
                 'code' => '<?php
                     $a = "b" + 5;',

diff --git a/tests/Traits/InvalidCodeAnalysisTestTrait.php b/tests/Traits/InvalidCodeAnalysisTestTrait.php
@@ -6,6 +6,7 @@
 use Psalm\Context;
 use Psalm\Exception\CodeException;
 
+use function array_key_exists;
 use function preg_quote;
 use function str_replace;
 use function strpos;
@@ -16,17 +17,22 @@
 use const PHP_VERSION_ID;
 
 /**
+ * @psalm-type psalmConfigOptions = array{
+ *     strict_binary_operands?: bool,
+ * }
  * @psalm-type DeprecatedDataProviderArrayNotation = array{
  *     code: string,
  *     error_message: string,
  *     ignored_issues?: list<string>,
- *     php_version?: string
+ *     php_version?: string,
+ *     config_options?: psalmConfigOptions,
  * }
  * @psalm-type NamedArgumentsDataProviderArrayNotation = array{
  *     code: string,
  *     error_message: string,
  *     error_levels?: list<string>,
- *     php_version?: string
+ *     php_version?: string|null,
+ *     config_options?: psalmConfigOptions,
  * }
  */
 trait InvalidCodeAnalysisTestTrait
@@ -43,12 +49,14 @@ abstract public function providerInvalidCodeParse(): iterable;
      * @dataProvider providerInvalidCodeParse
      * @small
      * @param list<string> $error_levels
+     * @param psalmConfigOptions $config_options
      */
     public function testInvalidCode(
         string $code,
         string $error_message,
         array  $error_levels = [],
-        ?string $php_version = null
+        ?string $php_version = null,
+        array $config_options = []
     ): void {
         $test_name = $this->getTestName();
         if (strpos($test_name, 'PHP80-') !== false) {
@@ -76,18 +84,22 @@ public function testInvalidCode(
             $code = str_replace("\n", "\r\n", $code);
         }
 
+        $config = Config::getInstance();
         foreach ($error_levels as $error_level) {
             $issue_name = $error_level;
             $error_level = Config::REPORT_SUPPRESS;
 
-            Config::getInstance()->setCustomErrorLevel($issue_name, $error_level);
+            $config->setCustomErrorLevel($issue_name, $error_level);
+        }
+        if (array_key_exists('strict_binary_operands', $config_options)) {
+            $config->strict_binary_operands = $config_options['strict_binary_operands'];
         }
 
         $this->project_analyzer->setPhpVersion($php_version, 'tests');
 
         $file_path = self::$src_dir_path . 'somefile.php';
 
-        // $error_message = preg_replace('/ src[\/\\\\]somefile\.php/', ' src/somefile.php', $error_message);
+        // $error_message = (string) preg_replace('/ src[\/\\\\]somefile\.php/', ' src/somefile.php', $error_message);
 
         $this->expectException(CodeException::class);
 

diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php
@@ -5,6 +5,7 @@
 use Psalm\Config;
 use Psalm\Context;
 
+use function array_key_exists;
 use function str_replace;
 use function strpos;
 use function strtoupper;
@@ -37,17 +38,22 @@
  * This is trait allows testing for the second case, when the value of
  * "throw_exception" will be "false".
  *
+ * @psalm-type psalmConfigOptions = array{
+ *      strict_binary_operands?: bool,
+ *  }
  * @psalm-type DeprecatedDataProviderArrayNotation = array{
  *     code: string,
  *     error_message: string,
  *     ignored_issues?: list<string>,
- *     php_version?: string
+ *     php_version?: string|null,
+ *     config_options?: psalmConfigOptions,
  * }
  * @psalm-type NamedArgumentsDataProviderArrayNotation = array{
  *     code: string,
  *     error_message: string,
  *     error_levels?: list<string>,
- *     php_version?: string
+ *     php_version?: string|null,
+ *     config_options?: psalmConfigOptions,
  * }
  */
 trait InvalidCodeAnalysisWithIssuesTestTrait
@@ -64,13 +70,16 @@ abstract public function providerInvalidCodeParse(): iterable;
      * @dataProvider providerInvalidCodeParse
      * @small
      * @param list<string> $error_levels
+     * @param psalmConfigOptions $config_options
      */
     public function testInvalidCodeWithIssues(
         string $code,
         string $error_message,
         array  $error_levels = [],
-        string $php_version = '7.4'
+        ?string $php_version = null,
+        array $config_options = []
     ): void {
+        $php_version ??= '7.4';
         $test_name = $this->getTestName();
         if (strpos($test_name, 'PHP80-') !== false) {
             if (version_compare(PHP_VERSION, '8.0.0', '<')) {
@@ -89,11 +98,15 @@ public function testInvalidCodeWithIssues(
             $code = str_replace("\n", "\r\n", $code);
         }
 
+        $config = Config::getInstance();
         foreach ($error_levels as $error_level) {
             $issue_name = $error_level;
             $error_level = Config::REPORT_SUPPRESS;
 
-            Config::getInstance()->setCustomErrorLevel($issue_name, $error_level);
+            $config->setCustomErrorLevel($issue_name, $error_level);
+        }
+        if (array_key_exists('strict_binary_operands', $config_options)) {
+            $config->strict_binary_operands = $config_options['strict_binary_operands'];
         }
 
         $this->project_analyzer->setPhpVersion($php_version, 'tests');

diff --git a/tests/Traits/ValidCodeAnalysisTestTrait.php b/tests/Traits/ValidCodeAnalysisTestTrait.php
@@ -5,6 +5,7 @@
 use Psalm\Config;
 use Psalm\Context;
 
+use function array_key_exists;
 use function str_replace;
 use function strlen;
 use function strpos;
@@ -14,6 +15,11 @@
 use const PHP_OS;
 use const PHP_VERSION_ID;
 
+/**
+ * @psalm-type psalmConfigOptions = array{
+ *     strict_binary_operands?: bool,
+ * }
+ */
 trait ValidCodeAnalysisTestTrait
 {
     /**
@@ -23,7 +29,8 @@ trait ValidCodeAnalysisTestTrait
      *         code: string,
      *         assertions?: array<string, string>,
      *         ignored_issues?: list<string>,
-     *         php_version?: string,
+     *         php_version?: string|null,
+     *         config_options?: psalmConfigOptions,
      *     }
      * >
      */
@@ -33,13 +40,15 @@ abstract public function providerValidCodeParse(): iterable;
      * @dataProvider providerValidCodeParse
      * @param array<string, string> $assertions
      * @param list<string> $ignored_issues
+     * @param psalmConfigOptions $config_options
      * @small
      */
     public function testValidCode(
         string $code,
         array $assertions = [],
         array $ignored_issues = [],
-        ?string $php_version = null
+        ?string $php_version = null,
+        array $config_options = []
     ): void {
         $test_name = $this->getTestName();
         if (strpos($test_name, 'PHP80-') !== false) {
@@ -71,8 +80,12 @@ public function testValidCode(
             $this->fail('Test case must have a <?php tag');
         }
 
+        $config = Config::getInstance();
         foreach ($ignored_issues as $issue_name) {
-            Config::getInstance()->setCustomErrorLevel($issue_name, Config::REPORT_SUPPRESS);
+            $config->setCustomErrorLevel($issue_name, Config::REPORT_SUPPRESS);
+        }
+        if (array_key_exists('strict_binary_operands', $config_options)) {
+            $config->strict_binary_operands = $config_options['strict_binary_operands'];
         }
 
         if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {