-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add ranges support #14
base: master
Are you sure you want to change the base?
Changes from 3 commits
6af5e8b
285543d
a7c3867
771b486
f3d3f26
e9ca9e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
<?php | ||
|
||
namespace QueryTranslator\Languages\Galach\Generators\Native; | ||
|
||
use LogicException; | ||
use QueryTranslator\Languages\Galach\Generators\Common\Visitor; | ||
use QueryTranslator\Languages\Galach\Values\Node\Term; | ||
use QueryTranslator\Languages\Galach\Values\Token\Range as RangeToken; | ||
use QueryTranslator\Values\Node; | ||
|
||
/** | ||
* Range Node Visitor implementation. | ||
*/ | ||
final class Range extends Visitor | ||
{ | ||
public function accept(Node $node) | ||
{ | ||
return $node instanceof Term && $node->token instanceof RangeToken; | ||
} | ||
|
||
public function visit(Node $node, Visitor $subVisitor = null, $options = null) | ||
{ | ||
if (!$node instanceof Term) { | ||
throw new LogicException( | ||
'Implementation accepts instance of Term Node' | ||
); | ||
} | ||
|
||
$token = $node->token; | ||
|
||
if (!$token instanceof RangeToken) { | ||
throw new LogicException( | ||
'Implementation accepts instance of Range Token' | ||
); | ||
} | ||
|
||
$domainPrefix = '' === $token->domain ? '' : "{$token->domain}:"; | ||
|
||
switch ($token->type) { | ||
case RangeToken::TYPE_INCLUSIVE: | ||
return $domainPrefix . '[' . $token->rangeFrom . ' TO ' . $token->rangeTo . ']'; | ||
|
||
case RangeToken::TYPE_EXCLUSIVE: | ||
return $domainPrefix . '{' . $token->rangeFrom . ' TO ' . $token->rangeTo . '}'; | ||
|
||
default: | ||
throw new LogicException(sprintf('Range type %s is not supported', $token->type)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
<?php | ||
|
||
namespace QueryTranslator\Languages\Galach\Values\Token; | ||
|
||
use QueryTranslator\Languages\Galach\Tokenizer; | ||
use QueryTranslator\Values\Token; | ||
|
||
/** | ||
* Range term token. | ||
* | ||
* @see \QueryTranslator\Languages\Galach\Tokenizer::TOKEN_TERM | ||
*/ | ||
final class Range extends Token | ||
{ | ||
const TYPE_INCLUSIVE = 'inclusive'; | ||
const TYPE_EXCLUSIVE = 'exclusive'; | ||
|
||
/** | ||
* Holds domain string. | ||
* | ||
* @var string | ||
*/ | ||
public $domain; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
public $rangeFrom; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
public $rangeTo; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
public $type; | ||
|
||
/** | ||
* @param string $lexeme | ||
* @param int $position | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No alignment in PHPDoc, it unnecessarily adds to diff when updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just run the php-cs-fixer with the current settings, I guess it was never run before on the code. |
||
* @param string $domain | ||
* @param string $rangeFrom | ||
* @param string $rangeTo | ||
* @param string $type | ||
*/ | ||
public function __construct($lexeme, $position, $domain, $rangeFrom, $rangeTo, $type) | ||
{ | ||
if (!in_array($type, [self::TYPE_EXCLUSIVE, self::TYPE_INCLUSIVE])) { | ||
throw new \InvalidArgumentException(sprintf('Invalid range type: %s', $type)); | ||
} | ||
|
||
parent::__construct(Tokenizer::TOKEN_TERM, $lexeme, $position); | ||
|
||
$this->domain = $domain; | ||
$this->rangeFrom = $rangeFrom; | ||
$this->rangeTo = $rangeTo; | ||
$this->type = $type; | ||
} | ||
|
||
/** | ||
* Returns the range type, given the starting symbol. | ||
* | ||
* @param string $startSymbol the start symbol, either '[' or '{' | ||
* | ||
* @return string | ||
*/ | ||
public static function getTypeByStart($startSymbol) | ||
{ | ||
if ('[' === $startSymbol) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having this check here means if someone customizes the symbol, this class will also need to be changed. So it should be rather done outside, in the |
||
return self::TYPE_INCLUSIVE; | ||
} | ||
|
||
if ('{' === $startSymbol) { | ||
return self::TYPE_EXCLUSIVE; | ||
} | ||
|
||
throw new \InvalidArgumentException(sprintf('Invalid range start symbol: %s', $startSymbol)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
<?php | ||
|
||
namespace QueryTranslator\Tests\Galach\Generators\Native; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use QueryTranslator\Languages\Galach\Generators\Common\Visitor; | ||
use QueryTranslator\Languages\Galach\Generators\Native\Range; | ||
use QueryTranslator\Languages\Galach\Values\Node\Mandatory; | ||
use QueryTranslator\Languages\Galach\Values\Node\Term; | ||
use QueryTranslator\Languages\Galach\Values\Token\Range as RangeToken; | ||
use QueryTranslator\Languages\Galach\Values\Token\Word; | ||
use QueryTranslator\Values\Node; | ||
|
||
class RangeTest extends TestCase | ||
{ | ||
/** | ||
* @var Visitor | ||
*/ | ||
public $visitor; | ||
|
||
protected function setUp() | ||
{ | ||
$this->visitor = new Range(); | ||
} | ||
|
||
public function acceptDataprovider() | ||
{ | ||
return [ | ||
[true, new Term(new RangeToken('[a TO b]', 0, '', 'a', 'b', 'inclusive'))], | ||
[false, new Term(new Word('word', 0, '', 'a'))], | ||
]; | ||
} | ||
|
||
/** | ||
* @param bool $expected | ||
* @param Node $token | ||
* | ||
* @dataProvider acceptDataprovider | ||
*/ | ||
public function testAccepts($expected, $node) | ||
{ | ||
$this->assertSame($expected, $this->visitor->accept($node)); | ||
} | ||
|
||
public function visitDataprovider() | ||
{ | ||
return [ | ||
['[a TO b]', new Term(new RangeToken('[a TO b]', 0, '', 'a', 'b', 'inclusive'))], | ||
['{a TO b}', new Term(new RangeToken('{a TO b}', 0, '', 'a', 'b', 'exclusive'))], | ||
]; | ||
} | ||
|
||
/** | ||
* @param string $expected | ||
* @param Node $token | ||
* | ||
* @dataProvider visitDataprovider | ||
*/ | ||
public function testVisit($expected, $node) | ||
{ | ||
$this->assertSame($expected, $this->visitor->visit($node)); | ||
} | ||
|
||
public function visitWrongNodeDataprovider() | ||
{ | ||
return [ | ||
[new Mandatory()], | ||
[new Term(new Word('word', 0, '', 'a'))], | ||
]; | ||
} | ||
|
||
/** | ||
* @param string $expected | ||
* @param Node $token | ||
* | ||
* @dataProvider visitWrongNodeDataprovider | ||
*/ | ||
public function testVisitWrongNodeFails($node) | ||
{ | ||
$this->expectException(\LogicException::class); | ||
$this->visitor->visit($node); | ||
} | ||
|
||
public function testVisitUnknownTypeFails() | ||
{ | ||
$this->expectException(\LogicException::class); | ||
$node = new Term(new RangeToken('{a TO b}', 0, '', 'a', 'b', 'unknown')); | ||
$this->visitor->visit($node); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
<?php | ||
|
||
namespace QueryTranslator\Tests\Galach\Values\Token; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use QueryTranslator\Languages\Galach\Values\Token\Range; | ||
|
||
class RangeTest extends TestCase | ||
{ | ||
public function failingStartSymbolDataprovider() | ||
{ | ||
return [ | ||
[''], | ||
['/'], | ||
['('], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider failingStartSymbolDataprovider | ||
* @param string $startSymbol | ||
*/ | ||
public function testGetTypeByStartFails($startSymbol) | ||
{ | ||
$this->expectException(\InvalidArgumentException::class); | ||
Range::getTypeByStart($startSymbol); | ||
} | ||
|
||
public function successfulStartSymbolDataprovider() | ||
{ | ||
return [ | ||
['inclusive', '['], | ||
['exclusive', '{'], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider successfulStartSymbolDataprovider | ||
* @param string $expectedType | ||
* @param string $startSymbol | ||
*/ | ||
public function testGetTypeByStartSucceeds($expectedType, $startSymbol) | ||
{ | ||
$this->assertSame($expectedType, Range::getTypeByStart($startSymbol)); | ||
} | ||
|
||
public function failingTypeDataprovider() | ||
{ | ||
return [ | ||
[''], | ||
[null], | ||
['other'], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider failingTypeDataprovider | ||
* @param string $type | ||
*/ | ||
public function testConstructorFailsWrongType($type) | ||
{ | ||
$this->expectException(\InvalidArgumentException::class); | ||
new Range('[a TO b]', 0, '', 'a', 'b', $type); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Symbols should be captured by the expression and contained in the token, so if they are customized this class does not need to know about it. And we should support mixed case as well,
{a TO b]
and[a TO b}
.It will probably mean a truckload of constructor arguments, but I'm OK with that :)