Added ceremonial break.

[mitchell-as]

DX-3088 Buildexpression unmarshalling may be dropping args

Function calls in JSON have a single argument object with a list of values. We already assert that the object is of length 1 (the list of args;

cli/pkg/buildscript/unmarshal_buildexpression.go

Lines 259 to 264 in 2deb608

	// m is a mapping of function name to arguments. There should only be one
	// set of arguments. Since the arguments are key-value pairs, it should be
	// a map[string]interface{}.
	if len(m) > 1 {
	return nil, errs.New("Function call has more than one argument mapping")
	}

), but a cursory glance at a later loop in the code may lead one to believe there is more than one object. I've added a ceremonial break with a comment for clarity.

We're not dropping any args; the interface contains a list of all arguments.

[Naatan]

This doesn't seem like it's solving anything. It's now defaulting to the first entry rather than the last. The issue here seems to be that it's not considering all arguments, which so far is a non-issue as no function calls exist at this level (yet!) that have more than one argument.

[mitchell-as]

No, this "non-fix" is making a point that the length of m is expected to be 1 (thanks to the assertion above). It is impossible for there to be more than one "argument". What you take to be an argument is actually a list of arguments (which you can see being processed below).

This is not a bug. If there will be more than one "thing" in the argument object, then we will have to file a task to expand what we're doing here.

Or am I completely missing something?

[Naatan]

My bad; I missed the comment about m>1.