AutoFilter Improvements (PHPOffice#2393)

* AutoFilter Improvements Fix issue PHPOffice#2378. The following changes are made: - NotEqual tests must be part of a custom filter. Documentation has been changed to indicate that. - Method setAndOr was replaced by setJoin some time ago. Documentation now reflects that change. - Documentation to indicate that string filters are not case-sensitive, same as in Excel. - Filters testing against numeric value now include a numeric test (not numeric for not equal, numeric for all others). - String filter had previously treated everything as a test for "equal". It now handles "not equal" and the variants of "greater/less" with or without "equal". - Documentation correctly stated that no more than 2 rules are allowed in a custom filter. Code did not enforce this restriction. It now does, throwing an exception if an attempt is made to add a third rule. - Deleted a lot of comments in Rule.php to make it easier to see what is not yet implemented (between, begins with, etc.). I may take these on in future. - Added a number of tests for the new functionality. * Not Sure Why Phpstan Results Differ Local vs Github Let's see if this change suffices. * Phpstan Still Not sure how to convince it. Let's try this. * Phpstan Solved Figured out the problem on my local machine. Expect this to work.
oleibman · Nov 16, 2021 · 2a12587 · 2a12587
1 parent 52585a9
commit 2a12587
Show file tree

Hide file tree

Showing 7 changed files with 392 additions and 76 deletions.
diff --git a/docs/topics/autofilters.md b/docs/topics/autofilters.md
@@ -99,6 +99,8 @@ results are unpredictable.
 Other filter expression types (such as cell colour filters) are not yet
 supported.
 
+String comparisons in filters are case-insensitive.
+
 ### Simple filters
 
 In MS Excel, Simple Filters are a dropdown list of all values used in
@@ -113,6 +115,8 @@ will be hidden.
 To create a filter expression, we need to start by identifying the
 filter type. In this case, we're just going to specify that this filter
 is a standard filter.
+*Please note that Excel regards only tests for equal as a standard filter;
+all others, including tests for not equal, must be supplied as custom filters.*
 
 ```php
 $columnFilter->setFilterType(
@@ -255,6 +259,7 @@ MS Excel uses `*` as a wildcard to match any number of characters, and `?`
 as a wildcard to match a single character. `U*` equates to "begins with
 a 'U'"; `*U` equates to "ends with a 'U'"; and `*U*` equates to
 "contains a 'U'".
+Note that PhpSpreadsheet recognizes wildcards only for equal/not-equal tests.
 
 If you want to match explicitly against `*` or `?`, you can
 escape it with a tilde `~`, so `?~**` would explicitly match for `*`
@@ -290,8 +295,8 @@ This defined two rules, filtering numbers that are `>= -20` OR `<=
 than OR.
 
 ```php
-$columnFilter->setAndOr(
-    \PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column::AUTOFILTER_COLUMN_ANDOR_AND
+$columnFilter->setJoin(
+    \PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column::AUTOFILTER_COLUMN_JOIN_AND
 );
 ```
 
@@ -501,7 +506,7 @@ hiding all other rows within the autofilter area.
 ### Displaying Filtered Rows
 
 Simply looping through the rows in an autofilter area will still access
-ever row, whether it matches the filter criteria or not. To selectively
+every row, whether it matches the filter criteria or not. To selectively
 access only the filtered rows, you need to test each row’s visibility
 settings.
 

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -6052,7 +6052,7 @@ parameters:
 
 		-
 			message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#"
-			count: 2
+			count: 1
 			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
 
 		-
@@ -6065,21 +6065,11 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
 
-		-
-			message: "#^Cannot access offset 'operator' on mixed\\.$#"
-			count: 2
-			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
-
 		-
 			message: "#^Cannot access offset 'time' on mixed\\.$#"
 			count: 1
 			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
 
-		-
-			message: "#^Cannot access offset 'value' on mixed\\.$#"
-			count: 9
-			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
-
 		-
 			message: "#^Cannot use array destructuring on mixed\\.$#"
 			count: 1
@@ -6120,11 +6110,6 @@ parameters:
 			count: 1
 			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
 
-		-
-			message: "#^Parameter \\#2 \\$subject of function preg_match expects string, mixed given\\.$#"
-			count: 1
-			path: src/PhpSpreadsheet/Worksheet/AutoFilter.php
-
 		-
 			message: "#^Parameter \\#3 \\$length of function array_slice expects int\\|null, mixed given\\.$#"
 			count: 1

diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter.php b/src/PhpSpreadsheet/Worksheet/AutoFilter.php
@@ -345,6 +345,7 @@ private static function filterTestInDateGroupSet($cellValue, $dataSet)
      */
     private static function filterTestInCustomDataSet($cellValue, $ruleSet)
     {
+        /** @var array[] */
         $dataSet = $ruleSet['filterRules'];
         $join = $ruleSet['join'];
         $customRuleForBlanks = $ruleSet['customRuleForBlanks'] ?? false;
@@ -357,38 +358,45 @@ private static function filterTestInCustomDataSet($cellValue, $ruleSet)
         }
         $returnVal = ($join == AutoFilter\Column::AUTOFILTER_COLUMN_JOIN_AND);
         foreach ($dataSet as $rule) {
+            /** @var string */
+            $ruleValue = $rule['value'];
+            /** @var string */
+            $ruleOperator = $rule['operator'];
+            /** @var string */
+            $cellValueString = $cellValue;
             $retVal = false;
 
-            if (is_numeric($rule['value'])) {
+            if (is_numeric($ruleValue)) {
                 //    Numeric values are tested using the appropriate operator
-                switch ($rule['operator']) {
+                $numericTest = is_numeric($cellValue);
+                switch ($ruleOperator) {
                     case Rule::AUTOFILTER_COLUMN_RULE_EQUAL:
-                        $retVal = ($cellValue == $rule['value']);
+                        $retVal = $numericTest && ($cellValue == $ruleValue);
 
                         break;
                     case Rule::AUTOFILTER_COLUMN_RULE_NOTEQUAL:
-                        $retVal = ($cellValue != $rule['value']);
+                        $retVal = !$numericTest || ($cellValue != $ruleValue);
 
                         break;
                     case Rule::AUTOFILTER_COLUMN_RULE_GREATERTHAN:
-                        $retVal = ($cellValue > $rule['value']);
+                        $retVal = $numericTest && ($cellValue > $ruleValue);
 
                         break;
                     case Rule::AUTOFILTER_COLUMN_RULE_GREATERTHANOREQUAL:
-                        $retVal = ($cellValue >= $rule['value']);
+                        $retVal = $numericTest && ($cellValue >= $ruleValue);
 
                         break;
                     case Rule::AUTOFILTER_COLUMN_RULE_LESSTHAN:
-                        $retVal = ($cellValue < $rule['value']);
+                        $retVal = $numericTest && ($cellValue < $ruleValue);
 
                         break;
                     case Rule::AUTOFILTER_COLUMN_RULE_LESSTHANOREQUAL:
-                        $retVal = ($cellValue <= $rule['value']);
+                        $retVal = $numericTest && ($cellValue <= $ruleValue);
 
                         break;
                 }
-            } elseif ($rule['value'] == '') {
-                switch ($rule['operator']) {
+            } elseif ($ruleValue == '') {
+                switch ($ruleOperator) {
                     case Rule::AUTOFILTER_COLUMN_RULE_EQUAL:
                         $retVal = (($cellValue == '') || ($cellValue === null));
 
@@ -404,7 +412,32 @@ private static function filterTestInCustomDataSet($cellValue, $ruleSet)
                 }
             } else {
                 //    String values are always tested for equality, factoring in for wildcards (hence a regexp test)
-                $retVal = preg_match('/^' . $rule['value'] . '$/i', $cellValue);
+                switch ($ruleOperator) {
+                    case Rule::AUTOFILTER_COLUMN_RULE_EQUAL:
+                        $retVal = (bool) preg_match('/^' . $ruleValue . '$/i', $cellValueString);
+
+                        break;
+                    case Rule::AUTOFILTER_COLUMN_RULE_NOTEQUAL:
+                        $retVal = !((bool) preg_match('/^' . $ruleValue . '$/i', $cellValueString));
+
+                        break;
+                    case Rule::AUTOFILTER_COLUMN_RULE_GREATERTHAN:
+                        $retVal = strcasecmp($cellValueString, $ruleValue) > 0;
+
+                        break;
+                    case Rule::AUTOFILTER_COLUMN_RULE_GREATERTHANOREQUAL:
+                        $retVal = strcasecmp($cellValueString, $ruleValue) >= 0;
+
+                        break;
+                    case Rule::AUTOFILTER_COLUMN_RULE_LESSTHAN:
+                        $retVal = strcasecmp($cellValueString, $ruleValue) < 0;
+
+                        break;
+                    case Rule::AUTOFILTER_COLUMN_RULE_LESSTHANOREQUAL:
+                        $retVal = strcasecmp($cellValueString, $ruleValue) <= 0;
+
+                        break;
+                }
             }
             //    If there are multiple conditions, then we need to test both using the appropriate join operator
             switch ($join) {
@@ -840,7 +873,7 @@ public function showHideRows()
 
                     break;
                 case AutoFilter\Column::AUTOFILTER_FILTERTYPE_CUSTOMFILTER:
-                    $customRuleForBlanks = false;
+                    $customRuleForBlanks = true;
                     $ruleValues = [];
                     //    Build a list of the filter value selections
                     foreach ($rules as $rule) {

diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php b/src/PhpSpreadsheet/Worksheet/AutoFilter/Column.php
@@ -176,6 +176,9 @@ public function setFilterType($filterType)
         if (!in_array($filterType, self::$filterTypes)) {
             throw new PhpSpreadsheetException('Invalid filter type for column AutoFilter.');
         }
+        if ($filterType === self::AUTOFILTER_FILTERTYPE_CUSTOMFILTER && count($this->ruleset) > 2) {
+            throw new PhpSpreadsheetException('No more than 2 rules are allowed in a Custom Filter');
+        }
 
         $this->filterType = $filterType;
 
@@ -305,6 +308,9 @@ public function getRule($index)
      */
     public function createRule()
     {
+        if ($this->filterType === self::AUTOFILTER_FILTERTYPE_CUSTOMFILTER && count($this->ruleset) >= 2) {
+            throw new PhpSpreadsheetException('No more than 2 rules are allowed in a Custom Filter');
+        }
         $this->ruleset[] = new Column\Rule($this);
 
         return end($this->ruleset);

diff --git a/src/PhpSpreadsheet/Worksheet/AutoFilter/Column/Rule.php b/src/PhpSpreadsheet/Worksheet/AutoFilter/Column/Rule.php
@@ -125,15 +125,7 @@ class Rule
         self::AUTOFILTER_RULETYPE_DYNAMIC_BELOWAVERAGE,
     ];
 
-    /*
-     *    The only valid filter rule operators for filter and customFilter types are:
-     *        <xsd:enumeration value="equal"/>
-     *        <xsd:enumeration value="lessThan"/>
-     *        <xsd:enumeration value="lessThanOrEqual"/>
-     *        <xsd:enumeration value="notEqual"/>
-     *        <xsd:enumeration value="greaterThanOrEqual"/>
-     *        <xsd:enumeration value="greaterThan"/>
-     */
+    // Filter rule operators for filter and customFilter types.
     const AUTOFILTER_COLUMN_RULE_EQUAL = 'equal';
     const AUTOFILTER_COLUMN_RULE_NOTEQUAL = 'notEqual';
     const AUTOFILTER_COLUMN_RULE_GREATERTHAN = 'greaterThan';
@@ -166,39 +158,17 @@ class Rule
         self::AUTOFILTER_COLUMN_RULE_TOPTEN_BOTTOM,
     ];
 
-    // Rule Operators (Numeric, Boolean etc)
-//    const AUTOFILTER_COLUMN_RULE_BETWEEN            = 'between';        //    greaterThanOrEqual 1 && lessThanOrEqual 2
+    //  Unimplented Rule Operators (Numeric, Boolean etc)
+    //    const AUTOFILTER_COLUMN_RULE_BETWEEN = 'between';        //    greaterThanOrEqual 1 && lessThanOrEqual 2
     // Rule Operators (Numeric Special) which are translated to standard numeric operators with calculated values
-//    const AUTOFILTER_COLUMN_RULE_TOPTEN                = 'topTen';            //    greaterThan calculated value
-//    const AUTOFILTER_COLUMN_RULE_TOPTENPERCENT        = 'topTenPercent';    //    greaterThan calculated value
-//    const AUTOFILTER_COLUMN_RULE_ABOVEAVERAGE        = 'aboveAverage';    //    Value is calculated as the average
-//    const AUTOFILTER_COLUMN_RULE_BELOWAVERAGE        = 'belowAverage';    //    Value is calculated as the average
     // Rule Operators (String) which are set as wild-carded values
-//    const AUTOFILTER_COLUMN_RULE_BEGINSWITH            = 'beginsWith';            // A*
-//    const AUTOFILTER_COLUMN_RULE_ENDSWITH            = 'endsWith';            // *Z
-//    const AUTOFILTER_COLUMN_RULE_CONTAINS            = 'contains';            // *B*
-//    const AUTOFILTER_COLUMN_RULE_DOESNTCONTAIN        = 'notEqual';            //    notEqual *B*
+    //    const AUTOFILTER_COLUMN_RULE_BEGINSWITH            = 'beginsWith';            // A*
+    //    const AUTOFILTER_COLUMN_RULE_ENDSWITH            = 'endsWith';            // *Z
+    //    const AUTOFILTER_COLUMN_RULE_CONTAINS            = 'contains';            // *B*
+    //    const AUTOFILTER_COLUMN_RULE_DOESNTCONTAIN        = 'notEqual';            //    notEqual *B*
     // Rule Operators (Date Special) which are translated to standard numeric operators with calculated values
-//    const AUTOFILTER_COLUMN_RULE_BEFORE                = 'lessThan';
-//    const AUTOFILTER_COLUMN_RULE_AFTER                = 'greaterThan';
-//    const AUTOFILTER_COLUMN_RULE_YESTERDAY            = 'yesterday';
-//    const AUTOFILTER_COLUMN_RULE_TODAY                = 'today';
-//    const AUTOFILTER_COLUMN_RULE_TOMORROW            = 'tomorrow';
-//    const AUTOFILTER_COLUMN_RULE_LASTWEEK            = 'lastWeek';
-//    const AUTOFILTER_COLUMN_RULE_THISWEEK            = 'thisWeek';
-//    const AUTOFILTER_COLUMN_RULE_NEXTWEEK            = 'nextWeek';
-//    const AUTOFILTER_COLUMN_RULE_LASTMONTH            = 'lastMonth';
-//    const AUTOFILTER_COLUMN_RULE_THISMONTH            = 'thisMonth';
-//    const AUTOFILTER_COLUMN_RULE_NEXTMONTH            = 'nextMonth';
-//    const AUTOFILTER_COLUMN_RULE_LASTQUARTER        = 'lastQuarter';
-//    const AUTOFILTER_COLUMN_RULE_THISQUARTER        = 'thisQuarter';
-//    const AUTOFILTER_COLUMN_RULE_NEXTQUARTER        = 'nextQuarter';
-//    const AUTOFILTER_COLUMN_RULE_LASTYEAR            = 'lastYear';
-//    const AUTOFILTER_COLUMN_RULE_THISYEAR            = 'thisYear';
-//    const AUTOFILTER_COLUMN_RULE_NEXTYEAR            = 'nextYear';
-//    const AUTOFILTER_COLUMN_RULE_YEARTODATE            = 'yearToDate';            //    <dynamicFilter val="40909" type="yearToDate" maxVal="41113"/>
-//    const AUTOFILTER_COLUMN_RULE_ALLDATESINMONTH    = 'allDatesInMonth';    //    <dynamicFilter type="M2"/> for Month/February
-//    const AUTOFILTER_COLUMN_RULE_ALLDATESINQUARTER    = 'allDatesInQuarter';    //    <dynamicFilter type="Q2"/> for Quarter 2
+    //    const AUTOFILTER_COLUMN_RULE_BEFORE                = 'lessThan';
+    //    const AUTOFILTER_COLUMN_RULE_AFTER                = 'greaterThan';
 
     /**
      * Autofilter Column.