vimeo · jack-worman · May 21, 2024 · May 21, 2024 · May 21, 2024 · May 21, 2024
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="dev-master@c488d40e6243536a42608bbe37245f07ebebe1ad">
+<files psalm-version="dev-master@16b24bdc94e052b5ce69fd232a77416a1f6ec3e6">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -2318,6 +2318,15 @@
       <code><![CDATA[public function getProjectDirectories(): array]]></code>
     </MethodSignatureMismatch>
   </file>
+  <file src="tests/Traits/InvalidCodeAnalysisTestTrait.php">
+    <InvalidArgument>
+      <code><![CDATA[testInvalidCode]]></code>
+      <code><![CDATA[testInvalidCode]]></code>
+      <code><![CDATA[testInvalidCode]]></code>
+      <code><![CDATA[testInvalidCode]]></code>
+      <code><![CDATA[testInvalidCode]]></code>
+    </InvalidArgument>
+  </file>
   <file src="tests/TypeParseTest.php">
     <RiskyTruthyFalsyComparison>
       <code><![CDATA[$param_type_1]]></code>

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -4,6 +4,7 @@
 
 namespace Psalm\Internal\Analyzer\Statements\Expression;
 
+use DateTimeInterface;
 use PhpParser;
 use Psalm\CodeLocation;
 use Psalm\Context;
@@ -19,6 +20,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;
@@ -35,6 +37,7 @@
 use UnexpectedValueException;
 
 use function in_array;
+use function sprintf;
 use function strlen;
 
 /**
@@ -49,6 +52,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());
 
@@ -165,7 +170,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);
@@ -257,6 +261,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
@@ -7,6 +7,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;
 
@@ -318,6 +319,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;',
@@ -1125,6 +1139,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
@@ -8,27 +8,33 @@
 use Psalm\Context;
 use Psalm\Exception\CodeException;
 
+use function array_key_exists;
 use function preg_quote;
+use function str_contains;
 use function str_replace;
-use function strpos;
 use function strtoupper;
 use function substr;
 
 use const PHP_OS;
 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
@@ -45,23 +51,25 @@ 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,
+        array $config_options = [],
     ): void {
         $test_name = $this->getTestName();
-        if (strpos($test_name, 'PHP80-') !== false) {
+        if (str_contains($test_name, 'PHP80-')) {
             if (PHP_VERSION_ID < 8_00_00) {
                 $this->markTestSkipped('Test case requires PHP 8.0.');
             }
 
             if ($php_version === null) {
                 $php_version = '8.0';
             }
-        } elseif (strpos($test_name, 'SKIPPED-') !== false) {
+        } elseif (str_contains($test_name, 'SKIPPED-')) {
             $this->markTestSkipped('Skipped due to a bug.');
         }
 
@@ -70,19 +78,23 @@ public function testInvalidCode(
         }
 
         // sanity check - do we have a PHP tag?
-        if (strpos($code, '<?php') === false) {
+        if (!str_contains($code, '<?php')) {
             $this->fail('Test case must have a <?php tag');
         }
 
         if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
             $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
@@ -7,6 +7,8 @@
 use Psalm\Config;
 use Psalm\Context;
 
+use function array_key_exists;
+use function str_contains;
 use function str_replace;
 use function strlen;
 use function strpos;
@@ -16,6 +18,11 @@
 use const PHP_OS;
 use const PHP_VERSION_ID;
 
+/**
+ * @psalm-type psalmConfigOptions = array{
+ *     strict_binary_operands?: bool,
+ * }
+ */
 trait ValidCodeAnalysisTestTrait
 {
     /**
@@ -25,7 +32,8 @@ trait ValidCodeAnalysisTestTrait
      *         code: string,
      *         assertions?: array<string, string>,
      *         ignored_issues?: list<string>,
-     *         php_version?: string,
+     *         php_version?: string|null,
+     *         config_options?: psalmConfigOptions,
      *     }
      * >
      */
@@ -35,32 +43,34 @@ 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,
+        array $config_options = [],
     ): void {
         $test_name = $this->getTestName();
-        if (strpos($test_name, 'PHP80-') !== false) {
+        if (str_contains($test_name, 'PHP80-')) {
             if (PHP_VERSION_ID < 8_00_00) {
                 $this->markTestSkipped('Test case requires PHP 8.0.');
             }
 
             if ($php_version === null) {
                 $php_version = '8.0';
             }
-        } elseif (strpos($test_name, 'PHP81-') !== false) {
+        } elseif (str_contains($test_name, 'PHP81-')) {
             if (PHP_VERSION_ID < 8_01_00) {
                 $this->markTestSkipped('Test case requires PHP 8.1.');
             }
 
             if ($php_version === null) {
                 $php_version = '8.1';
             }
-        } elseif (strpos($test_name, 'SKIPPED-') !== false) {
+        } elseif (str_contains($test_name, 'SKIPPED-')) {
             $this->markTestSkipped('Skipped due to a bug.');
         }
 
@@ -69,12 +79,16 @@ public function testValidCode(
         }
 
         // sanity check - do we have a PHP tag?
-        if (strpos($code, '<?php') === false) {
+        if (!str_contains($code, '<?php')) {
             $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') {