address PR feedback

aws · Jul 15, 2024 · 7031a52 · 7031a52
1 parent c02709f
commit 7031a52
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 13 deletions.
diff --git a/...odegen/src/main/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGenerator.java b/...odegen/src/main/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGenerator.java
@@ -130,7 +130,7 @@ private Variable visitMultiSelectList(MultiSelectListExpression expr, Variable c
     private Variable visitFilterProjection(FilterProjectionExpression expr, Variable current) {
         var unfiltered = visitProjection(new ProjectionExpression(expr.getLeft(), expr.getRight()), current);
         if (!(unfiltered.shape instanceof CollectionShape unfilteredCol)) {
-            throw new CodegenException("projection did not create a list");
+            throw new CodegenException("projection did not create a list: " + expr);
         }
 
         var member = expectMember(unfilteredCol);
@@ -197,7 +197,7 @@ private Variable visitFlatten(FlattenExpression tExpr, Variable current) {
 
         // inner HAS to be a list by spec, otherwise something is wrong
         if (!(inner.shape instanceof CollectionShape innerList)) {
-            throw new CodegenException("left side of projection did not create a list");
+            throw new CodegenException("projection did not create a list: " + tExpr);
         }
 
         // inner expression may not be a list-of-list - if so, we're done, the result is passed up as-is
@@ -229,7 +229,7 @@ private Variable visitProjection(ProjectionExpression expr, Variable current) {
             leftMember = expectMember(map);
         } else {
             // left of projection HAS to be an array/map by spec, otherwise something is wrong
-            throw new CodegenException("left side of projection did not create a list");
+            throw new CodegenException("projection did not create a list: " + expr);
         }
 
         var leftSymbol = ctx.symbolProvider().toSymbol(leftMember);
@@ -240,16 +240,14 @@ private Variable visitProjection(ProjectionExpression expr, Variable current) {
                 .generate(expr.getRight(), new Variable(leftMember, "v", leftSymbol));
 
         var ident = nextIdent();
-        // projections implicitly filter out nil evaluations of RHS
-        var needsDeref = isPointable(lookahead.type);
         writer.write("""
                 var $L []$T
                 for _, v := range $L {""", ident, ctx.symbolProvider().toSymbol(lookahead.shape), left.ident);
 
         writer.indent();
         // projected.shape is the _member_ of the resulting list
         var projected = visit(expr.getRight(), new Variable(leftMember, "v", leftSymbol));
-        if (needsDeref) {
+        if (isPointable(lookahead.type)) { // projections implicitly filter out nil evaluations of RHS
             writer.write("""
                     if $2L != nil {
                         $1L = append($1L, *$2L)
@@ -269,12 +267,9 @@ private Variable visitSub(Subexpression expr, Variable current) {
     }
 
     private Variable visitField(FieldExpression expr, Variable current) {
-        var optMember = current.shape.getMember(expr.getName());
-        if (optMember.isEmpty()) {
-            throw new CodegenException("expected member " + expr.getName() + " in shape " + current.shape);
-        }
+        var member = current.shape.getMember(expr.getName()).orElseThrow(() ->
+                new CodegenException("field expression referenced nonexistent member: " + expr.getName()));
 
-        var member = optMember.get();
         var target = ctx.model().expectShape(member.getTarget());
         var ident = nextIdent();
         writer.write("$L := $L.$L", ident, current.ident, capitalize(expr.getName()));

diff --git a/...en/src/test/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGeneratorTest.java b/...en/src/test/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGeneratorTest.java
@@ -275,7 +275,7 @@ public void testLengthFunctionStringPtr() {
 
     @Test
     public void testComparatorInt() {
-        var expr = "length(objectList) > `0`";
+        var expr = "length(objectList) > `99`";
 
         var writer = testWriter();
         var generator = new GoJmespathExpressionGenerator(testContext(), writer);
@@ -288,7 +288,7 @@ public void testComparatorInt() {
         assertThat(writer.toString(), Matchers.containsString("""
                 v1 := input.ObjectList
                 v2 := len(v1)
-                v3 := 0
+                v3 := 99 
                 v4 := int64(v2) > int64(v3)
                 """));
     }