diff --git a/src/Filter/Renderer.php b/src/Filter/Renderer.php index 513470ec..996cc948 100644 --- a/src/Filter/Renderer.php +++ b/src/Filter/Renderer.php @@ -32,9 +32,9 @@ public function __construct(Filter\Rule $filter) * * @return $this */ - public function setStrict($strict = true) + public function setStrict(bool $strict = true): self { - $this->strict = (bool) $strict; + $this->strict = $strict; return $this; } @@ -44,7 +44,7 @@ public function setStrict($strict = true) * * @return string */ - public function render() + public function render(): string { if ($this->string !== null) { return $this->string; @@ -54,7 +54,9 @@ public function render() $filter = $this->filter; if ($filter instanceof Filter\Chain) { - $this->renderChain($filter, $this->strict); + if ($this->strict || ! $filter->isEmpty()) { + $this->renderChain($filter, $this->strict); + } } else { /** @var Filter\Condition $filter */ $this->renderCondition($filter); @@ -71,12 +73,8 @@ public function render() * * @return void */ - protected function renderChain(Filter\Chain $chain, $wrap = false) + protected function renderChain(Filter\Chain $chain, bool $wrap = false): void { - if (! $this->strict && $chain->isEmpty()) { - return; - } - $chainOperator = null; switch (true) { case $chain instanceof Filter\All: @@ -107,13 +105,15 @@ protected function renderChain(Filter\Chain $chain, $wrap = false) foreach ($chain as $rule) { if ($rule instanceof Filter\Chain) { - $this->renderChain($rule, $this->strict || $rule->count() > 1); + if ($this->strict || ! $rule->isEmpty()) { + $this->renderChain($rule, $this->strict || $rule->count() > 1); + $this->string .= $chainOperator; + } } else { /** @var Filter\Condition $rule */ $this->renderCondition($rule); + $this->string .= $chainOperator; } - - $this->string .= $chainOperator; } if (! $chain->isEmpty() && (! $this->strict || ! ($chain instanceof Filter\Any && $chain->count() === 1))) { @@ -137,7 +137,7 @@ protected function renderChain(Filter\Chain $chain, $wrap = false) * * @return void */ - protected function renderCondition(Filter\Condition $condition) + protected function renderCondition(Filter\Condition $condition): void { $value = $condition->getValue(); if (is_bool($value) && ! $value) { diff --git a/tests/FilterTest.php b/tests/FilterTest.php index d1de3ea9..b7a6481f 100644 --- a/tests/FilterTest.php +++ b/tests/FilterTest.php @@ -687,4 +687,86 @@ public function testParserRespectsRedundantCharsInStrictMode() "Filter\Parser doesn't respect group operator for empty nested OR chains with non-empty siblings" ); } + + /* Non-Strict mode tests */ + + public function testRendererDoesNotDrawRedundantCharsInNonStrictMode() + { + $this->assertEquals( + '', + (new Renderer(Filter::all()))->render(), + "Filter\Renderer draws redundant parentheses for an empty root chain" + ); + $this->assertEquals( + 'foo=bar', + (new Renderer(Filter::equal('foo', 'bar')))->render(), + "Filter\Renderer draws redundant parentheses for a root chain with a single condition" + ); + $this->assertEquals( + 'foo=bar&bar=foo', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::equal('bar', 'foo') + )))->render(), + "Filter\Renderer draws redundant parentheses for a root chain with multiple conditions" + ); + $this->assertEquals( + 'foo=bar&bar=foo', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::all( + Filter::equal('bar', 'foo') + ) + )))->render(), + "Filter\Renderer draws redundant parentheses for nested chains with a single condition" + ); + $this->assertEquals( + 'foo=bar&bar=foo', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::any( + Filter::equal('bar', 'foo') + ) + )))->render(), + "Filter\Renderer draws redundant group operator for nested OR chains with a single condition" + ); + $this->assertEquals( + 'foo=bar', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::all() + )))->render(), + "Filter\Renderer draws redundant parentheses for empty nested chains" + ); + $this->assertEquals( + 'foo=bar&(bar=foo)', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::all( + Filter::all(), + Filter::equal('bar', 'foo') + ) + )))->render(), + "Filter\Renderer draws redundant parentheses for empty nested chains with non-empty siblings" + ); + $this->assertEquals( + 'foo=bar', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::any() + )))->render(), + "Filter\Renderer draws redundant group operator for empty nested OR chains" + ); + $this->assertEquals( + 'foo=bar&(bar=foo)', + (new Renderer(Filter::all( + Filter::equal('foo', 'bar'), + Filter::any( + Filter::any(), + Filter::equal('bar', 'foo') + ) + )))->render(), + "Filter\Renderer draws redundant group operator for empty nested OR chains with non-empty siblings" + ); + } }