fixup jmespath multiselect codegen #551
Conversation
| writer.write(""" | ||
| if $2L != nil { | ||
| $1L = append($1L, *$2L) | ||
| }""", ident, projected.ident); | ||
| $1L = append($1L, $3L$2L) |
There was a problem hiding this comment.
Pushed a commit to reflow this block to have the numbers go by order of first appearance, which coincidentally means that it's now append(2,3,1) but I think overall makes more sense.
| // of string, etc. | ||
| // we may need to pull the member shapes back out later, but they're not guaranteed to be in the model - so keep a | ||
| // shadow map of synthetic -> member to short-circuit the model lookup | ||
| private final Map<Shape, Shape> synthetics = new HashMap<>(); |
There was a problem hiding this comment.
For my general knowledge, but do shapes in the model usually get updated, or are they write-only? I would suspect that shapes are mostly a dictionary of things you've seen where you only ever add entries, but I'm not sure if they'd be a case where a shape would also get updated
There was a problem hiding this comment.
Every construct in smithy in-code is immutable. The only way to change something is by going toBuilder() and then re-build()ing to create a new instance. So there's no way to just add these synthetics to the model in place (nor do I think we should, that would run the risk of them getting picked up elsewhere). Not sure if that answers your question.
There was a problem hiding this comment.
It does, but just to clarify. I'm not asking about the Smithy interface, I'm asking about the general mental model for building shapes
I'm not asking to add the synthetics into the model in place. We're building here a lookup map, and I was curious about the Smithy model allowed updates to existing model shapes, something like "you had shape A which was a list(string). Now it's a list(bool)`. This would mean that a lookup map may also need to be updated instead of just being write-once. I suspect this isn't valid just because I can't think of a use case where you'd want that.
There was a problem hiding this comment.
Oh I see. Yeah, once we're in this code, the model (and all of its shapes) are essentially fixed.
| var expr = JmespathExpression.parse(def.getPath()); | ||
|
|
||
| return writer -> { | ||
| var generator = new GoJmespathExpressionGenerator(ctx, writer); | ||
|
|
||
| writer.write("func() {"); // contain the scope for each binding | ||
| var result = generator.generate(expr, new GoJmespathExpressionGenerator.Variable(input, "in")); | ||
|
|
||
| if (param.getType().equals(ParameterType.STRING_ARRAY)) { | ||
| // projections can result in either []string OR []*string -- if the latter, we have to unwrap |
There was a problem hiding this comment.
For context: discussed offline and this is done because projections don't have nulls, so they don't have *string members
| @@ -27,15 +27,15 @@ | |||
|
|
|||
| public final class ShapeUtil { | |||
| public static final StringShape STRING_SHAPE = StringShape.builder() | |||
| .id("smithy.api#String") | |||
| .id("smithy.api#PrimitiveString") | |||
There was a problem hiding this comment.
I know we talked about these offline, and since these tests work I guess this is fine, but I couldn't find any docs about the existence of this type
There was a problem hiding this comment.
OK I finally found it. It's in the prelude model, not the preamble model, I was searching the wrong thing - https://smithy.io/2.0/spec/model.html#prelude
[]string, the types will always align