802844 - Forbid passing complex objects as arguments to macros

Status: RESOLVED WONTFIX

(L20n :: General, defect, P2)

Description

[Staś Małolepszy :stas]

Instead of:

  <macro($arg) { $arg.prop == "foo" ? ... }>
  <entity1 "{{ macro(entity2) }}">

require the arguments passed to the macro to be of primitive values (string, number):

  <macro($arg) { $arg == "foo" ? ... }>
  <entity1 "{{ macro(entity2.prop) }}">

I think the intent was to limit the possibilities of what could be tested in a macro's conditional expression.  But if we fix bug 802839, we might not actually need this.

Comment 1

[Staś Małolepszy :stas]

I don't like this change.  It seems unnecessarily limiting.  If entities are objects, why would we want to limit the types that an argument to a macro can be?

I could see us throwing an exception when you pass a macro as an argument.  But again, why?

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Well, the thing here is that it makes it easier to have bugs that will be discovered really late.

Imagine a macro:

<macro($arg) {$arg..attr == "foo" ? $arg._prop : "Zero"}>

What if the $arg doesn't have attr or _prop ?

If we require the macro to pass str/int, then it'd be:

<macro($attr, $prop) {$attr == "foo" ? $prop : "Zero"}>

and we require explicit arg passing thus removing the risk here.

Comment 3

[Axel Hecht [:Pike]]

I'm not sure I'm either side.

Sure, limiting the arguments to simple values is rather restrictive. OTH, what are the use cases for complex values?

Is there any way in which passing in a macro to a macro can be used inside the outer macro? Like jsfunc.call or .apply in js to dereference the name to the actual call?

For tooling and editing support, restricted values are surely easier, but I doubt that you couldn't actually infer all type info you need, at least at the point where you try to use the macro with complex params. Not sure how to support editing macros to begin with, there's a good deal of chicken-and-egg between macro editing support and callsites. Once you have callsites, you can probably warn about things like your params not supporting sub-properties you try to reference, or autocomplete to references that exist in all passed in arguments so far.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

editing macros is less of a concern to me.

What is, is that if we want to allow passing complex objects, we need a way to operate on them. Things like ability to check if an entity has the argument, or if the value of the entity is a hash, or list and does it have the key.

What if you pass wrong arguments to a macro from inside of another macro.

That's a lot of debugging and we don't currently have too much logic to support a localizer/developer trying to recover from this.

I'd suggest that we stick to primitive values - string, number, maybe bool (?), maybe null (?) and this way we control this much earlier.

If we come up with a reasonable strategy in L20n.Next then we can extend this support.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

We decided to allow for entity ID passing to macro.

The reasoning behind it is that macros may often serve as a way to select the right value from a complex entity value based on the attributes of this entity.

Example:

<foo {
 *male: 'Resp 1',
  female: {
    animate: 'Resp 2',
   *inanimate: 'Resp 3',
  }
}
  _person: 'first'
>

In this case, passing all elements (especially if we forbid Hashes) as separate arguments will be extremely inefficient.
If we allow for passing whole entity to macro, we can actually build smart way to get the right value.

We will not allow (for now) to pass macro ID to a macro in order to reduce the potential complexity of the structure.

Comment 6

[Staś Małolepszy :stas]

I filed bug 816887 as a follow-up-of-sorts.