pharborist icon indicating copy to clipboard operation
pharborist copied to clipboard

Add getCase() method to SwitchNode

Open phenaproxima opened this issue 11 years ago • 5 comments

If I have this:

switch ($wangdoodle) { case 'emu': return 0; case 'kangaroo': return 1; case 'koala': return -33; default: return 'har'; }

And I call $switchNode->getCase('kangaroo'), I should get a CaseNode for that single case. Should also be able to get the default case with $switchNode->getDefaultCase().

phenaproxima avatar Aug 18 '14 23:08 phenaproxima

Issue here is case is followed by an expression, not just strings.

$a = 2;
switch ($a) {
  case 1 + 1:
    echo 'window', PHP_EOL;
    break;
}

To further help with this issue. What do you wish to do once you have the CaseNode?

Just also considering the case of fallthrough, etc. At the moment pharborist attaches statements to the last case it found. For example:

switch ($a) {
  case 'one':
  case 'two':
    $b = 'foo';
    break;
}

Then the $b = 'foo'; statement belongs to the case two node.

grom358 avatar Aug 19 '14 01:08 grom358

In the case of matching expressions, perhaps SwitchNode::getCase() could accept either a string, a StringNode, a T_STRING, or an ExpressionNode to check against. That kind of polymorphism is in keeping with the jQuery tradition, but it might be too complex to bother with.

Once I have the CaseNode, I imagine a use case would be extracting the logic it contains and moving it into a new function (for example, to convert implementations of hook_block_view() in DMU).

As far as handling fall-through goes...I vote that getCase() should NOT (at least not by default) return the fall-through case logic. The reason being that fall-through is run-time logic, not a matter of syntax, and Pharborist, being a parser, is concerned with what's been written down, not what actually ends up executing.

Perhaps getCase() could have a second optional boolean argument that, if TRUE, would return the fall-through case.

phenaproxima avatar Aug 19 '14 01:08 phenaproxima

Honestly, though...for the time being, this is hardly essential. Right now, DMU just avoids handling any kind of if/switch logic. Being able to walk through a switch-case like this would be an act of heroism on your part, as far as I'm concerned.

If you actually do decide to implement this, handling only string cases is fine in the interim (I'd be using this first for hook_block_view() conversion, which takes a string delta and nothing else).

phenaproxima avatar Aug 19 '14 02:08 phenaproxima

Marked as todo. For time being, something like:

$matchOn = "'two'";
$match = $switchNode->children(function ($node) use ($matchOn) {
  if ($node instanceof CaseNode::class) {
    return $node->getMatchOn()->getText() == (string) $matchOn;
  }
  return FALSE;
});
if ($match->count() > 0) {
  $caseNode = $match[0];
  // do something with caseNode
}

grom358 avatar Aug 19 '14 06:08 grom358

Thanks much! As I mentioned, this is more of a "nice to have a bit later" thing than a "need right now", so I can wait. :)

phenaproxima avatar Aug 19 '14 11:08 phenaproxima