$parse: the iseccst minerr is never thrown

[thorn0]

I accidentally stumbled upon this code in $parse:

function getStringValue(name, fullExpression) {
  // From the JavaScript docs:
  // Property names must be strings. This means that non-string objects cannot be used
  // as keys in an object. Any non-string object, including a number, is typecasted
  // into a string via the toString method.
  //
  // So, to ensure that we are checking the same `name` that JavaScript would use,
  // we cast it to a string, if possible.
  // Doing `name + ''` can cause a repl error if the result to `toString` is not a string,
  // this is, this will handle objects that misbehave.
  name = name + '';
  if (!isString(name)) {
    throw $parseMinErr('iseccst',
        'Cannot convert object to primitive value! '
        + 'Expression: {0}', fullExpression);
  }
  return name;
}

What caught my attention is that executing this line name = name + ''; can have only one of these two effects:

• either a string is assigned to the name variable, or
• an exception (TypeError: Cannot convert object to primitive value) is thrown by the JS engine if name.toString is 'broken' (doesn't return a string, isn't a function, etc.).

It's even mentioned in the comment in this code. This means this check if (!isString(name)) never passes, so $parseMinErr('iseccst', ...) is never thrown.

Plunker: http://plnkr.co/edit/xWFZbUxTeKszPd28apSf?p=preview

[Narretz]

@lgalfaso Can you take a look a this as the $parse master in residence?

[lgalfaso]

This is some leftover from the time that we used to do name = toString(name) (that had other issues and was changed to the current implementation). In this case, even the comment does not reflect how the method actually works.

PR welcome.

[thorn0]

Should we catch the engine's TypeError and throw iseccst instead? Wouldn't this hurt the performance?

[lgalfaso]

The if block should go away and the comment reflect what is going on and when it is expected this to throw. No try...catch as is check is in the critical path of $parse

[lgalfaso]

fixed with a6e9174