From 7031a526208644b2ee5c819d4b1650058a9a56b1 Mon Sep 17 00:00:00 2001
From: Luc Talatinian <lucix@amazon.com>
Date: Mon, 15 Jul 2024 14:32:35 -0400
Subject: [PATCH] address PR feedback

---
 .../codegen/GoJmespathExpressionGenerator.java  | 17 ++++++-----------
 .../GoJmespathExpressionGeneratorTest.java      |  4 ++--
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGenerator.java
index 9206ecbd..32a3cd8e 100644
--- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGenerator.java
+++ b/codegen/smithy-go-codegen/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,8 +240,6 @@ 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);
@@ -249,7 +247,7 @@ private Variable visitProjection(ProjectionExpression expr, Variable current) {
         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/codegen/smithy-go-codegen/src/test/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGeneratorTest.java b/codegen/smithy-go-codegen/src/test/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGeneratorTest.java
index 0e4e1e72..0c77721d 100644
--- a/codegen/smithy-go-codegen/src/test/java/software/amazon/smithy/go/codegen/GoJmespathExpressionGeneratorTest.java
+++ b/codegen/smithy-go-codegen/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)
                 """));
     }