Skip to content

Commit

Permalink
bug #3186 Fix twig compare (dpinheiro)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 3.x branch (closes #3186).

Discussion
----------

Fix twig compare

On twig 2.x the comparison between two variables was being done using the php operators (<, >, <=..), although on 3.x the function `twig_compare` was introduced and it had some issues

As an example
`'200' > 399` was returning `false` on twig 2.x, but on 3.x it was returning `true`.

Digging into the code I found out the comparison binaries (`GreaterBinary`, `LessBinary`, etc) were doing the wrong comparison, and the function `twig_compare` was also returning an wrong value if both variables had the same type

Now it's following the convention `-1` if first var is less, 0 if they are equal, or 1 if first var is greater

This is my first PR, so please comment if anything is wrong

Commits
-------

0d81c6e Fix twig compare
  • Loading branch information
fabpot committed Nov 7, 2019
2 parents 083e64f + 0d81c6e commit 3460024
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/Extension/CoreExtension.php
Expand Up @@ -989,7 +989,7 @@ function twig_compare($a, $b)
}

// fallback to <=>
return $b <=> $a;
return $a <=> $b;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/Binary/GreaterBinary.php
Expand Up @@ -24,7 +24,7 @@ public function compile(Compiler $compiler): void
}

$compiler
->raw('-1 === twig_compare(')
->raw('1 === twig_compare(')
->subcompile($this->getNode('left'))
->raw(', ')
->subcompile($this->getNode('right'))
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/Binary/GreaterEqualBinary.php
Expand Up @@ -24,7 +24,7 @@ public function compile(Compiler $compiler): void
}

$compiler
->raw('0 >= twig_compare(')
->raw('0 <= twig_compare(')
->subcompile($this->getNode('left'))
->raw(', ')
->subcompile($this->getNode('right'))
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/Binary/LessBinary.php
Expand Up @@ -24,7 +24,7 @@ public function compile(Compiler $compiler): void
}

$compiler
->raw('1 === twig_compare(')
->raw('-1 === twig_compare(')
->subcompile($this->getNode('left'))
->raw(', ')
->subcompile($this->getNode('right'))
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/Binary/LessEqualBinary.php
Expand Up @@ -24,7 +24,7 @@ public function compile(Compiler $compiler): void
}

$compiler
->raw('0 <= twig_compare(')
->raw('0 >= twig_compare(')
->subcompile($this->getNode('left'))
->raw(', ')
->subcompile($this->getNode('right'))
Expand Down
10 changes: 7 additions & 3 deletions tests/Extension/CoreTest.php
Expand Up @@ -285,10 +285,10 @@ public function provideCompareCases()

[0, '0', '0'],
[0, '0', '0.0'],
[1, '0', 'foo'],
[-1, '0', ''],
[-1, '0', 'foo'],
[1, '0', ''],
[0, '42', ' 42'],
[1, '42', '42foo'],
[-1, '42', '42foo'],

[0, 42, '000042'],
[0, 42, '42.0'],
Expand All @@ -310,6 +310,10 @@ public function provideCompareCases()
[0, -INF, '-INF'],
[0, INF, '1e1000'],
[0, -INF, '-1e1000'],

[-1, 10, 20],
[-1, '10', 20],
[-1, 10, '20'],
];
}
}
Expand Down

0 comments on commit 3460024

Please sign in to comment.