incompatible-binary-operands

jack-worman · May 21, 2024 · b65a89b · b65a89b
1 parent e1a0a4e
commit b65a89b
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 33 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="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;
@@ -272,6 +273,9 @@ public static function analyze(
                 && ($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(

diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php
@@ -316,28 +316,22 @@ function takesI(I $i): void
         $this->analyzeFile('somefile.php', new Context());
     }
 
-    public function testTwoRandomObjects(): void
-    {
-        $config = Config::getInstance();
-        $config->strict_binary_operands = true;
-
-        $this->addFile(
-            'somefile.php',
-            <<<'PHP'
-                <?php
-                $a = new \stdClass() > new \DateTimeImmutable();
-                PHP,
-        );
-
-        $this->expectException(CodeException::class);
-        $this->expectExceptionMessage(InvalidOperand::getIssueType());
-
-        $this->analyzeFile('somefile.php', new Context());
-    }
-
     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;',
@@ -1145,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') {