Bitwise Functions and 32-bit (#1900)

* Bitwise Functions and 32-bit

When running the test suite with 32-bit PHP, a failure was reported in BITLSHIFT.
In fact, all of the following are vulnerable to problems, and didn't report
any failures only because of a scarcity of tests:
- BITAND
- BITOR
- BITXOR
- BITRSHIFT
- BITLSHIFT

Those last 2 can be resolved fairly easily by using multiplication by a power of 2
rather than shifting. The first 3 are a tougher nut to crack, and I will continue
to think how they might best be approached. For now, I have added skippable tests
for each of them, which at least documents the problem.

Aside from adding many new tests, some bugs were correctd:
- The function list in Calculation.php pointed BITXOR to BITOR.
- All 5 functions allow null/false/true parameters.
- BIT*SHIFT shift amount must be numeric, can be negative, allows decimal portion
(which is truncated to integer), and has an absolute value limit of 53.
- Because BITRSHIFT allows negative shift amount, its result can overflow
(in which case return NAN).
- All 5 functions disallow negative parameters (except ...SHIFT second parameter).
This was coded, but the code had been thwarted by an earlier is_int test.

* Full Support for AND/OR/XOR on 32-bit

Previous version did not support operands 2**32 through 2**48.

src/PhpSpreadsheet/Calculation/Calculation.php

@@ -448,7 +448,7 @@ class Calculation
         ],
         'BITXOR' => [
             'category' => Category::CATEGORY_ENGINEERING,
-            'functionCall' => [Engineering\BitWise::class, 'BITOR'],
+            'functionCall' => [Engineering\BitWise::class, 'BITXOR'],
             'argumentCount' => '2',
         ],
         'BITLSHIFT' => [

src/PhpSpreadsheet/Calculation/Engineering/BitWise.php

@@ -7,6 +7,18 @@ use PhpOffice\PhpSpreadsheet\Calculation\Functions;
 
 class BitWise
 {
+    const SPLIT_DIVISOR = 2 ** 24;
+
+    /**
+     * Split a number into upper and lower portions for full 32-bit support.
+     *
+     * @param float|int $number
+     */
+    private static function splitNumber($number): array
+    {
+        return [floor($number / self::SPLIT_DIVISOR), fmod($number, self::SPLIT_DIVISOR)];
+    }
+
     /**
      * BITAND.
      *
@@ -28,8 +40,10 @@ class BitWise
         } catch (Exception $e) {
             return $e->getMessage();
         }
+        $split1 = self::splitNumber($number1);
+        $split2 = self::splitNumber($number2);
 
-        return $number1 & $number2;
+        return  self::SPLIT_DIVISOR * ($split1[0] & $split2[0]) + ($split1[1] & $split2[1]);
     }
 
     /**
@@ -54,7 +68,10 @@ class BitWise
             return $e->getMessage();
         }
 
-        return $number1 | $number2;
+        $split1 = self::splitNumber($number1);
+        $split2 = self::splitNumber($number2);
+
+        return  self::SPLIT_DIVISOR * ($split1[0] | $split2[0]) + ($split1[1] | $split2[1]);
     }
 
     /**
@@ -79,7 +96,10 @@ class BitWise
             return $e->getMessage();
         }
 
-        return $number1 ^ $number2;
+        $split1 = self::splitNumber($number1);
+        $split2 = self::splitNumber($number2);
+
+        return  self::SPLIT_DIVISOR * ($split1[0] ^ $split2[0]) + ($split1[1] ^ $split2[1]);
     }
 
     /**
@@ -93,19 +113,18 @@ class BitWise
      * @param int $number
      * @param int $shiftAmount
      *
-     * @return int|string
+     * @return float|int|string
      */
     public static function BITLSHIFT($number, $shiftAmount)
     {
         try {
             $number = self::validateBitwiseArgument($number);
+            $shiftAmount = self::validateShiftAmount($shiftAmount);
         } catch (Exception $e) {
             return $e->getMessage();
         }
 
-        $shiftAmount = Functions::flattenSingleValue($shiftAmount);
+        $result = floor($number * (2 ** $shiftAmount));
-
-        $result = $number << $shiftAmount;
         if ($result > 2 ** 48 - 1) {
             return Functions::NAN();
         }
@@ -124,19 +143,49 @@ class BitWise
      * @param int $number
      * @param int $shiftAmount
      *
-     * @return int|string
+     * @return float|int|string
      */
     public static function BITRSHIFT($number, $shiftAmount)
     {
         try {
             $number = self::validateBitwiseArgument($number);
+            $shiftAmount = self::validateShiftAmount($shiftAmount);
         } catch (Exception $e) {
             return $e->getMessage();
         }
 
-        $shiftAmount = Functions::flattenSingleValue($shiftAmount);
+        $result = floor($number / (2 ** $shiftAmount));
+        if ($result > 2 ** 48 - 1) { // possible because shiftAmount can be negative
+            return Functions::NAN();
+        }
 
-        return $number >> $shiftAmount;
+        return $result;
+    }
+
+    /**
+     * Validate arguments passed to the bitwise functions.
+     *
+     * @param mixed $value
+     *
+     * @return float|int
+     */
+    private static function validateBitwiseArgument($value)
+    {
+        self::nullFalseTrueToNumber($value);
+
+        if (is_numeric($value)) {
+            if ($value == floor($value)) {
+                if (($value > 2 ** 48 - 1) || ($value < 0)) {
+                    throw new Exception(Functions::NAN());
+                }
+
+                return floor($value);
+            }
+
+            throw new Exception(Functions::NAN());
+        }
+
+        throw new Exception(Functions::VALUE());
     }
 
     /**
@@ -146,25 +195,33 @@ class BitWise
      *
      * @return int
      */
-    private static function validateBitwiseArgument($value)
+    private static function validateShiftAmount($value)
     {
-        $value = Functions::flattenSingleValue($value);
+        self::nullFalseTrueToNumber($value);
 
-        if (is_int($value)) {
+        if (is_numeric($value)) {
-            return $value;
+            if (abs($value) > 53) {
-        } elseif (is_numeric($value)) {
+                throw new Exception(Functions::NAN());
-            if ($value == (int) ($value)) {
-                $value = (int) ($value);
-                if (($value > 2 ** 48 - 1) || ($value < 0)) {
-                    throw new Exception(Functions::NAN());
-                }
-
-                return $value;
             }
 
-            throw new Exception(Functions::NAN());
+            return (int) $value;
         }
 
         throw new Exception(Functions::VALUE());
     }
+
+    /**
+     * Many functions accept null/false/true argument treated as 0/0/1.
+     *
+     * @param mixed $number
+     */
+    public static function nullFalseTrueToNumber(&$number): void
+    {
+        $number = Functions::flattenSingleValue($number);
+        if ($number === null) {
+            $number = 0;
+        } elseif (is_bool($number)) {
+            $number = (int) $number;
+        }
+    }
 }

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/BitAndTest.php

@@ -2,26 +2,27 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
-use PhpOffice\PhpSpreadsheet\Calculation\Functions;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class BitAndTest extends TestCase
 {
-    protected function setUp(): void
-    {
-        Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
-    }
-
     /**
      * @dataProvider providerBITAND
      *
      * @param mixed $expectedResult
-     * @param mixed[] $args
      */
-    public function testBITAND($expectedResult, array $args): void
+    public function testBITAND($expectedResult, string $formula): void
     {
-        $result = Engineering::BITAND(...$args);
+        if ($expectedResult === 'exception') {
+            $this->expectException(CalcExp::class);
+        }
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A2', 24);
+        $sheet->getCell('A1')->setValue("=BITAND($formula)");
+        $result = $sheet->getCell('A1')->getCalculatedValue();
         self::assertEquals($expectedResult, $result);
     }
 

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/BitLShiftTest.php

@@ -2,26 +2,27 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
-use PhpOffice\PhpSpreadsheet\Calculation\Functions;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class BitLShiftTest extends TestCase
 {
-    protected function setUp(): void
-    {
-        Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
-    }
-
     /**
      * @dataProvider providerBITLSHIFT
      *
      * @param mixed $expectedResult
-     * @param mixed[] $args
      */
-    public function testBITLSHIFT($expectedResult, array $args): void
+    public function testBITLSHIFT($expectedResult, string $formula): void
     {
-        $result = Engineering::BITLSHIFT(...$args);
+        if ($expectedResult === 'exception') {
+            $this->expectException(CalcExp::class);
+        }
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A2', 8);
+        $sheet->getCell('A1')->setValue("=BITLSHIFT($formula)");
+        $result = $sheet->getCell('A1')->getCalculatedValue();
         self::assertEquals($expectedResult, $result);
     }
 

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/BitOrTest.php

@@ -2,26 +2,27 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
-use PhpOffice\PhpSpreadsheet\Calculation\Functions;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class BitOrTest extends TestCase
 {
-    protected function setUp(): void
-    {
-        Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
-    }
-
     /**
      * @dataProvider providerBITOR
      *
      * @param mixed $expectedResult
-     * @param mixed[] $args
      */
-    public function testBITOR($expectedResult, array $args): void
+    public function testBITOR($expectedResult, string $formula): void
     {
-        $result = Engineering::BITOR(...$args);
+        if ($expectedResult === 'exception') {
+            $this->expectException(CalcExp::class);
+        }
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A2', 8);
+        $sheet->getCell('A1')->setValue("=BITOR($formula)");
+        $result = $sheet->getCell('A1')->getCalculatedValue();
         self::assertEquals($expectedResult, $result);
     }
 

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/BitRShiftTest.php

@@ -2,26 +2,27 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
-use PhpOffice\PhpSpreadsheet\Calculation\Functions;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class BitRShiftTest extends TestCase
 {
-    protected function setUp(): void
-    {
-        Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
-    }
-
     /**
      * @dataProvider providerBITRSHIFT
      *
      * @param mixed $expectedResult
-     * @param mixed[] $args
      */
-    public function testBITRSHIFT($expectedResult, array $args): void
+    public function testBITRSHIFT($expectedResult, string $formula): void
     {
-        $result = Engineering::BITRSHIFT(...$args);
+        if ($expectedResult === 'exception') {
+            $this->expectException(CalcExp::class);
+        }
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A2', 8);
+        $sheet->getCell('A1')->setValue("=BITRSHIFT($formula)");
+        $result = $sheet->getCell('A1')->getCalculatedValue();
         self::assertEquals($expectedResult, $result);
     }
 

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/BitXorTest.php

@@ -2,26 +2,27 @@
 
 namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
 
-use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
-use PhpOffice\PhpSpreadsheet\Calculation\Functions;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PHPUnit\Framework\TestCase;
 
 class BitXorTest extends TestCase
 {
-    protected function setUp(): void
-    {
-        Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
-    }
-
     /**
      * @dataProvider providerBITXOR
      *
      * @param mixed $expectedResult
-     * @param mixed[] $args
      */
-    public function testBITXOR($expectedResult, array $args): void
+    public function testBITXOR($expectedResult, string $formula): void
     {
-        $result = Engineering::BITXOR(...$args);
+        if ($expectedResult === 'exception') {
+            $this->expectException(CalcExp::class);
+        }
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->setCellValue('A2', 8);
+        $sheet->getCell('A1')->setValue("=BITXOR($formula)");
+        $result = $sheet->getCell('A1')->getCalculatedValue();
         self::assertEquals($expectedResult, $result);
     }
 

tests/PhpSpreadsheetTests/Calculation/Functions/Engineering/MovedBitwiseTest.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Engineering;
+
+use PhpOffice\PhpSpreadsheet\Calculation\Engineering;
+use PHPUnit\Framework\TestCase;
+
+// Sanity tests for functions which have been moved out of Engineering
+// to their own classes. A deprecated version remains in Engineering;
+// this class contains cursory tests to ensure that those work properly.
+// If Scrutinizer fails the PR because of these deprecations, I will
+// remove this class from the PR.
+
+class MovedBitwiseTest extends TestCase
+{
+    public function testMovedFunctions(): void
+    {
+        self::assertEquals(1, Engineering::BITAND(1, 3));
+        self::assertEquals(3, Engineering::BITOR(1, 3));
+        self::assertEquals(2, Engineering::BITXOR(1, 3));
+        self::assertEquals(32, Engineering::BITLSHIFT(8, 2));
+        self::assertEquals(2, Engineering::BITRSHIFT(8, 2));
+    }
+}

tests/data/Calculation/Engineering/BITAND.php

@@ -1,20 +1,33 @@
 <?php
 
 return [
-    [
+    [5, '21, 39'],
-        0b101,
+    [64, '200, "84"'],
-        [0b10101, 0b100111],
+    [8, '72.00, 184.00'],
-    ],
+    ['#VALUE!', '"ABC", "DEF"'],
-    [
+    ['#VALUE!', '1, "DEF"'],
-        0b10001000,
+    ['#VALUE!', '"ABC", 1'],
-        [0b11001000, 0b10111000],
+    ['#NUM!', '12.00, 2.82E14'],
-    ],
+    [5123456789, '5123456789, 5123456789'],
-    [
+    [4831908629, '5123456789, 7123456789'],
-        0b00001000,
+    [21, '5123456789, 31'],
-        [0b01001000, 0b10111000],
+    ['#NUM!', '-5123456788, 1'],
-    ],
+    ['#NUM!', 'power(2, 50), 1'], // argument >= 2**48
-    [
+    ['#NUM!', '1, power(2, 50)'], // argument >= 2**48
-        '#VALUE!',
+    ['#NUM!', '-2, 1'], // negative argument
-        ['ABC', 'DEF'],
+    ['#NUM!', '2, -1'], // negative argument
-    ],
+    ['#NUM!', '-2, -1'], // negative argument
+    ['#NUM!', '3.1, 1'], // non-integer argument
+    ['#NUM!', '3, 1.1'], // non-integer argument
+    [0, '4, Q15'],
+    [0, '4, null'],
+    [0, '4, false'],
+    [1, '3, true'],
+    ['exception', ''],
+    ['exception', '2'],
+    [0, ', 4'],
+    [0, 'Q15, 4'],
+    [0, 'false, 4'],
+    [1, 'true, 5'],
+    [8, 'A2, 9'],
 ];

tests/data/Calculation/Engineering/BITLSHIFT.php

@@ -1,20 +1,34 @@
 <?php
 
 return [
-    [
+    [96, '3, 5'],
-        0b1100000,
+    [36, '9, "2"'],
-        [0b11, 5],
+    ['#VALUE!', '"ABC", 5'],
-    ],
+    ['#VALUE!', '5, "ABC"'],
-    [
+    ['#NUM!', '1, 48'], // result too large
-        0b10100,
+    ['#NUM!', '1.1, 2'], // first arg must be integer
-        [0b101, 2],
+    [4, '1, 2.1'], // second arg will be truncated
-    ],
+    ['#NUM!', '0, 54'], // second arg too large
-    [
+    [0, '0, 5'],
-        '#VALUE!',
+    ['#NUM!', '-16, 2'], // first arg can't be negative
-        ['ABC', 5],
+    [1, '4, -2'], // negative shift permitted
-    ],
+    [1, '4, -2.1'], // negative shift and (ignored) fraction permitted
-    [
+    [4, '"4", Q15'],
-        '#NUM!',
+    [4, '4, null'],
-        [0b01, 48],
+    [4, '4, false'],
-    ],
+    [8, '4, true'],
+    ['exception', ''],
+    ['exception', '2'],
+    [0, ', 4'],
+    [0, 'Q15, 4'],
+    [4, '4, q15'],
+    [4, '4, false'],
+    [8, '4, true'],
+    [0, 'false, 4'],
+    [16, 'true, 4'],
+    [16, 'A2, 1'],
+    [8000000000, '1000000000, 3'], // result > 2**32
+    [16000000000, '8000000000, 1'], // argument > 2**32
+    ['#NUM!', 'power(2,50), 1'], // argument >= 2**48
+    ['1', 'power(2, 47), -47'],
 ];

tests/data/Calculation/Engineering/BITOR.php

@@ -1,24 +1,34 @@
 <?php
 
 return [
-    [
+    [55, '21, 39'],
-        0b110111,
+    [248, '200, "184"'],
-        [0b10101, 0b100111],
+    [248, '72, 184'],
-    ],
+    ['#NUM!', '12.34, 56.78'], // non-integer argument
-    [
+    [60, '12.00, 56.00'],
-        0b11111000,
+    ['#VALUE!', '"ABC", "DEF"'],
-        [0b11001000, 0b10111000],
+    ['#VALUE!', '"ABC", 1'],
-    ],
+    ['#VALUE!', '1, "DEF"'],
-    [
+    ['#NUM!', '12.00, 2.82E14'],
-        0b11111000,
+    [5123456789, '5123456788, 1'],
-        [0b01001000, 0b10111000],
+    [7415004949, '5123456789, 7123456789'],
-    ],
+    ['#NUM!', '-5123456788, 1'],
-    [
+    ['#NUM!', 'power(2, 50), 1'], // argument >= 2**48
-        '#NUM!',
+    ['#NUM!', '1, power(2, 50)'], // argument >= 2**48
-        [12.34, 56.78],
+    ['#NUM!', '-2, 1'], // negative argument
-    ],
+    ['#NUM!', '2, -1'], // negative argument
-    [
+    ['#NUM!', '-2, -1'], // negative argument
-        60,
+    ['#NUM!', '3.1, 1'], // non-integer argument
-        [12.00, 56.00],
+    ['#NUM!', '3, 1.1'], // non-integer argument
-    ],
+    [4, '4, Q15'],
+    [4, '4, null'],
+    [4, '4, false'],
+    [5, '4, true'],
+    ['exception', ''],
+    ['exception', '2'],
+    [4, ', 4'],
+    [4, 'Q15, 4'],
+    [4, 'false, 4'],
+    [5, 'true, 4'],
+    [9, 'A2, 1'],
 ];

tests/data/Calculation/Engineering/BITRSHIFT.php

@@ -1,16 +1,35 @@
 <?php
 
 return [
-    [
+    [5, '20, 2'],
-        0b101,
+    [3, '52, "4"'],
-        [0b10100, 2],
+    ['#VALUE!', '"ABC", 5'],
-    ],
+    ['#VALUE!', '5, "ABC"'],
-    [
+    ['#NUM!', '1, -48'], // result too large
-        0b11,
+    ['#NUM!', '1.1, 2'], // first arg must be integer
-        [0b110100, 4],
+    [1, '4, 2.1'], // second arg will be truncated
-    ],
+    ['#NUM!', '0, 54'], // second arg too large
-    [
+    [0, '0, 5'],
-        '#VALUE!',
+    ['#NUM!', '-16, 2'], // first arg can't be negative
-        ['ABC', 5],
+    [4, '1, -2'], // negative shift permitted
-    ],
+    [4, '1, -2.1'], // negative shift and (ignored) fraction permitted
+    [4, '4, Q15'],
+    [4, '4, null'],
+    [4, '4, false'],
+    [2, '4, true'],
+    ['exception', ''],
+    ['exception', '2'],
+    [0, ', 4'],
+    [0, 'Q15, 4'],
+    [4, '4, q15'],
+    [4, '4, false'],
+    [2, '4, true'],
+    [0, 'false, 4'],
+    [0, 'true, 4'],
+    [16, 'true, -4'],
+    [4, 'A2, 1'],
+    [8000000000, '1000000000, -3'], // result > 2**32
+    [8000000000, '16000000000, 1'], // argument > 2**32
+    ['#NUM!', 'power(2,50), 1'], // argument >= 2**48
+    ['1', 'power(2, 47), 47'],
 ];

tests/data/Calculation/Engineering/BITXOR.php

@@ -1,24 +1,32 @@
 <?php
 
 return [
-    [
+    [50, '21, 39'],
-        0b110010,
+    [112, '200, "184"'],
-        [0b10101, 0b100111],
+    [216, '114.00, 170.00'],
-    ],
+    ['#VALUE!', '"ABC", "DEF"'],
-    [
+    ['#VALUE!', '"ABC", 1'],
-        0b01110000,
+    ['#VALUE!', '1, "DEF"'],
-        [0b11001000, 0b10111000],
+    ['#NUM!', '12.00, 2.82E14'],
-    ],
+    [5123456789, '5123456788, 1'],
-    [
+    [2583096320, '5123456789, 7123456789'],
-        0b11011000,
+    ['#NUM!', '-5123456788, 1'],
-        [0b01110010, 0b10101010],
+    ['#NUM!', 'power(2, 50), 1'], // argument >= 2**48
-    ],
+    ['#NUM!', '1, power(2, 50)'], // argument >= 2**48
-    [
+    ['#NUM!', '-2, 1'], // negative argument
-        '#VALUE!',
+    ['#NUM!', '2, -1'], // negative argument
-        ['ABC', 'DEF'],
+    ['#NUM!', '-2, -1'], // negative argument
-    ],
+    ['#NUM!', '3.1, 1'], // non-integer argument
-    [
+    ['#NUM!', '3, 1.1'], // non-integer argument
-        '#NUM!',
+    [4, '4, Q15'],
-        [12.00, 2.82E14],
+    [4, '4, null'],
-    ],
+    [4, '4, false'],
+    [5, '4, true'],
+    ['exception', ''],
+    ['exception', '2'],
+    [4, ', 4'],
+    [4, 'Q15, 4'],
+    [4, 'false, 4'],
+    [5, 'true, 4'],
+    [9, 'A2, 1'],
 ];