PHPOffice · oleibman · Jan 16, 2023 · Jan 9, 2023 · Jan 11, 2023 · Jan 11, 2023
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -80,16 +80,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Calculation/Internal/MakeMatrix.php
 
-		-
-			message: "#^Call to function is_string\\(\\) with null will always evaluate to false\\.$#"
-			count: 3
-			path: src/PhpSpreadsheet/Calculation/Logical/Operations.php
-
-		-
-			message: "#^Result of && is always false\\.$#"
-			count: 3
-			path: src/PhpSpreadsheet/Calculation/Logical/Operations.php
-
 		-
 			message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\LookupRef\\:\\:CHOOSE\\(\\) has parameter \\$chooseArgs with no type specified\\.$#"
 			count: 1

diff --git a/src/PhpSpreadsheet/Calculation/Logical/Operations.php b/src/PhpSpreadsheet/Calculation/Logical/Operations.php
@@ -33,23 +33,9 @@ class Operations
      */
     public static function logicalAnd(...$args)
     {
-        $args = Functions::flattenArray($args);
-
-        if (count($args) == 0) {
-            return ExcelError::VALUE();
-        }
-
-        $args = array_filter($args, function ($value) {
-            return $value !== null || (is_string($value) && trim($value) == '');
+        return self::countTrueValues($args, function (int $trueValueCount, int $count): bool {
+            return $trueValueCount === $count;
         });
-
-        $returnValue = self::countTrueValues($args);
-        if (is_string($returnValue)) {
-            return $returnValue;
-        }
-        $argCount = count($args);
-
-        return ($returnValue > 0) && ($returnValue == $argCount);
     }
 
     /**
@@ -74,22 +60,9 @@ public static function logicalAnd(...$args)
      */
     public static function logicalOr(...$args)
     {
-        $args = Functions::flattenArray($args);
-
-        if (count($args) == 0) {
-            return ExcelError::VALUE();
-        }
-
-        $args = array_filter($args, function ($value) {
-            return $value !== null || (is_string($value) && trim($value) == '');
+        return self::countTrueValues($args, function (int $trueValueCount): bool {
+            return $trueValueCount > 0;
         });
-
-        $returnValue = self::countTrueValues($args);
-        if (is_string($returnValue)) {
-            return $returnValue;
-        }
-
-        return $returnValue > 0;
     }
 
     /**
@@ -116,22 +89,9 @@ public static function logicalOr(...$args)
      */
     public static function logicalXor(...$args)
     {
-        $args = Functions::flattenArray($args);
-
-        if (count($args) == 0) {
-            return ExcelError::VALUE();
-        }
-
-        $args = array_filter($args, function ($value) {
-            return $value !== null || (is_string($value) && trim($value) == '');
+        return self::countTrueValues($args, function (int $trueValueCount): bool {
+            return $trueValueCount % 2 === 1;
         });
-
-        $returnValue = self::countTrueValues($args);
-        if (is_string($returnValue)) {
-            return $returnValue;
-        }
-
-        return $returnValue % 2 == 1;
     }
 
     /**
@@ -177,31 +137,36 @@ public static function NOT($logical = false)
     }
 
     /**
-     * @return int|string
+     * @return bool|string
      */
-    private static function countTrueValues(array $args)
+    private static function countTrueValues(array $args, callable $func)
     {
         $trueValueCount = 0;
+        $count = 0;
 
-        foreach ($args as $arg) {
+        $aArgs = Functions::flattenArrayIndexed($args);
+        foreach ($aArgs as $k => $arg) {
+            ++$count;
             // Is it a boolean value?
             if (is_bool($arg)) {
                 $trueValueCount += $arg;
-            } elseif ((is_numeric($arg)) && (!is_string($arg))) {
-                $trueValueCount += ((int) $arg != 0);
             } elseif (is_string($arg)) {
+                $isLiteral = !Functions::isCellValue($k);
                 $arg = mb_strtoupper($arg, 'UTF-8');
-                if (($arg == 'TRUE') || ($arg == Calculation::getTRUE())) {
-                    $arg = true;
-                } elseif (($arg == 'FALSE') || ($arg == Calculation::getFALSE())) {
-                    $arg = false;
+                if ($isLiteral && ($arg == 'TRUE' || $arg == Calculation::getTRUE())) {
+                    ++$trueValueCount;
+                } elseif ($isLiteral && ($arg == 'FALSE' || $arg == Calculation::getFALSE())) {
+                    //$trueValueCount += 0;
                 } else {
-                    return ExcelError::VALUE();
+                    --$count;
                 }
-                $trueValueCount += ($arg != 0);
+            } elseif (is_int($arg) || is_float($arg)) {
+                $trueValueCount += (int) ($arg != 0);
+            } else {
+                --$count;
             }
         }
 
-        return $trueValueCount;
+        return ($count === 0) ? ExcelError::VALUE() : $func($trueValueCount, $count);
     }
 }
diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/AndTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/AndTest.php
@@ -18,4 +18,22 @@ public function providerAND(): array
     {
         return require 'tests/data/Calculation/Logical/AND.php';
     }
+
+    /**
+     * @dataProvider providerANDLiteral
+     *
+     * @param mixed $expectedResult
+     * @param string $formula
+     */
+    public function testANDLiteral($expectedResult, $formula): void
+    {
+        $sheet = $this->getSheet();
+        $sheet->getCell('A1')->setValue("=AND($formula)");
+        self::assertSame($expectedResult, $sheet->getCell('A1')->getCalculatedValue());
+    }
+
+    public function providerANDLiteral(): array
+    {
+        return require 'tests/data/Calculation/Logical/ANDLiteral.php';
+    }
 }
diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/OrTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/OrTest.php
@@ -18,4 +18,22 @@ public function providerOR(): array
     {
         return require 'tests/data/Calculation/Logical/OR.php';
     }
+
+    /**
+     * @dataProvider providerORLiteral
+     *
+     * @param mixed $expectedResult
+     * @param string $formula
+     */
+    public function testORLiteral($expectedResult, $formula): void
+    {
+        $sheet = $this->getSheet();
+        $sheet->getCell('A1')->setValue("=OR($formula)");
+        self::assertSame($expectedResult, $sheet->getCell('A1')->getCalculatedValue());
+    }
+
+    public function providerORLiteral(): array
+    {
+        return require 'tests/data/Calculation/Logical/ORLiteral.php';
+    }
 }
diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/XorTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Logical/XorTest.php
@@ -18,4 +18,22 @@ public function providerXOR(): array
     {
         return require 'tests/data/Calculation/Logical/XOR.php';
     }
+
+    /**
+     * @dataProvider providerXORLiteral
+     *
+     * @param mixed $expectedResult
+     * @param string $formula
+     */
+    public function xtestXORLiteral($expectedResult, $formula): void
+    {
+        $sheet = $this->getSheet();
+        $sheet->getCell('A1')->setValue("=XOR($formula)");
+        self::assertSame($expectedResult, $sheet->getCell('A1')->getCalculatedValue());
+    }
+
+    public function providerXORLiteral(): array
+    {
+        return require 'tests/data/Calculation/Logical/XORLiteral.php';
+    }
 }
diff --git a/tests/data/Calculation/Logical/AND.php b/tests/data/Calculation/Logical/AND.php
@@ -4,9 +4,8 @@
     'no arguments' => [
         'exception',
     ],
-    // NULL
-    [
-        false,
+    'only argument is null reference' => [
+        '#VALUE!',
         null,
     ],
     // Boolean TRUE and NULL
@@ -87,28 +86,25 @@
         1,
         1,
     ],
-    [
-        '#VALUE!',
+    'string 1 is ignored' => [
+        true,
         '1',
         1,
     ],
-    // 'TRUE' String
-    [
+    'true string is ignored' => [
         true,
         'TRUE',
         1,
     ],
-    // 'FALSE' String
-    [
-        false,
+    'false string is ignored' => [
+        true,
         'FALSE',
         true,
     ],
-    // Non-numeric String
-    [
-        '#VALUE!',
+    'non-boolean string is ignored' => [
+        false,
         'ABCD',
-        1,
+        0,
     ],
     [
         true,

diff --git a/tests/data/Calculation/Logical/ANDLiteral.php b/tests/data/Calculation/Logical/ANDLiteral.php
@@ -0,0 +1,20 @@
+<?php
+
+return [
+    'both boolean true' => [true, 'true, true'],
+    'boolean and string-true' => [true, 'true, "true"'],
+    'true and non-boolean-string' => [true, 'true, "xyz"'],
+    'non-boolean-string and true' => [true, '"xyz", true'],
+    'empty-string and true' => [true, '"", true'],
+    'non-boolean-string and false' => [false, 'false, "xyz"'],
+    'false and non-boolean-string' => [false, '"xyz", false'],
+    'only non-boolean-string' => ['#VALUE!', '"xyz"'],
+    'only boolean-string' => [true, '"true"'],
+    'numeric-true and true' => [true, '3.1, true, true'],
+    'numeric-false and true' => [false, '0, true, true'],
+    'mixed boolean' => [false, 'true, true, true, false'],
+    'only true' => [true, 'true'],
+    'only false' => [false, 0.0],
+    'boolean in array' => [true, '{true}'],
+    'boolean-string in array' => ['#VALUE!', '{"true"}'],
+];
diff --git a/tests/data/Calculation/Logical/OR.php b/tests/data/Calculation/Logical/OR.php
@@ -4,9 +4,8 @@
     'no arguments' => [
         'exception',
     ],
-    // NULL
-    [
-        false,
+    'only argument is null reference' => [
+        '#VALUE!',
         null,
     ],
     // Boolean TRUE and NULL
@@ -87,21 +86,23 @@
         1,
         1,
     ],
-    // 'TRUE' String
-    [
+    'string 1 is ignored' => [
+        false,
+        0,
+        '1',
+    ],
+    'true string is ignored' => [
         true,
         'TRUE',
         1,
     ],
-    // 'FALSE' String
-    [
+    'false string is ignored' => [
         true,
         'FALSE',
         true,
     ],
-    // Non-numeric String
-    [
-        '#VALUE!',
+    'non-boolean string is ignored' => [
+        true,
         'ABCD',
         1,
     ],

diff --git a/tests/data/Calculation/Logical/ORLiteral.php b/tests/data/Calculation/Logical/ORLiteral.php
@@ -0,0 +1,20 @@
+<?php
+
+return [
+    'both boolean true' => [true, 'true, true'],
+    'boolean or string-true' => [true, 'true, "true"'],
+    'true or non-boolean-string' => [true, 'true, "xyz"'],
+    'non-boolean-string or true' => [true, '"xyz", true'],
+    'empty-string or true' => [true, '"", true'],
+    'non-boolean-string or false' => [false, 'false, "xyz"'],
+    'false or non-boolean-string' => [false, '"xyz", false'],
+    'only non-boolean-string' => ['#VALUE!', '"xyz"'],
+    'only boolean-string' => [true, '"true"'],
+    'numeric-true or true' => [true, '3.1, true, true'],
+    'numeric-false or true' => [true, '0, true, true'],
+    'mixed boolean' => [true, 'true, true, true, false'],
+    'only true' => [true, 'true'],
+    'only false' => [false, 0.0],
+    'boolean in array' => [true, '{true}'],
+    'boolean-string in array' => ['#VALUE!', '{"true"}'],
+];
diff --git a/tests/data/Calculation/Logical/XOR.php b/tests/data/Calculation/Logical/XOR.php
@@ -4,6 +4,10 @@
     'no arguments' => [
         'exception',
     ],
+    'only argument is null reference' => [
+        '#VALUE!',
+        null,
+    ],
     [
         false,
         true, true,
@@ -32,7 +36,7 @@
         true,
         true, true, true, false,
     ],
-    [
+    'ignore string other two should be true' => [
         false,
         'TRUE',
         1,
@@ -44,8 +48,28 @@
         1.5,
         0,
     ],
-    [
+    'only arg is string' => [
         '#VALUE!',
         'HELLO WORLD',
     ],
+    'true string is ignored' => [
+        true,
+        'TRUE',
+        1,
+    ],
+    'false string is ignored' => [
+        true,
+        'FALSE',
+        true,
+    ],
+    'string 1 is ignored' => [
+        true,
+        '1',
+        true,
+    ],
+    'non-boolean string is ignored' => [
+        true,
+        'ABCD',
+        1,
+    ],
 ];